docs(audit): check off CL4 + CC5 (Batch 21)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
+16
-13
@@ -1971,15 +1971,14 @@ split — but that style is unenforced and several "do the same thing two ways"
|
||||
*Fixed: every listed duplicate now has one home — renderer `lib/formatters`/`urlHelpers`/`id` +
|
||||
`@shared/format`, `MediaKind` single-sourced (L105); main `lib/exec.ts` (the five `execFile` wrappers) and
|
||||
now `lib/lineBuffer.ts` (the two stdout-split loops). The `lib/` dirs exist on both sides.*
|
||||
- [ ] **CC5 — Async patterns are mixed.** Three I/O idioms: callback-wrapped `new Promise` (execFile ×5),
|
||||
- [x] **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.
|
||||
*Partly done: the `execFile ×5` idiom is gone (one `execFileAsync`, CC4), so the app-code default is
|
||||
`async/await`. The one remaining event-emitter-wrapped Promise is the updater's `net.request` streaming
|
||||
download — its "wrap once in a helper" is **CL4**, deferred with it (it's security-critical installer-
|
||||
download code — host re-validation per redirect + SHA-256 verify + truncation guards — that can't be
|
||||
live-verified in this environment, so it isn't refactored unattended). Left open until CL4 lands.*
|
||||
*Closed with CL4 (Batch 21): the `execFile ×5` idiom was already gone (one `execFileAsync`, CC4), and the
|
||||
last event-emitter-wrapped Promise — the updater's `net.request` streaming download — is now wrapped once
|
||||
in `streamInstallerToFile`, extracted under 14 behavior-pinning tests. `async/await` + single wrappers hold
|
||||
across the app code.*
|
||||
- [x] **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
|
||||
@@ -2131,14 +2130,18 @@ filed. Several listed categories are **N/A** to this stack and worth recording s
|
||||
`startDownload` is now a linear pre-flight (binary checks → URL normalise → concurrency guard → cookies →
|
||||
spawn → metadata probe → wire). `getTitle` is a getter because the parallel metadata probe may fill the
|
||||
resolved title in after wiring is set up. Behaviour unchanged.*
|
||||
- [ ] **CL4 — Deep nesting: `updater.downloadAppUpdate`** — Promise → `net.request` → `response` → `data`
|
||||
- [x] **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.
|
||||
*Deferred (deliberate, needs a watched pass): this is the security-critical installer download —
|
||||
per-redirect host re-validation, streaming SHA-256 verification, truncation detection, idle-timeout, and
|
||||
backpressure teardown all interlock in that block. Extracting `streamToFile` is worthwhile but a mistake
|
||||
here is a security regression, and an app-update download can't be live-verified in this environment, so it
|
||||
isn't refactored unattended. Grouped with CC5's "wrap the event-emitter API once" — both land in a watched
|
||||
session on the updater.*
|
||||
*Fixed (Batch 21, pin-then-refactor): the security-critical invariants were first locked with 14
|
||||
behavior-pinning tests at the public `downloadAppUpdate` boundary
|
||||
([test/updaterDownload.test.ts](test/updaterDownload.test.ts), scripted `net.request` fake) — untrusted
|
||||
initial URL, per-hop redirect re-validation on both the checksum fetch and the installer, missing/malformed
|
||||
checksum refusal, SHA-256 mismatch + partial cleanup, truncation detection, idle-timeout, checksum-phase and
|
||||
mid-download cancel, HTTP-error status, combined-checksum line matching, and progress events. Then the
|
||||
nested block was extracted verbatim into `streamInstallerToFile` (redirects/hashing/timeout/truncation/
|
||||
teardown in one once-wrapped helper); `downloadAppUpdate` is now a linear pre-flight (trust-check →
|
||||
checksum → stream), with the B6 cancel-slot ownership kept beside the module state it guards via
|
||||
`onCancelReady`/`onSettled`. All pins passed before AND after the extraction — no behavior change.*
|
||||
- [x] **CL5 — Superfluous exports.** `PROGRESS_TEMPLATE` (L167) and `getManagedBinDir` are `export`ed but
|
||||
used only within their own module. **Fix:** make them module-private.
|
||||
- [x] **CL6 — Redundant wrappers** (minor): `MediaThumb`'s `kind === 'audio' ? 'audio' : 'video'` (L15),
|
||||
|
||||
Reference in New Issue
Block a user