6b0c515a9b
Follow-up hardening on the v3.3.80 queue-persistence fix after a deeper adversarial sweep + reviewer pass over the WHOLE subsystem (not just the diff). Two real defects: 1. Basename-collision lost work (regression introduced by v3.3.80 FIX A). restoreQueueStateFromConfig collapses jobs on the FULL path while the ts-gate keys on basename|hoster. Two genuinely different files with the same basename queued to the same hoster from different folders therefore share a gate key: if one was logged after savedAt, the ts-rule dropped BOTH — silently losing the still-pending one. Pre-FIX-A only 'done' jobs were dropped, so a pending file was never at risk. Fix: an ambiguity guard in partitionRestoredJobsByLog — the ts-rule is suppressed when a basename|hoster key maps to more than one distinct file path (the log records only basenames, so it can't say which physical file completed). The done-in-log rule is unchanged. Fails safe: worst case a visible ghost survives, never silent data loss. 2. selectedFiles re-materialization with removeFromQueueOnDone=ON (second mechanism, independent of the stale snapshot). When that setting is on, a completed job is stripped from queueJobs but its path stays in selectedFiles (syncSelectedFilesFromQueue only runs at batch-done, never on a mid-upload close). On restart the ts-gate operates on queueJobs and never sees it, then the startup updateUploadView -> buildQueuePreview re-creates it as a preview ghost AFTER the gate ran, and it re-persists with a fresh savedAt — sticky. Fix: completedSelectionKeys() seeds _completedUploadKeys (the set buildQueuePreview already consults) from the log at startup, keyed on full path, with the same ambiguity guard. Log-based so it survives a hard kill, consistent with FIX A. Also extracts the orphan-tmp sweep decision into lib/orphan-tmp.js (was untested inline code in main.js; behavior-preserving) and adds executable coverage for the paths that were previously only argued from logic: orphan-tmp sweep, config-store pendingQueue+savedAt round-trip, an end-to-end scenario in the exact user-reported shape (300 queued / ~200 finished mid-session), and a 3000+500-iteration property fuzz of the gate invariant including the lost-work guarantee. 359/359 green, ESLint clean, smoke-boot unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
88 lines
6.2 KiB
Markdown
88 lines
6.2 KiB
Markdown
# Queue-Persistenz Bug: fertige Dateien tauchen nach Neustart wieder auf
|
||
|
||
## Symptom
|
||
User: 300 Dateien, 100 übrig, Programm schließen + öffnen → manchmal sind bereits
|
||
fertig hochgeladene Dateien wieder in der Liste.
|
||
|
||
## Root Cause (verifiziert im Code)
|
||
- **RC-1 (Persist-Starvation, code-confirmed):** `persistQueueStateSoon()` setzt bei
|
||
jedem Progress-Event den Timer per `clearTimeout` zurück; Delay während Upload war
|
||
10000ms. Progress-Events feuern öfter als alle 10s → Timer feuert NIE während eines
|
||
aktiven Uploads. Der Disk-Snapshot bleibt auf dem Stand VOR Upload-Start stehen
|
||
(alle Jobs `preview`).
|
||
- **RC-2 (unzuverlässiger Close-Flush):** beforeunload-Sync-Flush existiert
|
||
(app.js:4605) und fängt den sauberen Close ab. Bei hartem Kill / Crash / OS-Kill
|
||
läuft er nicht → der stale Snapshot bleibt liegen.
|
||
- **RC-3 (Dedup-Asymmetrie):** `_autoDeduplicateFromLog` droppt beim Start nur Jobs
|
||
mit Status `done`. Die Ghosts aus dem stale Snapshot stehen aber als `preview` da
|
||
→ werden NICHT gedroppt → fertige Dateien erscheinen erneut.
|
||
|
||
## Fix (mechanismus-unabhängig, vom Kern auf)
|
||
- [x] **FIX A — Timestamp-gated Dedup (Kern-Fix, durable):** Beim Start jeden restored
|
||
Job droppen, dessen file+hoster im Log mit `ts >= floor(savedAt)` steht — egal ob
|
||
`preview` oder `done`. Fängt Ghosts auch nach hartem Kill (hängt vom Log ab, nicht
|
||
vom Snapshot). `lib/queue-dedup.js` additiver 3. Param `savedAt`; `buildPersistedQueueState`
|
||
stempelt `savedAt`; `restoreQueueStateFromConfig` merkt `_restoredSnapshotSavedAt`;
|
||
Log-Zeile → `ts` geparst; `_autoDeduplicateFromLog` reicht savedAt durch.
|
||
- [x] **FIX B — Throttle mit max-wait:** `lib/throttle-timer.js` (neu). Upload: delay 500
|
||
+ maxWait 20000 → Snapshot alle ~20s statt nie. Idle: reine Debounce. Fallback-Shim
|
||
honoriert maxWait (kein stilles Starvation-Reintro).
|
||
- [x] **FIX C — Close-Write-Härtung:** `save-global-settings-sync` renameSync-Retry bei
|
||
EBUSY/EPERM/EACCES + pid-unique tmp + tmp-cleanup. Startup-Sweep `_sweepOrphanConfigTmps`
|
||
räumt verwaiste `<config>.<pid>.tmp` toter PIDs (gegen Orphan-Akkumulation).
|
||
- [x] **Seam-Extraktion (Advisor #2):** `lib/upload-log.js` (neu) — `formatUploadLogLine`
|
||
+ `parseUploadLogLine` aus main.js gezogen; Test fährt den ECHTEN Writer→Reader→Gate-
|
||
Vertrag (kein Mirror) → fängt künftige Format-/Epoch-Brüche.
|
||
|
||
## Tests
|
||
- [x] `tests/throttle-timer.test.js`: Starvation ohne maxWait → 0 Fires; mit maxWait →
|
||
periodische Fires; last-write-wins (distinct fn); flushSync/cancel.
|
||
- [x] `tests/queue-dedup.test.js`: ts>=savedAt→DROP; ts<savedAt→KEEP; same-second→DROP;
|
||
max-ts; Multi-Hoster Teilabschluss (reale Bug-Form); ohne savedAt/ohne ts→Legacy.
|
||
- [x] `tests/upload-log.test.js`: realer Writer→Reader-Roundtrip + Seam-Drop/Keep.
|
||
- [x] 334/334 grün, ESLint clean, Smoke-Boot identisch zu Baseline (kein Regress).
|
||
|
||
## Review
|
||
- **Adversariale Multi-Agent-Review (4 Dimensionen, 15 Findings):** 14 refuted (meist
|
||
"ist korrekt"-Bestätigungen, Kommentar-Drift, Test-Härtungs-Vorschläge). 1 confirmed
|
||
(LOW): pid-unique tmp konnte bei Hard-Kill zwischen write und rename verwaisen → mit
|
||
Startup-Sweep behoben. Stale-Kommentare (queue-dedup Header + _autoDeduplicateFromLog)
|
||
auf die Zwei-Regel-Logik korrigiert.
|
||
- **Was bewiesen ist:** Komponenten-Logik (Unit-Tests inkl. realer Format-Seam),
|
||
Code-getraceter Wiring-Pfad, adversariale Gegenprüfung. Der Fix ist
|
||
MECHANISMUS-UNABHÄNGIG: greift egal ob der stale Snapshot von Starvation, einem
|
||
Mid-Upload-Close-Race ODER einem Hard-Kill kommt.
|
||
- **Ehrliche Einschränkung:** KEIN Live-Repro mit echtem byse-Key (Key unter anderem
|
||
Windows-Profil verschlüsselt, nicht entschlüsselbar). Symptom tritt nur auf bei
|
||
Close WÄHREND aktivem Upload oder Hard-Kill — ein sauberer Idle-Close war schon
|
||
vorher korrekt.
|
||
|
||
## Runde 2 — "noch intensiver" (v3.3.81)
|
||
- **Echte ausführbare Tests** für bisher nur logisch abgedeckte Pfade: `tests/orphan-tmp.test.js`
|
||
(Sweep-Entscheidung, extrahiert nach `lib/orphan-tmp.js`), config-store `pendingQueue`+`savedAt`
|
||
Roundtrip, `tests/queue-persistence-scenario.test.js` (exakte 300/100-Bug-Form + multi-hoster),
|
||
`tests/queue-dedup-property.test.js` (3000+500 Fuzz-Iterationen gegen die formale Invariante).
|
||
- **Breite adversariale Bug-Jagd** übers GANZE Subsystem: lief tooling-bedingt teils kaputt
|
||
(bug-analyzer ohne File-Tools, Socket-Fehler) → 3 unverifizierte Hypothesen SELBST am Code
|
||
geprüft:
|
||
- #2 (gedroppte done-Jobs reappear via buildQueuePreview) → REFUTED: `_completedUploadKeys`
|
||
(full path) == buildQueuePreview-Key; nach Gate räumt `syncSelectedFilesFromQueue` selectedFiles.
|
||
- #3 (done-File bleibt in selectedFiles → re-preview) → **REAL (Advisor-Catch) & gefixt.** Meine
|
||
erste Abweisung war zu schnell: `syncSelectedFilesFromQueue` läuft NICHT beim Mid-Upload-Close.
|
||
Bei `removeFromQueueOnDone=ON` werden fertige Jobs aus queueJobs entfernt, bleiben aber in
|
||
selectedFiles; `updateUploadView`→`buildQueuePreview` (Startup, Zeile 990) re-materialisiert sie
|
||
als Preview-Ghost NACH dem Gate → sticky. Fix: `completedSelectionKeys` (queue-dedup.js) seedet
|
||
beim Start `_completedUploadKeys` (full-path, log-basiert/hard-kill-durabel, gleiche Ambiguity-
|
||
Guard) → buildQueuePreview überspringt fertige (file|hoster)-Paare. Nur relevant bei
|
||
removeFromQueueOnDone=ON (Default OFF).
|
||
- #1 (basename-Kollision droppt PENDING Datei = Lost Work) → **REAL & gefixt.** FIX A's ts-Regel
|
||
keyt auf basename, Restore-Collapse auf full path → zwei gleichnamige Dateien aus verschiedenen
|
||
Ordnern an denselben Hoster: die geloggte droppte fälschlich auch die andere PENDING. **Ambiguity-
|
||
Guard** in queue-dedup.js: ts-Regel wird unterdrückt, wenn ein basename|hoster-Key auf mehrere
|
||
DISTINKTE Pfade zeigt (done-Regel unberührt). Fail-safe: schlimmstenfalls überlebt ein
|
||
sichtbarer Ghost, NIE stiller Datenverlust.
|
||
- **Un-gehuntete Bereiche selbst abgeklopft:** DST/Clock-Skew → Fehler nur in SICHERER Richtung
|
||
(Ghost bleibt, kein Lost Work), inhärente Grenze von Sekunden-Lokalzeit-Logs. Log-Discovery
|
||
readdir-Filter `startsWith(base)&&endsWith(ext)` fängt single/daily/session — keine verpassten
|
||
Log-Files. 353/353 grün, ESLint clean, 3× Suite ohne Flake.
|