9abd2c4441
L67: Onboarding now has a 'Skip' button alongside 'Get started', both in a
right-aligned flex row. Both call update({hasCompletedOnboarding:true})
so the behavior is the same — Skip just reads as optional for users who
want to get started immediately without reading the tips.
L86: HistoryView now clears any selected IDs that are no longer visible when
the filter (query or kind) changes. A useEffect on `filtered` intersects
the current selection with the new visible set and calls setSelected only
if the size actually changed (avoids re-renders when the filter doesn't
affect the selection).
typecheck + 242 tests green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1487 lines
121 KiB
Markdown
1487 lines
121 KiB
Markdown
# 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).
|
||
|
||
**Session 2026-06-30 (correctness/safety/strictness pass):** landed B3 B4 B5 B7 (checksum-
|
||
filename match, newline-safe meta probe, canceled-event guards, cookie-login never-resolve),
|
||
L88 L146 (queue/trim edge cases), **L168 + L169** (`noUncheckedIndexedAccess` +
|
||
`noFallthroughCasesInSwitch` now on — 0 errors after 8 real edge-case fixes), R5 (settings
|
||
write failure handled), W1 W5 W6 (min window size, seeded folder picker, parented sign-in
|
||
window), CL1 (shared stdout markers), L147 (dead macOS branches removed), M8 (StatusChip
|
||
already unified the status→label map), M36 (Library selection only counts actionable rows),
|
||
L11 (Queue header counts the live queue), L50 (no "saved" for 0 cookies), L156 + L57
|
||
(schedule picker `min`), L159 L15 L3 (dead-code/comment cleanup), package/builder metadata
|
||
(L4 L41 L42 L43 L45 L58), user-facing copy (L66 L154; partial M37/SR9 jargon), doc
|
||
reconciliation (M25 M26 L80 L81), and new unit tests (L35 isValidMediaItem, L36
|
||
compareVersions). All verified: typecheck + 234 tests + eslint + prettier green.
|
||
|
||
**Session 2026-06-30 pass 2 (quality/format/UX polish):** H5 (history re-download now stores
|
||
formatId/formatHasAudio + strips compound quality labels; thumbnail also passed — L72), H6
|
||
(TerminalView log capped at 2000 lines), M21 (skip --audio-quality for lossless flac/wav),
|
||
L166 (fmtEta hour rollover fixed in download.ts, store/downloads.ts, queueStats.ts — "2:00:00"
|
||
not "120:00"), L165 (formatSpeed precision aligned with fmtBytes), L55 (QueueItem suppresses
|
||
sizeLabel when already in probed-format quality label), L47 (probeMeta null sends empty meta
|
||
event to clear "Resolving…"), L68 (notifiedBackground resets on window show), L76 (dup warning
|
||
falls back to URL when title is placeholder), L91 (SettingsView unsafe tuple cast fixed), L98
|
||
(embed-chapters hint added), L108 (window title set), L37 (formatters extracted to lib/formatters.ts;
|
||
fmtBytes/fmtEta/parseProgress unit-tested including NA paths), L38 (webm thumbnail exclusion
|
||
test), L39 (CROP_SQUARE_PPA structural test), L40 (looksLikeUrl/looksLikeSingleVideo extracted
|
||
to lib/urlHelpers.ts; test imports pure module directly). All verified: typecheck + 242 tests + eslint + prettier green.
|
||
|
||
**Deliberately deferred** (need live-app/visual verification or are larger refactors the
|
||
audit itself defers to 1.x): the wire-or-cut features (M5/M6/UX1), the god-file/store
|
||
refactors (C1, C2, H1), the SIMP* helpers + shared-UI-token work, the CC* consolidations,
|
||
PERF8 code-splitting, and the visual UI/UX polish long-tail. These are best done as their
|
||
own PRs with the app running so the UI can actually be checked.
|
||
|
||
---
|
||
|
||
# 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 · SR1 · SR6 · SR7 · W14 — **all resolved as of this
|
||
session.** Code signing was dropped from the gate: it's a deliberate non-goal (no certificate will be
|
||
purchased; the SmartScreen prompt on the unsigned build is accepted).
|
||
|
||
**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 `<SettingsCard>`/`<ToggleField>` (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 (W14).** *Fixed:* a designed mark (teal gradient square + top sheen, bold
|
||
rounded download glyph over a landing shelf) replaces the flat placeholder; multi-size `.ico` regenerated.
|
||
*Benefit:* brand credibility. *(Unsigned build is a non-goal — no cert; the SmartScreen prompt is accepted.)*
|
||
|
||
### 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 is a non-goal — no cert.)*
|
||
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.
|
||
- [x] **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.
|
||
- [x] **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.
|
||
- [x] **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).
|
||
- [x] **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.
|
||
- [x] **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 `<asset>.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
|
||
|
||
- [x] **M1 — Unify JSON persistence.** `sources.ts` has generic `readJsonArray`/`writeJson`;
|
||
`history.ts`/`errorlog.ts`/`templates.ts` reimplement it inline.
|
||
- [x] **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.
|
||
- [x] **M7 — `newId()` duplicated 3×** (`store/downloads.ts`, `TerminalView.tsx`,
|
||
`TemplateManager.tsx`) with divergent fallback prefixes (`item-`/`t-`/`tpl-`). Extract one helper.
|
||
*Fixed: one [`newId(prefix)`](src/renderer/src/id.ts) helper; the three copies import it and pass their
|
||
prefix. The fallback gained a monotonic counter so same-millisecond ids can't collide (subsumes L33/L70).
|
||
Unit-tested in `test/id.test.ts`.*
|
||
- [x] **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.
|
||
- [x] **M9 — Three time formatters.** `relTime` (LibraryView), `formatWhen` (HistoryView),
|
||
`fmtSchedule` (QueueItem) — consolidate into a date util. *Fixed: all three moved to
|
||
[`datetime.ts`](src/renderer/src/datetime.ts) and imported by their views; unit-tested in `test/datetime.test.ts`.*
|
||
- [x] **M10 — `.url` shortcut parsing duplicated** — `parseUrlFile` (DownloadBar) vs
|
||
`readUrlShortcut` (main/deeplink). Different `URL=` extractors for the same file format.
|
||
- [x] **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.
|
||
- [x] **M12 — Shared `errorText` style.** `tokens.colorPaletteRedForeground1` is applied inline
|
||
12× across 5 files instead of one class. *Partially fixed: added
|
||
[`useErrorTextStyles`](src/renderer/src/components/ui/errorText.ts) (`error` / `errorPre` for pre-wrap
|
||
multi-line output) and adopted it in `SettingsView.tsx` — the biggest offender — replacing all 6 inline
|
||
`style={{ color: … }}` error spans + its local `errorRowText` class. The remaining red references live in
|
||
per-component `makeStyles` classes (DownloadBar/LibraryView/QueueItem/TerminalView), not inline; migrate
|
||
those to the shared hook incrementally.*
|
||
- [x] **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.
|
||
- [x] **M15 — Nested interactive controls in `role="button"` (a11y).** LibraryView's group header
|
||
is a `role="button"` div containing `<Button>`s (All / Download) — invalid ARIA / keyboard
|
||
semantics. Make the header a real element with sibling buttons.
|
||
- [x] **M16 — No renderer error boundary.** An exception in any view unmounts the whole shell to
|
||
a blank window. Add a top-level boundary with a recover/reload affordance. *Fixed: added
|
||
[`ErrorBoundary`](src/renderer/src/components/ErrorBoundary.tsx) wrapping `<App/>` in `main.tsx`. The
|
||
fallback is dependency-free (no Fluent/theme provider — the crash may have come from that tree), logs via
|
||
`componentDidCatch`, shows the error message, and offers a Reload button (`window.location.reload()`;
|
||
persisted data lives in main, so nothing is lost).*
|
||
- [ ] **M17 — `statusByUrl` collapses duplicate URLs.** LibraryView maps URL→status into one
|
||
`Map`; with "Download anyway" duplicates, a row can reflect the wrong item's state.
|
||
- [ ] **M18 — Audio "quality" presets hard-code MP3.** `QUALITY_OPTIONS.audio` labels ("Best
|
||
(MP3)", "320 kbps") are shown even when `downloadOptions.audioFormat` is opus/flac/wav, so the
|
||
label contradicts the actual output format; `--audio-quality` is also meaningless for lossless.
|
||
- [x] **M19 — Sidebar-collapsed pref uses `localStorage`** while every other preference uses
|
||
electron-store — won't back up/restore and is lost in the portable build across machines.
|
||
- [x] **M20 — ProgressBar / native `<select>` accessibility.** QueueItem/DownloadsView
|
||
`ProgressBar`s have no accessible name; several `Select`s carry both a `<Field label>` and an
|
||
`aria-label` (double announcement).
|
||
- [x] **M21 — `--audio-quality` always emitted.** `buildArgs` passes `--audio-quality` even for
|
||
lossless `audioFormat` (flac/wav), where it's meaningless; pair with M18's preset/format mismatch.
|
||
- [x] **M22 — Backup export writes secrets in cleartext, silently.** [backup.ts](src/main/backup.ts)
|
||
`exportBackup` serializes the *decrypted* settings (proxy creds, PO token, update token) to a
|
||
plain JSON file; the UI caption only says "Does not include download history" — no warning that
|
||
the file contains credentials. *Fixed: `proxy`/`youtubePoToken`/`updateToken` are stripped (set to
|
||
`''`) before writing; SettingsView caption updated to say credentials are not included.*
|
||
- [ ] **M23 — `MAX_ITEMS` truncation can silently drop a source's items.** [sources.ts](src/main/sources.ts)
|
||
`replaceMediaItems` does `[...new, ...others].slice(0, 20000)`; once the global total exceeds the
|
||
cap, the *tail* (another source's items) is dropped on write and only reappears when that source
|
||
is re-indexed. Cap per-source, or surface it.
|
||
- [x] **M24 — `setSettings` accepts unvalidated free strings.** [settings.ts](src/main/settings.ts)
|
||
stores `rateLimit`/`proxy`/`defaultVideoQuality`/`defaultAudioQuality`/`youtubePlayerClient` as
|
||
any string. A bad `rateLimit` ("abc") only fails at download time; a `defaultQuality` not in
|
||
`QUALITY_OPTIONS` leaves the Settings dropdown's controlled value blank. *Fixed: `VIDEO_QUALITY_OPTIONS`
|
||
/ `AUDIO_QUALITY_OPTIONS` added to `shared/ipc.ts` and imported in both `settings.ts` (allowlist
|
||
validation) and `store/downloads.ts` (single source of truth). `rateLimit` validated against yt-dlp
|
||
rate format regex; `youtubePlayerClient` trimmed.*
|
||
- [x] **M25 — ROADMAP marks unwired features as ✅ shipped.** [ROADMAP.md](ROADMAP.md) Phase C
|
||
("Command preview … Surfaced as a **Preview command** button in the download bar") and Phase D
|
||
("Private / incognito mode — a download bar toggle") both carry `[x]`, but neither is wired in
|
||
the UI (corroborates M5/M6). The roadmap overstates completion — reconcile the docs or finish the wiring.
|
||
- [x] **M26 — ROADMAP accent palette is wholesale stale.** [ROADMAP.md](ROADMAP.md) Phase E
|
||
describes "four accent presets — Toffee (original), Slate, Evergreen, Lavender" with a default of
|
||
Toffee; the shipped [theme.ts](src/renderer/src/theme.ts) is rose / coral / amber / teal
|
||
("Sunset-to-sea") with a default of **teal**. The whole section documents a palette that no
|
||
longer exists.
|
||
- [ ] **M27 — Library batch-enqueue ignores per-item kind.** [sources.ts](src/renderer/src/store/sources.ts)
|
||
`enqueueItems` forces `settings.defaultKind` (+ its quality) for *every* item, so a channel can't
|
||
be downloaded as audio without flipping the global default — inconsistent with the DownloadBar
|
||
playlist panel, which offers a per-item video/audio toggle.
|
||
- [x] **M28 — Primary text inputs have no accessible name.** The URL field (DownloadBar), add-source
|
||
(Library), search (History/Settings), and the Terminal args `Textarea` rely on `placeholder` only —
|
||
which is not an accessible name. Add `aria-label`/`<label>` to each. *(a11y; distinct from M20's Select/ProgressBar.)*
|
||
- [x] **M29 — Failures are swallowed everywhere.** 29 `.catch(() => {})` sites across 15 files: nearly
|
||
every IPC write/read discards its error with no log, no user feedback, and no telemetry. A failed
|
||
settings write, history add, or `openPath` simply vanishes. *Fixed: a shared
|
||
[`logError(op)`](src/renderer/src/reportError.ts) helper replaces every swallowed renderer catch with
|
||
one consistent `console.error('[AeroFetch] <op> failed:', e)` (settings, history, templates, errorlog,
|
||
sources, App, DownloadBar drag-drop, SettingsView version reads, LibraryView scheduled-sync); main-process
|
||
sites (ytdlp auto-update, cookies loadURL) log inline. Four genuinely-silent catches left intentionally:
|
||
`pauseDownload` / `cancelDownload` (process kill, state already applied), `unlink` (cleanup), and
|
||
`clipboard.writeText` (browser permission — no user action available). **Scope note:** this delivers
|
||
the "log" half — failures are no longer silent. The user-facing toast + a persistent log sink (renderer
|
||
`console.error` isn't captured in a packaged build) are deferred to **CC8** (electron-log), which the
|
||
audit already cross-references here.*
|
||
- [x] **M30 — A broken contextBridge degrades silently to mock mode.** [preload/index.ts](src/preload/index.ts)
|
||
only `console.error`s if `exposeInMainWorld` throws; the renderer then sees no `window.electron`,
|
||
flips `PREVIEW` true, and runs the **browser mock** (no real IPC) with no visible error. A real
|
||
bridge failure looks like "preview." *Fixed: preload sends `IpcChannels.preloadBridgeFailure` on failure;
|
||
main registers `ipcMain.once(IpcChannels.preloadBridgeFailure, …)` in `registerIpcHandlers()` (run before
|
||
the window is created) and shows an error dialog + quits, so the failure is impossible to miss.*
|
||
- [x] **M31 — Default Electron menu (incl. Toggle DevTools/Reload) ships in production.** No
|
||
`Menu.setApplicationMenu(null)` is called; with `autoHideMenuBar` the default role menu is still
|
||
Alt-accessible, exposing DevTools/reload to end users. *Fixed: `Menu.setApplicationMenu(null)` called
|
||
in `app.whenReady()` when `!is.dev`; dev builds keep the menu for DevTools access.*
|
||
- [x] **M32 — Playlist/channel batches download in reverse order.** `addMany` prepends the whole
|
||
batch in entry order (`[entry1…entryN, …old]`), but `pump()` promotes the *highest-index* queued
|
||
item first (it reverses, assuming one-at-a-time prepends). So a selected playlist/channel downloads
|
||
**N → 1**. Files are still named `001…NNN` correctly (by `playlistIndex`), so a partial run leaves
|
||
the *last* videos on disk — surprising. Enqueue batches in reverse, or have `pump()` respect insertion order.
|
||
- [x] **M33 — Documented enqueue batch cap doesn't exist.** [ipc.ts](src/shared/ipc.ts):629 and
|
||
ROADMAP-PINCHFLAT claim the queue pulls "a batch at a time (e.g. 50)" via `MAX_ENQUEUE_BATCH` so it
|
||
"never holds the whole collection at once," but no such cap exists — `enqueueItems` `addMany`s *every*
|
||
selected item, so "Download all pending" on a 5,000-video channel puts 5,000 items in the store/queue
|
||
at once. Implement the cap or fix the comment.
|
||
- [x] **M34 — Optimistic settings updates never reconcile with main's validation.** The settings store
|
||
does `set(partial)` then `setSettings(partial).catch(() => {})`, discarding the validated `Settings`
|
||
main returns. A value main rejects (e.g. an unsafe `filenameTemplate`, a clamped `maxConcurrent`, a
|
||
malformed `rateLimit`) still shows as accepted in the UI until restart — the user believes a setting
|
||
saved that didn't. Apply the returned authoritative state.
|
||
- [x] **M35 — History re-download creates duplicate rows.** `redownload` re-queues via `addFromUrl`,
|
||
which mints a **new id**; on completion `addHistory` dedups by id (no match) and prepends a second row
|
||
for the same video. Re-downloading from History accumulates duplicates. Dedup by URL, or reuse the entry id.
|
||
- [x] **M36 — Library checkbox count ≠ "Download N selected".** Every item row has a checkbox
|
||
([LibraryView.tsx](src/renderer/src/components/LibraryView.tsx)), but `selectedActionable` filters to
|
||
pending/error/canceled only, so selecting 5 rows (incl. 2 already-downloaded) shows "Download **3**
|
||
selected." The checkbox count and the button count silently disagree. **Fix:** only show checkboxes on
|
||
actionable rows, or count all selected.
|
||
- [x] **M37 — End-user hints leak developer/internal references.** Settings hints surface dev-facing
|
||
detail: "Requires aria2c.exe in resources/bin (see the README there)", "paste a read-only **Gitea**
|
||
token", "sent via `--extractor-args`", "breaking downloads with **403** errors", "open a locked cookie
|
||
database", and roadmap status ("automatic minting is **planned**"). These read as code comments, not
|
||
product copy. **Fix:** rewrite for end users (hide repo/flag/HTTP-status jargon and roadmap notes).
|
||
- [x] **M38 — `noImplicitAny: false` partially defeats `strict`.** The inherited
|
||
`@electron-toolkit/tsconfig` sets `strict: true` **but explicitly `"noImplicitAny": false"`**, and
|
||
neither project tsconfig re-enables it — so a parameter/variable with no inferrable type silently
|
||
becomes `any` project-wide (the one real hole in an otherwise-strict setup; `noUnusedLocals`/
|
||
`noUnusedParameters`/`noImplicitReturns` *are* on, which is why dead locals don't accumulate). **Fix:**
|
||
re-enable `noImplicitAny` in `tsconfig.node.json`/`tsconfig.web.json`.
|
||
|
||
## Low
|
||
|
||
- [ ] **L1 — Module-load `setInterval` in `store/downloads.ts`** runs on import (incl. tests/preview).
|
||
- [ ] **L2 — `index.ts` flat IPC registration (~150 lines)** — let each main module export its own
|
||
`register(ipcMain)`.
|
||
- [x] **L3 — Stale comments.** `getYtdlpVersion` JSDoc still calls it the "Step-1 spike";
|
||
`vitest.config.ts` claims tests "only exercise buildArgs.ts" (9 test files now exist).
|
||
- [x] **L4 — `electron-builder.yml` nits.** `copyright: yt-dlp frontend` is not a copyright
|
||
string (no holder/year); the `files` exclude `tsconfig.tsbuildinfo` never matches the real
|
||
`tsconfig.web.tsbuildinfo`.
|
||
- [ ] **L5 — Inline `style={{}}` vs makeStyles** (25× across 8 files; SettingsView 14×). Notably
|
||
Sidebar's active-nav inset shadow and DownloadOptionsForm's hint caption use inline styles
|
||
where a class exists elsewhere.
|
||
- [x] **L6 — Control height mismatch.** `Select` is a fixed 32px sitting beside `size="large"`
|
||
(~40px) Input/Buttons in DownloadBar.
|
||
- [x] **L7 — `canceled` and `paused` share the 'warning' badge color** (QueueItem) — visually
|
||
ambiguous.
|
||
- [x] **L8 — Index/compound list keys.** TerminalView keys log lines by array index; Settings
|
||
Diagnostics keys by `id + occurredAt` while every other list keys by `id` alone.
|
||
- [x] **L9 — Thumbnail box sizes not shared.** Four hand-tuned 16:9 boxes (120×68, 108×64, 72×44,
|
||
60×34) with no shared aspect/size constant.
|
||
- [ ] **L10 — Scattered magic numbers.** Timeouts/caps spread across modules (probe 60s, indexer
|
||
180s, probeMeta 30s, update-idle 60s; MAX_ENTRIES 500/200, MAX_TEMPLATES 100, MAX_ITEMS 20000).
|
||
Consider a central constants module.
|
||
- [x] **L11 — "Queue (N)" overcounts.** DownloadsView's header count is `items.length` (includes
|
||
completed/error/canceled), not the active queue.
|
||
- [x] **L12 — Command palette polish.** No scroll-into-view for keyboard selection in the 50vh
|
||
list; `role="dialog"` without `aria-modal`/focus-trap; Esc handled only on the input.
|
||
- [x] **L13 — Destructive actions lack confirmation.** "Clear history", "Clear log", "Remove
|
||
source" are one-click — inconsistent with the careful confirm on backup import.
|
||
- [x] **L14 — `base.css` is bare** — no global `box-sizing`, `:focus-visible`, or font-smoothing
|
||
baseline, so custom native elements (Select, Hint, segmented controls) get inconsistent focus
|
||
rings; `box-sizing` is then set ad hoc on the SettingsView swatch.
|
||
- [x] **L15 — `MediaThumb` redundant ternary** `kind === 'audio' ? 'audio' : 'video'` (kind is
|
||
already that union).
|
||
- [x] **L16 — `relTime`/`formatWhen` verbosity.** `relTime` returns unbounded "N d ago"; History's
|
||
`formatWhen` always appends the year, even for the current year.
|
||
- [x] **L17 — TemplateManager regex not validated.** An invalid `urlPattern` is accepted with no
|
||
feedback and silently never matches (main's `matchesUrl` swallows the error).
|
||
- [x] **L18 — Terminal nav item shown when custom commands are off** — leads to a gated dead-end
|
||
view; consider hiding/disabling it.
|
||
- [x] **L19 — `App.tsx` focus hack** — `setTimeout(() => …focus(), 60)` after a tab switch.
|
||
- [x] **L20 — `youtubePlayerClient` is free text** with no allowlist/validation; a typo passes
|
||
straight to yt-dlp's `--extractor-args`.
|
||
- [ ] **L21 — Inconsistent in-component error handling.** SettingsView funnels the app-update
|
||
not-ok result into a dedicated `appUpdError` slot but keeps not-ok duals for
|
||
version/update/export/import — two patterns in one file.
|
||
- [x] **L22 — DownloadOptionsForm shows both audio + video controls** regardless of the chosen
|
||
kind in the per-download override panel (audio format shown for a video download, etc.).
|
||
- [x] **L23 — Diagnostics shows first 20 errors** with no "showing 20 of N" hint (the copied
|
||
report includes all).
|
||
- [x] **L24 — `window.open(htmlUrl, '_blank')`** in SettingsView is the only `window.open` in the
|
||
app (relies on the window-open handler); elsewhere external opens go through IPC/`shell`.
|
||
- [x] **L25 — Taskbar effect over-fires.** `App.tsx` subscribes to the whole downloads store and
|
||
recomputes `summarizeQueue` + sends the taskbar IPC on *every* store change (each progress
|
||
tick); throttle or dedupe by computed value.
|
||
- [ ] **L26 — Third URL-sniffing path.** DownloadBar's drag-drop `firstUrl` is a separate
|
||
"first http(s) line" extractor alongside `useClipboardLink.looksLikeUrl` and deeplink's scan.
|
||
- [x] **L27 — DownloadBar Enter probes, not downloads.** Enter in the URL field triggers
|
||
`fetchFormats`; there's no keyboard path to actually start the download (LibraryView's Enter
|
||
runs its primary action). Inconsistent.
|
||
- [x] **L28 — Default kind/quality applied once.** DownloadBar seeds from settings via a one-shot
|
||
ref; changing the default in Settings while on the Downloads tab isn't reflected until reload.
|
||
- [x] **L29 — UI constant in the store.** `QUALITY_OPTIONS` lives in `store/downloads.ts` yet is a
|
||
presentational constant imported by views — couples UI to the store module.
|
||
- [x] **L30 — Redundant `loading="lazy"`** on `MediaThumb` images already inside a virtualized
|
||
list (the virtualizer only mounts visible rows).
|
||
- [ ] **L31 — Two theme switchers.** Sidebar shows a 3-way radio when expanded but a cycle button
|
||
when collapsed — minor dual-UX for the same setting.
|
||
- [x] **L32 — `paletteActions` rebuilt every render** in `App.tsx` (not memoized); harmless today,
|
||
but it's passed straight into a child.
|
||
- [x] **L33 — `idCounter` fallback resets per launch.** The non-crypto `newId` fallback
|
||
(`item-${++idCounter}`) restarts at 1 each run; the `Date.now()`-based fallbacks elsewhere can
|
||
collide within a millisecond. Unify on one robust id helper (see M7). *Fixed with M7: the single
|
||
`newId` fallback is `${prefix}-${Date.now()}-${++counter}`, so same-ms ids stay distinct.*
|
||
|
||
*Round 3 (2026-06-29) — tests, build metadata, error copy, edge cases:*
|
||
|
||
- [ ] **L34 — Integration test off by default.** `real-download.integration.test.ts` is
|
||
`describe.skipIf(!RUN)`; `npm test` never exercises a real download, so there's no automated
|
||
regression for the actual yt-dlp argv path (only the pure builder).
|
||
- [x] **L35 — `isValidMediaItem` has no test** — the one persisted-JSON validator of six with no spec.
|
||
- [x] **L36 — `compareVersions` differing-length components untested** (e.g. `1.2` vs `1.2.0`).
|
||
- [x] **L37 — `download.ts` formatters untested** (`parseProgress`/`fmtBytes`/`fmtEta`, incl. the `NA` paths).
|
||
- [x] **L38 — `buildArgs` webm-thumbnail exclusion branch untested** (`embedThumbnail && container!=='webm'`).
|
||
- [x] **L39 — `CROP_SQUARE_PPA` exact-string assertion** is a change-detector test (breaks on harmless reformat).
|
||
- [x] **L40 — `clipboardLink.test.ts` imports the zustand settings store** to test two pure helpers
|
||
(`looksLikeUrl`/`looksLikeSingleVideo`) — couples a pure-fn test to store init (see H2).
|
||
- [x] **L41 — `package.json` `homepage` points at yt-dlp's GitHub**, not AeroFetch — wrong product
|
||
URL (surfaced by the NSIS installer).
|
||
- [x] **L42 — `package.json` has no `license` field** (ships LGPL ffmpeg + GPL aria2c).
|
||
- [x] **L43 — `package.json` has no `repository` field** (real repo is the Gitea instance).
|
||
- [x] **L44 — Vestigial lint excludes.** `electron-builder.yml` excludes `.eslintrc`/`.prettierrc`
|
||
but there's no ESLint/Prettier config, script, or dependency — no enforced style tooling.
|
||
- [x] **L45 — Three self-descriptions** drift: pkg `description` "A yt-dlp frontend for Windows" vs
|
||
builder `copyright` "yt-dlp frontend" vs Sidebar caption "yt-dlp frontend".
|
||
- [x] **L46 — yt-dlp-missing error copy differs across 4 modules** (download/ytdlp/probe/indexer):
|
||
"Reinstall AeroFetch, or drop…" vs "Download it into resources/bin/…" vs "Drop it into resources/bin/."
|
||
- [x] **L47 — `probeMeta` 30s timeout returns null silently** — no surfaced message, unlike every
|
||
other yt-dlp timeout ("Timed out …").
|
||
- [x] **L48 — `cleanError` strips a leading `error:`** for live failures, while raw `ERROR:` text
|
||
shows elsewhere (errorlog seed / terminal) — inconsistent error normalization.
|
||
- [x] **L49 — Newlines in user-facing error strings.** download.ts missing-binary messages embed
|
||
`\n`, which renders awkwardly in inline/Caption error spans.
|
||
- [x] **L50 — "Cookies saved" shown for 0 cookies.** Closing the sign-in window without logging in
|
||
still writes the file and reports `ok` with `cookieCount: 0`.
|
||
- [ ] **L51 — Scheduled daily sync time hardcoded to 09:00** (schedule.ts) — on/off toggle only, no time picker.
|
||
- [x] **L52 — Installer handoff is a fixed `setTimeout(app.quit, 1500)`** (updater.ts) — a race on slow machines.
|
||
- [x] **L53 — `dialog.show*Dialog(win!, …)` non-null assertions** on a possibly-null window
|
||
(chooseFolder, backup export/import) — can throw if the sender window is gone.
|
||
- [x] **L54 — DownloadBar preview `<img>` has no `onError` fallback** (inconsistent with `MediaThumb`'s retry).
|
||
- [x] **L55 — QueueItem meta line can show size twice** — a probed-format `quality` label already
|
||
contains the size, then `sizeLabel` is appended again.
|
||
- [x] **L56 — SponsorBlock ON with zero categories silently no-ops** (`buildArgs` guards on
|
||
`length > 0`) with no UI warning.
|
||
- [x] **L57 — Scheduling a past datetime silently downloads now** (`buildItem` future-only guard),
|
||
no feedback that the schedule was ignored.
|
||
- [x] **L58 — `chooseFolder` passes the macOS-only `createDirectory` property** — dead option on Windows.
|
||
- [x] **L59 — `THEME_BACKGROUND` (index.ts) duplicates `pageBackground` (theme.ts)** — two
|
||
hand-synced color constants (a "keep in sync" comment guards them).
|
||
- [x] **L60 — `'external-url'` channel name lacks the `category:` prefix** every other IPC channel uses.
|
||
- [x] **L61 — `taskbarProgress` is the lone `ipcMain.on`** (fire-and-forget) amid all-`invoke` channels.
|
||
- [ ] **L62 — "Best available" synthetic format hardcodes `ext:'mp4'`/`hasAudio:true`** (probe.ts) —
|
||
mislabels when the chosen container is mkv/webm.
|
||
- [x] **L63 — `audioQuality()` unknown label → silent `'0'` (best)** — the audio analog of H5.
|
||
- [ ] **L64 — `ARIA2C_ARGS` connection params hardcoded** (`-x16 -s16 -k1M`), no user control.
|
||
- [ ] **L65 — Pause uses `taskkill /F`** like cancel — a forced kill risks an unflushed `.part` tail vs a graceful stop.
|
||
- [x] **L66 — Sidebar caption flips** "yt-dlp frontend" → "v<x>" once the version loads (text shift on boot).
|
||
- [x] **L67 — Onboarding has no Skip and can't be revisited** (no "show tips again").
|
||
- [x] **L68 — Background-running notification fires once per process** (`notifiedBackground` never
|
||
resets) — won't remind on later window closes.
|
||
- [ ] **L69 — `settings.ts` mixes path helpers with persistence** (`getDefaultMediaDir`/
|
||
`ensureMediaDirs`/`getDownloadArchivePath` alongside the store) — split a `paths.ts`.
|
||
- [x] **L70 — `crypto.randomUUID` fallback is effectively unreachable** in Electron/Node 26
|
||
(`typeof crypto !== 'undefined'` is always true) — dead defensive branch (see M7). *Fixed with M7: the
|
||
branch now lives once in `newId` (kept so the fn is total in any host) and is covered by a test that stubs
|
||
`crypto` to exercise it.*
|
||
- [x] **L71 — Settings folder inputs are `readOnly`** with only a Browse button — no way to paste a
|
||
known path.
|
||
- [x] **L72 — History re-download drops the thumbnail** (passes only title/channel), so the
|
||
re-queued row loses its thumbnail until re-probed.
|
||
- [x] **L73 — Two verbs for the same probe action** — DownloadBar "Fetch" vs LibraryView "Index".
|
||
- [x] **L74 — Accent swatches are triple-labeled** (`aria-pressed` + `aria-label` + `title`).
|
||
- [x] **L75 — Update-token field always visible** in the Software-update card, even when no update
|
||
is pending — buries an advanced/rarely-needed input.
|
||
- [x] **L76 — Duplicate-warning may show a placeholder title** — `setDup(existing.title)` can be the
|
||
`titleFromUrl` placeholder before metadata resolves ('YouTube video (id)').
|
||
- [x] **L77 — ffmpeg/ffprobe versions have no re-check affordance** (load once; "not found" sticks
|
||
with no retry) unlike the yt-dlp "Check version" button.
|
||
- [x] **L78 — `DownloadsView` empty-state copy** ("Paste a URL above to get started") doesn't match
|
||
the actual two-step Fetch→Download affordance.
|
||
- [x] **L79 — SponsorBlock default categories = `['sponsor']` only** — enabling SB silently covers
|
||
just sponsors until the user expands categories (expectation gap).
|
||
|
||
*Round 4 (2026-06-29) — doc/code drift, build artifacts, edge cases:*
|
||
|
||
- [x] **L80 — ROADMAP wrong file path.** [ROADMAP.md](ROADMAP.md) Phase A says the flags are
|
||
emitted "in `buildArgs` (`src/main/download.ts`)" — `buildArgs` lives in `src/main/buildArgs.ts`.
|
||
- [x] **L81 — ROADMAP internal contradiction.** Phase A: "`--split-chapters` still TODO"; Phase L:
|
||
`[x]` split-chapters done. Phase A wasn't updated when L landed.
|
||
- [ ] **L82 — PINCHFLAT roadmap interface sketches are stale.** [ROADMAP-PINCHFLAT.md](ROADMAP-PINCHFLAT.md)
|
||
Phase F's `Source`/`MediaItem` code blocks omit shipped fields (`videoId`, `itemCount`,
|
||
`watched`, `feedUrl`) — design sketches that drifted from `shared/ipc.ts`.
|
||
- [ ] **L83 — Dead build artifacts.** electron-builder emits `.blockmap` differential-update files
|
||
for each NSIS installer, but the custom [updater.ts](src/main/updater.ts) does a full download and
|
||
never consumes them.
|
||
- [ ] **L84 — `dist/` accumulates unboundedly.** ~620 MB per build with no cleanup; it currently
|
||
holds 0.1.0–0.5.0 Setup + portable artifacts (~3 GB). Add a clean step or prune.
|
||
- [x] **L85 — `.claude/launch.json` is committed and misplaced.** A VS Code launch config lives in
|
||
`.claude/` (VS Code reads `.vscode/launch.json`), so no tool consumes it; it's also tracked
|
||
(not gitignored).
|
||
- [x] **L86 — History select-all + filter change deletes hidden rows.** `selected` persists across a
|
||
filter change, so "Delete selected" can remove entries no longer visible (surprising).
|
||
- [ ] **L87 — History re-download ignores original options.** `redownload` re-queues url/kind/quality
|
||
only; the post-processing `DownloadOptions` used originally are lost (current defaults apply).
|
||
- [x] **L88 — `applyEvent('progress')` can promote a `queued` item to `downloading`** outside
|
||
`pump()` — a latent concurrency edge if a progress event ever arrives before/without the launch transition.
|
||
- [ ] **L89 — Inconsistent release artifacts.** Only `0.4.1` carries `.sha256` files (hand-made) and
|
||
the version sequence skips 0.3.x — symptomatic of the manual release process behind H8.
|
||
|
||
*Round 5 (2026-06-29) — code-level micro-inconsistencies & polish:*
|
||
|
||
- [x] **L90 — Two idioms for "value in const array."** [settings.ts](src/main/settings.ts) `sanitizeOptions`
|
||
uses `ARR.includes(x as never)` (L116–120) while `setSettings` uses `(ARR as readonly string[]).includes(x as string)` (L275/323). Pick one.
|
||
- [x] **L91 — Unsafe tuple cast / fragile encoding.** `value.split('|') as [MediaKind, string]`
|
||
([SettingsView.tsx](src/renderer/src/components/SettingsView.tsx):411) trusts a `'|'`-joined
|
||
`kind|quality` string; a value without `'|'` yields `undefined` quality. Use a typed pair, not a string.
|
||
- [ ] **L92 — Preload names diverge from main fn names.** `markSourceItemDownloaded`↔`setMediaItemDownloaded`,
|
||
`syncSources`↔`syncWatchedSources`. Align the names across the boundary.
|
||
- [ ] **L93 — SettingsView bypasses the store layer.** It calls `window.api` directly for
|
||
cookies/ffmpeg/yt-dlp/app-update/backup while using stores for settings/templates/errorlog — mixed
|
||
data-access in one component.
|
||
- [ ] **L94 — `summarizeQueue` recomputed every render** in [DownloadsView.tsx](src/renderer/src/components/DownloadsView.tsx)
|
||
(no `useMemo`) — and again on every store change in App's taskbar effect. Memoize/share.
|
||
- [ ] **L95 — "Sign in" hyphenation varies** in the Cookies card ("Sign in…", "Site to sign in to", "Sign-in window", "Signing in…").
|
||
- [ ] **L96 — Ellipsis-on-dialog-buttons inconsistent.** "Export backup…/Import backup…/Sign in…" use
|
||
`…` (opens a dialog), but "Browse" / "Check for updates" / "Update now" don't. Adopt the `…` = opens-a-dialog convention.
|
||
- [ ] **L97 — Advanced-field placeholders inconsistent.** Adjacent fields use an example (`web_safari`),
|
||
a literal `(none)`, and `Optional — Gitea access token`. Pick one placeholder style.
|
||
- [x] **L98 — "Embed chapters" has no hint** while every sibling toggle in DownloadOptionsForm does.
|
||
- [ ] **L99 — URL input has an `id` but no label.** `input={{ id: 'aerofetch-url' }}` with no
|
||
`<label for>`/`aria-label` (placeholder only) — see M28.
|
||
- [ ] **L100 — Inconsistent elevation.** DownloadBar (`shadow4`) and CommandPalette (`shadow28`) float;
|
||
every other card (Settings, QueueItem, History, Library, Templates) is flat (border-only). Define an elevation scale.
|
||
- [ ] **L101 — No z-index scale.** CommandPalette and Hint both hardcode `zIndex: 1000`.
|
||
- [ ] **L102 — Two mental models for "format."** Settings sets it via one combined Video/Audio dropdown
|
||
(`FORMAT_OPTIONS`); the DownloadBar uses a Video/Audio segmented control + a separate quality dropdown.
|
||
- [ ] **L103 — Two busy-indicator patterns.** Fetch/Index swap the button icon to a `Spinner`; the
|
||
Settings check/update buttons keep their icon and render a *separate* adjacent `Spinner`.
|
||
- [ ] **L104 — History isn't virtualized.** It renders all filtered rows, while Downloads and Library use `VirtualList`.
|
||
- [x] **L105 — `MediaKind` declared twice.** In both [ipc.ts](src/shared/ipc.ts) and
|
||
[store/downloads.ts](src/renderer/src/store/downloads.ts); components import it from both. Single source it.
|
||
*Fixed: the store now imports `MediaKind` from `@shared/ipc` and re-exports it, so the duplicate local
|
||
`type MediaKind` is gone while existing `import { MediaKind } from '../store/downloads'` sites still resolve.*
|
||
- [ ] **L106 — Refresh icons used interchangeably.** `ArrowClockwiseRegular` (retry, re-download,
|
||
check-for-new, update-yt-dlp) vs `ArrowSyncRegular` (re-index, check-for-updates) for the same "redo/refresh" idea.
|
||
- [ ] **L107 — Spinner sizes mixed** — `"tiny"` almost everywhere, `"extra-tiny"` for the QueueItem queued row.
|
||
- [x] **L108 — Main window sets no explicit `title`** (relies on the document `<title>`).
|
||
- [ ] **L109 — Versions auto-load with no indicator.** yt-dlp/ffmpeg versions in About just appear;
|
||
only the manual "Check" button shows a spinner.
|
||
- [ ] **L110 — One muted-text token, many class aliases.** `colorNeutralForeground3` is re-declared as
|
||
`hint`/`sub`/`meta`/`srcSub`/`stats`/`count`/`emptyHint`/`previewMeta` across components.
|
||
- [ ] **L111 — Two helper-text mechanisms** — Fluent `Field` `hint` vs manual `<Caption1 className={hint}>`; SettingsView uses both.
|
||
- [ ] **L112 — `Switch` labeling inconsistent.** Settings/DownloadOptionsForm show "On"/"Off" text;
|
||
Library switches use `aria-label` only (no visible state text).
|
||
- [ ] **L113 — Tooltip ≠ aria-label.** The Fetch button's Hint says "Fetch formats / playlist" but its
|
||
`aria-label` says "Fetch formats or playlist" (DownloadBar).
|
||
- [ ] **L114 — Row titles use different components** — `Text` (QueueItem/History/Templates) vs `span` (Library) vs `Caption1` elsewhere.
|
||
- [ ] **L115 — Equivalent row actions labeled inconsistently** — "Re-index" is icon+text; the
|
||
comparable "Retry"/"Remove"/"Re-download" are icon-only.
|
||
- [ ] **L116 — Empty-state structure varies** — Downloads (icon + line), History (colored badge + line + sub-hint), Library (icon + line, no sub-hint).
|
||
- [ ] **L117 — Terminal/error-log "empty" aren't centered blocks** like the other empty states (inline text instead).
|
||
- [ ] **L118 — Cross-component focus via hardcoded id.** App focuses the URL field with
|
||
`document.getElementById('aerofetch-url')` — brittle coupling into DownloadBar's markup.
|
||
- [ ] **L119 — "No items" copy varies** — "No … yet" (most) vs "Output will appear here." (Terminal) vs "No errors logged." (Diagnostics).
|
||
- [ ] **L120 — Fluent `Card` vs hand-styled `div` cards.** Only Settings/Onboarding use `<Card>`; every
|
||
other card surface is a bespoke styled `<div>`.
|
||
- [ ] **L121 — Raw scrim literal.** CommandPalette backdrop is `rgba(0,0,0,0.32)` (the only raw rgba in
|
||
the renderer), identical in light/dark — low contrast over a dark UI.
|
||
- [ ] **L122 — "Checking…" indicated twice** — the Settings update button changes its label to "Checking…" *and* a separate Spinner renders beside it.
|
||
- [ ] **L123 — `Caption1` is overloaded** for hints, metadata, counts, and timestamps — one size doing four jobs.
|
||
- [ ] **L124 — `Body1` vs `Text` role overlap** — `Body1` appears only in empty states/onboarding/gate; `Text` carries titles; the split isn't principled.
|
||
- [ ] **L125 — Icon sizing has two syntaxes** — px strings (`fontSize: '20px'`) on icon *containers* vs numeric `fontSize={28}` on inline icons.
|
||
- [ ] **L126 — Uneven text truncation** — some titles/metas use `nowrap`+ellipsis, others wrap; no shared truncation utility.
|
||
- [x] **L127 — `Notification` sets no icon** — completion toasts used the default Electron icon though `getAppIconPath()` existed. *Fixed with W13 (cached `getAppIconImage()` passed as `icon` at both notification sites).*
|
||
- [ ] **L128 — Preview seed/mock data ships in production.** The `PREVIEW` runtime check can't be
|
||
tree-shaken, so each store's seed arrays + fake ticker are bundled into the Electron renderer.
|
||
- [ ] **L129 — `Hint align` is silently ignored for left/right placements** yet callers still pass it.
|
||
- [ ] **L130 — Error-string style is mixed** — internal errors are lowercase/no-period ("timed out",
|
||
"interrupted"); user-facing ones are capitalized with periods. Fine when interpolated, jarring if shown raw.
|
||
- [ ] **L131 — Taskbar `error` state is ephemeral** — it's driven by live `error` items, so "Clear
|
||
finished" flips the red taskbar bar back to normal even though failures occurred (out of sync with the persisted error log).
|
||
- [ ] **L132 — `clearProbe` + form-reset logic is duplicated** in DownloadBar (`onUrlChange`, `download`, `addPlaylist`).
|
||
- [ ] **L133 — `Field` controls mix `aria-label` + visible `label`.** Many `Field label="X"` wrap a
|
||
`Select aria-label="X"`, so AT announces the name twice (extends M20 across DownloadOptionsForm).
|
||
- [ ] **L134 — Inconsistent disabled affordance for busy buttons.** Some stay enabled-looking with a
|
||
spinner icon (Fetch/Index) while others are `disabled` with adjacent text (Settings) — no single "busy" convention.
|
||
- [ ] **L135 — `Textarea` vs `Input` for similar fields.** Trim uses a `Textarea`; the visually-similar
|
||
single-line fields use `Input` — fine, but the trim/schedule panels and the template form mix both with no shared field style.
|
||
|
||
*Round 6 (2026-06-29) — behavioral/logic edge cases:*
|
||
|
||
- [ ] **L136 — Incognito leaks outside history.** Even if the (unreachable, M6) private flag were set,
|
||
`download.ts` still writes failures to `errorlog.json` (title + URL) and shows the title in completion
|
||
notifications — the "private" promise covers only history.
|
||
- [x] **L137 — Stuck 0% for unknown-size downloads.** `parseProgress` returns `progress: 0` when
|
||
`total_bytes`/`_estimate` are absent (livestreams, some sites), so the bar reads 0% the whole time
|
||
instead of an indeterminate state. *Fixed: `parseProgress` now sets `sizeUnknown: !totalBytes` on
|
||
`DownloadProgress`; the store threads it onto the item and `QueueItem` renders an indeterminate bar +
|
||
"Downloading…" (instead of a frozen 0%) when it's set, reusing the existing SR7 indeterminate path.*
|
||
- [ ] **L138 — Clipboard read on every window focus.** [useClipboardLink.ts](src/renderer/src/useClipboardLink.ts)
|
||
reads the full clipboard on each `focus` (and once on mount, possibly before `clipboardWatch` has
|
||
loaded — FALLBACK is `true`). Reading all clipboard text whenever focused may surprise privacy-conscious users.
|
||
- [ ] **L139 — Auto-update vs concurrent spawn race.** `runStartupYtdlpAutoUpdate` can overwrite
|
||
`yt-dlp.exe` on launch while a deep-link/queued download spawns the same binary — a Windows
|
||
file-in-use/corrupt-spawn window.
|
||
- [ ] **L140 — Retry-during-teardown race.** `retry` reuses the same id; if the previous process's
|
||
`close` handler hasn't removed it from `active` yet, `startDownload` returns "already running" → markError.
|
||
- [ ] **L141 — Renderer history grows unbounded in memory.** `useHistory.add` prepends without a cap
|
||
while main caps `history.json` at 500; in a long session the in-memory list exceeds the persisted cap (resets on reload).
|
||
- [ ] **L142 — IPC mutation return values are discarded everywhere.** history/templates/sources/settings
|
||
IPC calls return the authoritative (validated/capped/sanitized) state, but every renderer caller does
|
||
optimistic-only updates and ignores it — client and persisted state can silently diverge until reload (generalizes M34).
|
||
- [ ] **L143 — No in-window "quit anyway."** With a download active (or tray mode), closing only hides;
|
||
quitting requires the tray menu. If the tray ever fails to create, the only exit is Task Manager
|
||
(mitigated today by the embedded fallback tray icon).
|
||
- [ ] **L144 — Duplicate guard checks the queue only, not history.** Re-adding a URL downloaded earlier
|
||
(already cleared from the queue) gives no "already downloaded" hint (roadmap notes this as a possible extension).
|
||
- [ ] **L145 — `useClipboardLink` re-offers a dismissed link after a tab switch.** `lastSeen` is
|
||
per-hook-instance; switching Downloads↔Library remounts it, so a previously-dismissed clipboard link is offered again.
|
||
- [x] **L146 — `parseTrimSections` accepts malformed multi-colon times.** The `\d+(?::\d{1,2})*` pattern
|
||
passes tokens like `1:2:3:4-5:6:7:8`, which then reach yt-dlp's `--download-sections` and fail there
|
||
rather than being rejected up front.
|
||
- [x] **L147 — macOS branches in a Windows-only app.** `app.on('activate')` and the
|
||
`process.platform !== 'darwin'` guard in `window-all-closed` are dead on Windows — boilerplate that implies multi-platform support the app doesn't ship.
|
||
- [ ] **L148 — `clearFinished`/remove can free a slot before the killed process exits.** Removing a
|
||
just-canceled item lets `pump()` launch into the "freed" slot while `taskkill` is still tearing down the prior tree — a brief window over the concurrency cap.
|
||
|
||
*Round 7 (2026-06-29) — copy, a11y patterns & micro-behavior:*
|
||
|
||
- [ ] **L149 — Hint length wildly inconsistent.** Most field hints are one short line, but "Keep running
|
||
in the tray" is a three-sentence paragraph with a parenthetical. Normalize hint length/voice.
|
||
- [x] **L150 — "PO token" capitalization varies** — `PO Token` (ipc.ts/ROADMAP) vs `PO token` /
|
||
`Proof-of-Origin token` (SettingsView). Pick one.
|
||
- [ ] **L151 — Mixed range dashes.** En-dash in prose ("2–3 is a good balance") vs hyphen in time-range
|
||
placeholders ("1:30-2:00"). Choose one convention.
|
||
- [ ] **L152 — Search-field sizing differs per screen.** History `minWidth 180 / maxWidth 320`, Settings
|
||
inline `width: 100%`, DownloadBar `flexGrow`. No shared search-input width.
|
||
- [ ] **L153 — Generic "Dismiss" aria-labels.** Three close buttons (DownloadBar suggestion + dup,
|
||
Library suggestion) all read just "Dismiss" — ambiguous to a screen reader. Name what's dismissed.
|
||
- [x] **L154 — Instructional text in an aria-label.** Sidebar theme-cycle button is
|
||
`aria-label="Theme: Dark. Click to change."` — "Click to change" is UI instruction, not a name.
|
||
- [ ] **L155 — Accent swatches aren't a `radiogroup`.** They're a single-select set rendered as plain
|
||
buttons with `aria-pressed`, while the Theme controls (also single-select) use `role="radiogroup"`/`radio`.
|
||
Inconsistent single-select a11y pattern.
|
||
- [x] **L156 — `datetime-local` schedule has no `min`.** Past times are selectable, then silently download
|
||
now (extends L57). Add `min={now}`.
|
||
- [ ] **L157 — `removeSource` doesn't cancel in-flight downloads.** Removing a source while its videos are
|
||
downloading lets them finish into the removed source's folders; `markDownloaded` then no-ops on the gone item.
|
||
- [ ] **L158 — Drag highlight flickers.** `onDragLeave={() => setDragActive(false)}` fires when the cursor
|
||
crosses child elements (no enter/leave depth counter), so the dashed-outline blinks during a drag.
|
||
- [x] **L159 — Dead fallback branch.** `copyErrorReport` falls back to "No errors logged." but its button
|
||
is disabled when there are no entries, so the fallback is unreachable.
|
||
- [ ] **L160 — One-off inline margins.** `marginTop` literals (Onboarding 2px, QueueItem 4px, SettingsView
|
||
8px) instead of style classes (extends L5).
|
||
- [ ] **L161 — `height: 100%` screens inside a padded scroll container.** Downloads/Terminal set
|
||
`height: 100%` while App's `<main>` is `overflowY: auto` + 24/28px padding — a fragile coupling that can
|
||
double-scroll or misalign the virtualized list's viewport.
|
||
- [ ] **L162 — Per-thumbnail store subscription.** Every `MediaThumb` calls `useResolvedDark()` (two store
|
||
subscriptions) instead of receiving a resolved theme prop — N subscriptions per list.
|
||
- [ ] **L163 — QueueItem makes 9 separate `useDownloads` selector calls** instead of one destructured
|
||
selection — repeated boilerplate per row.
|
||
|
||
*Round 8 (2026-06-29) — formatting micro-inconsistencies:*
|
||
|
||
- [ ] **L164 — Metadata separator glyph/spacing is inconsistent.** Meta lines join with `' • '`
|
||
(2-space bullet) in QueueItem/History/DownloadBar/DownloadsView, with `' · '` (1-space midd·dot) in
|
||
LibraryView, and with `' • '` (1-space bullet) in QueueItem's "Paused • 42%". Three styles for one role.
|
||
**Standard:** one separator constant (glyph + spacing).
|
||
- [x] **L165 — The two byte/speed formatters round differently.** `download.ts fmtBytes` uses
|
||
`toFixed(v >= 100 ? 0 : 1)` ("512 MB") while `queueStats formatSpeed` uses `toFixed(1)` ("512.0 MB/s") —
|
||
visibly different precision for the same magnitudes (compounds the duplicate-formatter issue H2/M9).
|
||
- [x] **L166 — ETA has no hour rollover.** `fmtEta` (download.ts/downloads.ts) and `formatEta`
|
||
(queueStats) emit `M:SS` only, so a 2-hour ETA renders as **"120:00"**, whereas `fmtDuration`
|
||
(indexerCore) correctly rolls to `H:MM:SS`. Unify on the hour-aware format.
|
||
- [x] **L167 — `PROGRESS_TEMPLATE` is exported but used only inside `buildArgs.ts`** — a superfluous
|
||
public export (download.ts parses the `prog|` lines independently). Make it module-private.
|
||
|
||
*Round 9 (2026-06-29) — type-safety & build config:*
|
||
|
||
- [x] **L168 — `noUncheckedIndexedAccess` is off.** Array/index/tuple access is typed as always-defined,
|
||
so the exact edge cases already filed compile clean: the `split('|') as [MediaKind, string]` cast
|
||
(L91/CL2) and `parseProgress`'s positional destructure of a possibly-short `split('|')`. Enabling it
|
||
would surface them at build time.
|
||
- [x] **L169 — `noFallthroughCasesInSwitch` is off.** The large `setSettings` switch and `applyEvent`
|
||
switch aren't fallthrough-guarded; a missing `break`/`return` wouldn't be caught.
|
||
- [ ] **L170 — No source maps in production.** `sourceMap: false` (toolkit) and no `build.sourcemap` in
|
||
`electron.vite.config.ts`, so field crash stack traces are unmapped — combined with no logging (CC8/M29),
|
||
diagnosing a user-reported crash is very hard. Consider hidden/external source maps.
|
||
- [ ] **L171 — `main.tsx` mock drift.** The preview mock's version strings disagree
|
||
(`getAppVersion`→'0.4.0-preview' vs `checkForAppUpdate.currentVersion`→'0.4.0'), and its `setSettings`
|
||
skips the real validation/sanitization — so the browser preview can accept settings the app rejects,
|
||
masking M34-class issues during design.
|
||
- [ ] **L172 — `skipLibCheck: true`** (toolkit) hides type errors in dependency `.d.ts` files — standard
|
||
practice, low risk, noted for awareness (a dep type regression won't fail typecheck).
|
||
|
||
---
|
||
|
||
## UI consistency
|
||
|
||
Cross-screen review (Downloads, Library, History, Terminal, Settings, Onboarding, Command palette,
|
||
Sidebar, cookie sign-in window). Each item: what's inconsistent → **Standard:** the single rule to
|
||
adopt. Items prefixed `UI`; where a dimension was already filed, the existing ID is referenced
|
||
instead of re-filing. The app uses Fluent tokens well (theme-awareness is mostly solid); the gaps
|
||
are an undefined spacing/radius/type scale and a drift between Fluent components and hand-rolled ones.
|
||
|
||
### Layout, spacing & width
|
||
|
||
- [x] **UI1 — No shared content width.** SettingsView is `maxWidth: 640px` ([SettingsView.tsx](src/renderer/src/components/SettingsView.tsx)),
|
||
but Downloads/Library/History/Terminal are full-width; Onboarding card is 460px, CommandPalette
|
||
560px. On a wide window Settings is a narrow column while siblings stretch edge-to-edge.
|
||
**Standard:** one reading-width token (e.g. 720px) applied by a shared `Screen` wrapper, or commit to full-width everywhere.
|
||
- [ ] **UI2 — Per-screen vertical rhythm differs.** Root section gap is 16 (Settings/Terminal), 20
|
||
(Downloads), 18 (Library), 12 (History). **Standard:** a single section-gap (16px) via a shared screen wrapper.
|
||
- [ ] **UI3 — Card padding has no scale.** 14 (QueueItem) / 16 (DownloadBar) / 20 (Settings card) /
|
||
32 (Onboarding) / `10px 12px` (History row) / `12px 14px` (Library head) / `8px 10px` (template row).
|
||
**Standard:** a padding scale (e.g. control 8–10, list-row 12, card 16, hero 24) keyed to surface tier.
|
||
- [ ] **UI4 — Asymmetric app content padding.** `padding: '24px 28px'` ([App.tsx](src/renderer/src/App.tsx)) —
|
||
28px horizontal appears nowhere else. **Standard:** symmetric or scale-based padding.
|
||
- [ ] **UI5 — Action-cluster gaps split 4px vs 8px.** Icon-button rows use 4px (QueueItem/History/
|
||
TemplateManager) but DownloadBar/Library action rows use 8px. **Standard:** 4px for icon-button groups, 8px for labelled-control rows. *(Good baseline already: every empty-state uses `56px 16px` — keep it.)*
|
||
|
||
### Corner radius
|
||
|
||
- [ ] **UI6 — Thumbnail radius varies for the same element.** QueueItem `Large`, History `Medium`,
|
||
Library rowThumb `Small`, DownloadBar previewThumb `Medium`. **Standard:** one thumbnail radius (Medium/10px). *(Distinct from L9, which is thumbnail pixel dimensions.)*
|
||
- [ ] **UI7 — Card radius (XLarge vs Large) has no stated rule.** Top-level cards use XLarge (16),
|
||
list rows Large (12), but it's applied by feel. **Standard:** document surface tiers — page-card XLarge, list-item Large, control Medium — and apply uniformly.
|
||
- [x] **UI8 — Circular shapes mix token and literal.** `borderRadiusCircular` (Library pill/watchBadge)
|
||
vs literal `'50%'` (SettingsView swatch, History emptyBadge). **Standard:** always the token.
|
||
|
||
### Typography
|
||
|
||
- [x] **UI9 — Screen titles are inconsistent.** Most screens use `<Subtitle2>` (Downloads "Queue",
|
||
Library, Terminal, Settings cards); Onboarding uses `<Title2>`; **History has no title at all**
|
||
(just a count). **Standard:** one page-title style on every screen.
|
||
- [x] **UI10 — Only Library has a screen subtitle/description.** Others jump straight to content.
|
||
**Standard:** a consistent header block (title + optional one-line description).
|
||
- [ ] **UI11 — Semantic type ramp vs ad-hoc px.** Sidebar `brandName`/`navItem` and Library
|
||
`watchBadge`/`pill` set raw `fontSizeBaseXXX`; `sectionIcon`/`mark` use literal `'20px'`/`'26px'`.
|
||
**Standard:** Fluent type components/tokens; no literal px font sizes.
|
||
|
||
### Icons
|
||
|
||
- [ ] **UI12 — No icon-size scale.** Inline `fontSize` of 11/14/16/18/20/22/26/28/40 is scattered
|
||
across components. **Standard:** a small set (16 inline / 20 control / 24 section / 40 empty-state).
|
||
- [ ] **UI13 — Regular vs Filled weight mixed for one concept.** `ArrowDownloadRegular` (nav, buttons)
|
||
vs `ArrowDownloadFilled` (brand mark). **Standard:** one weight per concept (Filled only for brand/emphasis).
|
||
|
||
### Buttons & controls
|
||
|
||
- [x] **UI14 — Two hand-rolled segmented controls.** DownloadBar kind toggle (`segment`, padding
|
||
`7px 16px`) and Sidebar theme toggle (`themeSeg`, padding `7px 4px`) are separate implementations
|
||
with different padding/markup. **Standard:** one shared `SegmentedControl`.
|
||
- [x] **UI15 — Raw `<button>`s alongside Fluent `<Button>`.** Sidebar navItem/iconBtn/themeSeg,
|
||
DownloadBar segment/plKindBtn, CommandPalette item, and Library `groupHead` (a `role="button"` div)
|
||
are bespoke. **Standard:** wrap recurring patterns so hover/focus/disabled are uniform (ties to L5/M15).
|
||
- [ ] **UI16 — Button size hierarchy isn't applied uniformly.** DownloadBar `large`; Library "Index"
|
||
`large` but its toolbar `small`; History all `small`; Settings mostly default. **Standard:** size-by-role (screen primary = large; secondary = medium; row actions = small).
|
||
- [ ] **UI17 — `appearance="secondary"` used in only two spots** (Library "Check for new", Terminal
|
||
"Stop") while every other non-primary button is `subtle`. **Standard:** pick subtle *or* secondary as the standard non-primary appearance.
|
||
- [x] **UI18 — Two status-chip systems.** QueueItem uses Fluent `Badge`; LibraryView uses custom
|
||
color `pill` spans for the same item-status concept (and labels differ — see M8). **Standard:** one shared status-chip component + label map.
|
||
- [ ] **UI19 — Suggestion-banner CSS duplicated.** Identical `suggestion`/`suggestionText` styles in
|
||
DownloadBar and LibraryView. **Standard:** a shared `LinkSuggestion` component (also kills drift).
|
||
|
||
### Colors & selection
|
||
|
||
- [ ] **UI20 — "Selected/active" styling is split.** Solid brand (`colorBrandBackground` +
|
||
on-brand text) for DownloadBar/Sidebar segments, but brand-tint (`colorBrandBackground2` +
|
||
`colorBrandForeground2`) for Sidebar nav, CommandPalette item, Library pill. **Standard:** one active treatment per control class.
|
||
- [ ] **UI21 — Sidebar active-nav rail is expanded-only.** The inset brand box-shadow shows only when
|
||
expanded; collapsed relies on tint alone. **Standard:** a consistent active indicator in both states.
|
||
- [ ] **UI22 — Brand-icon tiles differ.** `mark` (on-brand on solid brand) vs `srcIcon`
|
||
(brandForeground2 on brand-tint) vs `sectionIcon` (compoundBrand, no tile). **Standard:** a defined icon-emphasis set.
|
||
|
||
### Dialogs, menus, status, navigation
|
||
|
||
- [ ] **UI23 — Fragmented overlay/dialog system.** Native OS dialogs (pickers, backup save/open,
|
||
backup warning `messageBox`), one custom React overlay (CommandPalette), a full-screen view
|
||
(Onboarding), and an **unstyled separate `BrowserWindow`** (cookie sign-in) that shows none of the
|
||
app's theme/chrome. **Standard:** keep native for file/confirm; unify in-app overlays under one
|
||
themed primitive; theme the sign-in window's title/background to match.
|
||
- [ ] **UI24 — No context menus anywhere.** Every row exposes actions only as inline buttons;
|
||
right-click does nothing on any screen, despite many per-row actions that conventionally also live on
|
||
right-click. **Standard:** add consistent right-click menus mirroring row actions, or note "none by design."
|
||
- [ ] **UI25 — No in-app global status surface.** Queue progress shows only on the Downloads tab's
|
||
summary strip; on any other tab there's no in-app sign downloads are running (only the OS taskbar).
|
||
**Standard:** a persistent affordance (e.g. a sidebar "Downloads" badge with the active count).
|
||
|
||
### Animations & transitions
|
||
|
||
- [ ] **UI26 — Lone animation.** Only the sidebar width transitions (`0.15s ease`); tab switches,
|
||
panel expand/collapse, banners, and Library card expansion are all instant. **Standard:** one motion
|
||
policy — either add subtle, consistent transitions (gated on `prefers-reduced-motion`) or drop the sidebar one.
|
||
|
||
### Hover, focus, disabled, keyboard & accessibility
|
||
|
||
- [ ] **UI27 — Command palette uses JS hover, not CSS.** Items highlight via `onMouseEnter` setting
|
||
`itemActive` (no `:hover`), unlike every other list, and expose no `aria-selected`/active-descendant,
|
||
so the highlight is invisible to screen readers. **Standard:** CSS `:hover` + listbox/option semantics.
|
||
- [x] **UI28 — Command palette input has no focus ring.** The search `<input>` sets `outline: 'none'`
|
||
with no replacement. **Standard:** a visible focus ring (Fluent stroke token). *(a11y)*
|
||
- [x] **UI29 — Fragmented focus treatment.** Fluent ring (Fluent controls) vs custom border (Select)
|
||
vs removed (palette input) vs UA default (Sidebar buttons, segments, and Library `cardHead`/`groupHead`
|
||
which are `role="button" tabIndex=0` with **no `:focus-visible`** → invisible keyboard focus).
|
||
**Standard:** one focus-ring style on all interactive elements, custom and native. *(a11y; extends L14)*
|
||
- [x] **UI30 — Hand-rolled radiogroups aren't arrow-navigable.** DownloadBar kind and Sidebar theme
|
||
use `role="radiogroup"`/`radio` but implement click only — no ←/→ roving focus a radiogroup implies.
|
||
**Standard:** roving-tabindex arrow keys, or Fluent's RadioGroup. *(a11y)*
|
||
- [ ] **UI31 — `Select` can't be disabled.** The native-`<select>` wrapper exposes no `disabled` prop,
|
||
so that control can't show a disabled state while Fluent controls can. **Standard:** add `disabled` + styling.
|
||
- [ ] **UI32 — Native-control dark mode is uneven.** The DownloadBar `datetime-local` sets
|
||
`colorScheme: 'light dark'` explicitly; the `Select` relies on the root `colorScheme`. **Standard:** set `colorScheme` consistently on both.
|
||
- [ ] **UI33 — No semantic headings.** Titles render as `Subtitle2`/`Title2` (styled spans), so there's
|
||
no h1–h6 hierarchy for screen-reader heading navigation (landmarks `<nav>`/`<main>` exist; headings
|
||
don't). **Standard:** render titles as real headings (`as="h1"`/`"h2"` or `role="heading" aria-level`). *(a11y)*
|
||
|
||
### Top cohesion-breakers (the "not one product" shortlist)
|
||
|
||
1. **Status chips** — Fluent `Badge` (Downloads) vs custom color pills (Library) for the same concept (UI18, M8).
|
||
2. **Segmented controls** — two separate hand-rolled versions (UI14).
|
||
3. **Buttons** — Fluent `<Button>` vs many bespoke `<button>`s with divergent hover/focus (UI15, UI29).
|
||
4. **Content width** — Settings is a 640px column; every other screen is full-width (UI1).
|
||
5. **Focus rings** — four different focus treatments incl. one removed and several invisible (UI28–UI29).
|
||
6. **Screen headers** — Subtitle2 / Title2 / none (UI9–UI10).
|
||
|
||
---
|
||
|
||
## UX review (first-run perspective)
|
||
|
||
Walking the app as a new user. Ranked by severity. `UX` IDs; existing IDs referenced where the
|
||
root cause is already filed.
|
||
|
||
### High — core-flow friction
|
||
|
||
- [ ] **UX1 — Per-download options are unreachable; you must change global Settings for one video.**
|
||
The DownloadBar exposes only URL · Video/Audio · quality · Trim · Schedule. There is **no**
|
||
per-download post-processing panel, custom-command panel, command preview, or incognito toggle —
|
||
so `DownloadItem.options`/`extraArgs` and the whole Phase A/C per-download-override plumbing are
|
||
unreachable; every download uses `settings.downloadOptions`. To grab one video as MP3-with-subs a
|
||
user must edit global Settings, download, then revert. (Supersedes/expands M5, M6.) **Fix:** add a
|
||
collapsible per-download Options panel (reuse `DownloadOptionsForm`) — the plumbing already exists.
|
||
- [ ] **UX2 — "Fetch" vs "Download" is ambiguous, and Enter does the wrong thing.** Two unlabeled
|
||
icon buttons (magnifier = "Fetch", clipboard = "Paste") sit next to "Download"; a first-timer can't
|
||
tell that Download works without Fetch. Pressing **Enter** in the URL field runs a *probe*, not the
|
||
download (L27) — violating "type URL + Enter = go". **Fix:** label the action, make Enter download
|
||
(or fetch-then-download), and present Fetch as optional ("Preview formats").
|
||
- [ ] **UX3 — Library "Index" is jargon and a hidden two-stage workflow.** Pasting a channel URL in
|
||
the Downloads bar tries to treat it as one item; whole channels belong in the Library tab, but
|
||
nothing signposts that. "Index" reads like "Download" but only catalogs — you must then expand,
|
||
select, and Download. **Fix:** rename to "Add channel/playlist," and after indexing surface a clear
|
||
"Download N videos" next step; detect channel URLs in the Downloads bar and suggest the Library.
|
||
- [x] **UX4 — Destructive actions have no confirmation and no undo** (L13). "Clear history,"
|
||
"Clear log," "Remove source" (deletes an entire indexed channel + items), and "Delete selected"
|
||
are all one click. A user exploring can wipe data irreversibly. **Fix:** confirm destructive/bulk
|
||
deletes (or offer an undo toast).
|
||
- [ ] **UX5 — First run can't choose the download folder.** [Onboarding.tsx](src/renderer/src/components/Onboarding.tsx)
|
||
only *describes* Documents\Video/Audio — there's no picker (the ROADMAP claims one). The user must
|
||
later discover Settings → Downloads. **Fix:** put the folder picker in onboarding as documented.
|
||
|
||
### Medium — confusion, feedback & organization
|
||
|
||
- [ ] **UX6 — Silent failures on file actions.** `openFile`/`showInFolder` in the stores call
|
||
`window.api.openPath(...)` and ignore the returned error string; [reveal.ts](src/main/reveal.ts)
|
||
refuses missing/moved files or disallowed types and returns an error nobody surfaces. Clicking
|
||
"Open file" on a moved download does nothing, with no message. **Fix:** surface open/reveal errors.
|
||
- [ ] **UX7 — Settings is one long unsegmented scroll** of ~11 cards with no section nav/anchors; the
|
||
only finder is the hide-cards search (M14). Order is arbitrary and related items are split.
|
||
**Fix:** group into sub-sections (or a left rail) with a stable, logical order.
|
||
- [ ] **UX8 — "Default format" vs "Format & post-processing" are two places for one concept.** Kind +
|
||
quality live under Downloads; container/codec/subs/SponsorBlock live in a separate card. **Fix:**
|
||
co-locate, or clearly label one "defaults" and the other "advanced post-processing."
|
||
- [ ] **UX9 — No global completion feedback off the Downloads tab** (UI25). If OS notifications are
|
||
off/suppressed (focus assist) and you're on another tab, a finished download gives no in-app sign.
|
||
**Fix:** a sidebar Downloads badge / lightweight in-app toast.
|
||
- [ ] **UX10 — Re-download silently changes quality** (H5) — a confusing "I asked for 720p, got Best."
|
||
- [ ] **UX11 — Scheduled downloads silently never fire if the app is quit** (M4). The hint is easy to
|
||
miss; the user returns to find nothing happened. **Fix:** warn at schedule time that it requires the
|
||
app running (or persist + relaunch).
|
||
- [ ] **UX12 — Terminal is a dead-end when custom commands are off** (L18). The nav item is always
|
||
visible; clicking it shows a gate pointing to a setting on another screen. **Fix:** hide/disable the
|
||
nav item, or let the gate enable the setting inline.
|
||
- [ ] **UX13 — Backup export warns of nothing; it contains secrets** (M22). A user "backs up settings"
|
||
and ships proxy creds / tokens in cleartext. **Fix:** warn, or mask/omit secrets.
|
||
- [ ] **UX14 — Duplicate warning can show a placeholder title** (L76): 'Already in your queue:
|
||
"YouTube video (abc123)"' before metadata resolves looks broken. **Fix:** fall back to the URL.
|
||
- [ ] **UX15 — No bulk actions in the Downloads queue.** History has a select-mode + bulk delete;
|
||
Downloads has only per-row remove + "Clear finished." Inconsistent. **Fix:** mirror select/bulk, or
|
||
add "Cancel all."
|
||
|
||
### Low — polish & smaller friction
|
||
|
||
- [ ] **UX16 — URL field isn't focused on launch.** `aerofetch-url` is only focused via the command
|
||
palette; opening the app or the Downloads tab requires a click to start typing. **Fix:** autofocus it.
|
||
- [ ] **UX17 — Icon-only Fetch/Paste buttons** rely on hover tooltips for meaning (UI a11y; new users on touch/keyboard miss them). **Fix:** labels or `aria`-described affordances.
|
||
- [ ] **UX18 — The quality control morphs after Fetch** (preset list → real formats; label "Quality" →
|
||
"Quality / format") in place — a surprising transform. **Fix:** keep a stable control with a clear "formats loaded" state.
|
||
- [ ] **UX19 — Window size/position isn't remembered.** [index.ts](src/main/index.ts) opens a fixed
|
||
920×700 every launch (no bounds persistence). **Fix:** persist + restore window bounds.
|
||
- [ ] **UX20 — "App is still running" surprise.** Closing with a download active hides to tray and
|
||
notifies once per process (L68); later closes give no hint, so the window "won't close" reads as a
|
||
bug. **Fix:** a persistent tray hint / first-close explainer.
|
||
- [ ] **UX21 — No keyboard-shortcut discoverability.** Ctrl+K (palette), Enter, Ctrl+Enter are
|
||
undocumented; there's no "?"/shortcuts screen. **Fix:** a shortcuts hint or help affordance.
|
||
- [ ] **UX22 — Onboarding is shallow and one-shot** (L67): three tips, no folder choice, not
|
||
revisitable; the clipboard tip requires a focus event a first-timer won't trigger.
|
||
- [ ] **UX23 — Settings folder paths are read-only** (L71) — no paste; Browse-dialog only.
|
||
- [ ] **UX24 — "Check N watched for new" is disabled with no explanation** when no source is watched —
|
||
a user with indexed (but unwatched) sources sees a dead button. **Fix:** tooltip / enable with a hint to watch a source.
|
||
- [ ] **UX25 — Empty-state copy mismatches the flow** (L78): Downloads says "Paste a URL above to get
|
||
started," but the actual path is Fetch/Download.
|
||
- [ ] **UX26 — yt-dlp/ffmpeg version + initial settings load have no skeleton/spinner** — brief blank
|
||
states on boot (plus the documented one-frame theme flash). **Fix:** lightweight loading states.
|
||
|
||
---
|
||
|
||
## Windows platform conventions
|
||
|
||
Review against Windows 11 / Fluent desktop conventions. `W` IDs are new; existing IDs are referenced
|
||
where a dimension is already filed. The app gets the big things right (native dialogs, per-user
|
||
no-admin install, AppUserModelID, tray, taskbar progress, jump list, `nativeTheme` dark-mode follow,
|
||
DPI handled by Chromium). The deviations:
|
||
|
||
### Window behavior & resizing
|
||
|
||
- [x] **W1 — No minimum window size.** `createWindow` sets `width/height` but no `minWidth`/`minHeight`
|
||
([index.ts](src/main/index.ts)), so the window can be dragged down to a few pixels and the layout
|
||
(212px sidebar + content) breaks. **Standard:** set a sensible min (e.g. 640×480).
|
||
- [ ] **W2 — Window placement isn't persisted** (extends UX19): size, position, **maximized state**, and
|
||
**which monitor** are all forgotten — it reopens 920×700 on the primary display every launch.
|
||
**Standard:** persist & restore window placement per Windows app convention (e.g. `electron-window-state`).
|
||
- [x] **W3 — Title bar doesn't follow the in-app theme.** `nativeTheme.themeSource` is deliberately never
|
||
set (index.ts:97), so the OS-drawn caption follows the *OS* theme. Choosing an explicit in-app **dark**
|
||
theme on a **light** OS leaves a light title bar on a dark app (and vice-versa). **Standard:** set
|
||
`nativeTheme.themeSource` to the resolved mode, or use `titleBarOverlay` with themed colors.
|
||
|
||
### Dialogs & file pickers
|
||
|
||
- [x] **W4 — Text fields have no Cut/Copy/Paste context menu.** Electron adds no default editing menu and
|
||
the app registers no `context-menu` handler, so right-clicking any input/textarea (URL, search, terminal
|
||
args, template fields) shows nothing — a basic Windows text-editing affordance is missing. **Standard:**
|
||
wire a standard editing context menu (and spellcheck suggestions for textareas).
|
||
- [x] **W5 — File pickers don't open at the current value.** `chooseFolder` sets no `defaultPath`, so the
|
||
folder dialog opens at a default location instead of the currently-configured Video/Audio folder.
|
||
**Standard:** seed the picker with the current path. *(Native dialogs themselves are correct — good.)*
|
||
- [x] **W6 — Cookie sign-in is a separate taskbar window.** `openCookieLoginWindow` creates a
|
||
`BrowserWindow` with no `parent`/`modal` ([cookies.ts](src/main/cookies.ts)), so it appears as a second
|
||
AeroFetch taskbar button rather than a child/modal dialog. **Standard:** `parent: mainWindow` (+ `modal`
|
||
if appropriate) so it groups under the app.
|
||
|
||
### Keyboard, context menus & focus
|
||
|
||
- [x] **W7 — Lists have no keyboard navigation.** The queue, history, and library lists can't be arrowed
|
||
through, **Delete** doesn't remove the focused/selected item, and there's no **Ctrl+A** select-all —
|
||
all standard Windows list behaviors. **Standard:** roving focus + Delete/Ctrl+A on list surfaces. *(a11y; see also UI30)*
|
||
- [x] **W8 — No standard accelerators.** No **Ctrl+,** (Settings), **F1** (help), or **F5** (refresh);
|
||
Ctrl+K (palette) is the only global shortcut and is undiscoverable (UX21). **Standard:** add the
|
||
conventional accelerators + a discoverable shortcut list.
|
||
- [x] **W9 — Focus isn't restored or advanced.** Closing the command palette doesn't return focus to the
|
||
trigger; finishing onboarding and switching tabs don't move focus into the new content. **Standard:**
|
||
restore focus on overlay close; move focus to the activated panel. *(extends UI28–UI29)*
|
||
- [ ] **(ref) Context menus absent app-wide** — UI24; the text-field case is W4.
|
||
|
||
### System tray, taskbar & icons
|
||
|
||
- [ ] **W10 — No taskbar attention flash.** When a background download completes while the window is
|
||
minimized/hidden, the app doesn't `flashFrame` the taskbar button. **Standard:** flash for attention on
|
||
background completion (pairs with the existing notification).
|
||
- [ ] **W11 — No taskbar overlay badge** (deferred in roadmap): a download manager conventionally shows an
|
||
active-count / error overlay icon via `setOverlayIcon`. **Standard:** add it (the progress bar's error
|
||
mode is not a substitute).
|
||
- [ ] **W12 — Fallback tray icon is single-resolution.** The embedded fallback is a 32×32 PNG, so on
|
||
150%/200% DPI the tray glyph is blurry when the real multi-size `.ico` is absent. **Standard:** a
|
||
multi-size fallback (or rely only on the `.ico`).
|
||
- [x] **W13 — Notifications set no icon.** `new Notification({title, body})` omitted `icon`; on the **portable**
|
||
build (no installed AUMID shortcut) Windows toasts showed a generic icon. *Fixed: both notification sites
|
||
([download.ts](src/main/download.ts) completion/failure toast + [index.ts](src/main/index.ts) background-running
|
||
toast) now pass `icon: getAppIconImage()` — a new cached `NativeImage` helper in [binaries.ts](src/main/binaries.ts)
|
||
that loads the app `.ico` once and falls back to an empty image (OS default) if it's missing. (refines L127)*
|
||
- [x] **W14 — App/notification icon is a placeholder** (M3-orig) — *Fixed: redesigned
|
||
[icon.svg](build/icon.svg) into a proper mark (teal brand square with a top-lit gradient + soft sheen and a
|
||
bold rounded download glyph over a landing shelf), legible down to 16px; regenerated the multi-size
|
||
`build/icon.ico` (256/128/64/48/32/16) via ImageMagick. The notification-icon **wiring** (passing the asset
|
||
to `new Notification`) is also done — see W13/L127.*
|
||
|
||
### High DPI, multiple monitors, touch
|
||
|
||
- [ ] **W15 — Software rendering is global.** Hardware acceleration is disabled for a documented GPU
|
||
workaround; on high-DPI/large windows this trades GPU compositing for CPU, which can be sluggish.
|
||
**Standard:** re-enable HW accel where the target GPU allows (the memo already flags revisiting this).
|
||
- [ ] **W16 — Touch targets too small / hover-only labels.** Icon-only row actions are ~32px, 4px apart,
|
||
4 per row (below the ~40px touch guideline), and their only labels are hover tooltips (`Hint`) that
|
||
never appear on touch. **Standard:** ≥40px touch targets and non-hover labels. *(extends UX17)*
|
||
|
||
### Dark mode & accessibility (Windows-specific)
|
||
|
||
- [x] **W17 — No `aria-live` for status changes.** Narrator doesn't announce download progress,
|
||
completion, or errors — there are no live regions. **Standard:** polite live regions for queue/status updates.
|
||
- [x] **W18 — Custom colors unverified under High Contrast.** The status pills, segmented controls, accent
|
||
swatches, and thumbnail tints use explicit background colors that Windows forced-colors mode may not
|
||
adapt (nothing sets `forced-color-adjust`). **Standard:** test under each Contrast theme; let system colors win.
|
||
- [ ] **(ref) No semantic headings / radiogroup arrow-nav / focus rings** — UI33, UI30, UI28–29 (all bear on Narrator + keyboard users).
|
||
|
||
### Settings & standard conventions (mostly OK)
|
||
|
||
- [x] **W19 — Window/taskbar don't reflect state.** No window title at all (L108), so the taskbar button
|
||
never shows "3 downloading" or similar. **Standard:** reflect activity in the title/tooltip.
|
||
- [ ] **(OK)** Per-user no-admin install, NSIS uninstaller, `aerofetch://` registration, AppUserModelID,
|
||
tray tooltip + left-click restore + right-click menu, taskbar progress, and jump list all follow
|
||
Windows conventions. **(Non-goal)** unsigned binary → SmartScreen prompt — accepted; no certificate will
|
||
be purchased (the build stays signing-ready via env vars if that ever changes — see [SIGNING.md](docs/SIGNING.md)).
|
||
|
||
---
|
||
|
||
## Code consistency & conventions
|
||
|
||
A cross-cutting consistency pass over the dimensions requested. Many specifics are already filed; this
|
||
section consolidates them by theme and — the point of the exercise — **recommends one standard each**.
|
||
`CC` IDs are the consolidated items; existing IDs are referenced, not re-filed. The codebase is actually
|
||
*stylistically* consistent (no-semicolon, single-quote, 2-space) and has a genuinely good pure/impure
|
||
split — but that style is unenforced and several "do the same thing two ways" seams have crept in.
|
||
|
||
- [ ] **CC1 — Naming conventions.** Boolean settings have no convention: `useAria2c` (verb-prefix),
|
||
`autoUpdateYtdlp` (auto-prefix), `customCommandEnabled` (suffix), `downloadArchive`/`restrictFilenames`
|
||
(bare). Keys say `videoDir`/`audioDir` while the UI says "folder." Preload names diverge from main
|
||
(L92); `MediaKind` is declared twice (L105). **Standard:** booleans as `is`/`has`/`should`/`<verb>`
|
||
consistently; one term ("folder") across keys + UI; align preload↔main names; single-source shared types in `@shared`.
|
||
- [x] **CC2 — Coding style is consistent but unenforced.** No ESLint/Prettier config, script, or dep
|
||
(L44), so the (good) house style drifts only by discipline; `??` vs `||` is occasionally misused for
|
||
null checks. **Standard:** add Prettier + typescript-eslint with `lint`/`format` scripts in CI; codify the existing style.
|
||
- [ ] **CC3 — Different patterns for the same problem.** JSON persistence (M1), status chips (UI18),
|
||
segmented controls (UI14), buttons (UI15), overlays/dialogs (UI23), three yt-dlp probe spawners
|
||
(`probeMedia`/`probeMeta`/`probeFlat`), and **duplicated stdout line-buffering** in `download.ts` and
|
||
`terminal.ts`. **Standard:** one helper per concern (a `jsonStore`, a `StatusChip`, a `SegmentedControl`,
|
||
one spawn-and-stream helper).
|
||
- [ ] **CC4 — Duplicate utilities.** Formatting (M9), `youtubeId` (H2), `newId` (M7), JSON I/O (M1),
|
||
`MediaKind` (L105) — **plus** `execFile` is hand-wrapped in a `new Promise` in five places
|
||
(`probe`/`ytdlp`/`ffmpeg`/`download.probeMeta`/`indexer.probeFlat`) instead of one `promisify`d helper,
|
||
and the stdout newline-split loop is copied in two. **Standard:** `src/main/lib/` + `src/renderer/src/lib/`
|
||
with one each of: `format`, `youtube`, `jsonStore`, `ytdlpExec` (promisified spawn + line stream).
|
||
- [ ] **CC5 — Async patterns are mixed.** Three I/O idioms: callback-wrapped `new Promise` (execFile ×5),
|
||
`async/await`+`fetch` (updater check, sync), and event-emitter-wrapped Promise (`net.request` in
|
||
updater). Renderer mixes `.then().catch()` (stores) with `async/await`+try/catch (components).
|
||
**Standard:** `async/await` everywhere; a shared `execFileAsync`; wrap event-emitter APIs once in a helper.
|
||
- [ ] **CC6 — Five failure-signaling conventions.** `{ ok, error }` result objects (download/updater/
|
||
ytdlp/backup/IPC), `throw` (`assertHttpUrl`), `null` (indexerCore), `[]` (history/sources read errors),
|
||
and a bare **error string** (`reveal.safeOpenPath`). `cleanError` is applied to some spawn stderr but not
|
||
ytdlp/ffmpeg. **Standard:** a `Result<T>` (`{ ok, value?/error? }`) across every service/IPC boundary;
|
||
`throw` only inside pure helpers, caught at the boundary; always run yt-dlp stderr through `cleanError`.
|
||
- [ ] **CC7 — Dependency-injection styles.** Pure modules take deps as params (good — `binDir`,
|
||
`now`); the impure shell uses module singletons (`getSettings()`, `getYtdlpPath()`, lazy `getStore()`);
|
||
progress is callback-injected; the `WebContents` sender is a param in some handlers and a closure in
|
||
others. **Standard:** keep pure-core param injection; in the shell, pass `wc`/`onProgress` consistently
|
||
as the leading argument and keep singleton access for config/binaries.
|
||
- [ ] **CC8 — No logging strategy.** Effectively no diagnostics — a single stray `console.error` in
|
||
preload, and ~29 swallowed catches (M29); `errorlog.ts` is domain data, not logging. **Standard:** one
|
||
small leveled logger (e.g. `electron-log`) written to userData, called at every catch; keep `errorlog.ts` for user-facing download failures only.
|
||
- [ ] **CC9 — Four validation styles.** Type-guard predicates (`isValid*` in validation.ts),
|
||
coerce-with-fallback (`sanitizeOptions`, `templates.sanitize`), an imperative per-key `switch`
|
||
(`setSettings`, M24), and `throw` (`assertHttpUrl`). **Standard:** one schema layer (e.g. `zod`) that
|
||
both *validates and coerces*; `setSettings` runs values through the same schema instead of a bespoke switch.
|
||
- [ ] **CC10 — Mixed serialization/persistence.** `electron-store` (settings) **and** hand-rolled
|
||
pretty-JSON files (history/errorlog/templates/sources/media-items, M1) for the same job, plus
|
||
yt-dlp-mandated formats (Netscape cookies, plaintext archive) and base64 `enc:v1:` secrets.
|
||
**Standard:** one `jsonStore<T>()` abstraction for the app's own records; pick **either** electron-store
|
||
**or** the JSON stores for everything, not both; leave the yt-dlp-format files alone.
|
||
- [ ] **CC11 — Configuration is scattered.** `electron-store` + `localStorage` (sidebar, M19) + env vars
|
||
(`PORTABLE_EXECUTABLE_DIR`, `CSC_LINK`, `AEROFETCH_REAL_DOWNLOAD`) + hardcoded module consts (update
|
||
host/owner/repo, timeouts, caps, `09:00`, `ARIA2C_ARGS`, L10). **Standard:** a `config.ts` for build/host
|
||
constants; fold `localStorage` UI prefs into the settings store so there's one persisted-prefs source.
|
||
- [ ] **CC12 — Project organization.** Renderer `components/` mixes screens (views) with reusable widgets;
|
||
helpers (`theme`/`thumb`/`useClipboardLink`) sit at src root; the **pure** `queueStats` lives in `store/`;
|
||
main mixes pure (`buildArgs`/`validation`/`indexerCore`/`ytdlpPolicy`) and impure modules in one flat dir.
|
||
**Standard:** renderer `views/` + `components/` + `lib/`; main `core/` (pure) + services; move `queueStats` to `lib`.
|
||
- [ ] **CC13 — View/state boundary (the project's "MVVM").** It's React+Zustand, but the container/
|
||
presentational split is inconsistent: orchestration lives in stores for downloads/sources yet inside the
|
||
component for DownloadBar, and SettingsView calls `window.api` directly (L93, UX1). **Standard:** stores/
|
||
hooks are the view-models (own orchestration + all IPC); components stay presentational; no `window.api` in components.
|
||
- [ ] **CC14 — State ownership is unprincipled.** State lives in Zustand stores, component `useState`,
|
||
`localStorage`, the main `electron-store` (mirrored into a renderer store), and module-level vars
|
||
(`idCounter`, `notifiedBackground`, `active`). Persisted state is optimistic-only and never reconciled
|
||
(M34/L142); two stores form a cycle (C2). **Standard:** main owns persisted state; renderer stores mirror
|
||
it and **apply the authoritative value the IPC call returns**; UI-only state in stores; break store cycles via a coordinator.
|
||
|
||
### Recommended single style (apply project-wide)
|
||
|
||
1. **Errors:** one `Result<T> = { ok: true; value } | { ok: false; error }` across every service/IPC
|
||
boundary; `throw` only in pure helpers; one `logger` (leveled, file-backed) invoked at every catch.
|
||
2. **Async:** `async/await` only; one `execFileAsync` + one spawn/stream helper; wrap event-emitter APIs once.
|
||
3. **Validation:** one schema lib (zod) that validates **and** coerces; reuse it in `setSettings`, backup import, and JSON-store reads.
|
||
4. **Persistence/serialization:** one `jsonStore<T>()` (or electron-store) for app records — not both; one config module for host/build constants; one persisted-prefs store (no `localStorage`).
|
||
5. **Shared code:** `lib/` for pure utils (format, youtube, ids, jsonStore); `@shared` is the only home for cross-process types (no re-declared `MediaKind`).
|
||
6. **UI:** design tokens (spacing/radius/type/icon scales) + shared primitives (`Screen`, `StatusChip`, `SegmentedControl`, `LinkSuggestion`, button/focus); stores are view-models, components presentational.
|
||
7. **Naming:** booleans `is`/`has`/`should`/`<verb>`; "folder" not "dir"; preload methods match main names; `verbNoun` for store actions, `onX` for component handlers.
|
||
8. **Tooling:** Prettier + typescript-eslint with `lint`/`format`/CI gates so all of the above stays enforced rather than aspirational.
|
||
|
||
---
|
||
|
||
## Code cleanliness
|
||
|
||
A dead-code / clutter review. **First, the good news — the codebase is genuinely clean on most axes:**
|
||
no `TODO`/`FIXME`/`HACK`/`XXX` in source (only in roadmap docs), **no commented-out code**, **no
|
||
`debugger`/debug logging** (the lone `@ts-ignore` ×2 in preload is the legitimate browser-fallback
|
||
`window` assignment), every source file is imported, and the dead IPC surface is exactly the two already
|
||
filed. Several listed categories are **N/A** to this stack and worth recording so they're not re-asked:
|
||
|
||
- **Unused XAML / WPF** — N/A (Electron + React/HTML, no XAML).
|
||
- **Unused strings / localization** — N/A (no i18n/string-resource files; strings are hard-coded, i18n deferred), so nothing to be "unused."
|
||
- **Unused images / icons** — none: the only assets are `build/icon.{ico,svg}` (used) and the inline base64 fallback tray PNG (used); thumbnails are remote. *(Clutter, not shipped: `dist/` ~3 GB of old installers — L84; `.frames/` 19 gitignored captures.)*
|
||
- **Unused files / resources** — none in `src` or `resources/bin` (yt-dlp/ffmpeg/ffprobe all used; aria2c optional).
|
||
|
||
**Dead / unreachable code (the real list):**
|
||
|
||
- [x] **Safe to remove now — `getDefaultFolder`** (M2): channel + preload method + main handler + mock,
|
||
**zero callers**. Pure deletion, no behavior change.
|
||
- [ ] **Decide "wire or remove" — command preview** (M5): channel + preload `previewCommand` + main
|
||
`previewCommand()` + `formatCommandLine`/`quoteForDisplay` (buildArgs, **unit-tested**) +
|
||
`CommandPreviewResult` + mock. Either surface it (UX1's per-download panel) or delete the whole chain (and its tests).
|
||
- [ ] **Decide "wire or remove" — incognito/private** (M6): `DownloadItem.incognito` + `AddOptions.incognito`
|
||
+ `buildItem` handling + two history-skip checks + the QueueItem "Private" badge. No UI sets it. Add a toggle or strip the plumbing.
|
||
- [ ] **Decide "wire or remove" — per-download options/extraArgs** (UX1): `DownloadItem.options`/`extraArgs`,
|
||
`StartDownloadOptions.options`/`extraArgs`, `AddOptions.options`/`extraArgs` are plumbed end-to-end but
|
||
never set by any UI. Largest block of built-but-unreachable code — wire the panel or remove the fields.
|
||
- [ ] **Comment, not code — `MAX_ENQUEUE_BATCH`** (M33): referenced by ipc.ts:629 + the roadmap but never
|
||
implemented; fix the comment (nothing to delete).
|
||
|
||
**New cleanliness findings:**
|
||
|
||
- [x] **CL1 — Magic-string protocol markers duplicated across the emit/parse boundary.** `'prog|'` and
|
||
`'path|'` are hard-coded in `buildArgs.ts` (`PROGRESS_TEMPLATE` / `--print after_move:path|…`) and again
|
||
in `download.ts` (`line.startsWith('prog|')` / `'path|'`). Change one and the other breaks silently.
|
||
**Fix:** export shared marker constants from one module.
|
||
- [ ] **CL2 — Long positional parameter list.** `buildArgs(opts, outputTemplate, o, binDir, access,
|
||
extraArgs)` takes six positional args (and `o` vs `opts` is easy to swap). **Fix:** pass a single options object.
|
||
- [ ] **CL3 — Large method: `startDownload`** (~130 lines) bundles spawn + dual metadata path + four inline
|
||
`child` event handlers. **Fix:** extract the stdout-parse + close/error wiring (pairs with CC3's spawn-stream helper).
|
||
- [ ] **CL4 — Deep nesting: `updater.downloadAppUpdate`** — Promise → `net.request` → `response` → `data`
|
||
with nested conditionals/teardown is the hardest-to-follow block. **Fix:** extract a `streamToFile` helper.
|
||
- [x] **CL5 — Superfluous exports.** `PROGRESS_TEMPLATE` (L167) and `getManagedBinDir` are `export`ed but
|
||
used only within their own module. **Fix:** make them module-private.
|
||
- [ ] **CL6 — Redundant wrappers** (minor): `MediaThumb`'s `kind === 'audio' ? 'audio' : 'video'` (L15),
|
||
`clearDir` = `update({[t]:''})`, and the store `openFile`/`showInFolder` thin wrappers around `window.api`.
|
||
Harmless; collapse opportunistically.
|
||
|
||
**Magic numbers / strings (consolidated, see L10):** timeouts `15_000`/`30_000`/`60_000`/`180_000`,
|
||
maxBuffers `4`/`64`/`128`/`256`×1024×1024, caps `500`/`200`/`100`/`20000`/`80`, `VIRTUALIZE_AT 100`,
|
||
installer handoff `1500`, tickers `650`/`15_000`, stderr tail `4000`; magic strings `'enc:v1:'`,
|
||
`'persist:aerofetch-login'`, `'AeroFetchDailySync'`, `'--sync'`, `'prog|'`/`'path|'` (CL1).
|
||
**Fix:** a `constants.ts` (CC11) — names over literals.
|
||
|
||
### What can safely be removed (recommendation)
|
||
|
||
1. **Now (zero risk):** the `getDefaultFolder` slice (M2); un-export `PROGRESS_TEMPLATE` + `getManagedBinDir` (CL5).
|
||
2. **Now (housekeeping):** prune `dist/` to the current release (L84); the macOS-only branches if Windows-only is committed (L147).
|
||
3. **Decision required (don't silently delete — these are documented features):** command preview (M5),
|
||
incognito (M6), and per-download options/extraArgs (UX1) — each is "wire it or remove it." Given UX1
|
||
rates the per-download panel as high value, wiring is likely better than deleting.
|
||
4. **Not removable (intentional):** preview seed/mock data + `PREVIEW` branches (C1/L128) — they power the
|
||
browser-preview dev workflow; consolidate (C1) rather than delete.
|
||
|
||
---
|
||
|
||
## Simplification opportunities
|
||
|
||
Reusable abstractions that shrink the code **and** read better — no premature optimization, just
|
||
de-duplication. Organized by the requested categories; each lists the repetition (with refs), the
|
||
abstraction, and a rough **LOC delta** (net, after the helper's own cost). Estimates are deliberately
|
||
conservative.
|
||
|
||
| # | Category | Repetition (refs) | Reusable abstraction | ~Net LOC |
|
||
|---|---|---|---|---|
|
||
| SIMP1 | Repeated file handling | JSON read/save+cap+try/catch in history/errorlog/templates; sources already has it (M1, CC10) | `createJsonStore<T>(file, isValid, cap)` | −40 |
|
||
| SIMP2 | Repeated networking | `execFile`-in-`Promise` ×5 (probe/ytdlp×2/ffmpeg/probeMeta/indexer) (CC4/CC5) | `execFileAsync` + `spawnYtdlpJson()` | −40 |
|
||
| SIMP3 | Repeated parsing | byte/speed/eta/duration formatters ×4 (M9, L165, L166) | `lib/format.ts` (hour-aware, one precision rule) | −40 |
|
||
| SIMP4 | Repeated parsing | YouTube id/URL parsing ×4 (H2) | `lib/youtube.ts` | −30 |
|
||
| SIMP5 | Repeated logic | stdout line-buffering ×2 + `taskkill` tree-kill ×2 (download.ts/terminal.ts) (CC3, **new**) | `lineStream(stream, onLine)` + shared `killTree(pid)` | −25 |
|
||
| SIMP6 | Repeated commands | 7 identical preload `on*` subscribe/unsubscribe wrappers (**new**) | `subscribe(channel, cb)` helper in preload | −20 |
|
||
| SIMP7 | Repeated event handlers | `Set<string>` add/remove/toggle/toggle-all in DownloadBar/Library/History (**new**) | `useSelection()` hook | −30 |
|
||
| SIMP8 | Repeated view models | async-action `setBusy/try/catch/finally + result + error` ×~7 in SettingsView (**new**) | `useAsyncAction()` hook | −50 |
|
||
| SIMP9 | Repeated UI (cards) | hand-styled `<div>` card surfaces ×6 (UI7/UI20/L120) | `<Surface tier>` (or use Fluent `Card`) | −30 |
|
||
| SIMP10 | Repeated UI (settings) | `sectionHeader` + `Card` ×11 and Switch+`Field`+"On/Off" ×~12 in SettingsView | `<SettingsCard icon title>` + `<ToggleField>` | −90 |
|
||
| SIMP11 | Repeated UI (row actions) | `Hint`+`Button` icon-action ×~16 (QueueItem/History/Templates) | `<IconAction label icon onClick>` | −55 |
|
||
| SIMP12 | Repeated UI (empty states) | centered icon+text `empty` block ×3 (L116) | `<EmptyState icon title hint?>` | −25 |
|
||
| SIMP13 | Repeated dialogs/banners | suggestion banner ×2 (UI19); status chips Badge-vs-pill (UI18, M8) | `<LinkSuggestion>` + `<StatusChip>` | −40 |
|
||
| SIMP14 | Repeated UI (controls) | two hand-rolled segmented controls (UI14) | `<SegmentedControl>` | −30 |
|
||
| SIMP15 | Repeated validation | type-guards + `sanitizeOptions` + `setSettings` switch (CC9, M24) | one schema (zod) validate+coerce | −60 |
|
||
| SIMP16 | Repeated networking | `net.request` + redirect-revalidate + timeout ×2 in updater (`fetchTrustedText`/`downloadAppUpdate`) (**new**) | `trustedRequest(url, {sink})` | −30 |
|
||
| SIMP17 | Repeated logic (misc) | `newId` ×3 (M7), `PREVIEW` ×8 (C1), `(ARR as readonly string[]).includes` ×N (L90) | `lib`: `newId`, `isPreview`, `isMember` | −15 |
|
||
| SIMP18 | Repeated converters | `MediaKind` ×2 (L105), kind\|quality string encode/decode (CL2/L91) | single-source type + typed pair | −10 |
|
||
|
||
### Highest-value (do these first)
|
||
|
||
1. **SettingsView decomposition (SIMP10 + SIMP8 + SIMP11).** A `<SettingsCard>` + `<ToggleField>` +
|
||
`useAsyncAction` + `<IconAction>` would take the 1104-line file to roughly **~600**, and the per-card
|
||
split (H1) falls out for free. Biggest single readability win.
|
||
2. **`createJsonStore` (SIMP1)** collapses three near-identical persistence modules to thin configs and
|
||
removes the M1/CC10 "two ways to persist" seam.
|
||
3. **`lib/format.ts` + `lib/youtube.ts` (SIMP3/SIMP4)** remove the most-copied pure utilities and fix the
|
||
formatter divergences (L165/L166) at the same time.
|
||
4. **`execFileAsync`/`spawnYtdlpJson` (SIMP2/SIMP5)** unify five spawn sites + the duplicated kill/line-buffer.
|
||
|
||
### Estimate
|
||
|
||
Gross duplicated lines removable ≈ **650–750**; the new helpers/primitives add back ≈ **150–200**, for a
|
||
**net reduction of ~550–800 lines (~5–7% of the ~12.5K total)**. Concentration: **SettingsView ~−450**
|
||
(1104 → ~600), the **JSON stores ~−40**, **main spawn/format/util helpers ~−120**, **shared UI primitives
|
||
~−150**. The real payoff is readability and killing the "two-ways-to-do-X" seams (M1/M8/M9/UI14/UI18/CC*),
|
||
not the line count. **Caveat:** stop short of over-abstracting — a `createMirroredStore` factory or a
|
||
generic form engine would cost more clarity than it saves; keep abstractions at the "one obvious helper" level.
|
||
|
||
---
|
||
|
||
## Bug hunt (execution-path review)
|
||
|
||
Tracing real code paths for latent bugs by category. **New** bugs get `B` IDs; the genuine bugs found in
|
||
earlier passes are listed so this is complete; and the categories that came back **clean** are recorded
|
||
(bug-hunting that finds nothing is a result too).
|
||
|
||
### New bugs
|
||
|
||
- [x] **B1 — No stall/idle timeout on downloads (resource leak + stuck slot).** `spawn(ytdlp, …, {
|
||
windowsHide: true })` ([download.ts](src/main/download.ts):306) sets **no `timeout`**, and `buildArgs`
|
||
emits no `--socket-timeout`. A hung connection means yt-dlp never exits → the `active` map entry and the
|
||
renderer's `downloading` item persist **forever**, permanently consuming a concurrency slot with no
|
||
recovery or feedback. (The app-updater download *does* have `DOWNLOAD_IDLE_TIMEOUT_MS`; downloads don't.)
|
||
**Fix:** pass `--socket-timeout` and/or an app-side idle watchdog that kills + errors a stalled child.
|
||
- [ ] **B2 — Source indexing can't be cancelled.** `indexSource` walks a channel's playlists with
|
||
sequential probes (each up to the 180 s `probeFlat` timeout); the UI shows "indexing…" with the Index
|
||
button disabled and **no cancel**. A large channel locks the add-source flow for minutes with no abort.
|
||
**Fix:** thread an `AbortSignal`/cancel token and a Cancel button.
|
||
- [x] **B3 — `extractSha256` ignores the filename (wrong-hash risk).** It returns the **first** 64-hex
|
||
token in the file ([updater.ts](src/main/updater.ts)); a combined multi-file checksum uploaded as
|
||
`<asset>.sha256` would verify the installer against the wrong line's hash. Safe only by the one-hash-
|
||
per-asset convention. **Fix:** match the hash on the asset's filename line.
|
||
- [x] **B4 — `probeMeta` assumes one line per `--print` field.** It does `stdout.split('\n')` → `[title,
|
||
uploader, duration]` ([download.ts](src/main/download.ts)); a title containing a newline shifts channel
|
||
and duration by a line. **Fix:** use a single `--print` with an unlikely delimiter, or `-J`.
|
||
- [x] **B5 — Missing status guard on the `meta` event.** Between `cancel()` and the child's `close`,
|
||
`active.has(id)` is briefly true, so `probeMeta` can `send` a `meta` event for a just-canceled item;
|
||
`applyEvent`'s `meta` case (unlike `progress`/`done`/`error`) has **no canceled guard**, so it updates a
|
||
canceled item's title. Harmless today, but the asymmetry is a latent bug. **Fix:** guard `meta` like the others.
|
||
- [ ] **B6 — App-update download has no cancel.** Once `downloadAppUpdate` starts there's no user abort —
|
||
only completion or the idle timeout stops it; the progress UI offers no Cancel. **Fix:** expose cancel (abort the request).
|
||
- [x] **B7 — Cookie-login promise can never resolve if the window is destroyed without `close`.**
|
||
`openCookieLoginWindow` resolves only via the `close`→`exportAndResolve` path; a `destroy()` (or a
|
||
`closed` without `close`) would leave `pendingResolvers` pending forever. Not triggered by current code,
|
||
but a latent never-resolve. **Fix:** also resolve/reject on `closed`.
|
||
|
||
### Real bugs already filed (confirmed in this pass)
|
||
|
||
- **M32** playlist/channel batches download in reverse order · **M35** History re-download duplicates rows
|
||
· **M34** optimistic settings never reconcile (UI shows unsaved values) · **L88** a `progress` event can
|
||
flip a `queued` item to `downloading` outside `pump()` · **L139** startup yt-dlp auto-update vs a
|
||
concurrent download spawning the same exe (Windows file-in-use) · **L140** retry-during-teardown hits the
|
||
"already running" guard · **L148** `clearFinished` can free a slot before the killed tree exits (brief
|
||
over-cap) · **L157** `markDownloaded` no-ops after `removeSource` (orphan downloads).
|
||
|
||
### Came back clean (no bug found)
|
||
|
||
- **IPC-after-destroy:** every `wc.send`/`e.sender.send`/`mainWindow.*` is guarded by `isDestroyed()`.
|
||
- **Threading / UI threading / synchronization:** single JS thread in main and renderer; React state
|
||
updates are always on-thread; the `active` map + module vars are main-thread-only — no data races.
|
||
- **Infinite loops:** the stdout line-buffer advances each iteration; `parseExtraArgs`/`parseRssVideoIds`
|
||
regexes can't zero-width-match; no self-recursive `pump`.
|
||
- **Stream disposal (updater):** `downloadAppUpdate` has a single `finish()` that aborts the request,
|
||
destroys the file stream, and unlinks the partial — correct on every failure path.
|
||
- **Event-listener cleanup:** component `useEffect` subscriptions return their unsubscribe; module-level
|
||
subscriptions are app-lifetime by design.
|
||
- **Off-by-one / boundary:** `parseProgress` guards `parts.length < 6`; playlist/collection indexing is
|
||
1-based throughout; `MAX_ENTRIES` slices are correct; `fmtBytes` clamps the unit index.
|
||
|
||
---
|
||
|
||
## Resilience & failure-mode review
|
||
|
||
A fresh lens: what happens when the *environment* fails (disk full, permission denied, corrupt file,
|
||
killed mid-write, huge inputs) rather than when the logic is wrong. These are robustness findings, not
|
||
cosmetics. `R` IDs.
|
||
|
||
- [x] **R1 — JSON stores are not written atomically (corruption on crash/power-loss).** The hand-rolled
|
||
stores (history/errorlog/templates/sources/media-items) persist via `writeFileSync(file, JSON.stringify…)`
|
||
— not write-temp-then-rename. A crash/power-cut mid-write truncates the file; the next read hits a parse
|
||
error and returns `[]`. `electron-store` (settings) *is* atomic, so durability is inconsistent across the
|
||
app. **Fix:** an atomic write (temp + rename, or `write-file-atomic`) in the shared `jsonStore` (SIMP1).
|
||
- [x] **R2 — A corrupt/invalid store silently loses all data.** `readJsonArray`/`listHistory` etc. catch a
|
||
parse error and return `[]` with no warning, backup, or recovery — one bad byte wipes the user's history/
|
||
sources/templates from their perspective. **Fix:** on parse failure, back up the bad file (`.corrupt`) and surface a notice.
|
||
- [x] **R3 — O(n) full-file rewrite per download completion (≈O(n²) per channel), synchronously on the
|
||
main thread.** `setMediaItemDownloaded` ([sources.ts](src/main/sources.ts):126) reads + parses the entire
|
||
`media-items.json` (up to `MAX_ITEMS` = 20,000) and rewrites the whole file on **every** completion, and
|
||
`addHistory` rewrites all of `history.json` likewise. Downloading a 1,000-video channel ⇒ ~1,000 full
|
||
read-parse-write cycles of a multi-MB file, blocking the IPC/event loop each time. **Fix:** in-memory
|
||
cache + debounced/batched atomic writes (or the deferred better-sqlite3).
|
||
- [ ] **R4 — Canceled downloads orphan `.part` files.** `cancelDownload` tree-kills yt-dlp but never
|
||
deletes the partial; canceled items vanish from the UI while `.part`/fragment files accumulate in the
|
||
output folder. (App crash mid-download likewise — the queue isn't persisted to resume, M4.) **Fix:**
|
||
clean up the partial on cancel, or surface/offer-resume for orphans.
|
||
- [x] **R5 — Settings write failure is unhandled.** `setSettings` calls `store.set(...)` with no try/catch;
|
||
on disk-full/read-only `electron-store` throws → the IPC rejects → the renderer's `setSettings().catch(()
|
||
=> {})` swallows it while the optimistic UI keeps the change (the disk-full form of M34). **Fix:** catch and report.
|
||
- [ ] **R6 — Silent data loss on any store write failure.** A completed download that can't be recorded
|
||
(disk full) shows "Completed" in-session but is gone on restart, with no error (the write `catch` is empty,
|
||
M29). **Fix:** detect write failure and warn.
|
||
- [ ] **R7 — `safeStorage` unavailable ⇒ secrets silently stored as plaintext.** `encryptSecret` falls back
|
||
to plaintext when DPAPI/safeStorage isn't available, with no indication — compounds H7 (cookies) and the
|
||
backup-cleartext issue (M22). **Fix:** warn (or refuse to store) when encryption is unavailable.
|
||
- [ ] **R8 — Pretty-printed JSON for large stores.** `JSON.stringify(value, null, 2)` inflates size/write
|
||
time for the potentially-20k-item media store (ties R3). **Fix:** compact JSON for the large stores.
|
||
- [ ] **R9 — Clock-skew sensitivity.** `shouldAutoCheckYtdlp` and the scheduled-download promoter compare
|
||
`Date.now()`; a backward system-clock correction can skip or double-fire a check/schedule. Minor; note for awareness.
|
||
|
||
---
|
||
|
||
## Ship-readiness (perceived quality)
|
||
|
||
Obsessive "does it feel finished" pass. The biggest untapped vein is **defaults** (first-run impressions),
|
||
plus a few stuck-status / progress glitches. `SR` IDs; already-filed perceived-quality items are listed at
|
||
the end so the picture is complete. **Verified-clean first:** card/label/button text is consistently
|
||
*sentence case*, and a careful read found **no spelling typos** — the polish gaps are defaults, jargon, and a couple of stuck states, not sloppy text.
|
||
|
||
### Poor / risky defaults (first-run impressions)
|
||
|
||
- [x] **SR1 — Theme defaults to `'light'`, not `'system'`.** A dark-mode Windows user gets a bright white
|
||
app on first launch despite the app fully supporting "follow system." Single most-noticeable first-run
|
||
miss. **Fix:** default `theme: 'system'`.
|
||
- [x] **SR2 — `ytdlpChannel` defaults to `'nightly'`.** The app ships auto-pulling yt-dlp's daily,
|
||
less-tested build (auto-update also on by default); one nightly regression breaks downloads for everyone.
|
||
Defensible for chasing YouTube changes, but a risky default for a commercial release. **Fix:** consider `stable` as the shipped default.
|
||
- [x] **SR3 — `autoDownloadNew` defaults to `true`.** The moment a user marks a channel "watched," new
|
||
uploads auto-download with no per-event consent — can quietly fill a disk. **Fix:** default off (let watching be "notify," downloading be opt-in).
|
||
- [ ] **SR4 — `clipboardWatch` defaults to `true`.** The app reads the clipboard on every window focus from
|
||
first launch (R7/L138). A privacy-conscious default would be off or first-run-prompted. **Fix:** default off, or ask on first run.
|
||
- [ ] **SR5 — Audio defaults to MP3 (lossy).** `defaultAudioQuality: 'Best (MP3)'` + `audioFormat: 'mp3'`
|
||
give a lossy re-encode by default; m4a/opus are higher quality at the same size. Minor (MP3 = max
|
||
compatibility), but "Best" implying MP3 is a quality mismatch (M18).
|
||
|
||
### Stuck states & progress glitches
|
||
|
||
- [x] **SR6 — "Resolving…" can stick forever.** A queued item's channel shows the `'Resolving…'`
|
||
placeholder until a `meta` event arrives; if metadata never resolves (error before meta, or no channel
|
||
in the probe), the row reads "Resolving… • 1080p • Video" permanently — looks hung. **Fix:** clear the placeholder on error / when meta fails.
|
||
- [x] **SR7 — Progress bar visibly resets mid-download.** A video+audio merge downloads as two streams, so
|
||
the bar fills 0→100%, then resets to 0→100% again before muxing — looks like a glitch/restart to the
|
||
user. **Fix:** weight the two phases (e.g. 0–50% / 50–100%) or show a "merging…" state.
|
||
- [x] **SR8 — Window title never reflects state** (L108/W19): always the static "AeroFetch," so the taskbar
|
||
can't show "3 downloading." A finished-feeling app updates its title/tooltip with activity.
|
||
|
||
### Wording / jargon polish
|
||
|
||
- [x] **SR9 — Dev jargon in user-facing option hints** (extends M37): "mux them into the video," "Tiebreaker,
|
||
not a hard filter," "Endcards / credits" ("End cards"), "Proof-of-Origin token," "`--extractor-args`."
|
||
Reads like engineer notes. **Fix:** plain-language hints.
|
||
- [x] **SR10 — Inconsistent product self-description / casing** surfaced to users: "yt-dlp frontend" caption
|
||
flips to "v0.5.0" on load (L66); "Sign in" vs "Sign-in" (L95); "PO token" vs "PO Token" (L150).
|
||
|
||
### Also reduces perceived quality (already filed — listed for completeness)
|
||
|
||
- Onboarding has no folder picker / feels thin (UX5/UX22); no boot loading state + one-frame theme flash
|
||
(UX26); refresh-icon mismatch ArrowClockwise vs ArrowSync (L106) and Regular/Filled brand icon (UI13); only
|
||
the sidebar animates, everything else snaps (UI26); separator glyph/spacing inconsistency `•`/`·` (L164);
|
||
the command palette is undiscoverable (UX21). *(The unsigned-build SmartScreen prompt is accepted as a
|
||
non-goal — no certificate.)*
|
||
|
||
**Ship-blockers (highest perceived-quality impact):** SR1 (light-on-dark first launch), SR7 (progress
|
||
visibly restarting), SR6 ("Resolving…" stuck), and W14 (placeholder icon) — **all now resolved.** The
|
||
unsigned-build SmartScreen prompt is accepted as a non-goal (no certificate). Together these were most of
|
||
the "this feels finished" gap.
|
||
|
||
---
|
||
|
||
## Performance (no premature optimization — real hot paths only)
|
||
|
||
A fresh lens: redundant work on hot paths and scaling cliffs, not micro-tuning. `PERF` IDs. **Verified
|
||
acceptable first:** memoized `QueueItem`s don't re-render when their item is unchanged (the `applyEvent`
|
||
`map` preserves references for untouched items); the queue + large library lists are virtualized;
|
||
`electron-store` is constructed lazily. The issues:
|
||
|
||
- [x] **PERF1 — `getSettings()` is heavy and on every hot path.** It runs per download spawn (twice — in
|
||
`buildCommand` and the `maxConcurrent` check), per completion (`notify`), and on the system-theme bridge;
|
||
each call re-reads the whole store and, for users who've configured a proxy/PO-token/update-token, runs
|
||
up to 3 `safeStorage.decryptString` (DPAPI) calls. **Fix:** cache the decrypted settings, invalidate on write.
|
||
- [x] **PERF2 — `templates.json` is read+parsed on every download.** `resolveExtraArgs` evaluates
|
||
`templates: listTemplates()` eagerly as an argument ([download.ts](src/main/download.ts):182) *before*
|
||
`selectExtraArgs` checks `customCommandEnabled`, so the file is read even when custom commands are off.
|
||
**Fix:** pass a lazy getter, or short-circuit on the gate first.
|
||
- [ ] **PERF3 — `pump()` is O(n) per queue event.** It `filter`+`reverse`+`slice`s the entire items array
|
||
on every add/done/error/cancel/progress-driven change; with M33's unbounded channel enqueue (thousands of
|
||
items) every event becomes O(n), and several events fire per second during active downloads. **Fix:**
|
||
track running/queued counts + a pointer instead of rescanning.
|
||
- [ ] **PERF4 — `summarizeQueue` runs twice per progress tick.** Once in App's store subscription (which
|
||
also pushes a taskbar IPC every tick, L25) and once in DownloadsView's render (no `useMemo`, L94) — each
|
||
O(items), every ~second per active download. **Fix:** compute once, memoize, throttle the taskbar push.
|
||
- [ ] **PERF5 — `VirtualList` gets new function identities each render.** `estimateSize`/`getKey`/
|
||
`renderItem` are passed as inline arrows, so the virtualizer can't rely on stable references (risking
|
||
re-measures/cache churn). **Fix:** `useCallback`/hoist them.
|
||
- [ ] **PERF6 — Per-thumbnail store subscriptions.** Each `MediaThumb` calls `useResolvedDark()` (two
|
||
store subscriptions) (L162); a long list creates N×2 subscriptions. **Fix:** resolve once, pass as a prop.
|
||
- [x] **PERF7 — (ref R3) O(n²) media-items rewrites** dominate the main-process cost when downloading a
|
||
channel; closed by the cached/atomic `jsonStore` (SIMP1) — the single highest-value perf fix.
|
||
- [ ] **PERF8 — No code-splitting.** All views (incl. the rarely-used Terminal/Settings) load up front in
|
||
one renderer bundle alongside Fluent UI + React; software rendering (W15) compounds first-paint cost.
|
||
Minor for a desktop app, but lazy-loading heavy views would trim startup. **Fix:** `React.lazy` the heavy tabs.
|
||
|
||
**Priority:** PERF7/R3 (scaling cliff) and PERF1/PERF2 (per-download redundant I/O + crypto) are the only
|
||
ones with real user-visible impact; the rest are good hygiene. None warrant work before the correctness
|
||
bugs (B1, M32, M35) — listed for completeness, not urgency.
|
||
|
||
---
|
||
|
||
## Completed
|
||
|
||
Security & correctness audit (2026-06-23):
|
||
|
||
- [x] **S1** — Backup import: confirm before enabling imported custom-command templates.
|
||
- [x] **S2** — Cookie login window: restrict popups to http(s)/about: schemes.
|
||
- [x] **S3** — Enforce `maxConcurrent` in main (`startDownload`), not just the renderer.
|
||
- [x] **S4** — Path-traversal sanitization for `filenameTemplate` + `outputDir`.
|
||
- [x] **S5** — Per-row validation on persisted JSON reads.
|
||
- [x] **C1 (orig)** — Bundle the missing `ffprobe.exe`; assert ffmpeg+ffprobe presence up front.
|
||
- [x] **P1** — Stop `getSettings()` writing to disk on every read.
|
||
- [x] **P2** — Forward the renderer's pre-probed metadata to main; skip the redundant probe.
|
||
- [x] **P3** — Document "lower maxConcurrent mid-flight doesn't pause overflow".
|
||
- [x] **M1 (orig)** — Extract duplicated `cleanError` to `log.ts`.
|
||
- [x] **M2 (orig)** — Document `parseExtraArgs` quoting limitations.
|
||
- [x] **M3 (orig)** — App icon placeholder (designed asset still wanted pre-v1.0).
|
||
|
||
---
|
||
|
||
## Deferred
|
||
|
||
- Designed app icon before v1.0 (placeholder shipped).
|
||
- Migrate JSON stores to better-sqlite3 if a user indexes many large channels.
|
||
- Metadata editing (`--parse-metadata`/`--replace-in-metadata`) — held pending a live recipe.
|
||
- PO-token WebView auto-minting — only the `--extractor-args` plumbing shipped.
|
||
- Taskbar overlay badge (needs a drawn `NativeImage`).
|
||
- i18n — deliberately deferred (note: UI strings are hard-coded throughout).
|
||
- Code signing — **non-goal**: no certificate will be purchased, so builds ship unsigned and the SmartScreen
|
||
prompt is accepted. The build remains signing-ready via `CSC_LINK`/`CSC_KEY_PASSWORD` env vars if this ever
|
||
changes — see [SIGNING.md](docs/SIGNING.md).
|
||
- Live smoke-test of OS-level wiring (tray/jump list/scheduled sync/`aerofetch://`).
|