feat(audit): L51 + L64 — daily-sync time picker + aria2c connection tuning

Two small self-contained features from the audit's deferred list: - L51: the Task Scheduler daily sync is no longer pinned to 09:00. New Settings.syncTime (24h HH:MM) with a native time input beside the Library "Daily sync" switch — persists on change, re-registers the task on blur (schtasks /Create /F overwrite is the time-change mechanism). The value reaches the schtasks /ST argv, so a shared isValidSyncTime guards it at both the settings write and in schedule.ts (unit-tested incl. injection-shaped input). Live-verified against the machine's real task. - L64: aria2c connection count (-x/-s) is user-tunable. New Settings.aria2cConnections (1-16, aria2c's own ceiling) exposed as a SpinButton in Settings -> Network while the aria2c toggle is on. ARIA2C_ARGS became the pure aria2cArgs(n?) with a defensive clamp; threaded through AccessOptions. The -k 1M split floor stays fixed — a free-text field would bypass the custom-command consent gate. typecheck + 309 tests + eslint + production build green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-01 22:57:20 -04:00
parent e19f988c72
commit 96800d5a17
12 changed files with 199 additions and 21 deletions
@@ -797,9 +797,16 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
  `\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.
-  *Deferred (feature): a time picker is a small new feature (UI + a Task Scheduler time argument), not a
-  cleanup — worth its own change; 09:00 is a reasonable default meanwhile.*
+- [x] **L51 — Scheduled daily sync time hardcoded to 09:00** (schedule.ts) — on/off toggle only, no time picker.
+  *Fixed. New `Settings.syncTime` (24h 'HH:MM', default '09:00'); a compact native time input appears next to
+  the Library's "Daily sync" switch when it's on. `setScheduledSync(enabled, time?)` threads the time into
+  `schtasks /ST`, guarded by a shared `isValidSyncTime` (in @shared/ipc, used by both main's settings
+  validation and schedule.ts — the value reaches the schtasks argv, so malformed input falls back to the
+  default rather than being forwarded; unit-tested incl. injection-shaped values). The picker persists via
+  settings on change and re-registers the task on blur (one `schtasks /Create /F` per commit, not per
+  keystroke — /F overwrite IS the time-change mechanism). Live-verified on this machine: re-registering the
+  real AeroFetchDailySync task with /ST 19:30 moved its start to 7:30 PM and back (the user's original 09:00
+  registration restored byte-exact afterwards).*
 - [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.
@@ -818,9 +825,15 @@ token system + shared primitives (UI/SIMP — high value, larger effort) · i18n
 - [x] **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.
-  *Deferred (feature): exposing aria2c tuning is a new advanced setting, not a cleanup; the defaults are
-  sensible and aria2c itself is already opt-in.*
+- [x] **L64 — `ARIA2C_ARGS` connection params hardcoded** (`-x16 -s16 -k1M`), no user control.
+  *Fixed. New `Settings.aria2cConnections` (1–16, default 16 — aria2c's own -x/-s ceiling), exposed as an
+  "aria2c connections" SpinButton in Settings → Network, shown only while the aria2c toggle is on (same
+  pattern as maxConcurrent). The const became a pure `aria2cArgs(connections?)` in buildArgs.ts that clamps/
+  rounds defensively (settings validation clamps too — `applySettings` mirrors the maxConcurrent case);
+  `ARIA2C_ARGS` remains exported as `aria2cArgs()` so existing references stay valid. Threaded through
+  `AccessOptions.aria2cConnections` from buildCommand. The `-k 1M` split floor stays fixed — the meaningful
+  user knob is connection count; a free-text field would bypass the customCommandEnabled consent gate.
+  Unit-tested: custom count reaches `--downloader-args`, clamp floor/ceiling/rounding/non-finite.*
 - [x] **L65 — Pause uses `taskkill /F`** like cancel — a forced kill risks an unflushed `.part` tail vs a graceful stop.
  *Verified acceptable in the Batch 15 live pass — no code change needed. Live test (a real 360p download paused
  via the same `taskkill /T /F` the app issues, then resumed with identical args): the paused `.part` was left
@@ -104,6 +104,8 @@ export interface AccessOptions {
  rateLimit: string
  /** absolute path to aria2c.exe; omit to use yt-dlp's built-in downloader */
  aria2cPath?: string
+  /** aria2c parallel connections (-x/-s), clamped to 1–16; omitted = 16 (L64) */
+  aria2cConnections?: number
  /** read auth cookies from an installed browser's own cookie store */
  cookiesFromBrowser?: CookieBrowser
  /** absolute path to a Netscape-format cookie file (the sign-in window's export) */
@@ -118,9 +120,23 @@ export interface AccessOptions {
  youtubePoToken?: string
 }

-// Tuned for typical CDN-served video: 16 connections split across 16 segments,
-// each at least 1MB so tiny files don't get split pointlessly.
-export const ARIA2C_ARGS = 'aria2c:-x 16 -s 16 -k 1M'
+/**
+ * The yt-dlp `--downloader-args` value for aria2c: N connections split across N
+ * segments, each at least 1MB so tiny files don't get split pointlessly. The
+ * connection count is user-tunable (Settings → Network, L64) and clamped here to
+ * aria2c's own -x/-s ceiling of 16 — settings validation clamps too, but this is
+ * pure-module defence for any other caller. Non-finite input → the default 16.
+ */
+export function aria2cArgs(connections?: number): string {
+  const n =
+    connections !== undefined && Number.isFinite(connections)
+      ? Math.min(16, Math.max(1, Math.round(connections)))
+      : 16
+  return `aria2c:-x ${n} -s ${n} -k 1M`
+}
+
+/** The default tuning (16 connections) — kept for tests/docs that reference it. */
+export const ARIA2C_ARGS = aria2cArgs()

 /**
 * Shell-like split of a raw "extra yt-dlp args" string (Phase C custom-command
@@ -295,7 +311,9 @@ function accessArgs(a: AccessOptions): string[] {
  const args: string[] = []
  if (a.proxy.trim()) args.push('--proxy', a.proxy.trim())
  if (a.rateLimit.trim()) args.push('--limit-rate', a.rateLimit.trim())
-  if (a.aria2cPath) args.push('--downloader', a.aria2cPath, '--downloader-args', ARIA2C_ARGS)
+  if (a.aria2cPath) {
+    args.push('--downloader', a.aria2cPath, '--downloader-args', aria2cArgs(a.aria2cConnections))
+  }
  // cookiesFromBrowser and cookiesFile are mutually exclusive sources — the UI
  // only ever sets one (driven by Settings.cookieSource), but browser wins if
  // a caller somehow sets both.
@@ -214,6 +214,7 @@ export function buildCommand(opts: StartDownloadOptions, cookiesFile?: string):
    proxy: settings.proxy,
    rateLimit: settings.rateLimit,
    aria2cPath,
+    aria2cConnections: settings.aria2cConnections,
    // L136/M6: incognito attaches no cookies from the browser either.
    cookiesFromBrowser:
      !opts.incognito && settings.cookieSource === 'browser' ? settings.cookiesBrowser : undefined,
@@ -270,7 +270,10 @@ export function registerIpcHandlers(getMainWindow: () => BrowserWindow | null):
    })
  )
  ipcMain.handle(IpcChannels.scheduledSyncGet, () => getScheduledSync())
-  ipcMain.handle(IpcChannels.scheduledSyncSet, (_e, enabled: boolean) => setScheduledSync(enabled))
+  // `time` (24h HH:MM) is validated inside setScheduledSync before touching schtasks (L51).
+  ipcMain.handle(IpcChannels.scheduledSyncSet, (_e, enabled: boolean, time?: string) =>
+    setScheduledSync(enabled, time)
+  )

  // Reflect overall queue progress on the Windows taskbar and window title (SR8).
  ipcMain.handle(IpcChannels.taskbarProgress, (_e, p: TaskbarProgress) => {
@@ -11,7 +11,7 @@

 import { execFile } from 'child_process'
 import { getSystem32Path } from './binaries'
-import type { ScheduledSyncStatus } from '@shared/ipc'
+import { isValidSyncTime, DEFAULT_SETTINGS, type ScheduledSyncStatus } from '@shared/ipc'

 const TASK_NAME = 'AeroFetchDailySync'
 /** The argv flag the scheduled task passes so startup knows it's a sync launch. */
@@ -46,10 +46,17 @@ export async function getScheduledSync(): Promise<ScheduledSyncStatus> {

 /**
 * Register or remove the daily-sync scheduled task. Creating runs AeroFetch with
- * `--sync` every day at 09:00 (overwriting any prior task of the same name).
+ * `--sync` every day at the given 24h 'HH:MM' time (overwriting any prior task of
+ * the same name — which is also how a time change re-registers, L51). The time is
+ * renderer-supplied over IPC, so it's validated here before reaching the schtasks
+ * argv; anything malformed falls back to the default rather than failing the toggle.
 */
-export async function setScheduledSync(enabled: boolean): Promise<ScheduledSyncStatus> {
+export async function setScheduledSync(
+  enabled: boolean,
+  time?: string
+): Promise<ScheduledSyncStatus> {
  if (enabled) {
+    const startTime = isValidSyncTime(time) ? time : DEFAULT_SETTINGS.syncTime
    const tr = `"${process.execPath}" ${SYNC_FLAG}`
    const r = await schtasks([
      '/Create',
@@ -57,7 +64,7 @@ export async function setScheduledSync(enabled: boolean): Promise<ScheduledSyncS
      '/SC',
      'DAILY',
      '/ST',
-      '09:00',
+      startTime,
      '/TN',
      TASK_NAME,
      '/TR',
@@ -14,6 +14,7 @@ import {
  DEFAULT_DOWNLOAD_OPTIONS,
  DEFAULT_SETTINGS,
  isYtdlpUpdateChannel,
+  isValidSyncTime,
  type Settings,
  type DownloadOptions,
  type SponsorBlockCategory,
@@ -266,6 +267,17 @@ function applySettings(s: Store<Settings>, partial: Partial<Settings>): void {
        if (Number.isFinite(n)) s.set('maxConcurrent', Math.min(5, Math.max(1, Math.round(n))))
        break
      }
+      case 'aria2cConnections': {
+        // aria2c caps -x/-s at 16; clamp like maxConcurrent rather than reject (L64).
+        const n = Number(value)
+        if (Number.isFinite(n)) s.set('aria2cConnections', Math.min(16, Math.max(1, Math.round(n))))
+        break
+      }
+      case 'syncTime':
+        // Reaches `schtasks /ST` when the daily sync is (re)registered — only a
+        // well-formed 24h HH:MM is ever stored (L51).
+        if (isValidSyncTime(value)) s.set('syncTime', value)
+        break
      case 'clipboardWatch':
      case 'useAria2c':
      case 'restrictFilenames':
@@ -218,11 +218,12 @@ const api = {
   *  match the main function `syncWatchedSources` (L92). */
  syncWatchedSources: (): Promise<SyncResult> => ipcRenderer.invoke(IpcChannels.sourcesSync),

-  /** Read / write the Windows daily-sync scheduled task. */
+  /** Read / write the Windows daily-sync scheduled task. `time` is the 24h 'HH:MM'
+   *  start time (L51); omitted = the default. Re-creating with a new time updates it. */
  getScheduledSync: (): Promise<ScheduledSyncStatus> =>
    ipcRenderer.invoke(IpcChannels.scheduledSyncGet),
-  setScheduledSync: (enabled: boolean): Promise<ScheduledSyncStatus> =>
-    ipcRenderer.invoke(IpcChannels.scheduledSyncSet, enabled),
+  setScheduledSync: (enabled: boolean, time?: string): Promise<ScheduledSyncStatus> =>
+    ipcRenderer.invoke(IpcChannels.scheduledSyncSet, enabled, time),

  /** Subscribe to live indexing progress. Returns an unsubscribe function. */
  onIndexProgress: (cb: (p: IndexProgress) => void): (() => void) => {
@@ -237,6 +237,7 @@ export function LibraryView(): React.JSX.Element {
  const syncing = useSources((s) => s.syncing)
  const downloadItems = useDownloads((s) => s.items)
  const autoDownloadNew = useSettings((s) => s.autoDownloadNew)
+  const syncTime = useSettings((s) => s.syncTime)
  const updateSettings = useSettings((s) => s.update)

  const [url, setUrl] = useState('')
@@ -294,11 +295,26 @@ export function LibraryView(): React.JSX.Element {
  async function toggleScheduled(next: boolean): Promise<void> {
    setScheduled(next) // optimistic
    if (PREVIEW) return
-    const res = await window.api.setScheduledSync(next)
+    const res = await window.api.setScheduledSync(next, syncTime)
    setScheduled(res.enabled)
    if (res.error) setError(res.error)
  }

+  // The native time input emits a complete 'HH:MM' or '' (cleared/partial) — only
+  // persist real times, so the picker can never store a malformed value (L51).
+  function onSyncTimeChange(value: string): void {
+    if (!/^([01]\d|2[0-3]):[0-5]\d$/.test(value)) return
+    updateSettings({ syncTime: value })
+  }
+
+  // Re-register the task with the new time once editing settles (blur), so typing
+  // through the segments doesn't spawn schtasks per keystroke (L51).
+  async function onSyncTimeCommit(): Promise<void> {
+    if (!scheduled || PREVIEW) return
+    const res = await window.api.setScheduledSync(true, useSettings.getState().syncTime)
+    if (res.error) setError(res.error)
+  }
+
  // Reset the selection (and any batch note) whenever the expanded source changes.
  useEffect(() => {
    setSelected(new Set())
@@ -578,6 +594,16 @@ export function LibraryView(): React.JSX.Element {
            onChange={(_, d) => toggleScheduled(d.checked)}
            aria-label="Daily background sync"
          />
+          {scheduled && (
+            <Input
+              type="time"
+              size="small"
+              value={syncTime}
+              onChange={(_, d) => onSyncTimeChange(d.value)}
+              onBlur={onSyncTimeCommit}
+              aria-label="Daily sync time"
+            />
+          )}
        </span>
      </div>

@@ -1,5 +1,14 @@
 import { useState } from 'react'
-import { Field, Input, Switch, Button, Card, Subtitle2, Spinner } from '@fluentui/react-components'
+import {
+  Field,
+  Input,
+  Switch,
+  SpinButton,
+  Button,
+  Card,
+  Subtitle2,
+  Spinner
+} from '@fluentui/react-components'
 import { GlobeRegular } from '@fluentui/react-icons'
 import { useSettings } from '../../store/settings'
 import { useSettingsStyles } from './settingsStyles'
@@ -9,6 +18,7 @@ export function NetworkCard(): React.JSX.Element {
  const proxy = useSettings((s) => s.proxy)
  const rateLimit = useSettings((s) => s.rateLimit)
  const useAria2c = useSettings((s) => s.useAria2c)
+  const aria2cConnections = useSettings((s) => s.aria2cConnections)
  const youtubePlayerClient = useSettings((s) => s.youtubePlayerClient)
  const youtubePoToken = useSettings((s) => s.youtubePoToken)
  const update = useSettings((s) => s.update)
@@ -53,6 +63,25 @@ export function NetworkCard(): React.JSX.Element {
        <Switch checked={useAria2c} onChange={(_, d) => update({ useAria2c: d.checked })} />
      </Field>

+      {useAria2c && (
+        <Field
+          label="aria2c connections"
+          hint="Parallel connections per download (1–16). More can be faster on throttled connections; fewer is gentler on servers."
+        >
+          <SpinButton
+            value={aria2cConnections}
+            min={1}
+            max={16}
+            onChange={(_, d) => {
+              const v = d.value ?? Number(d.displayValue)
+              if (typeof v === 'number' && Number.isFinite(v)) {
+                update({ aria2cConnections: Math.min(16, Math.max(1, Math.round(v))) })
+              }
+            }}
+          />
+        </Field>
+      )}
+
      <Field
        label="YouTube client (advanced)"
        hint="Override how AeroFetch identifies itself to YouTube when downloads start failing. Try web_safari, tv, or mweb. Leave blank for the automatic default."
@@ -319,6 +319,15 @@ export function isYtdlpUpdateChannel(v: unknown): v is YtdlpUpdateChannel {
  return typeof v === 'string' && (YTDLP_UPDATE_CHANNELS as readonly string[]).includes(v)
 }

+/**
+ * Runtime guard for a 24h 'HH:MM' sync time crossing the IPC boundary (L51) —
+ * the value reaches `schtasks /ST`, so anything malformed must never be forwarded.
+ * Shared so main's settings validation and schedule.ts use one definition.
+ */
+export function isValidSyncTime(v: unknown): v is string {
+  return typeof v === 'string' && /^([01]\d|2[0-3]):[0-5]\d$/.test(v)
+}
+
 export interface YtdlpUpdateResult {
  ok: boolean
  /** yt-dlp's own update output, e.g. "Updated to ... " or "yt-dlp is up to date" */
@@ -570,6 +579,8 @@ export interface Settings {
  rateLimit: string
  /** use bundled aria2c (resources/bin/aria2c.exe) as the external downloader */
  useAria2c: boolean
+  /** aria2c parallel connections per download (-x/-s), 1–16; only used when useAria2c is on (L64) */
+  aria2cConnections: number
  /** where auth cookies come from: none, a browser's store, or the sign-in window */
  cookieSource: CookieSource
  /** which browser to read cookies from when cookieSource is 'browser' */
@@ -603,6 +614,8 @@ export interface Settings {
  notifyOnComplete: boolean
  /** auto-enqueue newly-found videos from watched sources during a sync (Phase J) */
  autoDownloadNew: boolean
+  /** daily watched-source sync start time, 24h 'HH:MM' (Task Scheduler /ST) (L51) */
+  syncTime: string
  /** false until the first-run welcome screen has been dismissed */
  hasCompletedOnboarding: boolean
  /** keep AeroFetch running in the system tray when its window is closed (Phase O) */
@@ -650,6 +663,7 @@ export const DEFAULT_SETTINGS: Settings = {
  proxy: '',
  rateLimit: '',
  useAria2c: false,
+  aria2cConnections: 16,
  cookieSource: 'none',
  cookiesBrowser: 'chrome',
  youtubePlayerClient: '',
@@ -670,6 +684,7 @@ export const DEFAULT_SETTINGS: Settings = {
  // Default off so adding a watched channel doesn't silently fill the user's
  // disk on first use (SR3). Enable explicitly once they know what it does.
  autoDownloadNew: false,
+  syncTime: '09:00',
  hasCompletedOnboarding: false,
  minimizeToTray: false,
  launchAtStartup: false,
@@ -9,6 +9,7 @@ import {
  collectionOutputTemplate,
  CROP_SQUARE_PPA,
  ARIA2C_ARGS,
+  aria2cArgs,
  DEST_MARKER,
  type AccessOptions
 } from '../src/main/buildArgs'
@@ -105,6 +106,25 @@ describe('network options', () => {
    const argv = build(opts(), dlo(), NO_ACCESS)
    expect(argv).not.toContain('--downloader-args')
  })
+
+  it('threads a custom aria2c connection count into --downloader-args (L64)', () => {
+    const argv = build(opts(), dlo(), {
+      ...NO_ACCESS,
+      aria2cPath: 'C:/fake/bin/aria2c.exe',
+      aria2cConnections: 4
+    })
+    expect(hasSeq(argv, '--downloader-args', 'aria2c:-x 4 -s 4 -k 1M')).toBe(true)
+  })
+
+  it('aria2cArgs clamps to aria2c’s 1–16 ceiling and defaults to 16 (L64)', () => {
+    expect(aria2cArgs()).toBe('aria2c:-x 16 -s 16 -k 1M')
+    expect(aria2cArgs(8)).toBe('aria2c:-x 8 -s 8 -k 1M')
+    expect(aria2cArgs(0)).toBe('aria2c:-x 1 -s 1 -k 1M') // below floor → 1
+    expect(aria2cArgs(99)).toBe('aria2c:-x 16 -s 16 -k 1M') // above ceiling → 16
+    expect(aria2cArgs(7.6)).toBe('aria2c:-x 8 -s 8 -k 1M') // rounds
+    expect(aria2cArgs(NaN)).toBe('aria2c:-x 16 -s 16 -k 1M') // non-finite → default
+    expect(ARIA2C_ARGS).toBe(aria2cArgs()) // the exported default stays in sync
+  })
 })

 // --- Cookies: browser store vs. sign-in window's exported file --------------
@@ -8,7 +8,40 @@ import {
  isValidSource,
  isValidMediaItem
 } from '../src/main/validation'
-import type { HistoryEntry, ErrorLogEntry, Source, MediaItem } from '@shared/ipc'
+import {
+  isValidSyncTime,
+  type HistoryEntry,
+  type ErrorLogEntry,
+  type Source,
+  type MediaItem
+} from '@shared/ipc'
+
+// --- L51: daily-sync time (reaches `schtasks /ST`) ---------------------------
+
+describe('isValidSyncTime', () => {
+  it('accepts well-formed 24h HH:MM times', () => {
+    expect(isValidSyncTime('00:00')).toBe(true)
+    expect(isValidSyncTime('09:00')).toBe(true)
+    expect(isValidSyncTime('19:30')).toBe(true)
+    expect(isValidSyncTime('23:59')).toBe(true)
+  })
+
+  it('rejects out-of-range, partial, and 12h forms', () => {
+    expect(isValidSyncTime('24:00')).toBe(false)
+    expect(isValidSyncTime('12:60')).toBe(false)
+    expect(isValidSyncTime('9:00')).toBe(false) // must be zero-padded
+    expect(isValidSyncTime('09:0')).toBe(false)
+    expect(isValidSyncTime('9am')).toBe(false)
+    expect(isValidSyncTime('')).toBe(false)
+  })
+
+  it('rejects non-strings and injection-shaped values', () => {
+    expect(isValidSyncTime(undefined)).toBe(false)
+    expect(isValidSyncTime(900)).toBe(false)
+    expect(isValidSyncTime('09:00 /TN evil')).toBe(false) // nothing may trail the time
+    expect(isValidSyncTime('09:00\n')).toBe(false)
+  })
+})

 // --- S4: filename template path-traversal -----------------------------------