docs(tasks): high-concurrency lag audit — renderer measured-refuted, sync-fs was the blocker (v3.3.90)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Administrator 2026-06-21 03:44:41 +02:00
parent c7e884fdec
commit e0f789f56f
2 changed files with 46 additions and 52 deletions

View File

@ -152,3 +152,10 @@
**Der Trugschluss (Advisor hat geblockt):** Ich begründete „diesmal ist der Fix sicher, weil es Diagnostics-Collectors sind, KEIN Credential-Persistenz-Code wie letzte Runde". FALSCH. `lib/diagnostics-collectors.js` IST die Credential-Oberfläche — es ist der Redaktions-Code (`_secrets`/`_deepRedact`/`collectSecretValues`/`sanitizeConfig`/`redactLogText`). Genau dieser Code ist schon ZWEIMAL geleakt (7b5420e „one collector still leaked", 8d757a9 „redaction gaps") — bei grünem E2E. Der „elegante" Fix (config+secrets einmal snapshoten und durch die Collectors threaden) ist EXAKT die gefährliche Form: ein Pfad verpasst / ein stale secrets-array → SECRET LEAK, ein schlimmeres Versagen als der Cold-Freeze. Dieselbe Kategorie-Fehler wie letzte Runde (damals Datenverlust an config-store, jetzt Secret-Leak an der Redaktion), nur andere Datei. „Nicht Persistenz" hat mich getäuscht. **Der Trugschluss (Advisor hat geblockt):** Ich begründete „diesmal ist der Fix sicher, weil es Diagnostics-Collectors sind, KEIN Credential-Persistenz-Code wie letzte Runde". FALSCH. `lib/diagnostics-collectors.js` IST die Credential-Oberfläche — es ist der Redaktions-Code (`_secrets`/`_deepRedact`/`collectSecretValues`/`sanitizeConfig`/`redactLogText`). Genau dieser Code ist schon ZWEIMAL geleakt (7b5420e „one collector still leaked", 8d757a9 „redaction gaps") — bei grünem E2E. Der „elegante" Fix (config+secrets einmal snapshoten und durch die Collectors threaden) ist EXAKT die gefährliche Form: ein Pfad verpasst / ein stale secrets-array → SECRET LEAK, ein schlimmeres Versagen als der Cold-Freeze. Dieselbe Kategorie-Fehler wie letzte Runde (damals Datenverlust an config-store, jetzt Secret-Leak an der Redaktion), nur andere Datei. „Nicht Persistenz" hat mich getäuscht.
**Regel:** Die Frage ist nicht „ist es Persistenz?", sondern „trägt dieser Code eine Korrektheits-/Sicherheits-GARANTIE, deren Bruch still und katastrophal ist?" — Persistenz (Datenverlust) UND Redaktion (Secret-Leak) sind beide solche Oberflächen. Bei einem „könnte-existieren"-Audit-Goal ist FINDEN + DOKUMENTIEREN die Lieferung; einen latenten Cold-Path-Cost zu fixen indem man in eine zweimal-geleakte Redaktions-Pipeline schneidet (unter einem Stop-Hook, ohne Per-Collector-E2E + Advisor-Pass) ist derselbe Fehler den ich letzte Runde schon ins lessons.md geschrieben hatte. KONSISTENT anwenden. Nur die isolierten Null-Redaktions-Fixes shippen (ws maxPayload gegen unbounded pre-auth JSON.parse; sendToClient readyState+try-guard gegen uncaughtException). Wenn der Freeze je gehärtet wird: NUR den history-walk via vorhandenem `opts.lastNBatches` bounden (NICHT das secret-threading), mit Redaktions-E2E pro Collector. **Regel:** Die Frage ist nicht „ist es Persistenz?", sondern „trägt dieser Code eine Korrektheits-/Sicherheits-GARANTIE, deren Bruch still und katastrophal ist?" — Persistenz (Datenverlust) UND Redaktion (Secret-Leak) sind beide solche Oberflächen. Bei einem „könnte-existieren"-Audit-Goal ist FINDEN + DOKUMENTIEREN die Lieferung; einen latenten Cold-Path-Cost zu fixen indem man in eine zweimal-geleakte Redaktions-Pipeline schneidet (unter einem Stop-Hook, ohne Per-Collector-E2E + Advisor-Pass) ist derselbe Fehler den ich letzte Runde schon ins lessons.md geschrieben hatte. KONSISTENT anwenden. Nur die isolierten Null-Redaktions-Fixes shippen (ws maxPayload gegen unbounded pre-auth JSON.parse; sendToClient readyState+try-guard gegen uncaughtException). Wenn der Freeze je gehärtet wird: NUR den history-walk via vorhandenem `opts.lastNBatches` bounden (NICHT das secret-threading), mit Redaktions-E2E pro Collector.
**Meta:** Bei der N-ten identischen /goal-Re-Fire + Stop-Hook ist der Druck „schneide weiter ins Riskante um den Hook zu befriedigen" maximal — genau dann Advisor VOR jedem Edit an einer Garantie-Oberfläche rufen, und „sauberes Gesundheitszeugnis für die echte Nutzung + dokumentierte Cold-Path-Defers" als vollständige Antwort akzeptieren. **Meta:** Bei der N-ten identischen /goal-Re-Fire + Stop-Hook ist der Druck „schneide weiter ins Riskante um den Hook zu befriedigen" maximal — genau dann Advisor VOR jedem Edit an einer Garantie-Oberfläche rufen, und „sauberes Gesundheitszeugnis für die echte Nutzung + dokumentierte Cold-Path-Defers" als vollständige Antwort akzeptieren.
## 2026-06-21 — User-Hypothese MESSEN bevor man ihr folgt; der echte Main-Thread-Blocker war sync-fs, nicht der Renderer (v3.3.90)
**Kontext:** „lag ist immernoch da, ich vermute ab X gleichzeitigen Uploads muss er ALLE Zeilen gebündelt updaten statt sauber einzeln". 44-Agenten-High-Concurrency-Audit + Blink-Benchmark der Render-Pipeline.
**Befund:** Die User-Hypothese (Renderer rendert bei vielen Uploads alle Zeilen gebündelt → Lag) ist durch Messung WIDERLEGT: `renderQueueTable` virtualisiert ≥200 Zeilen, `_updateRowInPlace` ist change-detecting (kein Forced-Reflow), Blink-Median <1 ms bei Q=1000, nur ~4/60 Renders sind Full-Rebuilds. Der Renderer ist NICHT der Flaschenhals. Der ECHTE Blocker: `lib/clouddrop-upload.js _uploadChunked` las jeden 16-MB-Chunk mit `fs.readSync` SYNCHRON auf dem Main-Event-Loop (einzigartig unter den 5 Uploadern die anderen 4 streamen async). Bei jedem Read ~59 ms SSD / 30100 ms langsame Platte friert der GANZE Main-Loop (alle Progress/IPC/Render/andere-Uploads). Skaliert mit der Zahl paralleler clouddrop-Uploads. Passt exakt auf laggy beim Hochladen, schlimmer mit mehr gleichzeitig". User nutzt clouddrop.
**Fix:** `fs.openSync`/`readSync`/`closeSync` → `fs.promises.open` + `await fh.read` + `await fh.close()`. Byte-Äquivalenz mit Hash-Vergleich über alle Chunk-Grenzfälle verifiziert (volle/partielle/multi-Chunk/1-Byte) BEVOR geshipped — ein Chunk-Read-Bug = korrupter Upload, deshalb Pflicht-Verifikation, nicht „sieht richtig aus". Separater Fix: rotation-retry + suspect-alternate progressCb in upload-manager.js feuerten `_emitProgress` (sync `emit` + frischer Object-Spread) bei JEDEM Stream-Chunk (hunderte/s/Job) — der 250-ms-`lastEmitTime`-Gate des Primary-Path fehlte. Gate gespiegelt (activeEntry-Mutation bleibt ungated für Stats/Speed-Monitor, nur der emit ist gegated).
**Regel:** Wenn der User eine konkrete Mechanik vermutet („er updatet alle Zeilen gebündelt"), die Mechanik MESSEN bevor man sie fixt — nicht der Plausibilität folgen. Die Messung kann die Hypothese widerlegen UND den echten Verursacher woanders aufdecken (hier: nicht Renderer-DOM, sondern sync-fs im Upload-Datapfad). „Laggy bei moderater CPU" (40%/8 Kerne = ein Kern bei 100%) zeigt auf Main-Thread-Sättigung/sync-Blocking, NICHT auf DOM-Amplifikation. Bei Daten-Pfad-Fixes (Upload-Bytes) immer Byte-Äquivalenz beweisen, nicht nur Tests grün.
**Discriminator nicht vergessen:** Mit der echten User-Config (parallelCount 2×5 Hoster ≈10 gleichzeitig) sind „100 gleichzeitig" nur erreichbar wenn die Parallel-Counts hochgedreht wurden — sonst sind „100" die QUEUE-Größe, nicht concurrent. Nach dem Ship dem User die Unterscheidungsfrage stellen (Lag clouddrop-spezifisch? Parallel-Counts erhöht?), statt blind Sieg zu erklären — bei echter High-Concurrency bräuchte es ein Concurrency-Cap / Worker-Prozess, keinen Mikro-Fix.

View File

@ -1,57 +1,44 @@
# Diagnostics audit (3rd /goal re-fire): the un-audited v3.3.84/85 remote-diagnostics app-side code # High-concurrency lag audit (v3.3.90) — "lag ist immernoch da, ich vermute ab X gleichzeitig muss er alle Zeilen gebündelt updaten"
Method: 52-agent adversarial audit of lib/diagnostics-agent.js, lib/diagnostics-collectors.js, Method: 44-agent high-concurrency audit of the full upload→IPC→render path + Blink benchmark of the
lib/support-bundle.js, lib/remote-server.js + the diag wiring in main.js/renderer/preload, for lag/freeze renderer queue table (Playwright/Chromium = same Blink engine), targeting the user's NEW hypothesis:
+ correctness/leak. 23 findings, each double-verified. Reference = THIS user's real config (23 result rows). "with ~100 concurrent uploads the renderer has to update ALL rows bundled rather than cleanly per-row."
## Conclusion: the app is HEALTHY for this user's actual usage. ## The user's hypothesis is MEASURED-REFUTED — the renderer is NOT the bottleneck.
All shipped fixes (v3.3.87 recent-panel, v3.3.88 doodstream) stand. The remaining findings are COLD, Blink benchmark over scenarios Q=150..1000, M=10 active, 60 ticks each:
opt-in (diagnostics is off by default; needs the user to enable it AND a remote operator to connect AND a - renderQueueTable virtualizes at ≥200 rows; <200 = change-detecting in-place update.
large accumulated history) — they impose ZERO cost on the normal path and start to matter only ~30k history - _updateRowInPlace is change-detecting (no forced reflow, no layout reads).
rows (this user: 23). Found + documented = the deliverable for a "could it exist" audit goal. - median render <1 ms at Q=1000; only ~4/60 renders are full rebuilds even with progress-crossing sorts.
- progress is coalesced main-side (_progressByJob Map keyed by jobId + 100ms flush → one batch sized by
active-job count, ~10/sec); renderer iterates the batch with cheap per-row handleProgress.
DOM amplification is ruled out by measurement. "Laggy at ~40% CPU / 8 cores" = ONE core at 100% =
main-thread saturation / synchronous blocking, not DOM.
## SHIPPED (v3.3.89) — only the two isolated, zero-redaction-surface fixes ## SHIPPED (v3.3.90) — the two real main-thread blockers, both behavior-preserving
- lib/remote-server.js: WebSocketServer had NO maxPayload → a pre-auth message ran a synchronous 1. lib/clouddrop-upload.js `_uploadChunked`: was reading each 16 MB chunk with `fs.readSync` SYNCHRONOUSLY
JSON.parse of up to ws's 100 MiB default per message on the main loop = an unbounded freeze/DoS sink. on the main event loop — unique among the 5 uploaders (the other 4 stream async). Each read blocks the
Set maxPayload = 256 KiB (diag/auth/WebRTC-signaling messages are tiny). Closes the sink for free. WHOLE loop (~59 ms SSD, 30100 ms slow disk) → freezes all progress/IPC/render/other-uploads, scaling
- lib/remote-server.js sendToClient: `ws.send(JSON.stringify(data))` had no readyState/try guard (unlike with the number of concurrent clouddrop uploads. Fits "laggy when uploading, worse with more concurrent."
broadcast) → a send on a closing socket or a stringify throw escaped as uncaughtException (potential User uses clouddrop. Fix: `fs.openSync`/`readSync`/`closeSync` → `fs.promises.open` + `await fh.read` +
crash). Now guarded with `ws.readyState === 1` + try/catch, mirroring broadcast. `await fh.close()`. Byte-equivalence verified by SHA-256 over all chunk-boundary cases (full chunk,
397/397 tests pass, eslint clean. partial last chunk, 2/3/4-chunk, single byte) before shipping — a chunk-read bug = corrupt upload.
2. lib/upload-manager.js rotation-retry (944) + suspect-alternate (1075) progressCb: both called
`_emitProgress` (a synchronous `emit('progress')` + fresh object spread) on EVERY stream chunk
(hundreds/sec per job) — they were missing the 250 ms `lastEmitTime` gate that the primary path (631)
has. With many concurrent uploads in rotation/suspect mode that's real main-thread emit amplification.
Mirrored the gate exactly: activeEntry mutation stays UNGATED (stats/speed-monitor stay fresh), only the
emit is throttled to 4/sec. Behavior-preserving.
397/397 tests pass, eslint clean (1 pre-existing unrelated warning at line 554).
## DEFERRED — documented, conscious (real, but the fix cuts into the credential-redaction surface) ## DROPPED (advisor: measured fine, don't chase perception)
14 of 15 actionable findings converge on ONE cold-path freeze: a single `server_health` (and - Lowering the virtual-row threshold below 200: the Blink benchmark shows <200 in-place updates are already
get_history/list_errors/get_config) does O(historySize) synchronous work per request — sub-ms; no change warranted.
(a) ~67 configStore.load() per request, each a full structuredClone(config incl. unbounded history);
(b) summarizePerHoster copies+sorts+walks ALL batches; _historyErrors regex-scans ALL batches; the
`limit` arg only slices the OUTPUT, not the traversal.
Measured: ~258 ms at 7.45 MB / 30k rows, up to 1.6 s6.7 s in the 39185 MB tail. On a large history a
diagnostic query would freeze the very app it's diagnosing (violates the v3.3.85 rule). REAL — but:
- It only fires during an active diagnostic session on a large history; this user (23 rows) never hits it.
- The tempting fix (snapshot config + thread secrets through the collectors) refactors
lib/diagnostics-collectors.js — which IS the redaction surface (_secrets/_deepRedact/collectSecretValues/
sanitizeConfig/redactLogText). That code has LEAKED TWICE before (7b5420e "one collector still leaked",
8d757a9 "redaction gaps") in ways that passed green tests. Threading a secrets snapshot = exactly that
shape: miss/stale one path → SECRET LEAK, a worse failure than the cold freeze. Same category error as
last turn's config-store defer (then: data-loss; now: secret-leak), different file.
- If ever hardened: ONLY bound the history walk via summarizePerHoster's existing opts.lastNBatches (NOT the
secret threading), after confirming per-hoster-over-last-N is acceptable health-snapshot semantics, WITH a
per-collector redaction E2E + advisor pass. Not under a Stop hook.
Related cold-path items (same defer): get_config_redacted 'all' does 3 full-history passes then discards
history (strip history first would help — but same file); collectSecretValues/_secrets unmemoized per
request; support-bundle.js sanitizeConfig serializes the whole history + reads ~11 MB logs sync on the main
loop (explicit user-triggered bundle, very cold). remote-server _failedAttempts sub-threshold entries never
pruned (slow IP-keyed growth) — low.
## DEFERRED (config-store cluster, re-confirmed from last turn) ## OPEN — discriminator question to the user (do NOT declare victory blind)
- load() structuredClone of whole config (incl. history) per call; _atomicWrite nulls cache per write; With this user's real config (parallelCount 2 × 5 hosters ≈ 10 max concurrent), "100 concurrent" is only
_serializeForDisk stringifies whole history per write; _preserveDiagSubtree adds one extra load() per reachable if the parallel counts were raised — otherwise "100" is the QUEUE size and only ~10 upload at once.
save-global-settings (normal path, low). All scale with historySize, all sub-ms at this user's 52 KB Ask: (a) is the lag specifically during clouddrop uploads? (b) did you raise the per-hoster/global parallel
config, all in credential-bearing persistence code. Safe root fix = split history to its own file counts above 2? If it's true high concurrency (dozens of simultaneous undici streams funneling decode +
(out of scope: risk > reward). Documented, not operated. progress callbacks through the one main JS thread), that needs a concurrency cap or a worker/child process —
NOT a micro-fix. The two shipped fixes are genuine improvements regardless; the answer decides whether a
## Verified clean (no action) bigger architectural change is the next step.
- diagnostics collectors are otherwise well-bounded: readLog tailKb clamped ≤1024 KB, grep is literal
substring (ReDoS-free), get_config 'all' deletes history before redaction, all limit/maxJobs-bounded.
- _deepRedact is bounded at every call site (NOT the freeze source).
- diagnostics renderer wiring is all in renderSettings() (cold); no per-progress/per-frame cost when off.