Skip to content

feat(knowledge): scheduled video-digest source liveness lane - #2817

Merged
kyle-sexton merged 4 commits into
mainfrom
cursor/2797-video-liveness-8f91
Aug 16, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
cursor/2797-video-liveness-8f91

Conversation

@kyle-sexton

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

Copy link
Copy Markdown
Contributor

Summary

  • Adds an offline-compatible source-adapter liveness harness under plugins/knowledge/skills/youtube-digest/extraction/liveness/ (probes + fixtures + vitest) for the video-digest pipeline follow-up from PLAN Phase 4.
  • Adds advisory workflow .github/workflows/video-digest-source-liveness.yml (weekly schedule + dispatch for live yt-dlp probes; PR path runs --offline only). Not wired into ci.yml / ci-status.
  • Auth-required X probes skip without cookies (never fail). Red live runs annotate the job, write a step summary, and upload the liveness-report artifact — they never open or update GitHub issues. Knowledge plugin bumped to 0.12.5. Owner doc: plugins/knowledge/skills/youtube-digest/extraction/liveness/LIVENESS.md.

Closes #2797.

Test plan

  • npx vitest run liveness/run-source-liveness.test.js — 16 passed
  • node liveness/run-source-liveness.js --offline — 3 passed
  • Full extraction suite npm test — 286 passed / 52 files
  • CI: offline self-test job on this PR; confirm workflow is absent from ci-status

Related

  • PLAN Phase 4 deferred liveness (branch refactor/source-agnostic-video-digest)
  • Offline golden gate remains the merge-path conformance suite

Ship an offline-compatible probe harness and an advisory weekly workflow
so upstream yt-dlp / YouTube / X drift is visible without gating merges.
Auth-required X rows skip without cookies; knowledge 0.12.5. Closes #2797.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.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 45s —— View job


Security review (/review:security-review)

Reviewed all 12 changed files against origin/main (diff scope only), focused on the new advisory GitHub Actions workflow and the yt-dlp-shelling liveness harness.

  • Gather context (PR diff, changed files)
  • Run /review:security-review skill
  • Post findings

What I checked:

  • .github/workflows/video-digest-source-liveness.yml — trigger/permission scoping, secret handling, script-injection surface in run: blocks, issue-tracking logic (gh api + jq)
  • run-source-liveness.jsspawnAsync usage for the yt-dlp probe (args array, no shell — no injection), cookies-file env resolution, fixture loading
  • probes.json / fixture JSON files — no embedded credentials or PII
  • package.json diff — new npm scripts only, no dependency changes

Design notes (no action needed):

  • live-probe (the job with issues: write + the cookies secret) only runs on schedule / workflow_dispatch, never on pull_request — so a PR (including from a fork) touching probes.json or the harness can't reach secrets via the pull_request trigger. Good isolation.
  • Actions are commit-SHA pinned; GITHUB_SHA/GITHUB_RUN_ID/etc. are read as env vars rather than interpolated into ${{ }} inside run: blocks, avoiding the classic script-injection antipattern.
  • jq --arg is used correctly for the tracking-issue dedup logic — no unsanitized interpolation into the gh api/jq pipeline.

Finding posted inline:

  1. IMPORTANT.github/workflows/video-digest-source-liveness.yml#L87: the live-probe job (which has issues: write and the VIDEO_DIGEST_YT_DLP_COOKIES_FILE secret in its env) installs yt-dlp via pip install --user 'yt-dlp>=2026.6' with no upper bound and no hash verification. This repo's own convention elsewhere (ci.yml lines 551 and 870) uses --require-hashes --only-binary=:all: for pip installs. A compromised future yt-dlp PyPI release would run with access to the cookies secret and the job's token. Blast radius is bounded (advisory lane, issues: write only) but the fix is cheap — pin the version/hash to match the repo's existing pattern. Inline comment + fix link posted on the file.

No CRITICAL findings. No other IMPORTANT/SUGGESTION-level security issues found in this diff.
· branch cursor/2797-video-liveness-8f91

Red live runs annotate, summarize, and upload the probe report only.
Graduate the owner doc into the liveness harness directory so the
contract-slice prune gate stays clean.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Comment thread .github/workflows/video-digest-source-liveness.yml Outdated

@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: 17787a8879

ℹ️ 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 .github/workflows/video-digest-source-liveness.yml Outdated
Comment thread .github/workflows/video-digest-source-liveness.yml Outdated
@github-actions

Copy link
Copy Markdown
Contributor

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

@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cursor[bot]'s task in 3m 3s —— View job


Claude code review — /review:code-review

  • Load /review:code-review skill instructions
  • Gather context (diff, changed files, CLAUDE.md/REVIEW.md guidance)
  • Review new liveness harness code (run-source-liveness.js, tests, fixtures)
  • Review new GitHub Actions workflow (video-digest-source-liveness.yml)
  • Review plugin metadata/changelog updates
  • Post findings

This repo carries a .github/workflows/claude-security-review.yml lane, so this review is scoped to correctness/design/conventions/error-handling/observability/tests/docs per REVIEW.md; security findings are left to that lane.

Findings

One inline comment posted on run-source-liveness.js:

  • 🟡 probeLiveUrl discards the real spawn-failure reason (result.error from spawnAsync, e.g. yt-dlp missing/ENOENT) and the result.timedOut flag, falling back to a generic "yt-dlp exited non-zero" message. Since this harness's entire output is a human-facing drift report (job summary + artifact, no other signal), a real infra failure and a 120s timeout end up indistinguishable in the report — undermining the diagnostic value the workflow is built for. Suggested fix included inline.

What looked solid

  • runProbe's auth-required skip logic (live-only skip, offline still replays fixtures to keep the login-required classifier covered) is correctly separated and matches its own tests.
  • compareObservation correctly fails (rather than silently skips) when a non-auth probe unexpectedly returns login-required live — that's exactly the drift this lane should catch.
  • The workflow is genuinely non-blocking: confirmed it's absent from ci.yml's ci-status needs graph, pull_request only runs the offline self-test (no live network, no secrets needed), and the failure path only annotates/writes a summary/uploads an artifact — no gh issue create anywhere, matching the PR description and LIVENESS.md.
  • PIPESTATUS/set +e/set -e sequencing in the "Probe live sources" step is correct for capturing the piped node script's real exit code under tee.
  • Plugin version bump (0.12.4 → 0.12.5) and CHANGELOG.md entry are consistent with the added surface.

Not independently verified

The PR's Test plan (vitest 16/16, --offline 3/3, full suite 286/52) is author-claimed; I did not have node_modules available in this environment to re-run it myself, so I read the code/tests directly rather than executing them.

@github-actions

github-actions Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

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

cursoragent and others added 2 commits August 16, 2026 01:47
Pin yt-dlp with require-hashes, materialize cookie secret to a temp
file, reject bare --probes, chmod the harness shebang, and silence
actionlint SC2016 on markdown fences. Still never files GitHub issues.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
Satisfy the exec-bit hygiene gate for the shebang entrypoint.

Co-authored-by: Kyle Sexton <kyle-sexton@users.noreply.github.com>
@claude

claude Bot commented Aug 16, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cursor[bot]'s task in 2m 34s —— View job


Claude code review — /review:code-review

  • Load /review:code-review skill instructions
  • Gather context (diff since last code-review pass, changed files, CLAUDE.md/REVIEW.md guidance)
  • Review new liveness harness code (run-source-liveness.js, tests, fixtures)
  • Review new GitHub Actions workflow (video-digest-source-liveness.yml)
  • Review plugin metadata/changelog updates
  • Post findings

This lane already reviewed commit 8664569 (my prior pass) and flagged one open finding still on unchanged lines. Per REVIEW.md, security is scoped to /review:security-review (this repo carries that lane); this pass stays on correctness/maintainability. This review covers the delta since then — commits c2d3f83 ("harden video-digest liveness workflow hygiene") and 329b24e ("mark run-source-liveness.js executable") — against HEAD 329b24e.

What changed since the last review pass

  • .github/requirements-video-digest-liveness.txt (new) pins yt-dlp==2026.7.4 with a --require-hashes-compatible wheel hash, matching the repo's requirements-ci.txt convention. The "Install yt-dlp" step now installs --only-binary=:all: --require-hashes --requirement ... instead of the unbounded pip install yt-dlp>=2026.6.
  • New "Materialize yt-dlp cookies file (optional)" step writes the VIDEO_DIGEST_YT_DLP_COOKIES_FILE secret's contents to a mktemp file (chmod 600) and passes that file's path to the probe step, instead of passing the raw secret value as if it were already a path.
  • parseArgs in run-source-liveness.js now rejects --probes with no argument or with another flag immediately following it (--probes requires a path argument), with matching new tests.
  • run-source-liveness.js gained the executable bit (matches its #!/usr/bin/env node shebang).

All three previously-flagged Codex/security findings against the earlier commit are addressed by this delta:

No new bugs introduced by this delta. Checked: the --require-hashes install works for yt-dlp alone because it ships with no mandatory non-stdlib dependencies (unlike pyyaml/ruff in requirements-ci.txt, which list every transitive hash); the cookies temp file is never uploaded as part of the liveness-report artifact; the new fence=$(printf '\140\140\140') indirection in the drift-summary step avoids embedding literal backticks inside double-quoted run: strings (which would otherwise trigger command substitution).

Not re-flagged (pre-existing, unchanged lines, already an open thread)

probeLiveUrl (run-source-liveness.js:140-156, unchanged by this delta) still collapses spawnAsync's real failure reason (result.error, result.timedOut) into a generic "yt-dlp exited non-zero" message — see my inline comment from the prior pass. Not re-posting per the high-signal bar (pre-existing issue on untouched lines), but leaving it open since it wasn't addressed in this round.

Not independently verified

The PR's Test plan (vitest, --offline, full suite) is author-claimed; I read the code/tests directly rather than re-running them in this pass.

@kyle-sexton
kyle-sexton merged commit 7b43d6d into main Aug 16, 2026
46 checks passed
@kyle-sexton
kyle-sexton deleted the cursor/2797-video-liveness-8f91 branch August 16, 2026 02:05
kyle-sexton added a commit that referenced this pull request Aug 16, 2026
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>
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.

knowledge: scheduled liveness lane for video-digest source adapters

2 participants