fix(timeline): clear multi-select on drag start and lock in SSE edge-case coverage

- useTimelineDrag: onProjectBarMouseDown and single-alloc drag path now reset multiSelectRef + multiSelectState before starting a new drag, so the FloatingActionBar is dismissed immediately when an unrelated drag begins - FloatingActionBar.test.tsx: 4 regression tests for the null-render guard (count=0) and all three label variants - useTimelineSSE.test.ts: 2 new tests — tab hides during pending reconnect timer (clears timer, resyncs on next open) and first-ever connection fails before any open (retry open still resyncs correctly) - assistant-tools-user-admin-inventory-read.test.ts: add isActive to expected findMany select shape (already in production, test was stale) Co-Authored-By: claude-flow <ruv@ruv.net>
2026-04-02 21:16:10 +02:00
parent 8d9e26872b
commit e7e525df49
6 changed files with 231 additions and 63 deletions
@@ -1,94 +1,138 @@
-# Plan: App Base URL — configurable via env, no localhost fallback in production
+# Plan: Timeline Interaction Stability — P1 continuation

 Stand: 2026-04-02

 ---

+# Workstream C: Drag + Selection Interaction Conflicts
+
 ## Anforderungsanalyse

-Email-Links (Invite, Password Reset) werden aktuell mit `process.env["NEXTAUTH_URL"] ?? "http://localhost:3100"` gebaut. Das ist korrekt, wenn `NEXTAUTH_URL` gesetzt ist. Das Problem: wenn der Wert fehlt oder leer ist, enthalten Produktions-E-Mails localhost-Links. Außerdem gibt es keine `.env.example`, die dokumentiert, welche Variablen gesetzt werden müssen.
+Code-Review von `useTimelineDrag.ts` ergibt zwei tatsächliche Konflikte:

-**Was gebaut wird:**
-1. Zentrale `getAppBaseUrl()` Funktion in `packages/api` — liest `NEXTAUTH_URL`, wirft in production einen Fehler wenn nicht gesetzt, fällt in dev auf localhost zurück.
-2. Beide Router (`invite.ts`, `auth.ts`) verwenden diese Funktion statt duplizierter Inline-Logik.
-3. `.env.example` mit allen benötigten Variablen.
-4. Health-Route zeigt ob `NEXTAUTH_URL` konfiguriert ist.
+1. **`onProjectBarMouseDown`** (Zeile 488): Startet einen Projekt-Bar-Drag **ohne** das committed `multiSelectState` zurückzusetzen. → `FloatingActionBar` bleibt während des Drags sichtbar (da `selectedAllocationIds.length > 0` aus dem committed Multi-Select).

-**Betroffene Pakete:** `packages/api`, `apps/web`
+2. **`onAllocMouseDown` single-drag path** (Zeile 611, nach dem `if (isMultiSelected)` Block): Wenn eine Allocation per Mousedown angedraggt wird die NICHT in der Multi-Selektion ist, wird ebenfalls kein Reset ausgeführt. → `FloatingActionBar` bleibt sichtbar.

-**Audit-Ergebnis — intentionale localhost-Referenzen (NICHT ändern):**
- `apps/web/e2e/dev-system/` — alle E2E-Helfer, Specs, global-setup.ts
- `apps/web/playwright.dev.config.ts`
- `apps/web/src/middleware.test.ts`
- `.github/workflows/deploy-test.yml`
+3. **`onCanvasRightMouseDown`**: Startet neue Multi-Select-Session mit `selectedAllocationIds: []` — resettet Selektion korrekt. Kein Fix nötig.

---
+4. **ESC-Taste**: Bereits in `TimelineView.tsx` über `clearMultiSelect` abgehandelt. Kein Fix nötig.
+
+**Fix-Semantik**: Beide Handler brauchen ein **volles Reset** (`INITIAL_MULTI_SELECT`) — im Gegensatz zu `cancelTransientMultiSelectState`, das committed Selektionen bewahrt. Ein neuer, unabhängiger Drag soll die Selektion vollständig verwerfen.

 ## Betroffene Pakete & Dateien

 | Paket | Datei | Art |
 |-------|-------|-----|
-| `packages/api` | `src/lib/app-base-url.ts` | create |
-| `packages/api` | `src/router/invite.ts` | edit |
-| `packages/api` | `src/router/auth.ts` | edit |
-| `apps/web` | `src/app/api/health/route.ts` | edit |
-| root | `.env.example` | create |
-
---
+| `apps/web` | `src/hooks/useTimelineDrag.ts` | edit |
+| `apps/web` | `src/components/timeline/FloatingActionBar.test.tsx` | create |

 ## Task-Liste

- [ ] **Task 1:** `getAppBaseUrl()` in `packages/api/src/lib/app-base-url.ts` erstellen.
-  - Liest `process.env["NEXTAUTH_URL"]` (trimmed).
-  - Wenn gesetzt und nicht leer → gibt den Wert zurück (trailing slash entfernen).
-  - Wenn leer/fehlend **und** `NODE_ENV === "production"` → wirft `Error("NEXTAUTH_URL must be set in production — email links will be broken")`.
-  - Sonst (development/test) → gibt `"http://localhost:3100"` zurück und loggt einmalig eine Warnung.
-  - → Datei: `packages/api/src/lib/app-base-url.ts`
+- [ ] **C-1:** `useTimelineDrag.ts` — Multi-Select bei Drag-Start zurücksetzen.
+  - In `onProjectBarMouseDown` (nach `if (e.button !== 0) return;`, vor `createProjectDragState`):
+    ```ts
+    multiSelectRef.current = INITIAL_MULTI_SELECT;
+    setMultiSelectState(INITIAL_MULTI_SELECT);
+    ```
+  - In `onAllocMouseDown` im Single-Drag-Zweig (nach dem `if (isMultiSelected) { ... return; }` Block, vor `createAllocationDragState`):
+    ```ts
+    multiSelectRef.current = INITIAL_MULTI_SELECT;
+    setMultiSelectState(INITIAL_MULTI_SELECT);
+    ```
+  - `setMultiSelectState` ist bereits von `useState` (stabil) — kein Eintrag in dep-Array nötig (Konvention konsistent mit bestehender `onAllocMouseDown`-Implementierung, die `setMultiSelectState` ebenfalls ohne Dep-Eintrag nutzt).
+  - Ref-Reset ist nötig damit nachfolgende synchrone Ref-Lesungen im selben Event-Cycle den gecleansten State sehen.
+  - → Datei: `apps/web/src/hooks/useTimelineDrag.ts`

- [ ] **Task 2:** `invite.ts` auf `getAppBaseUrl()` umstellen.
-  - Ersetze `const baseUrl = process.env["NEXTAUTH_URL"] ?? "http://localhost:3100";` durch `const baseUrl = getAppBaseUrl();`
-  - → Datei: `packages/api/src/router/invite.ts` Zeile 53
-
- [ ] **Task 3:** `auth.ts` auf `getAppBaseUrl()` umstellen.
-  - Ersetze `const baseUrl = process.env["NEXTAUTH_URL"] ?? "http://localhost:3100";` durch `const baseUrl = getAppBaseUrl();`
-  - → Datei: `packages/api/src/router/auth.ts` Zeile 50
-
- [ ] **Task 4:** `.env.example` anlegen mit allen required und optionalen Variablen.
-  - Sections: App, Auth, Database, Redis, SMTP, AI, Dev-Tools (pgAdmin)
-  - Jede Variable: Kommentar (required/optional, Beschreibung), Beispielwert.
-  - `NEXTAUTH_URL` als REQUIRED mit Hinweis "must be the public URL (e.g. https://capakraken.example.com) — used in email links; do not use localhost in production"
-  - → Datei: `.env.example`
-
- [ ] **Task 5:** Health-Route um `baseUrl`-Check erweitern.
-  - Liest `NEXTAUTH_URL` direkt (kein `getAppBaseUrl()` — soll nie werfen, nur reporten).
-  - Fügt `"baseUrl": { "configured": bool, "isLocalhost": bool }` zum JSON-Response hinzu.
-  - `configured: false` wenn Var fehlt/leer; `isLocalhost: true` wenn Wert mit `http://localhost` beginnt.
-  - → Datei: `apps/web/src/app/api/health/route.ts`
-
---
+- [ ] **C-2:** `FloatingActionBar.test.tsx` erstellen — Regressions-Coverage.
+  - Test 1: `selectedAllocationCount=0, selectedResourceCount=0` → rendert `null` (Bar ist ausgeblendet)
+  - Test 2: `selectedAllocationCount=2, selectedResourceCount=0` → Bar sichtbar, Text "2 allocations selected"
+  - Test 3: `selectedAllocationCount=0, selectedResourceCount=1` → Bar sichtbar, Text "1 resource selected"
+  - Test 4: `selectedAllocationCount=1, selectedResourceCount=1` → Bar sichtbar, Text enthält "1 allocation" und "1 resource"
+  - Sichert den `totalCount === 0 → null`-Guard: wenn Multi-Select gecleart wird, verschwindet die Bar.
+  - → Datei: `apps/web/src/components/timeline/FloatingActionBar.test.tsx`

 ## Abhängigkeiten

- Task 2 **und** Task 3 benötigen Task 1 (Funktion muss existieren).
- Task 2 und Task 3 können **parallel** ausgeführt werden (unterschiedliche Dateien).
- Task 4 und Task 5 sind **unabhängig** von allen anderen und können parallel ausgeführt werden.
-
---
+- C-1 und C-2 sind **unabhängig** voneinander (C-2 ist reiner Render-Test der Komponente)

 ## Akzeptanzkriterien

 - [ ] `pnpm test:unit` läuft grün
 - [ ] `pnpm --filter @capakraken/web exec tsc --noEmit` — keine neuen TS-Errors
- [ ] `pnpm test:e2e:email` — alle 3 E2E-Tests bestehen weiterhin
- [ ] Wenn `NEXTAUTH_URL=https://capakraken.hartmut-noerenberg.com`: E-Mail-Links enthalten diese Domain
- [ ] Wenn `NEXTAUTH_URL` fehlt und `NODE_ENV=production`: `createInvite` / `requestPasswordReset` werfen einen klaren Fehler beim Link-Bau
- [ ] `GET /api/health` liefert `baseUrl.configured: true/false` und `baseUrl.isLocalhost: bool`
- [ ] `.env.example` existiert und dokumentiert alle Pflichtfelder
-
---
+- [ ] Projekt-Bar-Drag startet mit aktiver Selektion: `FloatingActionBar` wird sofort ausgeblendet
+- [ ] Single-Alloc-Drag auf nicht-selektierter Allocation: `FloatingActionBar` wird ausgeblendet
+- [ ] Rechtsklick-Drag auf Canvas: Selektion wird korrekt überschrieben (bereits korrekt)

 ## Risiken & offene Fragen

- **Unit-Tests für Router:** Bestehende Tests für `invite.ts` und `auth.ts` müssen `NEXTAUTH_URL` in der Testumgebung gesetzt haben (oder `NODE_ENV=test` → Dev-Fallback greift). Vorhandene Tests prüfen, ob `process.env["NEXTAUTH_URL"]` dort gesetzt wird.
- **`docker-compose.prod.yml`** delegiert ENV-Vars an `.env.production` (gitignored). Das `.env.example` deckt ab, was dort stehen muss — kein Code-Change nötig.
- **Trailing Slash:** NEXTAUTH_URL könnte mit oder ohne `/` enden. `getAppBaseUrl()` sollte trailing slashes normalisieren.
+- **Ref-Sync**: `multiSelectRef.current` wird bereits in `useTimelineDrag` auf Zeile 260 bei jedem Render mit `multiSelectState` synchronisiert. Der explizite Ref-Reset im Handler ist aber nötig, damit nachfolgende synchrone Zugriffe (z.B. innerhalb desselben Event-Handlers) nicht den alten Ref-Wert lesen. Kein Risiko.
+- **Dep-Array**: `INITIAL_MULTI_SELECT` ist eine Modul-Konstante (referenz-stabil), `setMultiSelectState` ist stabil aus `useState`. Beide müssen nicht in die `useCallback` dep-Arrays — das ist konsistent mit der bestehenden Nutzung in `onAllocMouseDown`.
+
+---
+
+# Workstream D: SSE Reconnect — fehlende Edge-Case-Tests
+
+## Anforderungsanalyse
+
+Vollständige Analyse von `useTimelineSSE.ts` und 8 bestehenden Tests in `useTimelineSSE.test.ts`.
+
+**Bereits abgedeckt:**
+- Malformed payloads → kein invalidate
+- Ping → kein invalidate, backoff-Reset
+- Initial open → kein resync
+- Error → reconnect → open → resync (genau einmal)
+- Double-open nach reconnect → resync nur einmal
+- Dispose während pending Timer
+- Hidden on mount, dann visible → connect ohne resync
+- Hide während aktiver Connection, dann visible → resync
+
+**Fehlende Edge Cases:**
+
+1. **Tab versteckt während pending reconnect-Timer** (kein aktives `es`, aber `reconnectTimeout.current` gesetzt):
+   `handleVisibilityChange` prüft `if (es || reconnectTimeout.current)` → setzt `shouldResyncOnOpen = true`, löscht Timer via `clearReconnectTimer()`. Bei visible wieder: `reconnectAttempts = 0`, `connect()` → neue Connection → open → resync. **Nicht getestet.**
+
+2. **Allererste Verbindung schlägt fehl** (kein `onopen` je gerufen, direkt `onerror`):
+   `shouldResyncOnOpen = true` nach dem ersten Fehler ohne vorangehendes open. Nach Retry → open → resync. Dokumentiert: auch wenn die initiale Verbindung nie erfolgreich war, wird beim ersten erfolgreichen open korrekt resynct. **Nicht explizit getestet.**
+
+3. **Mehrere konsekutive Errors ohne open** (implizit bereits abgedeckt im "resets reconnect backoff"-Test über `emitError(); emitError();`). Kein zusätzlicher Test nötig.
+
+## Betroffene Pakete & Dateien
+
+| Paket | Datei | Art |
+|-------|-------|-----|
+| `apps/web` | `src/hooks/useTimelineSSE.test.ts` | edit |
+
+## Task-Liste
+
+- [ ] **D-1:** `useTimelineSSE.test.ts` — zwei fehlende Edge-Case-Tests hinzufügen.
+  - **Test 9**: `"hides during pending reconnect timer — clears timer and resyncs on next open"`
+    - Sequence: connect → open → error (shouldResyncOnOpen=true, Timer 2s) → hide (Timer gecleart, shouldResyncOnOpen bleibt true, es=null) → show (reconnectAttempts=0, connect() → neue EventSource) → emitOpen() → resync
+    - Verifikation: `MockEventSource.instances` hat Länge 2; `invalidateQueries` wurde genau `getTimelineSseResyncKeys().length` mal mit den richtigen Keys aufgerufen
+    - Unterschied zu bestehendem hide-Test: dort wird `es` aktiv geschlossen; hier ist `es = null` und nur der Timer ist pending
+  - **Test 10**: `"resyncs after the first-ever connection attempt fails before any open"`
+    - Sequence: connect → emitError() (kein emitOpen davor, shouldResyncOnOpen=true) → advance timer 2000ms → neue EventSource → emitOpen() → resync
+    - Verifikation: `MockEventSource.instances` hat Länge 2; `invalidateQueries` aufgerufen mit resync-keys
+    - Dokumentiert: `shouldResyncOnOpen` wird auch beim allerersten Fehler gesetzt — nicht nur nach erfolgreicher erster Verbindung
+  - Kein Produktionscode geändert — reine Test-Ergänzung.
+  - → Datei: `apps/web/src/hooks/useTimelineSSE.test.ts`
+
+## Abhängigkeiten
+
+- D-1 ist vollständig unabhängig (nur Tests, kein Source-Code)
+
+## Akzeptanzkriterien
+
+- [ ] `pnpm test:unit` läuft grün (alle 10 SSE-Tests grün)
+- [ ] Beide neuen Tests decken bisher ungetestetem Verhalten ab (kein Produktionscode nötig)
+
+---
+
+# Parallele Ausführung
+
+**Workstream C** und **Workstream D** sind vollständig unabhängig:
+
+- **Agent 1** → C-1 + C-2 (sequenziell: C-1 zuerst, C-2 kann parallel)
+- **Agent 2** → D-1 (eigenständig)
+
+Keine gemeinsamen Dateien, kein Merge-Konflikt-Risiko.