# errors.md — Append-Only Error Log

> Read this file at the start of every session.
> Never delete entries. Only append.
> Every error, wrong assumption, or bad architectural choice goes here.

---

## How to Add an Entry

```markdown
## [YYYY-MM-DD HH:MM] Error: <short title>

**Context:** What were you trying to do?
**What went wrong:** Precise description of the failure.
**Root cause:** Why it happened (not just what happened).
**Fix applied:** What you changed to resolve it.
**Lesson learned:** One-sentence rule to prevent recurrence.

---
```

---

## [2026-05-05] Discussion attribuée au mauvais co-host (lead RU sans réservation)

**Context:** Après `php artisan messagerie:sync-latest`, une discussion entre Hichem (co-host) et Denis (guest) — un lead RU sans Reservation locale — est apparue dans le compte de Farouk au lieu d'Hichem.

**What went wrong:** `MessagingHistorySyncService::syncLatest()` itérait `[null, ...co-hosts]` et stampait chaque thread avec `$user->id` (l'utilisateur d'itération courant), sans jamais vérifier que le payload pointait vers ce co-host. Pour un thread sans `Reservation` locale, l'inbox-owner gagnait par ordre d'itération.

**Root cause:**
1. `syncLatest()` (l. 277) : `Discussion::upsertFromApiThread($thread, $parentId, $coHostId = $user->id)` — assignation arbitraire à l'utilisateur itéré.
2. La résolution était dupliquée 4 fois (`MessagingHistorySyncService`, `MessagingWebhookController`, `BackfillDiscussionCoHostCommand`, `RentalsService`) avec des stratégies divergentes.
3. `Discussion::where('thread_id', $X)->value('id')` n'était pas scopée par `co_host_id` → messages cross-tenant possibles.
4. `properties.co_host_id` est utilisé de façon ambiguë (parfois `users.id`, parfois `users.rentals_subuser_id`).

**Fix applied:** Refonte complète :
- Création du service `App\Services\Messaging\CoHostResolver` centralisant 5 stratégies ordonnées (`ReservationProperty`, `RuSubuserId`, `ContactName`, `InboxOwner`, `PrimaryFallback`) avec garde-fou anti-bug-Farouk : `InboxOwner` n'est utilisé que si le payload n'apporte aucun signal contradictoire.
- `Chat::upsertFromApiMessage($msg, $discussionId, ?int $expectedCoHostId)` bloque les attaches cross-tenant.
- `MessagingHistorySyncService` et `MessagingWebhookController` utilisent le resolver et détectent les drifts (`co_host_id` divergent → log WARN, pas d'écrasement).
- Migration `2026_05_05_120000_add_messaging_cohost_guards` : `UNIQUE` sur `users.rentals_subuser_id` + index composite `(co_host_id, last_message_date)` sur discussions.
- Commandes `messagerie:reclassify-discussions` (dry-run par défaut) et `messagerie:detach-orphan-chats` pour fix rétroactif.
- 17 nouveaux tests dont la reproduction exacte du scénario du bug.

**Lesson learned:** La résolution co-host doit utiliser des signaux du payload (IDs RU, noms, propriété), JAMAIS l'utilisateur d'itération comme assignation par défaut. Ne pas utiliser `inbox owner` comme fallback quand le payload contient un signal contradictoire.

---

## [2026-05-06] Bug Farouk-bis — sync sur DB fraîche ré-attribue à Farouk

**Context:** Après `migrate:fresh` + `php artisan messagerie:sync-history` (refactoré dans la même session pour pull via SOAP `listReservationsByDate` mois-par-mois), 4 discussions BookingCom appartenant à Hichem Mathlouthi (id=330) ont été créées avec `co_host_id=1` (Farouk, primary RU account). Constaté via `SELECT * FROM discussions WHERE contacts LIKE '%Mathlouthi%' AND co_host_id = 1`.

**What went wrong:** Le `CoHostResolver` censé être robuste (5 stratégies, post-mortem du bug d'origine) a deux trous structurels qui se cumulent :

1. `CoHostResolver::collectCandidateNames` ne lit que `Contacts[].Name`. Les payloads RU réels (BookingCom, Airbnb) n'envoient JAMAIS `Name` — ils utilisent `Contacts[].FirstName + LastName` (et frequemment inversent les deux : `FirstName="Mathlouthi", LastName="Hichem"`). Donc `tryContactName` ne voyait jamais le nom de Hichem dans les candidats.

2. `MessagingHistorySyncService::syncFromApi` appelait `resolver->resolveFromReservationId($reservationId)` une fois par réservation au lieu de `resolveFromThreadPayload($thread, null)` par thread. `resolveFromReservationId` n'expose que 2 stratégies sur 5 (ReservationProperty + InboxOwner). Sur DB fraîche : ReservationProperty fail (`reservations` vide), InboxOwner null (pas passé) → tombe systématiquement sur PrimaryFallback (Farouk).

**Root cause:** L'API `resolveFromReservationId` était une simplification trompeuse — son nom suggère "même résolution depuis un id de réservation" alors qu'elle saute 3 stratégies sur 5. Personne d'autre que `syncFromApi` ne l'appelait. Et `collectCandidateNames` avait été écrit en regardant des fixtures de test (qui utilisent `Name`) au lieu de payloads de production (qui n'utilisent que `FirstName/LastName`).

`syncLatest` n'a pas corrigé l'attribution au passage parce qu'avec le Bug 1, sa résolution tombait aussi sur PrimaryFallback → match avec l'existant → pas de drift détecté → silence.

**Fix applied:**
- **Fix A** (`CoHostResolver::collectCandidateNames`) : génère 2 candidats supplémentaires par contact à partir de `FirstName + LastName` dans les deux orientations.
- **Fix B** (`syncFromApi`) : résolution par-thread via `resolveFromThreadPayload($thread, null)` (5 stratégies). Le `co_host_id` résolu par thread est propagé à `Chat::upsertFromApiMessage` au lieu d'utiliser une valeur unique réservation-level.
- 3 nouveaux tests dont la reproduction exacte du payload BookingCom prod (id=133).
- Data-fix prod manuel : les 4 lignes existantes (id 133, 146, 147, 149) ne se corrigent pas par re-sync (drift detection refuse l'écrasement). À traiter via `UPDATE ... SET co_host_id = NULL` + `messagerie:sync-latest`.

**Lesson learned:** Les fixtures de test ne reflètent pas toujours les payloads réels — toujours valider avec un échantillon de prod réel avant de présumer la forme. Et : si une méthode publique d'un service expose moins que sa sœur (ex. `resolveFromReservationId` vs `resolveFromThreadPayload`), soit on l'enrichit pour atteindre la parité, soit on la supprime ; ne jamais laisser une "version dégradée" avec un nom qui n'avertit pas.

---

## [2026-05-11] Migration FK error: `event_invitations.client_id` vs `clients.id`

**Context:** Lancement de `php artisan migrate` pour créer la nouvelle table `event_invitations` (feature Soirée Loyalty WhatsApp RSVP).

**What went wrong:** `SQLSTATE[HY000]: General error: 1005 Can't create table 'event_invitations' (errno: 150 "Foreign key constraint is incorrectly formed")` sur l'ALTER `add constraint event_invitations_client_id_foreign foreign key (client_id) references clients (id)`. La table a été créée à moitié (sans la FK), bloquant un simple re-run.

**Root cause:** La migration utilisait `$table->foreignId('client_id')->nullable()->constrained('clients')->nullOnDelete()`. `foreignId()` génère `UNSIGNED BIGINT`, alors que la table `clients` (héritage : `0001_01_01_000003_create_current_clients_schema.php`) déclare `$table->integer('id')->autoIncrement()` — donc `SIGNED INT`. MySQL refuse la FK quand les types ne correspondent pas exactement (taille + signedness).

À noter : SQLite ne fait pas respecter cette règle, donc les tests passaient en `:memory:` malgré le bug.

**Fix applied:**
- Remplacement par `$table->integer('client_id')->nullable()->index()` (pas de FK DB-level — la relation Eloquent `EventInvitation::client()` reste fonctionnelle, juste sans cascade automatique).
- Nettoyage manuel avant re-run : `Schema::dropIfExists('event_invitations')` + suppression de la ligne `migrations` (la migration avait été marquée FAIL mais la table résiduelle bloquait le re-run).

**Lesson learned:** Avant `foreignId()->constrained()` sur une table legacy, toujours vérifier le type exact de `id` dans la migration cible. Pour les tables avec `integer().autoIncrement()` (signed INT), utiliser `$table->integer(...)->index()` sans FK, ou aligner les deux côtés. Et : SQLite en tests masque ce genre de mismatch — confirmer les migrations sur MySQL (au moins en dev) avant de pousser.

---