Skip to content

refactor(knowledge)!: make the single-video digest source-agnostic and rename it to video-digest - #2823

Merged
kyle-sexton merged 46 commits into
mainfrom
refactor/source-agnostic-video-digest
Aug 16, 2026
Merged

kyle-sexton merged 46 commits into
mainfrom
refactor/source-agnostic-video-digest

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

BREAKING: knowledge 0.13.0 — source-agnostic video digest; youtube-digestvideo-digest

The single-video digest skill now handles X (Twitter) video posts at full watch parity with
YouTube through an engine-layer source-adapter contract, and renames to /knowledge:video-digest.
course-digest receives no behavioral change. No on-disk consumer data migrates: the
youtube-watch epic queue directory and the acquisition lock directory are stable
storage-format identifiers (ADR 0011).

What landed

  • Adapter contract + static registry (extraction/adapters/): five required methods, a
    0..N acquisition envelope, a four-type error taxonomy, per-adapter yt-dlp extractor
    allow-lists (closes a delegation SSRF found in security review), path-safe slice keys, and a
    shared conformance suite + X golden eval insuring adapter three (ADR 0010).
  • X adapter: canonicalization by construction on every entry path, snowflake-aliasing
    detection persisted to slice metadata, provenance guard (refused-before-fetch delegations),
    429 silent-degradation detector, 0-video posts digest text-only end-to-end.
  • Transcript strategy seam: per-source defaults (captions for YouTube unchanged;
    captions+repair for X), an optional runtime-detected faster-whisper asr rung (never
    auto-installed) ratified by three recorded T5-ASR probes, and a named transcriptDegradation
    provenance field — degradation is never silent.
  • Env namespace: six YOUTUBE_* vars → VIDEO_DIGEST_* with a warn-once compatibility
    read; new --acquire-phase-gap launcher flag.
  • Hub split: SKILL.md body 37,620 → 13,328 chars (~3.3k est. tokens) with conditional
    spoke routing and a description widened to x.com/twitter.com triggers.
  • Rename discipline: structural git mv isolated from sweeps; a checked-in, self-tested
    regression sweep; CHANGELOG entry naming the five silently-failing consumer surfaces;
    plugin 0.12.0 → 0.13.0.

Verification

Every phase passed a fresh-context verifier against its acceptance criteria; Phases 1–2 also
passed independent security review (the CRITICAL SSRF finding was fixed at the contract seam
and re-verified) and Phase 1 an architecture review. Final outcome verification (fresh
context, whole-branch diff): CONFIRMED — all 5 success criteria, all scope boundaries.
Suite: 70 files / 467 tests green, tsc --noEmit clean in both extraction lanes with
checkJs: true, zero suppressions, check-skill PASS on video-digest and course-digest.
Live one-time evidence: anonymous X acquisition end-to-end (media + platform VTT + word-tag
cleanup) and YouTube slice-layout parity, recorded in the machine-local memory slice.

Breaking-change migration (the five silent surfaces)

  1. Cloud routines invoking /knowledge:youtube-digest — edit the routine prompt to
    /knowledge:video-digest (silent no-match otherwise).
  2. Scheduled tasks / /loop prompts — update stored prompts naming the old skill.
  3. Permission rulesSkill(knowledge:youtube-digest) is exact-match; an allow rule
    silently stops matching and a deny rule fails open (audit denies first). Rewrite to
    Skill(knowledge:video-digest).
  4. Agent SDK skills: allowlists — rename the entry (loud failure: skill absent).
  5. Bare /youtube-digest references — the freed bare name is squattable; sweep docs and
    saved prompts. Conversational routing keeps working: the description retains
    '/youtube-digest' as a migration alias.

Also: YOUTUBE_* env spellings are deprecated (compat read warns once per process; new name
wins); first run after upgrade reinstalls extraction deps once (stamp renamed).

Close-out pointers

  • ADRs: docs/adr/0012-dispatch-video-sources-through-a-static-adapter-registry.md,
    docs/adr/0013-keep-storage-format-identifiers-stable-across-renames.md (renumbered after
    main's merge-order claim on 0010/0011)
  • Follow-ups this effort filed were both drained to main by the autonomous worker lanes
    mid-flight: knowledge: scheduled liveness lane for video-digest source adapters #2797 → main 0.12.5 (liveness lane, old paths) and knowledge/video-digest: rename the user-facing 'resume <video-slug>' argument to a source-neutral name #2818 → main 0.12.6
    (BREAKING resume <slice-slug>). This branch merges main and carries both forward across
    the rename: the liveness tree is relocated into video-digest/ with all references
    updated, and the <slice-slug> argument is ported into the rewritten hub — main's
    breaking change is preserved, not reverted.
  • The branch-lived contract slice docs/topics/source-agnostic-video-digest/ is pruned before
    merge; full revision history on this branch. The approved PLAN and the deviation ledger are
    pasted below.
  • CI hygiene surfaced that an early design commit had accidentally tracked the
    .work/source-agnostic-video-digest/ research tree (~13.9k lines) against the repo's
    memory-tier convention (ADR 0004: .work/ never leaves its checkout). It is untracked via
    git rm --cached in 7eb5833a — content preserved on disk and in this branch's history.

No linked issue.

Related

Approved PLAN.md (final revision, pre-prune)
# PLAN — source-agnostic video digest

Status: APPROVED 2026-08-15 — all six [FALLBACK] rows accepted with their RECOMMENDED options
(no `name:` pin; docs/topics + .work refs KEEP; npm package renames; CI job id by pre-check;
version 0.13.0 + keywords; ASR never auto-installed). Design inputs: `design/design-threads.md` (primary),
`design/capability-matrix.md`, `design/inherited-decisions.md`, `design/hub-split-budget.md`,
`design/consumer-context.md`. Design gate: PASSED (`design/design-handoff-gate.md`, run 2,
2026-08-15). Stress-tested 2026-08-15 (cross-vendor Codex + fresh-context devils-advocate);
findings verified and folded in — see "Stress-test summary".

Conventions for this file: phase tags `[TODO]` / `[IN-PROGRESS]` / `[DONE]`. All sanity-check
commands run in **Git Bash from the repo root** unless stated otherwise. Durable evidence for
manual/one-time checks lands under `.work/source-agnostic-video-digest/evidence/<phase>/`
(machine-local memory slice, never committed).

## Brief

**Goal.** Make the single-public-video digest pipeline source-agnostic: X (Twitter) video joins
YouTube behind an engine-layer source-adapter contract; the skill is renamed
`youtube-digest``video-digest`; `course-digest` receives no behavioral or structural change.

**Why.** The per-source surface is measured at 6 stages (capability matrix, after the T4/T5
stage-2 collapse); everything else is shared. An `x-digest` sibling would duplicate ~325 lines
of source-agnostic pipeline and lose listing-budget triggers; an all-sources merge fails the
500-line hard cap. The adapter seam is the extraction layer.

**Storage invariant (A1, binding — stated verbatim so no phase erodes it).** One queue root,
one `claims/` namespace; the on-disk epic directory stays the literal `youtube-watch` (stable
storage-format identifier); **source is never a directory level** — it lives only in slice
metadata (`watch.json` `sourceUrl`); mixed-source batches share the one queue; **no migration**
of any consumer's `.work/` tree; every user-facing path rendering derives from the resolved
slice dir, never from the constant.

**Scope boundaries.**

- In: adapter contract + registry + YouTube/X adapters; transcript strategy seam; error
  taxonomy; conformance suite; hub split + widened description; naming hygiene; env-var
  namespace rename with compatibility read; skill rename with breaking-change discipline.
- Out (each with recorded trigger in design): A1 move (ii) epic parameterization;
  adapter-namespaced config; ASR-as-substrate (posture iv); priority/scoring dispatch;
  content-claim capability (reserved, unimplemented); LFS retention / sub-path templating;
  scheduled liveness lane (tracked follow-up only).
- `course-digest` untouched criterion, re-scoped (user-approved 2026-08-14): *no behavioral or
  structural change; mechanical cross-reference updates forced by the rename are exempt and
  enumerated in Phase 7.* Additionally (step-zero consequence): the `checkJs` flip may force
  **type-annotation-only** edits (JSDoc, tsconfig) in `course-digest/extraction` — zero
  runtime-behavior change, proven by its test suite.

**Success criteria.**

1. `watch <x-status-url>` and `transcript <x-status-url>` run end-to-end (full watch parity).
2. All existing YouTube behavior preserved (vitest + evals green; same slice layout; epic dir
   literal unchanged). YouTube and X slices coexist under one queue root, including under a
   non-default `--work-root`.
3. `tsc --noEmit` green in both extraction lanes **with `checkJs: true`**.
4. Rename ships with breaking-change discipline (CHANGELOG + announcement naming the five
   silent consumer surfaces); the sweep regression test catches the three known-miss vendored
   refs.
5. Hub `SKILL.md` **body** (after frontmatter; baseline 37,620 chars ≈ 9,405 est. tokens at
   chars/4) lands under 20,000 chars ≈ 5,000 est. tokens; description rules per Phase 6.

## Standards grounding

No standards index (`.claude/standards.yaml`, `docs/standards/README.md` absent) — ladder
rung 4 (inference), surfaced not persisted:

| Surface | Source loaded | Provenance |
|---|---|---|
| Skill caps + gates | `plugins/skill-quality/scripts/check-skill.sh:175-177` (1536 desc / 500 hard / 200 soft; token warrant governs the split per hub-split-budget.md) | repo (team) |
| Topic docs / PLAN lifecycle | `docs/conventions/topic-docs/README.md` + planning-plugin binding | repo (team) |
| Commit convention | `docs/conventions/commit-convention/README.md` + `.claude/source-control.md` resolution | repo (team) |
| Engineering philosophy (root-cause-only, zero suppressions, comment hygiene) | user-global CLAUDE.md + `melodic-software/standards` engineering-philosophy | org + user-global |

## Plan

### Phase 0: Switch the type lane on [DONE]

Step zero, before any contract work (T3 blocking precondition; user-directed).
Landed `aaa62cc7` (2026-08-15); strict-tier posture + `@satisfies` no-op recorded in
`DEVIATIONS.md`. Checker holes for Phase 4 grew to four: (1) `noImplicitAny` off — adapter
methods need explicit parameter annotations; (2) `**/*.test.js` excluded from the type lane, so
the conformance suite itself is untyped; (3) too-few-parameters accepted silently;
(4) interpolated `import()` resolves to `any`.

- [x] `course-digest/extraction/tsconfig.json:9``"checkJs": false``true`
- [x] `youtube-digest/extraction/tsconfig.json:9``"checkJs": false``true`
- [x] Fix every surfaced diagnostic at root cause. **Zero suppressions** (`@ts-ignore` /
  `@ts-expect-error` require explicit user approval + recorded justification; target: none).
- [x] Standardize contract-bearing annotations on `@satisfies`. (No applicable sites — lands
  with Phase 1 contract authorship; DEVIATIONS.md.)
- [x] Known checker holes recorded for Phase 4 to cover at runtime: too-few-parameters
  accepted silently; interpolated `import()` resolves to `any`.

**Sanity Check:**

- [x] `grep -rn '"checkJs": true' plugins/knowledge/skills/*/extraction/tsconfig.json` → 2 rows
- [x] `npx tsc --noEmit` exit 0 in both extraction dirs
- [x] `git grep -n -e '@ts-ignore' -e '@ts-expect-error' -- 'plugins/knowledge/skills/*/extraction'` → 0 rows (git-grep form per DEVIATIONS.md — the raw grep matches node_modules)
- [x] Both lanes' vitest suites exit 0 (no runtime change)

### Phase 1: Adapter contract, registry, YouTube adapter (behavior-preserving) [DONE]

Review: architecture
Review: security

Integration-first slice: after this phase the existing YouTube path runs **through** the
adapter seam with observably identical behavior.

**Pre-flight consumer check (FIRST item — the result envelope is a contract migration):**

- [x] Inventory every producer/consumer of acquisition metadata and result shape before
  defining the envelope: `run-watch.js`, `run-transcript.js`, `preflight-metadata.js` +
  `queue-claim.js` (queue lane), `watch-state.js` (state + resume prompts),
  `post-bootstrap-slice.js` / `snapshot-bootstrap.js`, `export-sheet-frame-index.js`,
  `harvest-links.js`, `evals/check-*.js`. Document each one's current single-result
  assumption; every touched assumption gets a work item below.

**Contract (`extraction/adapters/adapter-contract.js`):**

- [x] 5 required methods: `matchUrl(url)` (claim + canonicalization; no I/O);
  `extractSliceKey(url, metadata)` (signature fixed even though both sources need only `url`);
  `acquire(...)`; `harvestLinks(metadata)`; `acceptForEnqueue(url)`.
- [x] `acquire` specified executably: inputs = canonical URL + resolved slice dir + options
  (auth/throttle context supplied by the shared driver); required outputs = media path(s),
  caption path(s), metadata object — never specified by yt-dlp invocation. Shared call sites
  that drive it: `acquisition/acquire.js` via `acquire-with-retry.js` (the generic spawn
  wrapper stays shared machinery).
- [x] Declared attributes: `hosts` (registry keys); `extractorArgs` (string | null) **and**
  `comments` capability — BOTH `--write-comments` and `--extractor-args` at
  `build-yt-dlp-args.js:113-115` become adapter-declared, neither pushed unconditionally;
  `captionClass`; `errorPatterns` table; `transcriptStrategy` default; capabilities object
  (closed by default); reserved-unimplemented content-claim capability.
- [x] **Result envelope defined in the contract file** (T6): a collection of 0..N entry
  results + a shared metadata object; arity is a property of the result, never the adapter; a
  convenience accessor may collapse at a call site, the contract never does; open metadata
  namespace with reserved `source:`-prefixed keys for source-specific fields; transcript
  travels as a replayable file path. 0-entry results are well-formed (metadata-only), never
  null, never a throw.
- [x] Error taxonomy (T11) defined here: exactly **four** types — `UnsupportedSourceError`
  (dispatch-level, deliberately OUTSIDE the adapter error hierarchy), retryable source error,
  fatal source error, login-required. Degradation detail (e.g. 429-syndication) is **metadata
  on the retryable type**, never a fifth type. Concrete identifier names implementer's
  discretion.
- [x] `validateAdapter` + factory; contract stability posture declared in-file (private,
  versioned with the plugin); construction performs no network/filesystem I/O (cheap pure
  normalization permitted).

**Registry (`extraction/adapters/registry.js`):**

- [x] Static host-keyed map of **statically-imported** module references — never a computed
  dynamic import in any spelling (template, variable, concatenation, helper-mediated). The
  course-digest resolver shape (`config.js:44-51`, CWE-829/22) must not be replicated.
- [x] Unknown host fails closed with the supported-source list; regex evaluated only after
  owned-host selection; subdomain handling defined and tested.

**YouTube adapter (`extraction/adapters/youtube.js`):**

- [x] `extractVideoId` (from `acquire.js:116-138`) behind `matchUrl` / `extractSliceKey`
  (URL-authoritative, closing the redirect divergence).
- [x] Preflight acceptance: the **whole** YouTube-shaped pattern set at
  `preflight-metadata.js:58-63` and `:226-227` moves behind `acceptForEnqueue` /
  `errorPatterns` — not just the `Incomplete YouTube ID` line — so none of it applies YouTube
  semantics to X.
- [x] Link harvest incl. pinned comment; `extractorArgs =
  "youtube:max_comments=20,all,top;comment_sort=top"`; `comments` capability true;
  `errorPatterns` from `YOUTUBE_BOT_CHALLENGE_PATTERNS` (`acquire-yt-dlp-auth.js:8`).

**Shared rewiring:**

- [x] `acquire.js` routes source-id + acquisition through the registry/adapter
- [x] `build-yt-dlp-args.js:113-115` — comment flags + extractor-args from adapter declarations
- [x] `preflight-metadata.js` stage-13 delegates to `acceptForEnqueue`
- [x] `harvest-links.js` delegates to adapter `harvestLinks`
- [x] `run-watch.js:95,105` — slice key from `extractSliceKey`; slug FORMAT stays in shared
  `deriveVideoSlug`
- [x] Error classification: adapter `errorPatterns` consumed by the classification predicates
  in `spawn-yt-dlp-with-auth-fallback.js` (`:49` `isYoutubeBotChallengeError`, `:60`
  `isCookieProfileRetryableError` — the design's ":54 single site" citation names the spawn
  call; the predicates at :49/:60 are the actual seam, recorded here as a design-artifact
  precision fix, same intent). The **browser-cookie-profile fallback loop is gated on an
  adapter capability** — X (cookies-file only) must never iterate browser profiles. Cookie
  fallback fires on login-required classification only.

**Sanity Check:**

- [x] Conformance-style unit test asserts every registry value is a statically-imported module
  object (no thenable/specifier strings) AND `git grep -n "import(" -- plugins/knowledge/skills/youtube-digest/extraction/adapters` → 0 rows
- [x] Unknown-host unit test: dispatch of `https://vimeo.com/1` throws the dispatch-level
  unsupported-source error listing supported sources; exit non-zero via CLI wrapper test
- [x] Envelope tests: 0, 1, and N entries constructed and consumed through both `watch` and
  `transcript` code paths (fixture-driven, offline)
- [x] vitest exit 0; `npx tsc --noEmit` exit 0
- [x] One-time evidence (NOT a merge gate): manual run `transcript <known yt-url>` produces a
  slice layout-identical to a pre-change run; diff summary saved to
  `.work/source-agnostic-video-digest/evidence/phase1/yt-parity.md`

### Phase 2: X adapter [DONE]

Review: security

`extraction/adapters/x.js` + registration:

- [x] `matchUrl` claims `x.com` / `twitter.com` status URLs; **adapter-level canonicalization**
  re-derives the canonical status URL itself (T10 (ii)). Entry-path inventory it must cover
  (all reach dispatch through the shared seam): `run-watch.js` (`watch <url>`),
  `preflight-metadata.js` + `queue-claim.js` (`queue <url>` / `watch <n>`),
  `run-transcript.js`, `run-resume.js` / `watch-state.js` resume, and the recovery command
  emitted by `detect-recoverable-bootstrap.js` — canonicalization lives in the adapter so
  every path gets it by construction; test at least watch, queue, and transcript entries.
- [x] `extractSliceKey` → pair `(display_id, id)`. **Canonical identity = `display_id`** (the
  URL status id — the design invariant "slice key is the id captured from the URL"); `id`
  rides as the media discriminator. Same pair → same slice (no duplicates). Tolerates the
  link-post branch where `id` = twid. Snowflake timestamp delta (`(id >> 22) + 1288834974657`)
  flags quote/retweet aliasing; a flagged aliasing is recorded in slice metadata. Fixtures:
  original post, quote tweet, retweet, link post.
- [x] **Provenance guard**: any yt-dlp result whose `extractor` is not `twitter` is a
  **blocked delegation**, never followed (upstream #9715). The post itself then resolves as a
  0-video result (next item) — the guard blocks foreign media, it does not error the post.
- [x] 0..N per `twitter.py`'s five branches: N → collection; 1 → single-entry collection
  (never bare-object); `/video/<n>` pinned index honored; 0-with-outbound-link → provenance
  guard blocks the delegation, post yields a well-formed metadata-only 0-result with the
  blocked link recorded in provenance/harvested links; 0-no-link → metadata-only 0-result.
  Both 0-cases produce a **text-only digest with populated provenance** (T6 D-A).
- [x] `errorPatterns`: the three observed X failures (`No video could be found in this tweet`,
  `No video formats found!`, `Unsupported URL:`) → fatal-source (the first two are
  post-content facts; with the 0..N envelope the first typically resolves as a 0-result before
  spawn-level classification). **Login-required = exactly the three documented cases**
  (NSFW/age-restricted; protected account — cookie account must follow the author; any
  `not authorized` API message), all `raise_login_required`-shaped — only these gate the
  cookie fallback. Auth-dependence note carried into the source spoke: X auth-fallback
  windows are weeks-to-months, not days (design volatility measurement).
- [x] **429 silent-degradation compound detector**: warning text `Rate-limit exceeded;
  falling back to syndication endpoint` AND/OR missing `*_count` metadata AND multi-media
  collapse to one entry → classified retryable (degradation metadata set), never success.
  Fixtures: degraded response (positive), legitimate single-video post (negative — must NOT
  flag), boundary (counts present but warning seen).
- [x] `harvestLinks`: post-text links only (reply-chain harvest is agent-lane `/x:read`,
  optional — hub routing, Phase 6).
- [x] Captions: `--write-subs` never `--write-auto-subs`; raw `LANGUAGE` subtitle keys
  (`en`, `en-US`, `en-GB`, `und`) — no hardcoded `subtitles['en']`; literal `<X-word-ms`
  detection with `--convert-subs srt` cleanup path; `captionClass` declares platform-ASR.
- [x] `extractorArgs = null`; `comments` capability false; browser-cookie-fallback capability
  false (cookies file is the only auth route).

**Sanity Check:**

- [x] Offline fixture tests exit 0 covering: slice-key pair + all four identity fixtures,
  provenance-guard block + 0-result production, both 0-case digest paths, compound 429
  detector (3 fixtures), error-pattern mapping incl. login-required-only cookie gating,
  canonicalization via watch/queue/transcript entries
- [x] `git grep -n -e "automatic_captions" -e "write-auto-subs" -- plugins/knowledge/skills/youtube-digest/extraction/adapters/x.js` → 0 rows
- [x] Storage invariant test: X slice + YouTube slice created under one queue root in a temp
  `--work-root`; both resolve; `QUEUE.md` claims namespace shared; no source directory level
- [x] One-time evidence (NOT a merge gate): manual anonymous download of the design's verified
  public status; media + `.vtt` + `sourceUrl` recorded to
  `.work/source-agnostic-video-digest/evidence/phase2/x-live-probe.md`

### Phase 3: Transcript strategy seam [DONE]

`transcriptStrategy` = `captions` | `captions+repair` | `asr`; per-source default declared by
the adapter, pipeline-overridable.

- [x] YouTube default `captions` — behavior unchanged for every existing user.
- [x] `acquisition/select-caption.js` (shared ladder — editable, T12): consumes the adapter's
  declared `captionClass`; fixes X `.en.vtt``manual-en` misclassification; stage 2 stays
  shared.
- [x] **Selection rule (binding):** X caption-present → `captions+repair`; X caption-absent →
  `asr` **whenever the ASR capability is available**; capability absent → explicit degradation
  (digest without transcript, degradation stated in a named provenance field), never silent.
- [x] `captions+repair`: proper-noun repair over platform VTT; lexicon = post text
  (`description`) + harvested links.
- [x] ASR rung: faster-whisper large-v3, `batch_size=8`; optional closed-by-default
  capability; delivery = documented optional prerequisite + runtime detection, no
  auto-install.
- [x] **T5 probes — write a dated stub row here BEFORE running each; fill outcome after.**
  Evidence: `.work/source-agnostic-video-digest/evidence/phase3/`.
  - `[T5-ASR-ENTITY]` input: one known X clip with technical proper nouns; expected output:
    side-by-side entity transcription (X ASR vs faster-whisper); pass criterion: decision-grade
    verdict on entity fidelity (general WER is NOT an answer; "no advantage" is itself
    decision-grade). Governs any future upgrade to ASR-replace — no plan change either way.
  - `[T5-ASR-TIMESTAMPS]` input: known clip with platform VTT; expected: cue-boundary deltas;
    pass criterion: word-level timestamps usable for frame alignment (load-bearing — blocks
    the asr rung shipping as default-on for caption-absent if it fails; degradation path then
    covers caption-absent until resolved).
  - `[T5-ASR-LEXICON]` input: one clip with/without post-text `initial_prompt`; expected:
    entity-error delta; pass criterion: measurable proper-noun improvement → lexicon also
    feeds the ASR rung; else lexicon stays repair-only.
  - `[T5-ASR-ENTITY]` probe row — stub 2026-08-15 (clip: x.com/lispower1/status/1001551623938805763, phase-2 verified) · outcome: 2026-08-15, faster-whisper large-v3 (int8/CPU) ADVANTAGE — fixes 4 platform entity errors (`Collision``collusion`, `Muller``Mueller`, `Inform Ant``informant`, `Invest Gative``investigative`), introduces none; one non-entity ~0.9s hallucinated phrase noted. Governs future ASR-replace only; no plan change (evidence: `.work/.../evidence/phase3/t5-asr-entity.md`)
  - `[T5-ASR-TIMESTAMPS]` probe row — stub 2026-08-15 (same clip, platform VTT vs faster-whisper word timestamps) · outcome: 2026-08-15 PASS — 277/288 words aligned (96.2%), word-start delta median 21ms / p95 465ms / max 1.1s, all 48 cue boundaries within 421ms → word timestamps usable for frame alignment; asr rung stays default-on for caption-absent (evidence: `.work/.../evidence/phase3/t5-asr-timestamps.md`)
  - `[T5-ASR-LEXICON]` probe row — stub 2026-08-15 (same clip, with/without post-text `initial_prompt`) · outcome: 2026-08-15 FAIL — +1 proper-noun fix (`Spygate`) vs −1 entity regression (`feds``Fed`) plus a prompt-worsened repetition hallucination; no measurable net improvement → lexicon stays repair-only, `initialPrompt` not fed to the asr rung (evidence: `.work/.../evidence/phase3/t5-asr-lexicon.md`)

**Sanity Check:**

- [x] Fixture: X `.en.vtt` classifies per declared class; YouTube classification tests
  unchanged and green
- [x] Fixture: strategy resolution — per-source default; explicit pipeline override wins;
  caption-absent + capability-available selects `asr`; capability-absent exits 0 with the
  named provenance degradation field set
- [x] Three probe rows present with dated stub + outcome (grep `T5-ASR-` in this file → 3
  rows with `outcome:` filled)

### Phase 4: Conformance suite + fixtures [DONE]

- [x] Shared suite asserted once against the contract, star-imported into a thin per-adapter
  test file each adapter owns (SQLAlchemy shape).
- [x] Capability declarations skew closed; **explicit test: a fixture adapter omitting every
  optional capability passes the suite** (absence is a declaration, not a failure).
- [x] Runtime `validateAdapter` covers Phase 0's checker holes (method arity, required-method
  presence, attribute shapes).
- [x] CI collision test: full-registry round-trip — each adapter's canonical example URL
  resolves to that adapter; duplicate/overlapping host claims fail (insurance for adapter
  three, stated as such).
- [x] X golden eval fixture under `evals/` with defined assertions: expected 0..N result
  shape, slice-key pair, provenance fields; wired into the offline CI eval gate.
- [x] Conformance = offline, fixture-based, CI-gated. Liveness lane = tracked follow-up work
  item (never on the merge path); file it in the tracker at phase close.

**Sanity Check:**

- [x] Mutation probes: deleting a required method from a test double fails the suite;
  registering a second adapter claiming `x.com` fails the collision test
- [x] CI green with suite + X golden eval wired into the existing extraction test step
- [x] Tracker item for the liveness lane exists (search-before-create; record number here)
  — **#2797** (2026-08-15; never-gates-merges constraint stated in the body)

### Phase 5: Naming hygiene + env namespace [DONE]

Runs AFTER Phase 3 lands (shares `run-watch.js` / `acquire.js` territory with P1–P3 — see
Execution shape). Epic constant `YOUTUBE_WATCH_EPIC_DIR` **unchanged** per the storage
invariant.

**Display/staged-artifact hygiene:**

- [x] `watch/watch-state.js:169,193` — resume-prompt paths render from the resolved slice dir,
  never the epic constant
- [x] `watch/export-sheet-frame-index.js:75``"{tmp}/youtube-sheets-unknown"` → source-
  neutral literal
- [x] Temp prefixes → `video-*`: `watching/run-watching-pipeline.js:33-34`
  (`youtube-frames-` / `youtube-sheets-`), `watch/run-watch.js:83-85` (incl.
  `youtube-extraction-`), `transcript/run-transcript.js:30` (`youtube-extraction-`)
- [x] **KEEP** `acquisition/acquire-throttle.js:34` lock dir
  `youtube-extraction-acquire-locks` — stable cross-version coordination identifier (same
  A1 (4) rule as the epic constant; renaming it breaks mutual exclusion across the upgrade
  boundary and silently defeats `max_concurrent_acquires`). Recorded in Decisions table.
- [x] Repo-wide staged-artifact literal audit (not just `extraction/`): any `youtube-`-named
  literal that can reach a **staged** artifact is fixed; every deliberate survivor gets a KEEP
  row with reason here

**Env namespace (A2 (c)) — all six, enumerated:**

| Old (read site) | New |
|---|---|
| `YOUTUBE_WORK_ROOT` (`lib/work-root.js:19`; forwarded by `run.mjs`; documented in SKILL.md work-root section) | `VIDEO_DIGEST_WORK_ROOT` |
| `YOUTUBE_YT_DLP_JS_RUNTIMES` (`acquisition/build-yt-dlp-args.js`) | `VIDEO_DIGEST_YT_DLP_JS_RUNTIMES` |
| `YOUTUBE_YT_DLP_COOKIES_FILE` (`acquisition/build-yt-dlp-args.js`) | `VIDEO_DIGEST_YT_DLP_COOKIES_FILE` |
| `YOUTUBE_YT_DLP_COOKIES_FROM_BROWSER` (`acquisition/build-yt-dlp-args.js`) | `VIDEO_DIGEST_YT_DLP_COOKIES_FROM_BROWSER` |
| `YOUTUBE_MAX_CONCURRENT_ACQUIRES` (`acquisition/acquire-throttle.js:23`) | `VIDEO_DIGEST_MAX_CONCURRENT_ACQUIRES` |
| `YOUTUBE_ACQUIRE_PHASE_GAP_SEC` (`acquisition/acquire.js:24` — also enters the `run-args.js` flag map + docs, closing the sixth-knob gap) | `VIDEO_DIGEST_ACQUIRE_PHASE_GAP_SEC` |

- [x] One shared `resolveEnvWithLegacy(newName, oldName)` helper in `lib/`; **every read site
  above** resolves through it (new wins; old works and warns once per process — note
  `run.mjs` re-execs a child, so "once" is per-process by design; state that in the helper's
  doc comment)
- [x] Writers updated: `lib/run-args.js` flag map sets the NEW names; `run.mjs` forwards new
  names
- [x] `userConfig` keys keep their names; the four `"… (youtube-digest)"` titles change in
  Phase 7
- [x] CHANGELOG deprecation for old names (folds into Phase 7's breaking entry)
- [x] SKILL.md work-root/env documentation updates are **handed to the Phase 6 lane** as a
  reconciliation input (P6 owns skill markdown)

**Sanity Check:**

- [x] `git grep -n "resolveEnvWithLegacy" -- plugins/knowledge/skills/youtube-digest/extraction` → ≥ 7 rows (helper + six read sites)
- [x] Env compat tests: old alone works + warns once per process; new wins when both set; all
  six reachable (flag map test covers `--acquire-phase-gap`)
- [x] `git grep -n -e "youtube-sheets-unknown" -e "youtube-frames-" -e "youtube-sheets-" -e "youtube-extraction-" -- plugins/knowledge/skills/youtube-digest/extraction ':!*acquire-throttle*'` → 0 rows (lock-dir KEEP excluded)
- [x] Fixture: X resume prompt contains resolved slice path; zero epic-constant prose
  interpolations

### Phase 6: Hub split + widened description [DONE]

Warrant: **body** 37,620 chars ≈ 9,405 est. tokens ≈ 1.9× the < 5,000-token recommendation
(chars/4; the repo's own fleet-measurement method). The 200-line soft cap is NOT the warrant.

- [x] Execute the hub-split-budget.md moves table (the single authoritative partition):
  Watch action 148 → ~30-line phase spine (rest → `context/watch-pipeline.md`); Queue 61 → ~8
  (→ `context/watch-queue.md`); Output contract + artifact landing → new spoke; yt-dlp &
  throttle overrides → `reference/sources/youtube.md`; slug derivation + eval fixtures trimmed
- [x] Source spokes: `reference/sources/{youtube,x}.md`; any spoke > 100 lines gets a TOC
- [x] Hub routing table: **explicit conditional rows** — "read `reference/sources/x.md` when
  the URL is an x.com/twitter.com status", "read `reference/sources/youtube.md` when the URL
  is YouTube", "read `context/watch-pipeline.md` for the watch action only". A `transcript`
  run loads no watch spoke; a YouTube run loads no X file; `/x:read` reply-chain harvest
  routed here as optional
- [x] `reference/variation-matrix-backlog.json` disposition decided (KEEP in place —
  `vendor/video-digestion/TUNING.md:5` points at it; if it moves, Phase 7 inventory gains
  that row)
- [x] Widened `description` (T2a, `xlsx` shape): four literal hosts + **natural-language
  triggers** ("watch this video", "digest this video/post", etc.) + explicit `Do NOT` clause
  naming course platforms (preserves `course-digest` boundary); retains `youtube` /
  `youtu.be` tokens; third person
- [x] **Record the pre-change trigger-token baseline** (the current description's token set)
  to `.work/source-agnostic-video-digest/evidence/phase6/trigger-token-baseline.md` — Phase
  7's manual continuity check validates against this artifact
- [x] X source section added to the hub within budget
- [x] **Reconciliation gate (before Phase 7):** after P3 lands, diff the spokes' claims
  (adapter behavior, error semantics, ASR/degradation, env-var names from P5) against the
  final contract + fixtures; fix drift. Parallel drafting is allowed; this gate is what makes
  it safe

**Sanity Check:**

- [x] Body size: chars after frontmatter ÷ 4 < 5,000 (small Node/awk snippet, checked in with
  the sweep script or run in Git Bash; record the number here)
- [x] `check-skill.sh` exit 0 on the skill
- [x] Blocking routing assertion (not check 15, which only warns): every file under
  `reference/sources/` and every `context/*.md` spoke is cited in SKILL.md **with a
  conditional "when" clause** — script-checked, not eyeballed
- [x] Description: contains all four hosts, ≥ 2 natural-language trigger phrases, a `Do NOT`
  line naming course platforms, `youtube`, `youtu.be`; combined `description` +
  `when_to_use` frontmatter fields ≤ 1,536 chars (the check-skill DESC_CHAR_CAP definition)
- [x] Reconciliation gate outcome recorded here (dated line)
  — **2026-08-16: gate PASS.** All 12 draft markers resolved across two passes (pre-reconciliation
  `c5730c6b` after P1/P2; final gate `78f521b5`+`d7975e00`+`b8217201` after P3/P5); body 13,328
  chars ≈ 3,332 est. tokens; check-skill 0 errors / 4 pre-existing warning classes; routing
  assertion 10/10 conditional citations; fresh-context verification PASS 9/9 including an
  11-claim spoke-vs-code audit with zero contradictions.

### Phase 7: Rename `youtube-digest``video-digest` [DONE]

Deliberate breaking change (T2b). Terminal phase. **Resume note: the `git mv` commit is the
point of no return — after it, the pre-flight baseline command targets the NEW directory and
CI is red until the ci.yml rows land; a resuming session mid-phase continues the inventory,
never re-runs pre-flight against the old paths.**

**Pre-flight (FIRST item — reproducible baseline):**

- [x] Run: `git grep -c "youtube-digest" -- ':!docs/topics' ':!.work'` and record file+
  occurrence counts here (approval-time measure: 30 files / 112 occurrences repo-wide
  incl. the skill dir; in-skill: 14 files / 65). Any file NOT in the inventory below gets a
  row before the sweep proceeds. (`docs/topics/` and `.work/` are excluded as immutable
  historical/session records — Decisions table.)
  **Recorded (2026-08-16, pre-mv):** 33 files / 114 occurrences (delta vs approval-time =
  Phase 6's hub split redistributing SKILL.md content into spokes); un-inventoried files were
  the Phase-6-created spokes, covered by the in-skill glob rows — dispositions in DEVIATIONS.md.
- [x] Pre-check the CI job id: `gh api "repos/{owner}/{repo}/branches/main/protection"` (or
  rulesets) — is `youtube-extraction` a required status check? Required → KEEP job id with
  comment; not required → rename job id alongside the paths. Record outcome here.
  **Outcome (2026-08-15, run early while P3 probes were in flight):** branch protection 404
  (no legacy protection); rulesets `base`/`ci-gate`/`signing` active on the default branch;
  `ci-gate` requires exactly `pr-title / pr-title`, `pr-issue-linkage / pr-issue-linkage`,
  `do-not-merge / do-not-merge`, `ci-status``youtube-extraction` is NOT a required check
  → **rename the job id alongside the paths.**

**Structural move (single commit):**

- [x] `git mv plugins/knowledge/skills/youtube-digest plugins/knowledge/skills/video-digest`
- [x] Frontmatter `name:` — per the FALLBACK decision below (default: NO pin; T2b term
  deviation recorded with evidence)

**In-skill content sweep (14 files / 65 occurrences — same commit series):**

- [x] `SKILL.md` (23 occ — self-references become `/knowledge:video-digest`; the
  `${CLAUDE_PLUGIN_ROOT}/skills/video-digest/extraction/run.mjs` launcher lines)
- [x] `watch/watch-state.js:2,145,158`**user-facing**: `:158` emits
  `# Continue /youtube-digest watch` into resume prompts (same failure class as the recovery
  command)
- [x] `watch/detect-recoverable-bootstrap.js:112` — the twelfth entry point: emits
  `skills/youtube-digest/extraction/run.mjs` into a user-run recovery command
- [x] `watch/run-watch.js:3`, `watch/run-resume.js:3`, `lib/slice-lanes.js:2` (comments/docs)
- [x] `context/*.md` (5 files), `templates/*.md` (2 files), `evals/evals.json`

**Outside-skill sweep:**

- [x] `plugins/knowledge/.claude-plugin/plugin.json`**4 occurrences, all userConfig
  titles** (`:44,50,56,62`; no skill-path key exists — discovery is by directory)
- [x] `plugins/knowledge/.claude-plugin/plugin.json``keywords`: add `x`/`twitter`/`video`;
  plugin `description` updated off "YouTube pipeline" (marketplace discoverability)
- [x] `plugins/knowledge/CHANGELOG.md` (new entry; historical entries KEEP verbatim)
- [x] `plugins/knowledge/README.md`, `skills/setup/SKILL.md`, `skills/docpage-digest/SKILL.md`,
  `skills/map-corpus/SKILL.md`
- [x] `.github/workflows/ci.yml`**5 occurrences** (`:855` comment, `:888`
  cache-dependency-path, `:892,896,900` working-directory); job id per pre-check
- [x] `scripts/docs-only-paths.txt:21` — inertness-proof comment path updated; re-run
  `scripts/check-docs-only.test.sh`
- [x] `docs/knowledge-integration-design.md:12`, `docs/MIGRATION-PLAYBOOK.md:1455`,
  `docs/CLOUD-SESSIONS.md`
- [x] `extraction/package.json` — per the FALLBACK decision below (default: rename
  `@melodic/youtube-extraction``@melodic/video-extraction` + description; regen lockfile;
  ci.yml cache key rides the path change)
- [x] `extraction/setup-deps.mjs:64``.youtube-extraction.stamp``.video-extraction.stamp`
  (added by the Phase 5 staged-literal audit — invisible to the `youtube-digest` sweep; rides
  the npm package rename; costs one harmless dependency reinstall; DEVIATIONS.md)

**`course-digest` mechanical refs (the exemption's exhaustive enumeration):**

- [x] `course-digest/SKILL.md:2` (description — verify no course-digest trigger-keyword
  regression via `check-skill.sh` on course-digest)
- [x] `course-digest/SKILL.md:138`
- [x] `course-digest/context/storage-schema.md:7`
- [x] `course-digest/evals/evals.json:19,21,25` (case renamed
  `youtube-url-routes-to-video-digest-skill` + expected_output + criterion)
- [x] `course-digest/reference/adapters/discovery-checklist.md:65,221`
- [x] `course-digest/extraction/setup-deps.mjs:50` (comment)

**Vendored stragglers (sweep regression set — fix regardless):**

- [x] `vendor/video-digestion/README.md:5` (`/youtube`)
- [x] `vendor/repo-analysis/README.md:5` (`/youtube`)
- [x] `vendor/video-digestion/TUNING.md:55` (`/youtube`)
- [x] `vendor/video-digestion/TUNING.md:5` (path ref
  `skills/youtube-digest/reference/variation-matrix-backlog.json`)

**Breaking-change discipline:**

- [x] CHANGELOG breaking entry naming the **five silent consumer surfaces**: cloud routines;
  scheduled tasks / `/loop`; `Skill(knowledge:youtube-digest)` permission rules (exact-match —
  a **deny rule fails open**); Agent SDK `skills:` allowlists (the one loud failure);
  bare-`/name` squatting / docs. Plus the env-name deprecations (Phase 5).
- [x] Out-of-band announcement (release notes / PR body) with a migration line per surface
- [x] `plugin.json` version 0.12.0 → 0.13.0
- [x] Manual trigger-token continuity check against the Phase 6 baseline artifact (the HEAD
  keyword diff is blind post-rename); result recorded here

**Sweep script (checked in, e.g. `extraction/scripts/` or repo `scripts/` per conventions):**
`git grep -n "youtube-digest" -- ':!.git' ':!docs/topics' ':!.work'` filtered by an **explicit
inline allowlist**: CHANGELOG historical entries; this PLAN's own baseline lines. Exit
non-zero on any other hit. Self-test seeds a stale ref in a temp file under `vendor/` and
asserts the sweep catches it.

**Sanity Check:**

- [x] Sweep script exit 0; its self-test (seeded `vendor/` ref) exits non-zero
- [x] `git grep -n "/youtube\b" -- plugins/knowledge/vendor` → 0 rows (Git Bash)
- [x] `git grep -c "youtube-digest" -- .github/workflows/ci.yml` → 0 (static assertion — CI
  green alone does not prove textual migration)
- [x] `bash scripts/check-docs-only.test.sh` exit 0
- [x] `check-skill.sh` exit 0 on `video-digest` AND on `course-digest`
- [x] Fixture tests: `detect-recoverable-bootstrap` emits the new path; `watch-state` resume
  prompt emits `/knowledge:video-digest`
- [x] CI green on the branch

## Blast radius

**HIGH.** Published marketplace plugin; breaking rename with five silently-failing consumer
surfaces (deny rules fail open — security-relevant); CI workflow changes; 30+ files outside
the skill dir; new cross-cutting contract. Mitigants: no on-disk data migration (storage
invariant); rename isolated to the terminal phase with a self-tested sweep script; CI loud on
path misses; conformance suite offline and CI-gated.

## Stress-test summary

Two independent reviews of the draft, 2026-08-15; all findings verified against the tree
before adoption:

- **Codex (cross-vendor, /codex:rescue; artifacts inlined after a sandbox failure):** 3
  CRITICAL / 16 IMPORTANT / 2 SUGGESTION. Adopted: 0-with-link semantics (T6), storage
  invariant stated + tested, envelope schema + consumer pre-flight, acquire call-site
  definition, static-import verification, canonical identity rule, four-type taxonomy
  clarity, three login-required cases, compound 429 detector, mandatory strategy selection +
  probe pass criteria, entry-path inventory, six env vars enumerated, P5 serialization,
  Git-Bash-only sanity commands, live probes demoted to evidence, resumability
  (evidence paths + IN-PROGRESS + mid-P7 note), static ci.yml assertion, description checks,
  blocking routing assertion, capability-absence fixture, doc reconciliation gate. Rejected
  (verified false): "plugin.json has a skill path key" (4 title occurrences only);
  "select-caption.js not named" (it was).
- **Devils-advocate (fresh-context sub-agent):** 2 CRITICAL / 4 HIGH / 9 MEDIUM / 5 LOW.
  Adopted: P5 fence rewritten to the six env READ sites; `.work/` + `scripts/docs-only-paths.txt`
  in the sweep scope; `youtube-extraction-` prefix + `run-transcript.js:30`; lock-dir KEEP;
  in-skill 14-file/65-occurrence sweep + `watch-state.js:158`; `name:` pin re-decided (gate
  evidence contradicts the pin's rationale — FALLBACK row); reproducible pre-flight baseline;
  `--write-comments` adapter-declared; predicates `:49`/`:60` as the real classification seam
  plus browser-fallback capability gate; `TUNING.md:5` path ref; `@melodic/youtube-extraction`
  package identity decision; ci job-id/branch-protection pre-check; preflight `:58-63` full
  pattern set; token-basis fix (body-only). Rejected (verified false): caps line-number
  correction (they ARE at `:175-177`).

## Execution shape

### Dependency graph

- P0 → P1 → P2 → P3 → P4 (contract chain)
- P5 after P3 (shares `run-watch.js`, `acquire.js`, `run-transcript.js` territory with P1–P3);
  may run parallel with P4 (P4 = test/eval files; P5 = the enumerated source files)
- P6 drafts parallel to P2–P5; its **reconciliation gate** (post-P3, incl. P5's env names)
  must pass before P7
- P7 terminal, gated by all

### Recommended shape

> Wave A: P0 (solo gate)
> Wave B (parallel, 2 lanes): {P1 → P2 → P3} ∥ {P6 drafting}
> Wave C (parallel, 2 lanes): {P4} ∥ {P5}, then P6 reconciliation gate
> Wave D: P7 (solo)
> Cost note: 2 parallel lanes ≈ 2× token burn during Waves B/C; the saving is the full
> markdown lane off the critical path.

Sequential fallback: on any scope-fence violation, concurrent-edit race, or cannot-complete
report, abort the affected agent and run its phase sequentially at the next wave boundary;
other lanes continue.

### Per-phase routing

| Phase | Surface | Basis |
|---|---|---|
| 0 | sub-agent worker | mechanical flip + diagnostic fixes; escalates if any fix would need a suppression |
| 1 | main-session | judgment-heavy: contract authorship, security posture, shared rewiring |
| 2 | main-session | judgment-heavy: provenance guard, degradation detection, identity rules |
| 3 | main-session | design-adjacent seam + manual probes |
| 4 | sub-agent worker | suite shape fully specified; mechanical once contract is fixed |
| 5 | sub-agent worker | enumerated sites; fence below |
| 6 | sub-agent worker | measured moves table; fence below; reconciliation gate verified main-session |
| 7 | sub-agent worker | checkbox-inventory sweep; fence below |

**Scope fences (ALLOWED whitelists; everything else FORBIDDEN, incl. PLAN.md — main session
edits status tags only — and commit/push):**

| Agent | ALLOWED |
|---|---|
| P0 | both `extraction/` trees (tsconfig + JSDoc-only edits) |
| P4 | `extraction/adapters/*.test.js`, new suite files, `evals/**`, CI test-step wiring |
| P5 | `lib/work-root.js`, `lib/run-args.js`, `lib/` (new env helper), `acquisition/build-yt-dlp-args.js`, `acquisition/acquire-throttle.js` (env only — lock dir KEEP), `acquisition/acquire.js` (env line only), `run.mjs`, `watch/watch-state.js`, `watch/export-sheet-frame-index.js`, `watching/run-watching-pipeline.js`, `watch/run-watch.js` (temp-prefix lines only), `transcript/run-transcript.js` (temp-prefix line only), plus co-located tests |
| P6 | `skills/youtube-digest/{SKILL.md,context/**,reference/**}` only |
| P7 | the checkbox-inventory rows exactly, `skills/video-digest/**` (in-skill sweep), the sweep script |

Every worker brief carries the divergence-escalation clause verbatim (planning plugin
plan-template.md).

## Decisions made (gate-passed)

| Decision | What it changes in the plan | Basis |
|---|---|---|
| [EXEC-SHAPE] Rename terminal (P7) | One sweep; CI paths break in exactly one commit series | T2a design-bound "strictly prior to rename"; minimal churn |
| [EXEC-SHAPE] P5 serialized after P3; two-lane Waves B/C | Routing + fences above | Verified file overlap (`run-watch.js`, `acquire.js`, `run-transcript.js`) |
| [EXEC-SHAPE] Temp prefixes → `video-frames-` / `video-sheets-` / `video-extraction-` | Phase 5 literals | Matches new skill name; any source-neutral literal satisfies the design |
| [EXEC-SHAPE] Error-type identifier names implementer's discretion | Phase 1 detail | Design delegates "concrete identifier names are PLAN-level detail" |
| [EXEC-SHAPE] KEEP `youtube-extraction-acquire-locks` lock dir | Phase 5 exclusion | Stable cross-version coordination identifier — renaming breaks mutual exclusion across the upgrade boundary (A1 (4) rule; verified `acquire-throttle.js:34`) |
| [EXEC-SHAPE] Classification seam = predicates `:49`/`:60`, not the `:54` spawn call; browser-fallback gated on adapter capability | Phase 1 wiring | Verified in `spawn-yt-dlp-with-auth-fallback.js`; design's ":54" cites the spawn line — same intent, precise seam |
| [FALLBACK — confirm or override] `docs/topics/**` and `.work/**` KEEP their `youtube-digest` refs (immutable historical/session records); sweep excludes both | Phase 7 scope | Repo precedent (`shadowed-skill-renames/PLAN.md:32` retains pre-rename names); `.work/` is git-tracked here and contains 6 research files with the literal |
| [FALLBACK — confirm or override] **Drop the `name:` pin** — deviates from T2b's recorded "pin `name: video-digest`" term | Phase 7 frontmatter row | Verified gate evidence: `check-skill.sh:284` hard-FAILs a name≠directory mismatch, so the pin cannot buy future directory-rename freedom in this repo, and a matching pin draws a WARN + registers the bare `/video-digest` alias — the same squatting surface the breaking-change list flags. If the bare alias is wanted, override to "pin, same commit as `git mv`, alias rationale recorded" |
| [FALLBACK — confirm or override] Rename npm package `@melodic/youtube-extraction``@melodic/video-extraction` (+ description, lockfile regen) | Phase 7 row | Package identity survives every grep sweep otherwise; verified `package.json:2,6`. KEEP-with-reason is the cheaper alternative if lockfile churn is unwanted |
| [FALLBACK — confirm or override] CI job id `youtube-extraction`: decided by the branch-protection pre-check (required check → KEEP + comment; else rename) | Phase 7 pre-flight | Job id invisible to sweeps; protection state not readable from the worktree — `gh` check required |
| [FALLBACK — confirm or override] Plugin version 0.12.0 → 0.13.0 (pre-1.0 minor-as-breaking) + keywords/description refresh | Phase 7 rows | SemVer 0.x convention; no marketplace doc found mandating otherwise |
| [FALLBACK — confirm or override] ASR = documented optional prerequisite + runtime detection, never auto-install | Phase 3 delivery | T5: optional closed-by-default capability with explicit degradation; auto-install lands a multi-GB dependency the design rejected for existing users |

## Open questions

- None blocking. The `[T5-ASR-*]` probes run inside Phase 3 with recorded outcomes; the CI
  job-id question resolves via the Phase 7 pre-check.

## Handoff to implementation

### User-approval gates

- ~~The six `[FALLBACK]` rows above — confirm or override at plan approval.~~ Approved
  2026-08-15, RECOMMENDED options as recorded in the Status line.
- Any scope expansion beyond type-annotation-only edits in `course-digest` — STOP and surface.
- Any suppression (`@ts-ignore` etc.) — explicit approval + recorded justification.
- Mid-flight pivots that change acceptance criteria — dated scope-change note here + ask.

### Execution shape ([EXEC-SHAPE] tagged)

Waves, routing, and fences as in "Execution shape". PLAN.md is main-session-only; workers
report back.

### Mechanical work

- Commit boundaries: ≥ 1 commit per phase; Phase 7 splits the `git mv` (structural) from the
  reference sweep (Tidy First); PLAN status-tag updates ride each phase's commit.
- Verification: phase Sanity Checks gate each boundary; `npx tsc --noEmit` + vitest + affected
  eval checks at every boundary.
- The Phase 7 sweep is a checked-in, self-tested script — never copy-paste commands.
- Close-out at PR time via `/planning:plan close-out` (PLAN into PR body `<details>`, ADR
  graduation, prune slice with pointer). ADR candidates (write when the decision crystallizes):
  static-registry dispatch (T3) and the storage-format-identifier rule (A1 (4)) — both pass
  the hard-to-reverse / surprising / real-trade-off test.
DEVIATIONS.md — the autonomous-run deviation ledger (final revision, pre-prune)
# Deviations log — source-agnostic video digest

Autonomous-run deviation ledger per the dispatch discipline: Moderate divergences resolved with
the conservative option are logged here at deviation time and reviewed at PR time.

## 2026-08-15 — Phase 0: strict tier pinned off (planned: bare `checkJs` flip)

- **Planned:** flip `checkJs: false → true` in both extraction tsconfigs; fix surfaced diagnostics.
- **Wrong premise:** TypeScript 6.0 defaults `strict` to `true`; neither tsconfig declared it, so
  the flip alone dragged in the full strict tier (408 diagnostics course-digest, 135 youtube-digest).
  Full strict is unreachable inside the phase fence: 48 TS7016 diagnostics trace to the vendored
  `@melodic/video-digestion`, whose only root-cause fix is typing the vendor package
  (`plugins/knowledge/vendor/**`, outside scope); the alternatives are a blanket `any` module stub
  (a suppression in declaration form) or hand-copied upstream declarations.
- **Done instead:** both lanes pin `checkJs: true`, `strict: false`, `strictNullChecks: true`, and
  extend `include` with `**/*.mjs`. `strict` pinned explicitly (caret-ranged `typescript` could move
  the default again); `strictNullChecks` re-enabled deliberately because Phase 1's result envelope
  and four-type error taxonomy are discriminated unions, which the checker silently stops narrowing
  without it (proof: `acquire.js:328` errored without the flag).
- **Blast radius:** type-lane only; zero runtime change (course 91/91, youtube 270/270 vitest green).
  Success criterion 3 ("tsc --noEmit green with checkJs: true") holds as written.
- **Follow-up:** full-strict adoption deferred; trigger = typing or replacing the vendored package.

## 2026-08-15 — Phase 0 sanity-command precision fix

The suppression grep as written in PLAN Phase 0 (`grep -rn … plugins/knowledge/skills/*/extraction`)
matches 116 third-party rows under `node_modules` once deps are installed. Equivalent intent,
corrected form: `git grep -n -e '@ts-ignore' -e '@ts-expect-error' -- 'plugins/knowledge/skills/*/extraction'`
→ 0 rows (tracked source only). Later phases use the `git grep` form.

## 2026-08-15 — Phase 1 remediation: dispatch seam lives in registry.js, not acquire.js

PLAN Phase 1 wrote "`acquire.js` routes source-id + acquisition through the registry/adapter".
The review round found the resulting `acquire.js → registry → youtube.js → acquire.js` static
cycle to be an initialization hazard (empirically verified: entry-order-dependent TDZ throw),
held together only by an unstated nothing-dereferences-adapter-at-module-eval invariant. Fix:
`extractVideoId` moved into the YouTube adapter (its URL grammar) and the `acquireMedia`
dispatch moved into `adapters/registry.js`; `acquire.js` no longer imports the registry, the
graph is acyclic, and the registry consistency check runs at plain module init. Same intent
(all acquisition routes through the adapter seam), sharper seam location. Landed `5020fd4a`.

## 2026-08-16 — Phase 7: sweep-allowlist grew to four classes; in-spirit sweep additions

- The sweep script's allowlist could not be the PLAN's two rows: the Phase 6 trigger-token
  baseline mandates the `'/youtube-digest'` description token verbatim (migration alias), and
  the two fixture `.not.toContain("youtube-digest")` assertions plus the sweep script pair
  self-reference the literal. Allowlist = CHANGELOG, the SKILL.md trigger line, the fixture
  negative assertions, the sweep pair — each named inline in the script.
- Resume prompt emits `# Continue /knowledge:video-digest watch` (PLAN's fully-qualified
  fixture spec won over the brief's bare `/video-digest` wording).
- In-spirit sweep additions beyond the inventory (all mechanical): ci.yml youtube-extraction
  comments/`needs:` row/echo renamed with the job id; TUNING.md `youtube-extraction/`
  shorthand headings rode the npm package rename; README skill-row prose updated off
  YouTube-only; setup-deps user-facing output strings. Pre-flight baseline measured 33 files /
  114 occurrences (approval-time 30/112; delta = Phase 6's hub split redistribution).
- Post-verification accuracy fix (orchestrator, close-out): the two yt-dlp cookie userConfig
  descriptions still described YouTube-only cookie semantics; updated for X (cookies-file-only,
  no browser-profile iteration).

## 2026-08-15 — Phase 5: two granted fence extensions (env-table miss + acceptance-grep reach)

- **Auth-lane env reads (behavioral):** the PLAN's six-var table named
  `acquisition/build-yt-dlp-args.js` as the read site for the yt-dlp cookie vars, but
  `acquisition/acquire-yt-dlp-auth.js` also reads two of them via the exported constants.
  Renaming without routing those reads through `resolveEnvWithLegacy` silently broke the
  auth-fallback lane for legacy-only users (proven by three failing tests before the fix:
  the fallback loop fired 6 spawns instead of 1). Fence extended to those two expressions +
  the co-located test; the warn-once set is keyed per legacy name process-wide so
  builder+auth reads cannot double-warn.
- **Fixture literals (mechanical):** the acceptance grep for `youtube-{frames,sheets,extraction}-`
  literals reaches four test files whose production sources were outside the enumerated
  fence (`lib/temp-session-paths.test.js`, `watch/detect-recoverable-bootstrap.test.js`,
  `watch/snapshot-bootstrap.test.js`, `watch/sanitize-slice-temp-paths.test.js`). Literal-only
  renames granted (applied by a scoped `sed` global replace; content verified by grep + full
  suite).
- **New P7 inventory row (from the staged-literal audit):** `setup-deps.mjs`
  `.youtube-extraction.stamp` install stamp is invisible to the `youtube-digest` sweep and was
  absent from the P7 inventory — added there (rename alongside the npm package rename; costs
  one harmless dependency reinstall).

## 2026-08-15 — Phase 2: contract grew two review-driven declarations

Security review (two independent reviewers; the cross-checked SSRF finding was CRITICAL) drove
two additive contract attributes not in the PLAN's Phase 1 attribute list:

- **`allowedExtractors` (required, string|null):** yt-dlp's link-post delegation (upstream
  #9715) meant the probe pass fetched an attacker-chosen outbound URL — cookie-bearing —
  before the provenance guard could read the result. The fix restricts every spawn (probe,
  media, queue preflight) with `--use-extractors` from an adapter declaration; X declares
  `twitter.*` (family verified live: twitter, :amplify, :broadcast, :card, :shortener,
  :spaces), YouTube declares null. A refused delegation (`ERROR: No suitable extractor found
  for URL <url>` — the live-verified refusal shape) is parsed into the well-formed 0-result
  with the blocked link in provenance; a foreign info JSON on disk is now a hard failure.
- **`mediaOptional` capability (closed-by-default):** maps to `--ignore-no-formats-error` in
  BOTH acquire and preflight arg builders, so a valid 0-video X post enqueues metadata-only
  and digests text-only end-to-end (T6 D-A), removing the queue-vs-watch asymmetry. YouTube
  unchanged.

Also from review, recorded here: X fatal patterns gained `/Media #\d+ is not a video/` and
sibling `/Video #\d+ is unavailable/` (deterministic index-selected conditions, not in the
PLAN's three observed literals — additive, same permanence class); the self-referential
`/X acquisition degraded/` retryable pattern was removed (an adapter table describes source
stderr only).

**Deferred with trigger:** the 429 `countsMissing` heuristic classifies a permanently-degraded
post retryable on every attempt. Today no queue-level retry consumes adapter retryable
patterns, so no loop exists. Trigger: any future queue-retry lane adopting adapter patterns
must add an attempt cap or degraded-accept path (doc note lives on the detector).

## 2026-08-15 — Phase 0: `@satisfies` standardization had no applicable sites

The only contract-bearing `@type` object literal in either tree is `utils.js:111` (Node stdlib
`ParseArgsOptionsConfig`, frozen course-digest lane). The `@satisfies` convention lands with
Phase 1's contract authorship instead of a retrofit.

🤖 Generated with Claude Code

kyle-sexton and others added 30 commits August 15, 2026 01:05
…ract

Design slice for folding X (Twitter) video into the single-public-video
lane as an engine-layer source adapter, without duplicating the pipeline
and without touching course-digest.

Artifacts under docs/topics/source-agnostic-video-digest/design/:
capability-matrix, design-threads (14), inherited-decisions,
hub-split-budget, consumer-context. Research slices under
.work/source-agnostic-video-digest/ — four topics, each gate-checked and
adversarially verified by an independent context.

Resolved this pass:

- Source is slice metadata, not a directory level. No per-source epic
  dirs; default path unchanged, so no migration and no orphaning.
- select-caption.js is NOT a managed surface. The inherited "never edit
  there" constraint was unsupported; standards' sync manifest declares 37
  components and none is in plugins/knowledge.
- No authoritative source states a 200-line SKILL.md target. The real
  constraint is the spec's < 5,000 tokens, which this hub exceeds at
  ~1.9x while passing the 500-line gate.
- Dispatch: a static host-keyed registry, not a computed dynamic import.
- Multi-media posture reversed: return a uniform 0..N collection; arity
  is a property of the result, never of the adapter.

Defects found in existing code:

- checkJs is false in both extraction trees with zero @ts-check
  directives, so CI's tsc --noEmit passes while checking nothing.
- resolveAdapter interpolates unvalidated input into a module specifier
  (CWE-829, CWE-22). Low severity today; high once an adapter is
  selected from a URL.
- twitter.py returns linked off-platform videos merged with the tweet's
  full provenance, so a digest would cite a foreign video as the X post.

Open: T5's choice of transcript fallback. Platform-captions-only is
eliminated (X captions are absent more often than present with no
predictor); ASR feasibility clears on consumer hardware (~2m37s for a
2-hour video, batched).

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ff gate

Resolve the eleven threads the gate FAILed on, adopting each thread's
recorded recommendation under user delegation (2026-08-15):

- T1 adapter home: extraction/adapters/ (contracts are different
  abstractions; vendor/ stays source-agnostic)
- T2a: widened description ships with the feature, xlsx shape
- T2b: rename to video-digest; pin name:; breaking-change terms bind
  the PLAN; three stale /youtube refs are the sweep regression test
- A2: env rename with compatibility read; sixth env var named;
  adapter-namespaced config TAGGED-DEFERRED with trigger
- T3: static host-keyed registry; type lane on is step zero
- T4: five required methods + declared attributes; stage 2 collapses
  to shared (per-source stages 7 -> 6, capability-matrix updated)
- T5: directional with [T5-ASR-*] tags - transcriptStrategy seam;
  YouTube captions, X captions+repair / asr-on-absent; (iv) rejected
- T6: D-A closed - no-video post yields text-only digest
- T7: reference/sources/ spokes with explicit conditional routing
- T9: shared conformance suite, closed-by-default capabilities,
  X golden fixture
- T10: sub-decision (ii) adapter-level URL canonicalization
- T11: four distinct error types; per-adapter pattern table; cookie
  fallback gated on login-required class only

Add design-handoff-gate.md recording both gate runs (FAIL then PASS)
and the root cause: research-gate-passed was conflated with
thread-resolved in the prior session's handoff.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
…tion plan

Eight-phase PLAN for the youtube-digest -> video-digest refactor: type lane
on (checkJs), adapter contract + static host-keyed registry, X adapter,
transcript strategy seam, conformance suite, naming/env hygiene with
compatibility reads, hub split + widened description, and the terminal
breaking rename with its enumerated sweep.

Stress-tested by a cross-vendor Codex review (3C/16I/2S) and a fresh-context
devils-advocate pass (2C/4H/9M/5L); all findings verified against the tree,
three rejected on evidence, the rest folded in. Approved 2026-08-15 with all
six FALLBACK rows at their recommended options.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Phase 0 of the source-agnostic video digest work: the adapter contract
landing in youtube-digest/extraction needs the checker actually reading the
JS, so flip checkJs first and clear every diagnostic it surfaces.

TypeScript 6.0 defaults strict to true, so flipping checkJs alone dragged in
the full strict tier (408 diagnostics in course-digest, 135 in youtube-digest)
which the repo never opted into. Most of that is TS7016 against the vendored
@melodic/video-digestion, whose only root-cause fix is typing the vendor
package -- out of scope here, and the alternatives (a blanket `any` module
stub, or hand-written declarations mirroring upstream) are both worse than an
explicit posture. So strict is pinned false explicitly rather than left to the
compiler default, which a caret-ranged typescript upgrade can move again.

strictNullChecks is pinned back on: without it TypeScript silently stops
narrowing discriminated unions, and the adapter contract's result envelope and
error taxonomy are exactly that. It costs 14 further diagnostics, all fixed.

include gains **/*.mjs -- the launchers (run.mjs, setup-deps.mjs, the hook
shims) matched no include pattern and were never type-checked at all.

Every fix is a type annotation or a tsconfig change; no runtime logic changed,
proven by both suites passing unchanged (course-digest 91, youtube-digest 270).
No @ts-ignore or @ts-expect-error was added.

Notable root causes, as opposed to silenced symptoms:

- PreflightResult declared `@property {string} handle - raw channel @handle`;
  the bare @handle parsed as a new JSDoc tag and truncated the typedef, so
  displayTitle and displayChannel were never declared.
- 13 JSDoc import() specifiers named vendor subpaths with a .js suffix that the
  vendor's exports map does not publish -- they would throw
  ERR_PACKAGE_PATH_NOT_EXPORTED if they were ever runtime imports.
- WatchState under-declared four frameSelection/artifactPaths properties that
  both writers set and the resume-prompt reader consumes.
- spawnWithAcquireRetry forwards extra options to spawn but declared only its
  own retry options, so every cwd-passing caller was a type error.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Phase 0 landed in aaa62cc and passed fresh-context acceptance verification.
DEVIATIONS.md records the strict-tier posture call (TypeScript 6.0 defaults
strict on; pinned false with strictNullChecks re-enabled), the suppression-grep
precision fix, and the @Satisfies deferral to Phase 1.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…seam

Adapter contract (5 required methods, declared attributes, 0..N result
envelope, four-type error taxonomy), static host-keyed registry that fails
closed on unknown hosts, and a YouTube adapter owning URL grammar, enqueue
acceptance, error patterns, extractor args, and link-harvest composition.
Shared machinery consumes adapter declarations: comment/extractor-args flags
and the browser-cookie fallback loop are closed by default and gated on
adapter capabilities; cookie fallback fires on login-required classification
only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…envelope

Contract validation (arity, closed capabilities, reserved content-claim),
static-import registry conformance with fail-closed unknown-host dispatch and
subdomain rules, YouTube adapter claims/declarations, CLI wrapper unknown-host
exits, and fixture-driven 0/1/N envelope consumption through both the watch
and transcript code paths. JSDoc type references in adapters/ use the @import
tag so the no-dynamic-import grep gate stays clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… draft)

Hub SKILL.md body drops from 37,620 to 13,008 chars (~3,252 est. tokens,
under the <5,000-token guidance) by moving the watch-action detail to
context/watch-pipeline.md, queue detail to context/watch-queue.md, the
output contract to a new context/output-contract.md spoke, and per-source
material to reference/sources/{youtube,x}.md, each cited from the hub with
a conditional "when" routing clause. The description widens to cover
x.com/twitter.com status URLs with natural-language triggers and an
explicit course-platform Do NOT boundary.

Draft stage: 12 RECONCILE markers flag implementation-dependent claims for
the post-implementation reconciliation gate before the Phase 7 rename.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Recovery reads sourceUrl from the slice's persisted watch.json instead of
synthesizing a YouTube URL; production consumes yt-dlp flags and spawn
classification straight off the adapter declarations
(adapterSourceDeclarations + classifyErrorDetail), collapsing the private
side-channel object; the URL grammar moves into the YouTube adapter and
dispatch (acquireMedia) into the registry, making the adapter module graph
acyclic so the registry consistency check runs at module init; dispatch fails
closed on a null URL claim from the owning adapter; transcript.txt pairs with
the primary (media-bearing) entry and the CLI's legacy single-entry fields
follow that entry at N>1; the slice README records the URL-authoritative
slice key; deriveVideoSlug rejects non-path-safe slice keys and yt-dlp argv
gets a -- end-of-options sentinel before the URL (verified against yt-dlp
2026.07.04); caller-less writeTranscriptArtifacts removed; a shared
adapter-test harness (describe-factory) carries the contract-generic
assertions so the next adapter's tests compose instead of copy-pasting.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Phase 1 landed in e61fb53 + 56b11a6 and was hardened in 5020fd4 after
security and architecture review (recovery-lane sourceUrl from slice state,
N>1 transcript handling, side-channel collapse onto adapter declarations,
acyclic dispatch graph, null-claim fail-closed, path-safe slice keys, argv
sentinel, shared adapter-test harness). Fresh-context verification passed
10/10; DEVIATIONS.md records the registry-side dispatch-seam supersession.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…st seam

Second implementer of the source-adapter contract: x.com/twitter.com status
URLs claim, canonicalize (adapter-level, every entry path by construction),
and acquire through the shared yt-dlp machinery with a probe-first pass so
the provenance guard blocks foreign-extractor delegations before any media
download. Snowflake slice identity (display_id canonical, media id as
discriminator, flagged quote/retweet aliasing recorded in slice metadata),
compound 429 syndication-degradation detector (retryable, never success),
0..N envelope arities incl. both text-only 0-cases, raw-language caption
keys with the word-timing-tag cleanup pass, and login-required cookie
gating limited to the three documented cases.

Shared seams consumed: per-source yt-dlp caption/no-formats declarations in
build-yt-dlp-args (subLangs, omitAutoSubs, ignoreNoFormatsError,
convertSubs) and canonical-URL preflight probing; registry registers the
x module statically. Storage invariant covered by a mixed-source test:
one queue root, shared claims namespace, source never a directory level.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Keeps the adapters directory free of `import(` in any spelling so the
static-registry audit grep stays a zero-row assertion.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Live probe of the design's verified public status shows one <X-word-ms>
tag per cue wrapping the text, with ms durations and character_ranges as
attributes; the fixture now mirrors that shape.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
SSRF guard: adapters declare a yt-dlp extractor allow-list
(allowedExtractors, contract attribute; X = twitter.*) that every probe,
media, and preflight invocation pushes as --use-extractors, so a link
post's delegated foreign URL is refused by yt-dlp WITHOUT any fetch
(verified empirically: refusal line "ERROR: No suitable extractor found
for URL <url>"). The refusal is parsed before classification and the
post resolves as a well-formed 0-media result with the blocked link
recorded; a foreign info JSON on disk is now a hard provenance failure
(defense-in-depth), and a successful spawn that wrote no info JSON fails
instead of laundering into a durable 0-result.

Also: caption cleanup is atomic (tagged originals move aside and are
unlinked only after the cleanup pass succeeds with output; failure fails
the acquisition); status-id/media-index grammar bounded (\d{1,25} /
\d{1,3}) closing the BigInt sink and slice-key length path; /video/0
survives canonicalization; login-required patterns anchored to
[twitter]-tagged ERROR lines so attacker-influenced URLs on stderr can
never gate cookie retries; preflight fails closed on a null URL claim
and markdown-escapes stderr-derived note/reason cells.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Structural media-pass gate (foreign-free in the condition, not only the
earlier return); AcquireContext.workDir documented as empty-and-exclusive
per acquisition; blocked-delegation harvest links URL-parsed with an
http/https scheme allowlist; the self-referential retryable pattern
removed (the table describes source stderr signatures only); counts-
missing degradation judged only on post-level payloads (playlist or
metadata-only info), never a media-entry stand-in, with an attempt-cap
note; deterministic pinned-index failures (Media #n is not a video,
Video #n is unavailable) mapped fatal; metadata-only info collected as
an array; inspection re-derived after the media pass so degradation,
ordering, and entry paths never read stale state; srt-to-vtt conversion
scoped to the post's own media ids.

New mediaOptional capability (contract + validator): 0-media posts are
well-formed for every yt-dlp consumer — acquisition AND queue preflight
now push --ignore-no-formats-error from the adapter declaration, so a
valid 0-video X post enqueues as metadata-only instead of rejecting
(T6 D-A text-only digest). Quote-tweet and retweet aliasing get distinct
fixtures completing the four named identity cases.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Phase 2 landed across db73ef0, 7b88443, 6f2ef44, then hardened in
8b0ec7b and 898a288 after two independent security reviews and a
fresh-context acceptance pass (11/11). DEVIATIONS.md records the two
review-driven contract additions (required allowedExtractors closing the
delegation SSRF at every spawn; closed-by-default mediaOptional aligning
queue and watch on 0-video posts), the added deterministic fatal patterns,
and the deferred countsMissing retry-cap trigger.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ase 3)

The shared caption ladder now consumes the adapter's declared captionClass
(platform-asr: EN -> und, every rung auto-class), fixing the X .en.vtt ->
manual-en misclassification that routed X platform ASR through the manual
cleaner. transcriptStrategy resolves per entry from the adapter default plus
an explicit --transcript-strategy override: captions unchanged (YouTube
default), captions+repair layers deterministic proper-noun repair over the
platform VTT (lexicon = post text + harvested links), and caption-absent
entries take the optional faster-whisper asr rung when the runtime-detected
toolchain is present — never auto-installed — else degrade explicitly via the
named transcriptDegradation provenance field (watch.json + CLI output),
never silently.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ract

Settles 8 of the 12 draft-stage RECONCILE markers whose facts landed with
phases 1-3: slice key is the URL status id alone (the id pair rides as
metadata), the two 0-video cases split (a link post's own text is
unrecoverable after an unfetched allow-list refusal), the provenance
guard's two mechanisms documented with their opposite outcomes, the fatal
table grown to five patterns, mediaOptional and the twitter.* allow-list
added to the capability table, YouTube's caption class and /v/<id> +
subdomain acceptance corrected, and shared-vs-adapter failure attribution
made explicit. Folds in the phase-2 handoff notes (anonymous-acquisition
posture, 0-video queue behavior, run-harvest --url). Four markers remain,
retagged to name what they wait on (P3 probes, P5 env names, P5 naming).

Records one open question in output-contract.md: source:snowflakeAliasing
is written onto the acquisition envelope but persists to no slice
artifact.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ice metadata

The three Phase 3 probes ran against the phase-2 verified X clip with
faster-whisper large-v3 (int8, CPU): TIMESTAMPS passes (96.2% word
alignment, median start delta 21ms, every platform cue boundary within
421ms), so the asr rung stays default-on for caption-absent entries;
LEXICON fails (one Spygate fix against a feds->Fed regression and a
prompt-worsened repetition hallucination), so the post-text lexicon stays
repair-only and initialPrompt remains unfed; ENTITY records a clear
faster-whisper advantage over platform ASR for any future ASR-replace
decision. PLAN probe rows carry the dated outcomes.

The envelope's source:-prefixed metadata (snowflake aliasing, blocked
delegations, counts) now persists into watch.json via a contract-owned
subset helper, closing the gap where a flagged aliasing was unreadable
after the run; an unflagged run writes no key.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Phase 3 landed in 0545158 (strategy seam) and 122924f (T5-ASR probes +
source-metadata persistence) and passed fresh-context verification. Probe
outcomes: TIMESTAMPS pass ratifies the asr rung default-on for
caption-absent entries; LEXICON fail keeps the post-text lexicon
repair-only; ENTITY records a faster-whisper advantage for any future
ASR-replace decision. The Phase 7 CI pre-check outcome block that rode
122924f's PLAN staging was this session's own pre-flight record, landed
early by design.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…en eval (plan phase 4)

Extend the shared adapter suite into the full conformance surface
(canonicalization idempotence, per-host fixture coverage, harvest
well-formedness) with a findHostCollisions check matching the registry's
owned-host rule; add a capability-omitting fixture adapter proving
declarations skew closed, plus mutation probes showing the suite and the
collision test bite. Strengthen runtime validateAdapter for the type
lane's checker holes: duplicate hosts, empty-string extractor
declarations (which would silently drop the SSRF guard), stateful g/y
error patterns, and slug-shaped ids.

Add a full-registry round-trip test (canonical example URL per adapter —
insurance for adapter three) and close the review-named test debt:
cleanup-pass-succeeded-without-srt, preflight null-claim fail-closed,
blocked-delegation scheme gate, YouTube argv never carrying
--use-extractors / --ignore-no-formats-error, and a CLI-level run-watch
fixture proving capability-absent ASR degradation exits 0 with
transcriptDegradation recorded.

Pin the X golden behavior in evals/fixtures/x-status-goldens.json with
an offline vitest gate (result shape, slice-key pair, persisted source:*
provenance incl. snowflake aliasing) — picked up by the existing CI test
step, no workflow change needed — and a matching model-graded eval case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pace

Rename the extraction lane's user-visible YouTube-specific surfaces for the
source-agnostic pipeline (Phase 5):

- Env namespace: all six YOUTUBE_* knobs become VIDEO_DIGEST_*; a shared
  resolveEnvWithLegacy helper keeps every legacy spelling working with a
  once-per-process deprecation warning (new name wins when both are set).
  The auth-fallback lane resolves through the same helper so legacy-only
  cookie config still gates the browser-profile loop.
- Launcher: run.mjs / run-args.js forward the new names and gain
  --acquire-phase-gap, closing the sixth-knob gap.
- Temp prefixes: youtube-{frames,sheets,extraction}- -> video-*, and the
  staged fallback literal video-sheets-unknown. The acquire lock dir and the
  youtube-watch epic dir keep their literals (stable cross-version
  coordination/storage identifiers).
- Resume prompts render paths from the resolved slice dir, never the epic
  constant, so non-default work roots always yield resumable paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Phase 4 (cfb7de4) and Phase 5 (c9fa5d5) both passed fresh-context
verification (10/10 and 8/8). Records the liveness-lane tracker item
(#2797), the Phase 5 staged-literal audit's KEEP survivors, the two
granted fence extensions (the auth-lane env reads the six-var table
missed, proven by failing tests; fixture-literal renames the acceptance
grep requires), and a new Phase 7 inventory row for the setup-deps
install stamp that no youtube-digest sweep can see.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rkers

Phases 3-5 landed, so every deferred marker is answerable. Each resolution is
written from the landed code rather than the phase briefs.

- Resume argument: `resume <video-slug>` keeps its name through the rename;
  renaming a user-facing argument is outside approved scope.
- ASR prerequisite: faster-whisper is documented as optional and `watch`-only.
  The rung needs the media file as well as the capability
  (`asrAvailable && mediaAvailable`), and the `transcript` action never
  downloads media, so the hub row states the code's rule and cannot contradict
  the X spoke.
- Env namespace: name `VIDEO_DIGEST_WORK_ROOT` concretely and state the
  `YOUTUBE_`-spelling compatibility read. No variable enumeration — `run-args.js`
  owns that list.
- X strategy: the landed selection table was already correct, so only the
  T5-ASR-LEXICON delta is added — the lexicon is repair-only and never reaches
  ASR as an `initial_prompt`.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both defects misstated code the previous commit cited.

The prerequisites paragraph said the `asr` rung runs only for caption-absent
entries. `resolveTranscriptStrategy`'s `requested === "asr"` branch carries no
`captionPresent` guard, so an explicit `--transcript-strategy asr` runs ASR with
captions present; both entry paths are now stated. Its absence behavior was also
too narrow — the seam falls back to a caption strategy when a caption exists and
only then completes without a transcript.

The X spoke said feeding the lexicon to ASR as an `initial_prompt` made
proper-noun accuracy worse. The T5-ASR-LEXICON probe found +1 fix and -1
regression: no net proper-noun gain, with a materially worsened hallucination.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The output contract still called source:snowflakeAliasing envelope-only
and unreadable after a run; phase 3 landed sourceMetadataSubset
persistence into watch.json, settling the question for watch runs. The
transcript action keeps no run-state file, so its CLI output remains the
only carrier there.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Hub split drafted parallel to implementation, reconciled in two passes,
and verified fresh-context (9/9, including an 11-claim spoke-vs-code
audit with zero contradictions). Body lands at 13,328 chars against the
20,000-char budget with all routing conditional and the widened
description inside the frontmatter cap.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…igest

Structural move only (git mv, no content changes). The skill now digests
X posts at parity with YouTube; the name stops claiming YouTube-only
scope. Reference sweeps follow in separate commits.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…o-digest

Self-references become /knowledge:video-digest; run.mjs launcher lines
point at skills/video-digest/; the user-facing resume prompt and the
bootstrap recovery command emit the new paths, each pinned by a fixture
test. The description keeps the '/youtube-digest' trigger phrase as the
migration alias per the Phase 6 trigger-token baseline.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Plugin manifest: four userConfig titles now say (video-digest), keywords
gain x/twitter, the description drops "YouTube pipeline", and the version
bumps 0.12.0 -> 0.13.0 (pre-1.0 minor-as-breaking). CHANGELOG gains the
breaking entry naming the five old-name consumer surfaces (deny permission
rules fail open) plus the VIDEO_DIGEST_* env deprecations. CI job id
youtube-extraction renames to video-extraction (not a required status
check per the recorded ruleset pre-check). The extraction npm package
renames @melodic/youtube-extraction -> @melodic/video-extraction with a
regenerated lockfile, and the setup-deps install stamp becomes
.video-extraction.stamp (one harmless dependency reinstall).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
kyle-sexton and others added 7 commits August 15, 2026 21:35
…gest

Exactly the rename exemption's enumerated cross-references: routing lines
in SKILL.md, storage-schema.md, discovery-checklist.md, the setup-deps
comment, and the renamed eval case youtube-url-routes-to-video-digest-skill.
No behavioral or structural change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The three known-miss /youtube consumer refs, the TUNING.md path to
variation-matrix-backlog.json, and two youtube-extraction/ shorthand
headings that ride the npm package rename.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rename

git-grep gate over tracked and untracked files with an explicit inline
allowlist (CHANGELOG, the '/youtube-digest' trigger phrase, the sweep
itself, and the fixture negative assertions); the self-test seeds a stale
ref under vendor/ and asserts the sweep catches it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The terminal rename landed across six commits (structural git mv isolated
from the sweeps, Tidy First) and passed fresh-context verification 13/13.
Records the pre-flight baseline (33 files / 114 occurrences), the
sweep-allowlist growth to four classes (the trigger-token migration alias
made the PLAN's two-row allowlist unsatisfiable), and the in-spirit sweep
additions. Also corrects the two yt-dlp cookie userConfig descriptions
that still described YouTube-only cookie semantics, inaccurate now that
the X adapter is cookies-file-only.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two decisions from the source-agnostic video-digest effort pass the ADR
admission test (hard to reverse, surprising without context, real
trade-off): dispatching sources through a static adapter registry rather
than a computed dynamic import, and keeping storage-format identifiers
(the youtube-watch epic dir, the acquisition lock dir) stable across the
skill rename.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Close-out per the topic-docs lifecycle: the branch-lived contract slice
(PLAN.md, DEVIATIONS.md, design/) is published in the PR description and
its durable outcomes graduated to docs/adr/0010 and 0011; follow-ups live
in tracker issues #2797 and #2818. The git history of this branch retains
every revision.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Composes both sides' intent across the youtube-digest -> video-digest rename:

- Version supremacy: plugin.json stays 0.13.0 (> main's 0.12.6) with this
  branch's widened description; CHANGELOG keeps main's 0.12.1-0.12.6 entries
  verbatim below the 0.13.0 entry, which now notes the 0.12.6 <slice-slug>
  resume argument carries forward under /knowledge:video-digest.
- Resume-arg port (main #2821): the <slice-slug> rename is ported into the
  renamed hub's SKILL.md (description, argument-hint, action router, resume
  block, handoff message), run-resume.js usage strings, and the resume eval
  expectation - main's breaking change is preserved, not reverted.
- Liveness relocation (main #2817): extraction/liveness/ and the #2703
  scripts/run-tests.sh facade are git mv'd into video-digest/ with every
  path reference updated (LIVENESS.md, probes.json, the source-liveness
  workflow, ci.yml facade calls); the harness's VIDEO_DIGEST_-first cookie
  env resolution and its legacy YOUTUBE_ fallback are kept as designed.
- ADR renumber: main owns 0010/0011 by merge order; this branch's ADRs
  move to 0012/0013 (dispatch-video-sources, keep-storage-format).
- check-rename-sweep allowlists the two rename ADRs (historical-record
  class, same rationale as the CHANGELOG); main-side stale skill refs in
  knowledge/reference and docs-hygiene are updated to video-digest.

Gates: tsc clean, vitest 486/486 (71 files, +19 liveness), rename sweep +
self-test green, check-skill PASS (video-digest, course-digest),
markdownlint clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@cursor

cursor Bot commented Aug 16, 2026

Copy link
Copy Markdown

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 41s —— View job


Claude is reviewing this PR

  • Gather context (PR body, changed files, diff)
  • Confirm PR open, security lane present (scoping security findings out of this lane)
  • Deep review across 6 areas: adapter contract/registry/adapters, acquisition layer, transcript strategy, watch pipeline, course-digest scope-boundary check, CI/scripts/docs
  • Post inline findings anchored to changed lines
  • Post summary comment with cross-cutting findings

@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 36s —— View job


Security Review in progress

  • Gather context (PR body, changed files, diff against origin/main)
  • Load /review:security-review skill instructions
  • Hunt for findings across: adapter contract/registry/adapters (SSRF fix verification), yt-dlp spawn/acquisition layer, path/env/slice-key safety, harvesting/transcript/course-digest changes
  • Adversarially verify any surviving candidates
  • Post final findings (inline + summary)

@github-actions

github-actions Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Claude has reviewed this PR 4 times. The lane skips further automatic reviews after 5; deleting this comment resets the count.

@github-actions

github-actions Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Last security-reviewed head: 8a8fda0956de9448d6ee4dbdafaad0e8f5ff2566. On the next push, the relevance gate compares only the commits since this SHA; delete this comment to force a full re-review.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 207a6ba201

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread plugins/knowledge/skills/video-digest/extraction/adapters/x.js Outdated
kyle-sexton and others added 6 commits August 16, 2026 00:18
The topic-docs convention keeps .work/ as the never-committed memory
tier, but the slice was committed before the self-ignore guard healed
.work/.gitignore. Untracking it also clears the markdownlint and
editorconfig hygiene findings, all of which sat in these files.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eo-digest

Respell "unparseable" to typos' preferred "unparsable", mark the
osaDistance transposition datum with the blessed spellchecker
directive, drop the upstream tracker reference from the x.js header
per comment hygiene, and state the video-id class explicitly in the
youtube reference prose (`[A-Za-z0-9_-]{11}`) so the shell-portability
lint stops reading `\w` as a GNU-only construct; the JS pattern is
unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
shellcheck SC2292 (style) flags POSIX [ ] tests in bash scripts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
scripts/sync-plugin-options-docs.py regeneration after the branch's
userConfig cookie-description edits.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Changelog-parity bump for the derivability status board's
templates/sources.md row now pointing at the renamed video-digest
skill path.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three graded cases covering the read-only check contract, the
guidance-only library_dir apply path, and install-deps verification
by re-probe.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 20s —— View job


Claude is reviewing this PR

  • Gather context (PR body, changed files, diff)
  • Read repo CLAUDE.md and review conventions (empty — no repo-specific overrides)
  • Deep review across 5 areas (in progress, parallel agents): adapter contract/registry/YouTube adapter · X adapter · acquisition/auth layer · watch pipeline + env rename · course-digest scope-boundary + CI/scripts
  • Post inline findings anchored to changed lines
  • Post summary comment with cross-cutting findings

@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 3m 3s —— View job


Security Review

  • Gather context (PR body, changed files, diff origin/main...HEAD)
  • /review:security-review skill did not load in this environment (repeated Execute skill error) — proceeded with an equivalent manual security review of the same scope
  • Hunt for findings across adapter contract/registry/dispatch, yt-dlp spawn/auth-fallback, SSRF provenance guard, path/slice-key safety, CI workflow diff
  • Verify candidate findings against the actual code paths
  • Post findings

Note on process: the /review:security-review skill tool errored on every invocation attempt (Execute skill: review:security-review), so I could not follow its exact packaged instructions. I instead performed the equivalent review by hand against origin/main...HEAD, focused on the security-relevant surfaces this PR calls out itself (SSRF fix at the adapter seam, cookie/auth handling, static-registry dispatch, path-safe slice keys).


Findings

1. Confirmed — X adapter's browserCookieFallback: false is not honored on the initial yt-dlp call, only on the retry loop

  • resolveYtDlpAuthArgs (build-yt-dlp-args.js#L70-L103) reads VIDEO_DIGEST_YT_DLP_COOKIES_FROM_BROWSER straight from env whenever authOverride.cookiesFromBrowser is undefined — i.e. on every call except the fallback loop's per-profile retries.
  • spawnYtDlpWithAuthFallback (spawn-yt-dlp-with-auth-fallback.js#L64-L96) only gates the iteration over browser profiles on allowBrowserCookieProfileFallback (from the adapter's browserCookieFallback capability). The very first buildArgs() call at L70/L73 carries no override, so it still resolves --cookies-from-browser from env unconditionally.
  • preflight-metadata.js (preflight-metadata.js#L170) has the same pattern.
  • X declares browserCookieFallback: false specifically because "cookies file is the only auth route" (x.js#L913), and the PLAN states the invariant explicitly: "X (cookies-file only) must never iterate browser profiles."
  • Impact: any user who configures yt_dlp_cookies_from_browser (a common workaround for YouTube's bot-challenge flow — this is exactly what browserCookieFallbackProfiles/hasExplicitYtDlpCookieConfig in acquire-yt-dlp-auth.js is built around) will have that local browser profile's cookies sent to x.com/twitter.com on every X acquisition and preflight call — not just YouTube — even though the adapter contract was explicitly designed to keep X on cookies-file-only auth. This crosses the capability boundary the PR itself introduced to prevent exactly this (unintended-account auth, and the "cookie account must follow the author" business rule for protected posts in the PLAN). It's a real, adapter-capability-boundary bypass, not a hypothetical.
  • This matches @chatgpt-codex-connector's independently-filed comment on build-yt-dlp-args.js:90 — I traced the call path and confirm it: the gate in spawnYtDlpWithAuthFallback only protects the fallback loop, never the base call.
  • Fix direction: gate the base --cookies-from-browser resolution (not just the retry loop) on the adapter's browserCookieFallback capability, e.g. thread source.allowBrowserCookieProfileFallback into resolveYtDlpAuthArgs/buildYtDlpArgs and drop cookiesBrowser when the capability is false (cookies-file still applies unconditionally, since that's the X-safe path).
  • Fix this →

2. Lower-severity, correctness (not exploitable) — partial caption-cleanup failure silently drops captions for some entries on multi-video X posts

  • Corroborating @chatgpt-codex-connector's comment on x.js:811: the check at x.js#L807 (srtPaths.length === 0) only fails when none of the tagged VTTs converted; if 1-of-N succeeds, all N tagged-original backups are deleted at L820 regardless, silently losing captions for the other entries. This contradicts the surrounding comment's stated "fail loud" intent, but it's a data-integrity bug, not a security vulnerability (no untrusted-input escalation, no injection, no auth bypass) — flagging for completeness only.

Areas reviewed and found solid:

  • SSRF fix (Phase 2 finding): allowedExtractors: "twitter.*" pushes --use-extractors on every spawn (x.js#L905), plus a defense-in-depth provenance check on the result that hard-fails on any non-twitter info JSON (x.js#L695-L705) — this closes the yt-dlp delegation SSRF the PR describes.
  • Static registry dispatch (registry.js): host-keyed map of statically-imported modules only, fails closed on unknown host, no dynamic import in any form — verified no CWE-829/22 dynamic-import path exists.
  • Slice-key path safety: X status IDs are constrained to \d{1,25} by the URL parser regex (x.js#L173-L174); the metadata-derived fallback path is separately bounded by an explicit ^[A-Za-z0-9_-]{1,64}$ check (x.js#L951-L955) before it's ever used as a directory segment.
  • yt-dlp argv construction: URL always passed after a -- sentinel (build-yt-dlp-args.js#L184-L186), all args passed as array elements to a non-shell spawn — no shell-injection surface.
  • CI workflow diffs (ci.yml, video-digest-source-liveness.yml): purely mechanical path/job-id renames, no new triggers, permissions, or injectable contexts introduced.

Summary

One confirmed finding worth fixing before merge: the X adapter's cookies-file-only auth policy (browserCookieFallback: false) is bypassed on the initial yt-dlp call whenever a user has yt_dlp_cookies_from_browser configured — the capability gate only protects the automatic retry loop, not the base request. This is a real regression against the adapter-capability boundary this PR itself introduces, though it requires a specific env configuration to trigger (not exploitable by an external/remote attacker — it's a local-cookie-scoping/authorization-boundary bug, not RCE/injection/exfiltration to a third party). The SSRF fix, static-registry dispatch, and slice-key path safety are all correctly implemented and verified independently. A second, lower-severity correctness bug (partial caption-cleanup silently drops captions on multi-video posts) is noted for completeness but is not security-relevant.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 24s —— View job


Claude is running /review:code-review

  • Gather context (PR body, diff origin/main...HEAD, changed files)
  • Attempt to invoke /review:code-review skill (errored on load — proceeding with the equivalent manual review scoped the same way: correctness + maintainability, security findings excluded since a security lane already ran on this PR)
  • Review adapter contract / registry / YouTube adapter / X adapter (in progress)
  • Review acquisition/auth layer + transcript strategy seam (in progress)
  • Verify course-digest "no behavioral change" claim (in progress)
  • Review CI/scripts/docs sweep + watch pipeline hub (in progress)
  • Post inline findings anchored to changed lines
  • Post summary comment

kyle-sexton and others added 2 commits August 16, 2026 01:36
…cookie capability

The env-configured browser-cookie profile was pushed into EVERY
source's yt-dlp argv, bypassing the capability that gates the
automatic fallback loop — an X (cookies-file-only) acquisition or
preflight could carry --cookies-from-browser. resolveYtDlpAuthArgs
now takes the same allowBrowserCookieProfileFallback declaration
(closed by default) on both argv paths; a cookies FILE stays allowed
for every source. Conformance tests pin X argv browser-cookie-free
across all acquisition modes and preflight, with YouTube unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ting backups

On a multi-video X post the caption-cleanup gate was satisfied by ONE
converted .srt, then the cleanup deleted EVERY .tagged-original
backup — silently losing captions for entries whose conversion
produced nothing. The gate now requires a converted counterpart for
every tagged VTT; a partial conversion fails the acquisition
(fail-loud, matching the existing cleanup-failure path) with all
backups intact.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 2m 49s —— View job


Claude is running /review:code-review

  • Gather context (PR body, diff origin/main...HEAD, changed files)
  • Attempt to invoke /review:code-review skill (errored on load again — proceeded by reading the skill's SKILL.md + REVIEW.md directly and following those instructions manually)
  • Confirm scope split: .github/workflows/claude-security-review.yml exists, so this lane excludes security findings (already covered by the prior security-review lane pass on this PR — both of its findings are fixed in the two newest commits, verified below)
  • Review adapter contract / registry / YouTube adapter / X adapter (in progress)
  • Review acquisition/auth layer + transcript strategy seam (in progress)
  • Verify course-digest "no behavioral change" claim + CI/scripts/docs sweep (in progress)
  • Post inline findings anchored to changed lines
  • Post summary comment

Branch

@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @kyle-sexton's task in 1m 54s —— View job


Security Review

  • Gather context (PR body, diff origin/main...HEAD, changed files)
  • Invoke /review:security-review — skill tool errored on load again (Execute skill: review:security-review); proceeded with an equivalent manual review, scoped per the custom instructions (repo/PR/HEAD SHA 8a8fda0956de9448d6ee4dbdafaad0e8f5ff2566)
  • Scope the review to what changed since the last security-reviewed head (9361b8c, per the bot's tracking comment): docs/CATALOG.md (docs-only regen) plus two fix commits — 4c6762a and 8a8fda0
  • Verify both fixes against the exact findings raised in the prior security-review pass
  • Adversarially re-check for regressions or incomplete gating introduced by the fixes

Findings

None outstanding. Both issues raised in the prior security-review round on this PR are now fixed, verified correct, and covered by targeted regression tests.

1. Fixed — X's cookies-file-only auth policy is now honored on every argv path

Commit 4c6762a threads allowBrowserCookieProfileFallback (from adapter.capabilities.browserCookieFallback, via the single adapterSourceDeclarations() source of truth in acquire.js#L43-L52) into resolveYtDlpAuthArgs (build-yt-dlp-args.js#L79-L104), so --cookies-from-browser is now gated on the capability on every call path — not just the retry loop:

X declares browserCookieFallback: false (x.js#L924); YouTube declares true (youtube.js#L96). A cookies file stays allowed for both sources unconditionally — only the browser-profile argument is capability-gated, matching the "cookies-file-only" invariant. New conformance coverage pins X argv browser-cookie-free across all four acquisition modes + preflight even with the env var set, and confirms YouTube is unaffected (adapter-argv-conformance.test.js#L58-L91).

2. Fixed — partial caption-cleanup no longer deletes backups for unconverted entries

Commit 8a8fda0 changes the cleanup gate at x.js#L800-L821 from "at least one .srt exists" to "every tagged VTT has its own converted .srt counterpart" (unconverted = taggedVtts.filter(...), fails loud if unconverted.length > 0). Backup deletion at L829-831 only runs after this check passes, so a partial conversion on a multi-video post now fails the acquisition with all .tagged-original backups intact, rather than silently discarding captions for the unconverted entries. Regression tests cover both the partial-failure case (both backups survive, error names the missing entry) and the full-success case (all backups cleared) — x.test.js#L746-L799.

No regressions introduced by either fix: the override path used by the auth-fallback loop is now also gated on the same capability, but since the loop itself only invokes with an override when the capability is already true (spawn-yt-dlp-with-auth-fallback.js#L69), this is defense-in-depth, not a behavior change for YouTube.

Summary

Both findings from the prior security-review pass on this PR (X's browserCookieFallback: false capability bypassed on the base yt-dlp/preflight call, and partial caption-cleanup silently dropping captions on multi-video X posts) are fixed correctly at the root cause, applied consistently across every call site via the shared adapterSourceDeclarations() seam, and locked in with targeted regression tests. No new findings; nothing else changed in scope since the last security-reviewed head besides a docs-only catalog regen.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Both review findings verified and fixed:

  • build-yt-dlp-args.js browser-cookie gate — fixed in 4c6762a: resolveYtDlpAuthArgs now takes the adapter's browser-cookie capability (the same declaration that gates the automatic fallback loop, flowing through adapterSourceDeclarations) and emits --cookies-from-browser only when it is true — env-configured and override-supplied alike. Every argv path (all acquisition modes and preflight) is gated identically; a cookies file stays allowed for X. New conformance tests assert X argv never carries the flag with the env set while YouTube argv still does.
  • x.js caption-cleanup backups — fixed in 8a8fda0: every tagged VTT must have its own converted SRT before any .tagged-original backup is deleted; a partial conversion on a multi-video post now fails the acquisition fail-loud with all backups intact (tests cover the partial and complete cases).

Suite 494/494, tsc clean.

@kyle-sexton
kyle-sexton merged commit 1199240 into main Aug 16, 2026
46 checks passed
@kyle-sexton
kyle-sexton deleted the refactor/source-agnostic-video-digest branch August 16, 2026 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant