From d7b57378065d416b65d5dbb9a87ed15024581fb3 Mon Sep 17 00:00:00 2001
From: debont80 <dmbont23@mailbox.org>
Date: Tue, 30 Jun 2026 15:11:13 -0400
Subject: [PATCH] Self-review fixes: revert ineffective/regression-risk changes
 from audit batch

Addressing issues found reviewing this session's audit commits:

REGRESSIONS FIXED
- updater.ts (L52): reverted the spawn(detached) installer launch. Raw
  CreateProcess/spawn fails with ERROR_ELEVATION_REQUIRED if the NSIS build
  flips to perMachine (the config comment invites this), and it silently
  dropped shell.openPath's launch-error detection. Restored shell.openPath
  (ShellExecute honors the elevation manifest) and kept the L52 goal by
  replacing the 1500ms timer with setImmediate(app.quit).
- LibraryView.tsx (M15): native <button> doesn't inherit color (UA sets
  ButtonText), so the source-card chevron (currentColor) would render wrong
  in dark mode. Added color: colorNeutralForeground1 to cardHead.

INEFFECTIVE CHANGE REVERTED
- base.css (W18): forced-color-adjust:auto on * is the CSS default (no-op),
  and [class*="backdrop"] never matches Fluent/Griffel's hashed atomic class
  names. Reverted; W18 unmarked (needs real High-Contrast testing).

WEAK FIX REVERTED
- Onboarding.tsx (L67): "Skip" and "Get started" called the identical handler
  on a single-screen onboarding. Removed the duplicate button; L67 unmarked
  (real ask is a "show tips again" revisit affordance).

STYLE / CORRECTNESS
- Stripped UTF-8 BOMs accidentally added to Select/CommandPalette/App/
  Onboarding by the PowerShell sanitizer; left pre-existing BOMs untouched.
- DownloadBar.tsx: merged the duplicate @shared/ipc import; removed a stray
  double blank line.
- qualityOptions.ts (L29): restored the original `satisfies Record<MediaKind,
  readonly string[]>` constraint + the noUncheckedIndexedAccess rationale
  comment (the extraction had dropped both for a weaker `as const`).
- downloads.ts: removed double blank line left by the QUALITY_OPTIONS move.
- binaries.ts: corrected YTDLP_MISSING_MSG doc comment ("at startup" -> the
  actual call sites).

typecheck + 242 tests + eslint all green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 CODE-AUDIT.md                                 |  4 +--
 src/main/binaries.ts                          |  2 +-
 src/main/updater.ts                           | 17 ++++-----
 src/renderer/src/App.tsx                      |  2 +-
 src/renderer/src/assets/base.css              | 11 ------
 .../src/components/CommandPalette.tsx         |  2 +-
 src/renderer/src/components/DownloadBar.tsx   |  9 +++--
 src/renderer/src/components/LibraryView.tsx   |  3 ++
 src/renderer/src/components/Onboarding.tsx    | 29 +++++----------
 src/renderer/src/components/Select.tsx        |  2 +-
 src/renderer/src/qualityOptions.ts            |  7 ++--
 src/renderer/src/store/downloads.ts           | 35 +++++++++----------
 12 files changed, 54 insertions(+), 69 deletions(-)

diff --git a/CODE-AUDIT.md b/CODE-AUDIT.md
index ec70dba..c0f4129 100644
--- a/CODE-AUDIT.md
+++ b/CODE-AUDIT.md
@@ -501,7 +501,7 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
 - [ ] **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").
+- [ ] **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`/
@@ -1042,7 +1042,7 @@ DPI handled by Chromium). The deviations:
 
 - [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
+- [ ] **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).
diff --git a/src/main/binaries.ts b/src/main/binaries.ts
index 81be1fb..8b3cde5 100644
--- a/src/main/binaries.ts
+++ b/src/main/binaries.ts
@@ -80,7 +80,7 @@ export function getFfprobePath(): string {
   return join(getBinDir(), 'ffprobe.exe')
 }
 
-/** User-facing error shown whenever yt-dlp.exe is not found at startup. */
+/** User-facing error shown by download/probe/index/update when yt-dlp.exe is missing. */
 export const YTDLP_MISSING_MSG =
   'yt-dlp.exe is missing. Open Settings → Software update to re-download it.'
 
diff --git a/src/main/updater.ts b/src/main/updater.ts
index 79d1bb2..4e54665 100644
--- a/src/main/updater.ts
+++ b/src/main/updater.ts
@@ -1,5 +1,4 @@
-import { app, net, type WebContents } from 'electron'
-import { spawn } from 'child_process'
+import { app, net, shell, type WebContents } from 'electron'
 import { createWriteStream, type WriteStream } from 'fs'
 import { stat, unlink } from 'fs/promises'
 import { join, normalize, dirname } from 'path'
@@ -426,12 +425,14 @@ export async function runAppUpdate(filePath: string): Promise<{ ok: boolean; err
       return { ok: false, error: 'Refused to run an unexpected file.' }
     }
     await stat(target) // throws if the file is missing
-    // Spawn the installer detached + unref'd so it outlives our process even on a
-    // slow machine. shell.openPath goes through ShellExecute but gives us no child
-    // PID to wait on; spawning directly lets us call unref() before we quit.
-    const child = spawn(target, [], { detached: true, stdio: 'ignore', shell: false })
-    child.unref()
-    // Let the IPC response reach the renderer before we exit.
+    // Hand the installer to the OS via ShellExecute, which (unlike a raw
+    // CreateProcess/spawn) honors the NSIS elevation manifest — needed if the
+    // build ever flips to perMachine. openPath resolves once the launch is
+    // initiated, so the installer process already exists when we quit.
+    const err = await shell.openPath(target)
+    if (err) return { ok: false, error: err }
+    // Quit on the next tick so this IPC response reaches the renderer first; the
+    // launched installer is independent of our process and survives the quit.
     setImmediate(() => app.quit())
     return { ok: true }
   } catch (e) {
diff --git a/src/renderer/src/App.tsx b/src/renderer/src/App.tsx
index c3fbc19..ecc44b1 100644
--- a/src/renderer/src/App.tsx
+++ b/src/renderer/src/App.tsx
@@ -1,4 +1,4 @@
-import { useState, useEffect, useMemo, useRef } from 'react'
+import { useState, useEffect, useMemo, useRef } from 'react'
 import { FluentProvider, makeStyles, tokens } from '@fluentui/react-components'
 import { Sidebar, type TabValue } from './components/Sidebar'
 import { DownloadsView } from './components/DownloadsView'
diff --git a/src/renderer/src/assets/base.css b/src/renderer/src/assets/base.css
index 89b5870..c0ee042 100644
--- a/src/renderer/src/assets/base.css
+++ b/src/renderer/src/assets/base.css
@@ -2,17 +2,6 @@
 *::before,
 *::after {
   box-sizing: border-box;
-  /* Let Windows High Contrast mode override all colors by default (W18). */
-  forced-color-adjust: auto;
-}
-
-/* CommandPalette backdrop uses a hardcoded rgba — replace it with the standard HC
-   overlay so the backdrop remains perceptible under forced-colors. */
-@media (forced-colors: active) {
-  [class*="backdrop"] {
-    background-color: Canvas;
-    border: 2px solid ButtonText;
-  }
 }
 
 :focus-visible {
diff --git a/src/renderer/src/components/CommandPalette.tsx b/src/renderer/src/components/CommandPalette.tsx
index ba0e525..e844b8e 100644
--- a/src/renderer/src/components/CommandPalette.tsx
+++ b/src/renderer/src/components/CommandPalette.tsx
@@ -1,4 +1,4 @@
-import { useEffect, useRef, useState } from 'react'
+import { useEffect, useRef, useState } from 'react'
 import { makeStyles, mergeClasses, tokens, shorthands } from '@fluentui/react-components'
 import { useFocusStyles } from './ui/focusRing'
 
diff --git a/src/renderer/src/components/DownloadBar.tsx b/src/renderer/src/components/DownloadBar.tsx
index 022c702..5a0e0be 100644
--- a/src/renderer/src/components/DownloadBar.tsx
+++ b/src/renderer/src/components/DownloadBar.tsx
@@ -27,7 +27,12 @@ import {
   CalendarClockRegular,
   WarningRegular
 } from '@fluentui/react-icons'
-import type { MediaInfo, FormatOption, PlaylistInfo } from '@shared/ipc'
+import {
+  parseUrlShortcutContent,
+  type MediaInfo,
+  type FormatOption,
+  type PlaylistInfo
+} from '@shared/ipc'
 import { useDownloads, type MediaKind } from '../store/downloads'
 import { QUALITY_OPTIONS } from '../qualityOptions'
 import { sameVideo } from '../store/queueStats'
@@ -36,12 +41,10 @@ import { Select } from './Select'
 import { Hint } from './Hint'
 import { SegmentedControl } from './ui/SegmentedControl'
 import { IconButton } from './ui/IconButton'
-import { parseUrlShortcutContent } from '@shared/ipc'
 import { useClipboardLink, looksLikeUrl, firstUrlInText } from '../useClipboardLink'
 import { logError } from '../reportError'
 import { THUMB_LG } from '../thumbSizes'
 
-
 /** Pull the URL= target out of a dropped Windows .url Internet Shortcut file. */
 function parseUrlFile(content: string): string | null {
   const url = parseUrlShortcutContent(content)
diff --git a/src/renderer/src/components/LibraryView.tsx b/src/renderer/src/components/LibraryView.tsx
index f770d59..23b8a80 100644
--- a/src/renderer/src/components/LibraryView.tsx
+++ b/src/renderer/src/components/LibraryView.tsx
@@ -127,6 +127,9 @@ const useStyles = makeStyles({
     width: '100%',
     border: 'none',
     backgroundColor: 'transparent',
+    // A native <button> doesn't inherit color; set it so the chevron (currentColor)
+    // matches the page foreground in dark mode instead of UA ButtonText.
+    color: tokens.colorNeutralForeground1,
     fontFamily: tokens.fontFamilyBase,
     textAlign: 'left',
     cursor: 'pointer',
diff --git a/src/renderer/src/components/Onboarding.tsx b/src/renderer/src/components/Onboarding.tsx
index f1f9fe6..2098b87 100644
--- a/src/renderer/src/components/Onboarding.tsx
+++ b/src/renderer/src/components/Onboarding.tsx
@@ -1,4 +1,4 @@
-import {
+import {
   Card,
   Title2,
   Body1,
@@ -72,11 +72,6 @@ const useStyles = makeStyles({
     flexShrink: 0,
     marginTop: '2px',
     color: tokens.colorCompoundBrandForeground1
-  },
-  btnRow: {
-    display: 'flex',
-    justifyContent: 'flex-end',
-    gap: '8px'
   }
 })
 
@@ -131,21 +126,13 @@ export function Onboarding(): React.JSX.Element {
           ))}
         </div>
 
-        <div className={styles.btnRow}>
-          <Button
-            appearance="subtle"
-            onClick={() => update({ hasCompletedOnboarding: true })}
-          >
-            Skip
-          </Button>
-          <Button
-            appearance="primary"
-            icon={<RocketRegular />}
-            onClick={() => update({ hasCompletedOnboarding: true })}
-          >
-            Get started
-          </Button>
-        </div>
+        <Button
+          appearance="primary"
+          icon={<RocketRegular />}
+          onClick={() => update({ hasCompletedOnboarding: true })}
+        >
+          Get started
+        </Button>
       </Card>
     </div>
   )
diff --git a/src/renderer/src/components/Select.tsx b/src/renderer/src/components/Select.tsx
index 7b29c2a..6b175b1 100644
--- a/src/renderer/src/components/Select.tsx
+++ b/src/renderer/src/components/Select.tsx
@@ -1,4 +1,4 @@
-import { makeStyles, mergeClasses, tokens, shorthands } from '@fluentui/react-components'
+import { makeStyles, mergeClasses, tokens, shorthands } from '@fluentui/react-components'
 
 // A native <select> styled to match Fluent inputs. We use this instead of
 // Fluent's <Dropdown> because Fluent's portal/popover menu is a composited
diff --git a/src/renderer/src/qualityOptions.ts b/src/renderer/src/qualityOptions.ts
index c4cdd85..3cc4bb7 100644
--- a/src/renderer/src/qualityOptions.ts
+++ b/src/renderer/src/qualityOptions.ts
@@ -1,6 +1,9 @@
-import { VIDEO_QUALITY_OPTIONS, AUDIO_QUALITY_OPTIONS } from '@shared/ipc'
+import { VIDEO_QUALITY_OPTIONS, AUDIO_QUALITY_OPTIONS, type MediaKind } from '@shared/ipc'
 
+// `satisfies` (not an annotation) so the tuple element types survive: under
+// noUncheckedIndexedAccess a `readonly string[]` index yields `string | undefined`,
+// but the const tuples keep `QUALITY_OPTIONS[kind][0]` known-defined (L168).
 export const QUALITY_OPTIONS = {
   video: VIDEO_QUALITY_OPTIONS,
   audio: AUDIO_QUALITY_OPTIONS
-} as const
+} satisfies Record<MediaKind, readonly string[]>
diff --git a/src/renderer/src/store/downloads.ts b/src/renderer/src/store/downloads.ts
index 0085744..700e282 100644
--- a/src/renderer/src/store/downloads.ts
+++ b/src/renderer/src/store/downloads.ts
@@ -40,10 +40,10 @@ export interface DownloadItem {
   speed?: string
   eta?: string
   sizeLabel?: string
-  /** yt-dlp reported no total size — render the bar indeterminate, not 0% (L137) */
+  /** yt-dlp reported no total size -- render the bar indeterminate, not 0% (L137) */
   sizeUnknown?: boolean
   /** true once the main download stream finished and yt-dlp is fetching the
-   *  remaining stream / merging / post-processing — the bar goes indeterminate
+   *  remaining stream / merging / post-processing -- the bar goes indeterminate
    *  instead of visibly restarting 0→100% for the second stream (SR7) */
   finishing?: boolean
   filePath?: string
@@ -55,21 +55,20 @@ export interface DownloadItem {
   options?: DownloadOptions
   /** per-download custom-command override (omitted = use the persisted default template) */
   extraArgs?: string
-  /** raw trim spec — time ranges to keep (parsed to --download-sections in main) */
+  /** raw trim spec -- time ranges to keep (parsed to --download-sections in main) */
   trim?: string
   /** epoch ms to auto-start a 'saved' item; undefined = parked indefinitely (Phase M) */
   scheduledFor?: number
-  /** private download — completion is never recorded to history */
+  /** private download -- completion is never recorded to history */
   incognito?: boolean
   /** metadata already fetched at probe time; forwarded to main so it skips a redundant probe */
   probedMeta?: DownloadMeta
-  /** media-manager folder context — files this into <channel>/<playlist>/<NNN> - <title> */
+  /** media-manager folder context -- files this into <channel>/<playlist>/<NNN> - <title> */
   collection?: CollectionContext
-  /** when set, the source MediaItem id this download came from — marked downloaded on completion */
+  /** when set, the source MediaItem id this download came from -- marked downloaded on completion */
   mediaItemId?: string
 }
 
-
 // Placeholder channel shown while metadata is still being fetched. Tracked as a
 // constant so the meta/error/done handlers can recognise and clear it instead of
 // letting it stick forever when a probe returns no channel (SR6).
@@ -204,7 +203,7 @@ const seed: DownloadItem[] = PREVIEW
       {
         id: newId('item'),
         url: 'https://youtube.com/watch?v=dQw4w9WgXcQ',
-        title: 'Building a Desktop App with Electron — Full Course',
+        title: 'Building a Desktop App with Electron -- Full Course',
         channel: 'DevChannel',
         durationLabel: '1:42:08',
         kind: 'video',
@@ -306,7 +305,7 @@ function startFakeTicker(
       if (finished?.mediaItemId) {
         useSources.getState().markDownloaded(finished.mediaItemId, finished.filePath)
       }
-      get().pump() // a slot just freed — promote the next queued item
+      get().pump() // a slot just freed -- promote the next queued item
     }
   }, 650)
 }
@@ -350,9 +349,9 @@ export const useDownloads = create<DownloadState>((set, get) => {
   // Promote queued items (oldest first) into free slots, then launch them.
   //
   // Note: this only gates *future* promotions. Lowering maxConcurrent while N
-  // downloads are already active does NOT pause the overflow — the running
+  // downloads are already active does NOT pause the overflow -- the running
   // processes finish on their own, and the lower cap takes effect only as slots
-  // free up. (audit P3 — documented behaviour, not a bug.)
+  // free up. (audit P3 -- documented behaviour, not a bug.)
   function pump(): void {
     const items = get().items
     const running = items.filter((i) => i.status === 'downloading').length
@@ -385,7 +384,7 @@ export const useDownloads = create<DownloadState>((set, get) => {
       // The queue is a newest-first array and pump() promotes the highest-index
       // (oldest) queued item first, so a batch must be stored last-entry-first for
       // the playlist/channel to download 1→N instead of N→1 (M32). Entry 1 then
-      // sits at the bottom of the list — the same way repeated single adds stack.
+      // sits at the bottom of the list -- the same way repeated single adds stack.
       const built = entries.map((e) => buildItem(e.url, e.kind, e.quality, e.opts)).reverse()
       set((s) => ({ items: [...built, ...s.items] }))
       pump()
@@ -455,8 +454,8 @@ export const useDownloads = create<DownloadState>((set, get) => {
     },
 
     prioritize: (id) => {
-      // Reposition the item so pump() — which promotes the highest-index queued
-      // item first (oldest-first over a newest-first array) — picks it next.
+      // Reposition the item so pump() -- which promotes the highest-index queued
+      // item first (oldest-first over a newest-first array) -- picks it next.
       set((s) => {
         const idx = s.items.findIndex((i) => i.id === id)
         if (idx < 0 || s.items[idx]?.status !== 'queued') return {}
@@ -552,7 +551,7 @@ export const useDownloads = create<DownloadState>((set, get) => {
           switch (ev.type) {
             case 'meta':
               // A late meta event can arrive between cancel() and the child's
-              // close — don't overwrite a canceled item's fields (B5; mirrors the
+              // close -- don't overwrite a canceled item's fields (B5; mirrors the
               // canceled guard on the done/error cases below).
               if (i.status === 'canceled') return i
               return {
@@ -566,7 +565,7 @@ export const useDownloads = create<DownloadState>((set, get) => {
               }
             case 'progress':
               // Only a launched (downloading) item should take progress. Never
-              // promote a 'queued' item here — pump() owns the queued→downloading
+              // promote a 'queued' item here -- pump() owns the queued→downloading
               // transition and the concurrency cap (L88); a stray progress event
               // for a non-downloading item is ignored.
               if (i.status !== 'downloading') return i
@@ -634,7 +633,7 @@ export const useDownloads = create<DownloadState>((set, get) => {
           useSources.getState().markDownloaded(item.mediaItemId, item.filePath)
         }
       }
-      // A finished item frees a slot — promote whatever is queued next.
+      // A finished item frees a slot -- promote whatever is queued next.
       if (ev.type === 'done' || ev.type === 'error') pump()
     }
   }
@@ -658,7 +657,7 @@ if (!PREVIEW) {
   window.api.onDownloadEvent((ev) => useDownloads.getState().applyEvent(ev))
 } else {
   // Browser preview: animate any seed item that starts out 'downloading' so the
-  // queue is live (and its completion promotes the queued seed item — a real
+  // queue is live (and its completion promotes the queued seed item -- a real
   // demo of the concurrency cap).
   useDownloads
     .getState()