diff --git a/tasks/lessons.md b/tasks/lessons.md index b53b3f5..61bbcec 100644 --- a/tasks/lessons.md +++ b/tasks/lessons.md @@ -130,3 +130,12 @@ **Was der Downloader anders macht (und warum es Tailscale ermöglicht):** Host steckt IM Code (`{v,h,p,t,n,fp?,s?}`), nicht extern. Zwei Bind-Modi: lokal (127.0.0.1) ODER Netzwerk (0.0.0.0) — letzteres NUR mit nicht-leerer **fail-closed IP-Allowlist**: leere Allowlist = nur Loopback; geprüft am ECHTEN socket.remoteAddress (NIE forwarded-Header), `::ffff:`-normalisiert, CIDR-Matching. Tailscale wird NICHT autodetektiert — es ist nur eine der `os.networkInterfaces()`-IPs, erreicht über den Tunnel; Allowlist (auf den Tailnet, z.B. `100.64.0.0/10`) + Token sind das Gate, WireGuard ist die Verschlüsselung. **Regel:** Bei "mach es wie X": X lokalisieren (grep), die security-kritischen Teile mit einem Subagenten verbatim mappen, dann replizieren. Eine fail-closed Allowlist (Loopback immer erlaubt, leer=loopback-only, real peer IP) ist das richtige Modell für netzwerk-erreichbare read-only Diagnose über einen vertrauten Tunnel — plaintext-Transport ist ok, WENN der Tunnel (Tailscale/WireGuard) verschlüsselt UND die Allowlist+Token den Zugriff gaten. Den HAPPY-Path (allowlisted non-loopback peer über echten 0.0.0.0-Socket) auch LIVE testen, nicht nur per Komposition aus Unit+Wiring. **Prozess-Stolperstein:** Test NACH dem Feature-Commit hinzugefügt → release_gitea.mjs brach ab ("uncommitted tracked changes"). Vor jedem Release: `git status --porcelain | grep -v '^??'` muss leer sein. Tracked-aber-uncommitted (auch ein nachgereichter Test) blockt den Build. + +## 2026-06-21 — "Gefühlt laggy nach Zeit, CPU/RAM normal" = ERST messen, dann in echtem Blink profilen (v3.3.87) +**Kontext:** User: Programm fühlt sich nach langer Laufzeit mit vielen Uploads zäh an, CPU ~40%/8 Kerne, RAM 6/32 GB — beide normal/stabil. Erste Hypothese (Haupt­prozess-Config-I/O skaliert mit wachsender History) war für DIESEN User FALSCH. +**Was es wirklich war (gemessen + profiliert):** `renderRecentUploadsPanel` hatte einen Append-only-Fastpath, gegated auf `rows.length > _recentLastRenderedLen`. `maybeAddSessionFile` capped per push-then-slice (2000→2001→zurück auf 2000). Ab dem Cap ist `rows.length` auf 2000 fixiert → Gate für IMMER false → JEDE Completion fiel in den Full-`innerHTML`-Rebuild von 2000 Zeilen. Blink-Messung (Playwright, table-layout:fixed, gleiche Engine wie Electron): **~80 ms pro Completion** → wiederkehrender 80-ms-Freeze. Fix (append-evict, Gate auf `pendingAppends>0`, Overflow vom DOM-Boden evicten): **80 ms → 7,4 ms** (>10×), DOM bleibt exakt == Daten (Cap/Reihenfolge/keine Dupes), über 5000 Completions verifiziert. +**Regel 1 — Magnituden NICHT raten, LESEN:** „wächst über Zeit" ist eine Annahme über GRÖSSE. Die echte electron-config.json war 52 KB (History 23 Zeilen) — ein einziger `node`-Read killte die ganze Config-I/O-Theorie. Bevor man eine „skaliert-mit-X"-Ursache fixt: X am echten Artefakt messen (Dateigröße, Array-Länge, Job-Count im persistierten State). +**Regel 2 — Im ECHTEN Renderer-Engine profilen, nicht analytisch raten:** jsdom rendert kein Blink-Layout. Playwright (Chromium = Electron-Blink) mit `performance.now()` um (a) Rebuild und (b) erzwungenes Relayout nach Style-Write liefert die Zahl, die entscheidet: 3 ms = unsichtbar, 80 ms = DIE Ursache. Dieselbe Messung ist Fix-Auswahl UND Vorher/Nachher-Verifikation (das Goal verlangt „verifiziere dass behoben" — ein grüner Test beweist Korrektheit, NICHT dass der Lag weg ist). +**Regel 3 — Multi-Agent-Findings gegen primäre Evidenz prüfen (Control-Char-Falsch­positiv):** Der Hunt meldete HIGH-ish einen „_sessionFileKeys delete-key separator mismatch". Beim Versuch ihn zu fixen matchte der Edit-`old_string` NICHT. Char-Code-Dump (`HAS_U0001: True`) zeigte: die Zeile hat ECHTE U+0001-Zeichen — die Read-Tools der Verifier-Agenten rendern Steuerzeichen unsichtbar, sie schlossen fälschlich „keine Separatoren". KEIN Bug. **Wenn ein Fix-`old_string` nicht matcht obwohl Grep ihn zeigt: Char-Codes dumpen, bevor man dem Tool misstraut — die Quelle kann unsichtbar von der Read-Anzeige abweichen.** +**Regel 4 — Den negligible-aber-realen Befund mit Zahl ABLEHNEN, nicht aus dem Bauch:** queueJobs O(N)-Scan pro Render (wächst unbounded, da removeFromQueueOnDone=false UND Folder-Monitor EINEN Batch via addJobs am Leben hält → 500-Cap-Prune feuert nie) — real, aber Blink-gemessen <0,1 ms bei 3000 Jobs. Den riskanten Inkremental-Counter-Refactor mit DIESER Zahl skippen, nicht mit „fühlt sich klein an". +**Wie anwenden:** Append-only-Optimierungen, die auf Längenwachstum gaten, brechen still an JEDEM Cap (push-then-slice fixiert die Länge) — stattdessen die Anzahl NEUER Items zählen und am Boden evicten. „Mach es wie die Queue-Tabelle (virtualisieren)" war hier NICHT nötig: die Messung zeigte stehende 2000 Zeilen kosten median 0,4 ms; nur der Rebuild war teuer. Simplest-Fix der die gemessene Ursache trifft schlägt die größere Architektur-Änderung. diff --git a/tasks/todo.md b/tasks/todo.md index 506df08..151cbd9 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -1,36 +1,52 @@ -# Tailscale / network-bind diagnostics — match the downloader (rd-diagnostics-mcp) +# Long-running lag — root cause + fix (symptom: UI laggy over time, CPU 40%/RAM 6GB both normal) -Goal: make the MHU read-only diagnostics reachable like the Real-Debrid-Downloader does over -Tailscale — host embedded in the connection code, two bind modes (local / network), and a -fail-closed IP allowlist as the access gate. No Tailscale auto-detection (the downloader has none); -Tailscale is just one of the offered interface IPs reached over the tunnel, gated by the allowlist. +## What "laggy after time, low CPU, stable RAM" actually was (MEASURED, not assumed) +First instinct (main-process config I/O scaling with history) was WRONG for this user: the +real config is tiny (history 23 rows / 4.8 KB, total 52 KB, queue ~153 jobs). Measured with a +one-off node read of the live electron-config.json. So serialize-cost theories were dead on arrival. -## Plan -- [ ] lib/ip-allowlist.js — fail-closed allowlist (normalizeIp/::ffff:, isLoopback, ipv4ToInt, matchIpRule exact+CIDR+wildcard, evaluateClientAllowed: loopback always, empty=loopback-only). + unit tests. -- [ ] remote-server.js — accept config.allowlist; reject non-allowlisted peers at connection (close 4005). + protocol test. -- [ ] config-store.js — diagnostics subtree: bindMode ('local'), publicHost (''), allowlist ([]). -- [ ] main.js — bindMode->host (local=127.0.0.1, network=0.0.0.0, network requires non-empty allowlist); buildDiagnosticCode with host (h); getSuggestedRemoteHosts (os.networkInterfaces); pass allowlist; IPC save-settings/status. -- [ ] gateway/code.js — decode h/p/t/n/fp/s (tolerant of old port/token/label). + test. -- [ ] gateway/index.js — connect_server takes host from the code; host arg optional override. -- [ ] renderer/app.js — bind-mode selector, publicHost input + suggested-host chips, allowlist textarea (network), network-requires-allowlist validation. -- [ ] docs/remote-diagnostics-setup.md — network mode + allowlist + Tailscale (set allowlist to your tailnet, e.g. 100.64.0.0/10). -- [ ] Tests: ip-allowlist unit, remote-server allowlist protocol, gateway decode, integration (network bind + allowlist), adversarial fail-closed (empty allowlist rejects non-loopback). -- [ ] Release v3.3.86 (gitea + github mirror). +A 13→18-agent leak hunt + adversarial verify + a Playwright/Blink microbenchmark found the +真 cause: -## Review (done) -All steps implemented and verified. lib/ip-allowlist.js (fail-closed, ::ffff:, CIDR incl. -100.64.0.0/10) + 8 unit tests. remote-server.js rejects non-allowlisted peers (close 4005), -opt-in via config.allowlist (existing remote-control unaffected) + 2 protocol tests (fail-closed -wiring + loopback-always-allowed). Code now carries the host (mhu1_{v,h,p,t,n,fp?,s?}); gateway -decode is tolerant of the legacy long keys; connect_server takes the host from the code (host arg -optional override) — proven end-to-end by the integration harness connecting with NO host arg. -Renderer: bind-mode selector + public-host input + suggested-host chips + allowlist textarea + -network-requires-allowlist validation. Docs rewritten for Tailscale (set allowlist to the tailnet, -put the Tailscale IP/MagicDNS in the code address). 393 app tests + 9 gateway tests + e2e + -integration + adversarial all green, lint 0 errors. +### ROOT CAUSE (confirmed + profiled): recent-uploads panel append-only path defeats itself at the cap +- `renderRecentUploadsPanel` had a cheap append-only fast path gated on `rows.length > _recentLastRenderedLen`. +- `maybeAddSessionFile` caps `sessionFilesData` by push-then-slice (2000 → 2001 → sliced back to 2000). +- So once past SESSION_FILES_CAP, `rows.length` is pinned at 2000 → the gate is FALSE forever → + EVERY completion fell through to `tbody.innerHTML = rows.map(...).join('')` — a full ~2000-row + rebuild. Cap is per (link × file × hoster), so 4–5 hosters hit 2000 at only ~400–500 files. +- Blink measurement (table-layout:fixed, same engine as Electron): full 2000-row rebuild = **~80 ms** + on EVERY completion past the cap. At several completions/sec that is a repeating ~80 ms main-thread + freeze → exactly "fine fresh, gets laggy after many uploads, CPU/RAM fine." -## Security model shift -v3.3.85 hard-locked loopback. This change replaces that with the downloader's model: network bind -(0.0.0.0) is allowed ONLY with a non-empty fail-closed IP allowlist (empty => loopback only). The -allowlist (real socket peer, ::ffff: normalized, CIDR) + token are the gate; the tunnel -(Tailscale/WireGuard) is the confidentiality layer. Plaintext ws:// — document the trust boundary. +## Fix (shipped) — append-evict, keeps the panel append-only past the cap +- Track newly-pushed rows in `_recentPendingAppends` (incremented in maybeAddSessionFile), consumed + every render. Gate the fast path on `pendingAppends > 0` (not length-delta) so it survives the cap. +- Prepend the new rows, then evict the same overflow count from the DOM bottom (oldest, = data front + eviction in date-desc) to honour the cap. DOM work back to O(added). +- Gate behind an explicit `appendOnly` flag passed ONLY by scheduleRecentRender's rAF, so selection/ + delete/clear/sort/batch-done renders stay full + correct (no wrong-row eviction, no double-prepend). +- VERIFIED in Blink over a simulated 5000-completion session: per-frame render **80 ms → median 7.4 ms** + (>10×), DOM stays exactly == data (cap held, newest-on-top, oldest evicted, ZERO duplicates). + +## Also shipped earlier (29d1944) — defensible, but NOT the cause for this user +- T1 ConfigStore in-memory cache (mtime/size keyed) + lean _serializeForDisk (clones only hosters) + + copyFileSync .bak. T3 cache logMode/logFilePath (drop load() from the 500 ms log flush). + +## Investigated and DISMISSED with evidence +- `_sessionFileKeys` "delete-key separator mismatch" (workflow flagged it HIGH-ish): FALSE POSITIVE. + Line uses REAL U+0001 chars (char-code dump: `HAS_U0001: True`); the verifier agents' Read rendered + the control chars invisibly and wrongly concluded "no separators". Keys match at runtime. No leak. +- queueJobs O(N) per-render scans (grows unbounded since removeFromQueueOnDone=false AND folder-monitor + keeps ONE batch alive via addJobs so the 500-cap prune never fires): REAL but Blink-measured at + **<0.1 ms even at 3000 jobs** → imperceptible. Incremental-counter refactor NOT worth the risk. Skipped. +- "Continuous standing relayout of 2000 rows": Blink median 0.4 ms regardless of row count; its spikes + were caused by the rebuild (same root cause). Not a standing cost → no need to virtualize/lower cap. + +## Deferred (separate, not this symptom) +- doodstream-upload.js `_debugLog`: ungated SYNC fs.appendFileSync + statSync ~10–20×/upload on the + main loop. CONSTANT cost (does not grow over a session), only when doodstream is active. Real freeze + contributor but NOT the reported progressive lag — convert to buffered-async like main.js, separately. + +## Verification summary +- 397/397 tests pass, eslint 0 errors. +- Blink before/after: recent-panel render 80 ms → 7.4 ms; correctness asserted (cap/order/no-dupes).