feat(audit): Batch 25 — project reorg (CC12), leading DI args (CC7), CC10 by-design
CC12: renderer views/ (5 screens + Onboarding + settings cards) + lib/ (theme/thumb/datetime/id/qualityOptions/useClipboardLink/queueStats); main core/ (buildArgs/validation/indexerCore/ytdlpPolicy). git mv preserved history; all src+test imports updated. Build emits view chunks from views/, 344 tests + typecheck green, live probe rendered all screens. CC7: wc/onProgress is the leading arg everywhere — downloadAppUpdate(wc,url), indexSource(onProgress,url,signal), indexSourceCancelable(onProgress,url). CC10: closed by-design — jsonStore backs all records; settings stay in electron-store for DPAPI secret encryption (a deliberate two-store split). Also closes the UI24/W4 context-menu cross-reference. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
+27
-18
@@ -1845,7 +1845,9 @@ DPI handled by Chromium). The deviations:
|
||||
- [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.
|
||||
- [x] **(ref) Context menus absent app-wide** — UI24; the text-field case is W4. *Both referents resolved:
|
||||
UI24 is "none by design" (portal-based menus flicker on the dev GPU; every row action is a visible button
|
||||
with keyboard support) and W4 added the Cut/Copy/Paste editing menu on text fields.*
|
||||
|
||||
### System tray, taskbar & icons
|
||||
|
||||
@@ -1998,16 +2000,17 @@ split — but that style is unenforced and several "do the same thing two ways"
|
||||
broader "one `Result<T>` across every boundary" holds already for the service/IPC layer (they return
|
||||
`{ ok, error }`); collapsing the remaining `null`/`[]`/bare-string internal signals into it is a low-value
|
||||
sweep left to the incremental track — the specific `cleanError` inconsistency this item names is closed.*
|
||||
- [ ] **CC7 — Dependency-injection styles.** Pure modules take deps as params (good — `binDir`,
|
||||
- [x] **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.
|
||||
*Deferred (low-value restyle): the substantive half already holds — the pure core
|
||||
(`buildArgs`/`validation`/`indexerCore`/`ytdlpPolicy`) takes deps as params and the shell uses singletons
|
||||
for config/binaries, which is the recommended split. What's left is purely cosmetic argument-ordering
|
||||
(`wc`/`onProgress` position), not worth the churn/risk across every handler unattended; recorded as the
|
||||
convention for new handlers.*
|
||||
*Fixed (Batch 25): the injected `wc`/`onProgress` is now the LEADING argument on every shell function that
|
||||
takes one — `downloadAppUpdate(wc, url)`, `indexSource(onProgress, url, signal)`,
|
||||
`indexSourceCancelable(onProgress, url)` — matching the existing `startDownload(wc, opts)` /
|
||||
`runTerminal(wc, id, args)` / download.ts `send(wc, ev)`/`notify(wc, …)`. Call sites updated; the updater
|
||||
pins were updated for the flip and stay green. Singleton access for config/binaries is kept as the
|
||||
recommended split.*
|
||||
- [x] **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.
|
||||
@@ -2033,16 +2036,19 @@ split — but that style is unenforced and several "do the same thing two ways"
|
||||
[test/settingsSchema.test.ts](test/settingsSchema.test.ts); the whole suite passes unchanged. The jsonStore
|
||||
row guards deliberately stay type-guard predicates (hot read path filtering untrusted disk rows — no
|
||||
coercion wanted), documented in the schema header.*
|
||||
- [ ] **CC10 — Mixed serialization/persistence.** `electron-store` (settings) **and** hand-rolled
|
||||
- [x] **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.
|
||||
*Partly closed / deferred: the `jsonStore<T>()` abstraction now exists (`createJsonStore`, M4/R1) and backs
|
||||
every app record (history/errorlog/templates/sources/media-items/queue), so the "hand-rolled per file"
|
||||
divergence is gone. The remaining electron-store↔jsonStore split is a **deliberate** keep: settings live in
|
||||
electron-store for its DPAPI-encrypted secret fields, records in jsonStore. Fully migrating settings off
|
||||
electron-store is a 1.x call, not forced here.*
|
||||
*Resolved as deliberate-by-design (Batch 25): the `jsonStore<T>()` abstraction (`createJsonStore`, M4/R1)
|
||||
already backs every app record (history/errorlog/templates/sources/media-items/queue), so the
|
||||
"hand-rolled per file" divergence is gone. The remaining electron-store↔jsonStore split is a deliberate
|
||||
KEEP, not drift: settings live in electron-store for its DPAPI-encrypted secret fields (proxy/PO-token/
|
||||
update-token via safeStorage, now flowing through the CC9 schema before encryption), records live in
|
||||
jsonStore. Migrating settings off electron-store would mean re-implementing at-rest secret encryption for
|
||||
no functional gain, so the two-store split stays on purpose. The yt-dlp-format files (Netscape cookies,
|
||||
plaintext archive) correctly stay in their mandated formats.*
|
||||
- [x] **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
|
||||
@@ -2051,14 +2057,17 @@ split — but that style is unenforced and several "do the same thing two ways"
|
||||
now live in [config.ts](src/main/config.ts), imported by the updater — deploy identity in one file,
|
||||
runtime tunables in constants.ts (L10), user prefs in settings (localStorage was folded in by M19). The
|
||||
env vars are genuinely environmental (portable mode, CI signing, integration-test opt-in) and stay env vars.*
|
||||
- [ ] **CC12 — Project organization.** Renderer `components/` mixes screens (views) with reusable widgets;
|
||||
- [x] **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`.
|
||||
*Deferred to 1.x: a file-tree reorg is broad import churn across nearly every module for no behavior change —
|
||||
exactly the kind of sweeping move that wants its own reviewed PR, not an unattended batch. The `lib/`
|
||||
convention is already established (both sides have one and new pure utils land there); the wholesale
|
||||
`views/`+`core/` split is the 1.x task. Standard recorded.*
|
||||
*Fixed (Batch 25): the reorg landed. Renderer — the five screens + Onboarding + `useTerminalRun` +
|
||||
the `settings/` cards moved to `views/`; the root helpers (`theme`/`thumb`/`thumbSizes`/`datetime`/`id`/
|
||||
`qualityOptions`/`useClipboardLink`) and the pure `queueStats` moved to `lib/` (which already held
|
||||
`urlHelpers`/`formatters`/etc). Main — the pure core (`buildArgs`/`validation`/`indexerCore`/`ytdlpPolicy`)
|
||||
moved to `core/`, leaving the flat `src/main` root for services. `git mv` preserved history; every import
|
||||
(src + test) updated. Verified: typecheck 0, full suite (344) green, production build emits the four view
|
||||
chunks from their new `views/` home, and the live preview probe rendered all five screens post-move.*
|
||||
- [x] **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/
|
||||
|
||||
Reference in New Issue
Block a user