# Code Audit Living checklist of audit findings for AeroFetch. Security & correctness were reviewed 2026-06-23 (all fixed — see Completed). The architectural review (2026-06-29) added the structural items; a second polish pass (2026-06-29) added the smaller inconsistencies. Items carry stable IDs so we can check them off as they land this session. Severity = structural leverage / risk of future bugs, not "app is broken" (it isn't). --- # 1.0 Release-Readiness Audit (lead-engineer synthesis) *A pre-release synthesis of the ~385 catalogued findings below — root causes, a release gate, and the consequential issues per dimension (severity · reasoning · fix · benefit). The detailed, ID'd backlog follows. No rewrite required; everything here is incremental.* ## Verdict **Not 1.0-ready as-is, but close — roughly 3–4 focused PRs away.** The foundation is genuinely strong: disciplined security model (context isolation, sandbox, argv-injection defense, host-pinned signed-checksum updater), a clean pure/impure architecture, and good unit coverage of the pure logic. What blocks a *commercial* 1.0 is a thin layer of (a) real reliability bugs, (b) naive persistence, (c) built-but-unwired features, (d) first-run/perceived-quality gaps, and (e) the absence of enforcement/observability tooling. None require rearchitecting. ## Root-cause themes (the 385 findings collapse to 8) 1. **Built-but-unwired features** — command preview (M5), incognito (M6), per-download options/extraArgs (UX1) are plumbed end-to-end with no UI; the ROADMAP even marks them ✅ (M25). *Decide wire-or-cut.* 2. **"Two ways to do X" with no enforcement** — persistence (M1), validation (CC9), async (CC5), errors (CC6), UI primitives (UI14/UI18/UI19), formatters (M9) — and no ESLint/Prettier to hold any line (CC2). 3. **Renderer-optimistic state never reconciles + a store cycle** — M34/L142 (settings show unsaved values), C2 (downloads↔sources circular import). 4. **Naive JSON persistence** — non-atomic writes (R1), corrupt→silent total data loss (R2), O(n²) full-file rewrites per completion (R3/PERF7), no cache. 5. **Download-engine reliability gaps** — no stall timeout (B1), reverse-order batches (M32), orphan `.part` on cancel (R4), retry-during-teardown race (L140). 6. **First-run / perceived quality** — light-default theme (SR1), nightly-default yt-dlp (SR2), placeholder icon (W14), stuck "Resolving…" (SR6), progress bar visibly restarting (SR7), unsigned build (SIGNING.md). 7. **Accessibility & Windows-native feel** — focus rings (UI28/29), no list keyboard nav (W7), title-bar theme (W3), text-field context menu (W4), no `aria-live` (W17), input labels (M28). 8. **No enforcement / observability** — no lint (CC2), `noImplicitAny:false` (M38), no logging (CC8/M29), no production source maps (L170). ## Release gate **MUST fix before 1.0 (blockers)** — correctness, data safety, security, trust: B1 · M32 · M35 · M34 · R1 · R2 · R3 · H7 · H8 · code-signing · SR1 · SR6 · SR7 · W14. **SHOULD fix for 1.0 (high):** wire-or-cut the dead features (M5/M6/UX1) · a11y cluster (UI28/29, W4, W7, W17, M28) · W3 native theming · lint + `noImplicitAny` (CC2/M38) · dev-jargon copy (M37/SR9) · destructive confirmations (UX4) · PERF1/PERF2 (per-download redundant work). **DEFER to 1.x:** the ~150 Low items · the simplification refactors (SIMP*, do incrementally) · the UI token system + shared primitives (UI/SIMP — high value, larger effort) · i18n · sqlite migration. ## By dimension (severity · reasoning · fix · benefit) ### Reliability - **[Critical] No download stall timeout (B1).** *Reasoning:* `spawn` has no timeout + no `--socket-timeout`; a dead connection hangs forever and permanently consumes a concurrency slot. *Fix:* `--socket-timeout` + an idle watchdog that kills+errors via the existing `killTree`. *Benefit:* downloads self-recover on flaky networks. - **[Critical] Non-atomic writes + silent corruption→data-loss (R1/R2).** *Reasoning:* `writeFileSync` isn't atomic; a crash mid-write corrupts the store and the next read returns `[]`, silently wiping history/ sources. *Fix:* atomic write (temp+rename) + back up a corrupt file before resetting, in one `jsonStore`. *Benefit:* user data survives crashes; no silent loss. - **[High] Reverse-order batch downloads (M32)** & **history duplicates (M35).** *Reasoning:* `addMany`+`pump` promote highest-index first; `redownload` mints a new id defeating dedup. *Fix:* reverse the batch enqueue; dedup history by URL. *Benefit:* playlists download 1→N; clean history. - **[High] Optimistic state never reconciles (M34).** *Reasoning:* renderer keeps a value main rejected. *Fix:* apply the validated `Settings` the IPC returns. *Benefit:* the UI never lies about what's saved. ### Performance - **[High] O(n²) media-items rewrite (R3/PERF7).** *Reasoning:* every completion re-reads+rewrites the whole (≤20k-item) file synchronously. *Fix:* in-memory cache + batched atomic writes (the same `jsonStore`). *Benefit:* channel downloads stop hitching the main process. - **[Medium] Per-download redundant work (PERF1/PERF2).** *Reasoning:* `templates.json` read + settings decrypt on every spawn even when unused. *Fix:* gate `listTemplates()` on the consent flag; cache decrypted settings. *Benefit:* lower latency/IO per download. - **[Low] `summarizeQueue`/`pump` O(n) per tick (PERF3/PERF4).** *Fix:* memoize + count-based pump. *Benefit:* scales to large queues. ### Maintainability - **[High] Triplicated Settings/`Api` + the preview mock (C1).** *Reasoning:* a 4th touch-point per IPC method; drift-prone. *Fix:* `DEFAULT_SETTINGS` in `@shared`; one typed `mockApi`. *Benefit:* one place to change. - **[High] Store circular dependency (C2).** *Fix:* a coordinator/event bus. *Benefit:* removes a latent init crash. - **[Medium] Duplicated persistence/validation/async/formatters (M1/CC9/CC5/M9).** *Fix:* the `lib/` helpers in SIMP1–SIMP5. *Benefit:* ~−500 LOC and fewer divergence bugs. - **[Medium] God files (H1).** *Fix:* SettingsView via ``/`` (SIMP10). *Benefit:* 1104→~600 lines, reviewable. ### Consistency - **[Medium] Two ways to do X** across persistence, status chips, segmented controls, buttons, errors (M1/UI18/UI14/UI15/CC6). *Fix:* one shared primitive/standard each (see "Recommended single style"). *Benefit:* the app reads as one product. - **[Low] Naming/copy drift** (CC1, L95/L150, separators L164). *Fix:* the conventions in CC1. *Benefit:* professional finish. ### Readability - **[Medium] Magic strings/numbers + long signatures + deep nesting (CL1/CL2/CL4, L10).** *Fix:* `constants.ts`, options-objects, extracted helpers. *Benefit:* faster comprehension, fewer transpose bugs (CL2). ### Polish (perceived quality) - **[High] First-run defaults (SR1/SR2/SR3).** *Reasoning:* light-on-dark first launch, nightly yt-dlp by default, auto-download-new on. *Fix:* `theme:'system'`, `ytdlpChannel:'stable'`, `autoDownloadNew:false`. *Benefit:* the first 10 seconds feel native and safe. - **[High] Stuck "Resolving…" (SR6) & restarting progress bar (SR7).** *Fix:* clear placeholder on error; weight the two merge phases. *Benefit:* nothing looks hung/glitchy. - **[High] Placeholder icon + unsigned build (W14, SIGNING.md).** *Fix:* designed icon; purchase + wire a cert (already env-ready). *Benefit:* no SmartScreen scare; brand credibility. ### User experience - **[High] Per-download options are unreachable (UX1).** *Reasoning:* must change global settings to tweak one download. *Fix:* wire the existing `DownloadOptionsForm` into the bar (plumbing exists). *Benefit:* reclaims a whole feature. - **[High] Destructive actions have no confirmation/undo (UX4).** *Fix:* confirm Clear/Remove/Delete. *Benefit:* prevents data loss. - **[Medium] Silent failures on Open/Show (UX6)** and **no global download status off the Downloads tab (UX9).** *Fix:* surface errors; sidebar badge. *Benefit:* the app feels responsive and honest. ### Accessibility & Windows-native (UX-critical for a Windows product) - **[High] Fragmented/invisible focus + no list keyboard nav (UI28/29, W7).** *Fix:* one focus-ring + roving list focus + Delete/Ctrl+A. *Benefit:* keyboard- and Narrator-usable. - **[Medium] Title bar ignores in-app theme (W3); text fields have no context menu (W4); no `aria-live` (W17); inputs lack accessible names (M28).** *Fix:* sync `themeSource`; add an editing context menu; live regions; `aria-label`s. *Benefit:* feels like a native, accessible Windows app. ### Developer experience - **[High] No enforcement tooling (CC2) + `noImplicitAny:false` (M38).** *Reasoning:* style/type safety rely on discipline; implicit `any` is allowed. *Fix:* Prettier + typescript-eslint + CI; re-enable `noImplicitAny`. *Benefit:* the conventions stay true; regressions caught pre-merge. - **[Medium] No logging + no prod source maps (CC8/M29/L170).** *Fix:* one leveled file logger at every catch; hidden source maps. *Benefit:* field crashes become diagnosable instead of invisible. - **[Low] Stale roadmap/docs (M25/M26, L80–L82); release checksum is manual (H8).** *Fix:* reconcile docs; generate `.sha256` in `build:win`. *Benefit:* trustworthy docs; updates don't silently fail to install. ## Suggested PR sequence to 1.0 1. **Correctness & data safety:** B1, M32, M35, M34, and the cached/atomic `jsonStore` (R1/R2/R3/PERF7). 2. **Security & trust:** H7 (encrypt cookies), H8 (auto-generate checksums), code-signing. 3. **First-run polish:** SR1/SR2/SR3 defaults, SR6/SR7 status, W14 icon, W3 title-bar theme. 4. **Wire-or-cut + a11y + tooling:** UX1 (per-download panel) or remove M5/M6; focus/keyboard (UI28/29, W7, W4); Prettier+ESLint+`noImplicitAny` (CC2/M38). 5. **Incremental cleanup (post-1.0):** the SIMP refactors and the Low/UI/UX long tail, behind the new lint gate. --- ## Critical - [ ] **C1 — Single source of truth for the IPC mock + Settings defaults.** `const PREVIEW` redeclared in 8 files; `main.tsx` reimplements the entire `Api` (~180 lines); the full `Settings` object is hand-maintained in 3 places (`main/settings.ts` `DEFAULTS`, `renderer/store/settings.ts` `FALLBACK`, `main.tsx` `MOCK_SETTINGS`). Add `DEFAULT_SETTINGS` to `shared/ipc.ts`, extract the preview mock into one `mockApi.ts` typed as `Api`, centralize `isPreview`. - [ ] **C2 — Break the `downloads ↔ sources` store circular dependency.** `downloads.ts` imports `useSources`; `sources.ts` imports `useDownloads`. Works only via lazy `.getState()`. Introduce a coordinator/event bus that owns cross-store reactions. ## High - [ ] **H1 — Decompose god files.** `SettingsView.tsx` (1104, ~11 cards, ~30 selector subs + ~20 useState), `DownloadBar.tsx` (858, ~17 state hooks), `store/downloads.ts` (614). - [ ] **H2 — Consolidate scattered utilities.** `youtubeId` ×2; 4 YouTube-URL-parsing variants; byte/speed/eta/duration formatting across 4 modules. - [ ] **H3 — Fix `probe.ts → download.ts` dependency direction** (`fmtBytes` import); resolves with H2's shared formatter. - [ ] **H4 — Carry raw numbers across the progress boundary.** `DownloadProgress` ships formatted `speed`/`eta` strings; `queueStats.ts` re-parses them back to numbers. - [ ] **H5 — History re-download silently changes quality.** `HistoryView.redownload` passes the stored `quality` (for format-picker downloads this is a *label* like "720p · mp4 · 184 MB") back as `quality` with no `formatId`; `buildArgs.videoFormat()` has no matching case and falls back to `bv*+ba/b` (Best). The queued item shows "720p…" while actually fetching Best. - [ ] **H6 — TerminalView log grows unbounded.** `setLines((ls) => [...ls, …])` with no cap; a verbose run (`-F`, `--verbose`) streams thousands of lines into React state. Every other log/list in the app is capped — cap this too (and/or virtualize). - [ ] **H7 — `cookies.txt` written in plaintext with no restrictive perms.** [cookies.ts](src/main/cookies.ts) `writeFileSync(getCookiesFilePath(), …)` stores live auth/session cookies unencrypted under `userData`. In the **portable build** that's `AeroFetch-data/` next to the exe (USB stick / shared Downloads folder), so anyone with folder access can read a logged-in session. Settings secrets are DPAPI-encrypted (settings.ts) but cookies are not. Encrypt at rest or document the exposure for the portable/shared-PC scenario the app explicitly targets. - [ ] **H8 — Release checksum is a manual step the updater hard-requires.** [updater.ts](src/main/updater.ts) sets `REQUIRE_CHECKSUM = true` and refuses any update lacking a `.sha256`, but `build:win` (`electron-vite build && electron-builder --win`) never generates one. Evidence in `dist/`: only `0.4.1` has `.sha256` files (hand-made); the current `0.5.0` build does not. If a release is published without manually adding the checksum, **every client's in-app update fails** ("This release has no checksum … refusing to install"). Generate the `.sha256` in the build/release script. ## Medium - [ ] **M1 — Unify JSON persistence.** `sources.ts` has generic `readJsonArray`/`writeJson`; `history.ts`/`errorlog.ts`/`templates.ts` reimplement it inline. - [ ] **M2 — Remove dead code: `getDefaultFolder` / `download:default-folder`** (channel + preload + handler + mock, zero callers). - [ ] **M3 — Remove duplicated completion side-effect in `store/downloads.ts`** (history write + `markDownloaded` in both `applyEvent('done')` and the preview ticker). Subsumed by C2. - [ ] **M4 — Decide queue persistence explicitly.** Scheduler/queue is renderer-memory only; `saved`/scheduled items don't survive a quit. Persist or document as a non-goal in code. - [ ] **M5 — Command-preview feature is fully built but unwired.** `CommandPreviewResult` type + `command:preview` channel + `previewCommand` (download.ts) + `formatCommandLine`/ `quoteForDisplay` (buildArgs) + preload method all exist, but no renderer component calls `window.api.previewCommand` (only the `main.tsx` mock references it). Wire it into the DownloadBar Options panel, or remove the dead chain. - [ ] **M6 — Private/incognito mode is plumbed but unreachable.** `incognito` flows through `AddOptions` → `buildItem` → history-skip → the QueueItem "Private" badge, but nothing in the UI ever sets `incognito: true`. Add the toggle or remove the dead plumbing. - [ ] **M7 — `newId()` duplicated 3×** (`store/downloads.ts`, `TerminalView.tsx`, `TemplateManager.tsx`) with divergent fallback prefixes (`item-`/`t-`/`tpl-`). Extract one helper. - [ ] **M8 — Two download-status→label maps.** `STATUS_BADGE` (QueueItem) and `STATUS_LABEL` (LibraryView) independently map the same statuses (completed = "Completed" vs "Downloaded"). One shared map. - [ ] **M9 — Three time formatters.** `relTime` (LibraryView), `formatWhen` (HistoryView), `fmtSchedule` (QueueItem) — consolidate into a date util. - [ ] **M10 — `.url` shortcut parsing duplicated** — `parseUrlFile` (DownloadBar) vs `readUrlShortcut` (main/deeplink). Different `URL=` extractors for the same file format. - [ ] **M11 — Inconsistent clipboard access.** Reads use `navigator.clipboard.readText` (paste button) *and* `window.api.readClipboard` (suggestion watcher); writes use `navigator.clipboard.writeText` (copy report). Pick one strategy. - [ ] **M12 — Shared `errorText` style.** `tokens.colorPaletteRedForeground1` is applied inline 12× across 5 files instead of one class. - [ ] **M13 — Inconsistent secret-field masking.** `updateToken` uses `type="password"`; `proxy` (may carry `user:pass@`) and `youtubePoToken` (a token) are plain-text Inputs. - [ ] **M14 — Settings search mutates React-owned DOM.** The search toggles each card's `el.style.display` directly; fragile (breaks if a card ever gets a conditional `style`) and matches on `textContent` incl. hidden text. Prefer state-driven filtering. - [ ] **M15 — Nested interactive controls in `role="button"` (a11y).** LibraryView's group header is a `role="button"` div containing `