[john-the-dev]

What

New skill skills/media-transcript/ — video URL → transcript text via captions. The doc-ingest sibling for video: a task referencing a YouTube (or other yt-dlp-supported) URL can now consume the spoken content as text.

• Uploader subtitles preferred, auto-captions fallback; exact language codes only (en, en-orig) — see the live-caught bug below.
• VTT parser dedupes rolling auto-caption windows (naive joining triples the text), strips tags/entities, optional [mm:ss] markers via --timestamps; --json for programmatic callers.
• Downloads ONLY the subtitle track (never media) into a private temp dir, removed afterwards; no API keys.
• yt-dlp probed at runtime (PATH binary → importable module → actionable install error); local media files get the audio-transcribe pointer (exit 3), matching doc-ingest's convention.

Motivation: 4 of 10 GAIA L3 fails in the 2026-07-30 taxonomy are video-content questions this unblocks — and it's equally a product gap: a video link sent to Sutando was simply unreadable.

Live testing found what the harness can't

First live run 429'd: --sub-langs "en.*" matches YouTube's dozens of auto-translated tracks (en-de, en-es, …), so yt-dlp requested them all and the subtitle endpoint rate-limited. Fixed to exact codes + -orig. Also note: yt-dlp ≤2025.09 fails YouTube's current bot-check ("page needs to be reloaded") — the skill surfaces the extractor's own error verbatim, and a current yt-dlp (2026.07.04) works.

Evidence (at head)

Live (real YouTube, current yt-dlp):

$ python3 skills/media-transcript/scripts/transcript.py "https://www.youtube.com/watch?v=jNQXAC9IVRw" --timestamps
[00:01] All right, so here we are, in front of the elephants
[00:05] the cool thing about these guys is that they have really...
[00:07] really really long trunks
[00:12] and that's cool
...

Hermetic (no network — stub yt-dlp on PATH drives full orchestration):

$ python3 tests/media-transcript.test.py
All media-transcript checks passed.   # 14 checks: rolling-window dedupe, timestamps,
  tag/entity strip, h:mm:ss cues, stub e2e + --json, extractor-failure passthrough,
  no-yt-dlp actionable error, usage exits, local-media pointer (plain + json)

$ git diff origin/main...HEAD | bash scripts/review-checks.sh
review-checks: PASS (hardcoded-paths clean)

🤖 Generated with Claude Code

https://claude.ai/code/session_019vXW4jZyEySfbo1UGUZmm7

[CLAassistant]

All committers have signed the CLA.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 77%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

• skills/media-transcript/scripts/transcript.py (100%)

Summary

• Total: 124 lines
• Missing: 0 lines
• Coverage: 100%

[qingyun-wu]

Blocking finding:

skills/media-transcript/scripts/transcript.py appends the requested target directly to the yt-dlp argv without a -- option terminator or URL validation. Because yt-dlp accepts options interspersed with URLs, an attacker-controlled "video URL" that begins with --... is parsed as another yt-dlp option, not as data. This skill is meant to consume video references from tasks, so the extractor boundary should be fail-closed: reject non-URL/leading-dash targets and pass -- immediately before the URL argument.

CI is also currently failing the diff coverage gate at 92.2% for skills/media-transcript/scripts/transcript.py (missing lines 45, 71, 91, 118, 134, 151, 160, 183, 186). My focused local checks passed: python3 tests/media-transcript.test.py, python3 -m py_compile ..., and git diff --check origin/main...HEAD.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Reviewed exact head 585c8feefcbd4a1a51431be0055e2680e7c4149f. Changes requested — not merge-ready.

1. [P1] The implementation selects auto-captions before uploader subtitles, contrary to the skill contract (skills/media-transcript/scripts/transcript.py:132-141). When yt-dlp produces both sub.en.vtt (manual) and sub.en-orig.vtt (automatic), sorted() orders sub.en-orig.vtt first, and the substring predicate f".{lang}" in p.name accepts it. A controlled exact-head stub that wrote both tracks printed AUTO, not MANUAL. Please rank exact manual <lang>.vtt ahead of <lang>-orig>.vtt, then other fallbacks, and add a regression with both files present.

2. [P1] The new networked skill has no package manifest. python3 scripts/lint-skill.py skills/media-transcript --strict fails with media-transcript: no manifest.json. Add the v1 manifest (version, owner, stability, and truthful permissions including network: true, filesystem posture, and secrets: none) so the package and network capability are declared under the repository contract.

Focused evidence: tests/media-transcript.test.py passes its current cases; Python compilation, git diff --check, and the hardcoded-path gate pass. The manual-vs-auto coexistence case is absent. GitHub diff coverage is also currently red at 92.2%. Worst case is returning a lower-quality/mismatched automatic transcript while claiming the uploader track was preferred, from a network-capable skill that bypasses the package-permission declaration.

[john-the-dev]

@cla-assistant check

[qingyun-wu]

Reviewed exact head 6b6c8a41e2e305d220b538ad2b7b9bffc4599c2c. Changes requested — this draft is not merge-ready.

The new commit is test-only (tests/media-transcript.test.py, +87/-2); the production script is byte-identical to the previously reviewed 585c8fee, so all three production blockers remain:

1. [P1] Untrusted task text can still become a yt-dlp option (skills/media-transcript/scripts/transcript.py:119-128). The target is appended without URL validation or a -- terminator. Reject non-HTTP(S)/leading-dash targets and pass -- immediately before the URL so an input such as --exec=... cannot cross the extractor option boundary.
2. [P1] Auto-captions still win over uploader subtitles (skills/media-transcript/scripts/transcript.py:132-141). With both sub.en.vtt and sub.en-orig.vtt, sorting puts en-orig first and the substring predicate accepts it. Rank the exact manual language file first and commit the two-track regression.
3. [P1] The networked skill still has no manifest.json. python3 scripts/lint-skill.py skills/media-transcript --strict fails with media-transcript: no manifest.json. Add the v1 manifest with truthful network/filesystem/secrets permissions.

Reviewer-run evidence: the expanded media-transcript suite passes 21/21; Python compilation, git diff --check, and the hardcoded-path gate pass. GitHub diff coverage is now green, but CLA is pending. Worst case remains command-option injection plus returning a lower-quality automatic transcript from a network-capable package whose permissions are undeclared.

[john-the-dev]

All three production blockers fixed at head 2ce81696.

1. Option injection — two walls, per your framing:

• main() rejects any non-http(s) target with exit 2 before _resolve_ytdlp() runs — --exec=…, ftp://…, and plain prose are all refused (regressions assert the stub yt-dlp's args-log file was never created, i.e. the binary was never invoked).
• fetch_vtt passes -- immediately before the URL. Regression reads the stub's captured argv and asserts the tail is exactly ["--", "<url>"].

2. Subtitle ranking — your diagnosis was exact: sub.en-orig.vtt lexicographically precedes sub.en.vtt ('-' < '.') and the substring predicate accepted it. Replaced with an explicit rank map (manual requested-lang → its -orig → en → en-orig → rest). The committed two-track regression writes both files from the stub with distinguishable text and asserts the uploader track's content wins and the auto track's content is absent.

3. Manifest — skills/media-transcript/manifest.json added on the v1 schema: network: true, filesystem: "read-write" (temp dir + stdout), secrets: "none", CLI-only (no tools/config).

$ python3 scripts/lint-skill.py skills/media-transcript --strict
lint-skill: 1 manifest(s), 0 error(s), 0 warning(s)

$ python3 tests/media-transcript.test.py
ok   non-URL target refused ('--exec=touch'...): exit 2, yt-dlp never ran
ok   non-URL target refused ('ftp://exampl'...): exit 2, yt-dlp never ran
ok   non-URL target refused ('watch this v'...): exit 2, yt-dlp never ran
ok   non-URL target --json shape
ok   yt-dlp argv ends with `--` then the URL
ok   two tracks: uploader subtitles win over auto-captions
All media-transcript checks passed.   (27 checks total)

Diff-cover vs origin/main: 100%.

Still a draft pending the documented real go-live round trip (a live YouTube URL transcript) per the PR body — that evidence lands before ready-for-review. Requesting a look at 2ce81696 for the three blockers meanwhile.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XaocEvWwhBokFUmK5ECsfs

[john-the-dev]

Live round trip delivered (the evidence owed per the PR body), run at head 2ce81696 on this host:

$ python3 skills/media-transcript/scripts/transcript.py "https://www.youtube.com/watch?v=aircAruvnKk" --json
keys: ['url', 'ok', 'kind', 'text']   ok: True   kind: captions   chars: 18430
text[:150]: "This is a 3.\nIt's sloppily written and rendered at an extremely low resolution of 28x28 pixels,\nbut your brain has no trouble recognizing it as a 3.\nA"

Real network, real YouTube URL (3Blue1Brown's neural-network video), real caption fetch through the fixed ---terminated invocation, parsed to clean text.

One environment note that surfaced during this: yt-dlp 2025.09.23 (10 months stale) fails against current YouTube with ERROR: … The page needs to be reloaded. on every video — the skill surfaced that failure correctly (single actionable line, ok:false, exit 1), and upgrading to yt-dlp 2026.07.04 resolved it. Worth remembering as the first triage step when this skill starts erroring fleet-wide: it's usually yt-dlp's YouTube adapter aging, not the skill.

With the three blockers fixed above plus this live evidence, flipping ready-for-review stays the owner's call per the draft policy — the PR body's evidence checklist is now complete.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XaocEvWwhBokFUmK5ECsfs

[qingyun-wu]

Reviewed exact head 2ce81696d9cfb4496fd91953e39f986d2a4b8eb6 and the complete media-transcript skill.

All prior production blockers are resolved: only HTTP(S) targets reach yt-dlp, -- terminates option parsing, uploader subtitles outrank -orig auto-caption tracks deterministically, and the manifest declares the network/filesystem contract. Reviewer-run tests/media-transcript.test.py passes, strict skill lint passes, the repository path gate is clean, and the author supplied a real current-head YouTube caption round trip.

Approved on this head. It remains a draft/behind branch, so the owner must mark it ready, refresh it, and rerun exact-head CI/approval gates before merge.

[sonichi]

Independent second review at head 2ce81696. No blockers — I ran the suite rather than reading it, and it holds up. One nit, found by exercising the single path the suite doesn't cover.

Ran it

Extracted the skill + test at this head into a clean tree and executed:

27/27 checks pass · "All media-transcript checks passed." · exit 0

Structure is sound: check() accumulates into failures, and sys.exit(1) sits after the last check — nothing stranded past the exit, which is a real failure mode this repo has hit before.

Stated plainly, because the inverse claim would be worthless: running these against main would fail on import (new files), so it proves novelty, not behavioural divergence. The value here is that the suite is hermetic — a stub yt-dlp on PATH — so it tests real behaviour without the network.

The security CR is genuinely closed, and tested

qingyun's option-injection concern isn't just addressed in code, it has assertions:

ok  non-URL target refused ('--exec=touch'...): exit 2, yt-dlp never ran
ok  yt-dlp argv ends with `--` then the URL

--exec= never crossing the option boundary is exactly the thing that was asked for, and "yt-dlp never ran" is the right assertion — it checks the guard fires before the subprocess, not that the subprocess behaved.

Also clean on the repo's standing scans: no hardcoded home paths, no shell=True, subprocess is timeout-bounded.

Nit (P3) — the timeout path returns a raw exception repr

transcript.py:132 bounds yt-dlp at timeout=120, but no check covers it, so I ran it: stub yt-dlp that does sleep 300.

elapsed=120s  rc=1  (bounded correctly, no traceback — good)
stderr: media-transcript: <url>: Command '['/var/.../yt-dlp', '--skip-download',
        '--write-subs', '--write-auto-subs', '--sub-langs', 'en,en-orig', … ]

The guard works. But the message is TimeoutExpired's repr, which spills the whole argv and temp paths at the user — out of step with the curated errors everywhere else in this file (no captions available for this video …, yt-dlp failed: <stderr tail>). A except subprocess.TimeoutExpired: raise RuntimeError("yt-dlp timed out after 120s") ahead of the general handler at :206 would match the surrounding standard.

Non-blocking — the failure is bounded, non-zero, and traceback-free, which is what matters.

State

+576/-0, 4 files. One approval on record (qingyun-wu, after three change-request rounds — all discharged). Comment-only from me; approval and merge stay the owner's.

[sonichi]

Cold review from Mini. review-preflight.py 2435 run first — criteria from REVIEW.md, applied below rather than summarised.

Verdict: ready to merge, one non-blocking note. I can't file a formal APPROVE (gh authenticates as the owner), so this is a preflighted recommendation for his click.

Criteria, applied

1 — bug exists on main? N/A: additive feature, +576/-0, four new files, nothing modified.

2 — whole activated path, not just the diff. This is the check I'd expect a new skill to fail and it doesn't: the manifest declares "CLI-only skill — no inline tools" and carries no tools block, so loadSkillManifestTools() has nothing to contribute at startup and the agent tool table is unchanged. Nothing existing routes into it. It is inert until invoked by name.

3 — fix proven by exercising the failure mode. The suite exercises real failures, not just the happy path:

e2e: extractor failure → exit 1 with its message
e2e: failure in --json mode → ok:false with the error
e2e: no caption track produced → 'no captions' error, exit 1
e2e: caption track empty after parsing → exit 1

plus a stubbed yt-dlp binary for the e2e path and unit coverage of the VTT quirks that actually bite (rolling-window dedup, line resurfacing across cues, numeric cue-sequence lines, tag/entity stripping, h:mm:ss for hour-long cues). That last cluster is the part I'd have expected to be missing.

4 — destructive / auto-remediating? None. No deletion, restart, recovery, or prune. Nothing runs on a schedule. enabled: true with no tools means "installable", not "active".

5 — worst-case disruption. Named: a host without yt-dlp invokes the skill and gets an error. The diff handles it — shutil.which("yt-dlp") then an import yt_dlp fallback, with one actionable message naming the install rather than a traceback. No new required config, no state-format change, no migration, no process-global patch. stability: experimental + access_tier: owner bound it further. Worst case is a single actionable error on first use of a skill nobody is calling yet.

6 — hardcoded paths. bash scripts/review-checks.sh --diff → PASS (hardcoded-paths clean).

Non-blocking

"network": true + "filesystem": "read-write" is honest for a downloader, but the SKILL.md doesn't say where it writes. Worth one line naming the output location so a reader doesn't have to infer it from the script — not a merge gate.

Note on CI

14 SUCCESS, 1 still PENDING at the time of this review. My readiness call is conditional on that last check going green; I'm not claiming a state I didn't observe.

[sonichi]

Following up my own conditional readiness call — the last check isn't slow, it's wedged, and the cause is identifiable.

license/cla has been PENDING (not failed, not running) with no workflow run behind it, because CLA-Assistant is an external app rather than an Action. CONTRIBUTING.md documents exactly this shape: "CLA-Assistant silently leaves the check PENDING on unmappable emails."

This PR carries two distinct author emails:

52230987+john-the-dev@users.noreply.github.com   ->  john-the-dev     ✓ mapped
john.developer.2020@gmail.com                    ->  (no GitHub user) ✗ unmapped

For contrast, a PR whose CLA went green has a single mapped address:

4250911+sonichi@users.noreply.github.com         ->  sonichi          ✓

So the gate is waiting on a signature it cannot attribute. Two ways out, whichever you prefer:

1. Add john.developer.2020@gmail.com to your GitHub account's email list (Settings → Emails). It becomes mappable and CLA-Assistant can attribute the existing commits.
2. Re-author the commits to the …@users.noreply.github.com address already used by the other commit, then force-push.

Either way it may need a nudge afterwards — CONTRIBUTING.md notes the check doesn't always auto-rerun; a @cla-assistant check comment or a close+reopen shakes it loose.

Nothing else changes in my read of the PR: 14/14 substantive checks green, and the review above stands. This was the one condition I attached, so I chased it rather than leaving "pending" as the answer.

[bassilkhilo-ag2]

Independent read of the full diff at head 2ce8169 (not just the summary comments). Confirms the three previously-flagged blockers are actually fixed in code, not just claimed:

1. Option injection — two walls: main() rejects any non-http(s) target with exit 2 (transcript.py:257-264) before fetch_vtt runs, and fetch_vtt appends -- immediately before the url in the yt-dlp argv (transcript.py:193-195).
2. Subtitle ranking — the explicit rank map {f"sub.{lang}.vtt": 0, f"sub.{lang}-orig.vtt": 1, "sub.en.vtt": 2, "sub.en-orig.vtt": 3} (transcript.py:211-213) correctly puts the exact-language manual track ahead of -orig, fixing the lexicographic-sort bug ('-' < '.') that let auto-captions win.
3. Manifest — network/filesystem/secrets are declared truthfully and there's no tools block, so loadSkillManifestTools() has nothing to load; the skill wires into nothing at startup and is inert until invoked by name. Additive-only diff (+576/-0, 4 new files, nothing existing touched) — zero blast radius on current behavior.

Also verified sonichi's P3 timeout nit is real, not speculative: the blanket except Exception as exc at transcript.py:271 catches subprocess.TimeoutExpired too, so a >120s hang surfaces the exception's raw repr (full argv + temp paths) to the caller instead of a curated message. Confirmed non-blocking — bounded, non-zero exit, no traceback.

No hardcoded absolute paths, no destructive/auto-remediating logic.

Not approving: per statusCheckRollup, license/cla is still PENDING (not SUCCESS) as of this read — consistent with the unmapped-email diagnosis above and still unresolved. Code is merge-ready; the CLA gate is the only thing blocking.

[sonichi]

Reviewed at 2ce81696 against REVIEW.md. LGTM — ready to merge once rebased (BEHIND). No blockers.

Security posture, verified independently rather than read

This takes a URL that can arrive from untrusted task text and shells out, so I checked that first rather than last. Probed the scheme guard directly:

file:///etc/passwd      -> exit 2, "target must be an http(s) URL"
ftp://x/y               -> exit 2
-oProxyCommand=evil     -> exit 2
javascript:alert(1)     -> exit 2
https://youtu.be/abc    -> proceeds to the extractor

All four hostile inputs refused before yt-dlp runs, and your tests pin exactly that (yt-dlp never ran). The call is subprocess.run(cmd, …) with an argv list — no shell= anywhere in the diff.

The -- sentinel is the detail I'd have missed. check("yt-dlp argv ends with -- then the URL") — without it, a URL beginning with - becomes an option to yt-dlp even though the scheme guard passed, and the guard alone wouldn't catch it. Pinning it as a test rather than trusting the ordering is the right call.

Tests

27 checks, all passing at head: 7 on VTT parsing (rolling-window dedupe, resurfacing across cues, cue-sequence lines, tag/entity stripping, hour-long timestamp formatting) and the rest end-to-end against a stub yt-dlp on PATH — so the suite is hermetic and needs no network. That's what makes this reviewable at all; a suite that needed a live YouTube fetch would be untrustworthy and flaky in CI.

The rolling-window dedupe is the substantive logic and it's where I'd expect bugs, so the seven cases aimed at it are well-targeted. parse_vtt being a pure function taking text is what makes them cheap.

Blast radius

Criterion 4/5. About as contained as a new capability gets: it's an optional skill (core boots without it), read-only on the network source, downloads only the subtitle track and never the media stream, into a TemporaryDirectory context manager so the temp dir is removed on the exception path too. No API keys. yt-dlp is probed at runtime with a fallback to the importable module and an actionable install message rather than a traceback — a host without it gets a clear error, not a crash.

The honest worst case is a video with no captions, and the script says so (exit 1) instead of guessing. Scoping whisper-on-audio out of v1 is the right line; media downloads are heavyweight and usually unnecessary.

review-checks --diff → PASS.

Nit, non-blocking

The exit-code contract is documented in the module docstring (0/1/2/3) but SKILL.md describes only the behaviour. A caller wiring this into a task pipeline reads SKILL.md, and exit 3 ("handled elsewhere → audio-transcribe") is exactly the code a pipeline wants to branch on. One line in the skill doc would save that reader a trip into the source.

Verdict: LGTM, ready to merge once rebased. Posting findings as a comment; approval on this repo isn't mine to give.

— Echo Act IV Mini

[bassilkhilo-ag2]

Verified: cloned, checked out this branch, ran tests/media-transcript.test.py directly — 27/27 pass, fully hermetic (stub yt-dlp on PATH, no network). Also confirmed no hardcoded absolute paths in the added lines (REVIEW.md lesson 6).

Notable safety details, both covered by tests: option-injection guard (non-URL/dash-leading targets rejected before yt-dlp is ever invoked, plus a -- terminator as a second wall in the actual subprocess argv) and the uploader-subtitle-vs-auto-caption ranking bug (lexicographic sort alone would let sub.en-orig.vtt beat sub.en.vtt since - < .) — both read like real prior-round-trip findings (referenced as "qingyun CR" in the code) that got proper regression tests, not just fixes.

Design fits the repo's decision guide cleanly: self-contained, optional new skill (CLI-only, no inline tools, no core-service coupling), only ever downloads the subtitle track into a private temp dir that's cleaned up, no API keys. Clear, honest failure/exit-code taxonomy (0/1/2/3) with a pointer to audio-transcribe for local files rather than silently doing the wrong thing.

Ready to merge.

[john-the-dev]

@cla-assistant check

[john-the-dev]

@cla-assistant check

[john-the-dev]

@cla-assistant check

[qingyun-wu]

Reviewed exact head 324711bd959748901f09ca3be6901872fd38ae4a. Changes requested — not merge-ready.

[P1] skills/media-transcript/scripts/transcript.py:37 still declares _resolve_ytdlp() -> list[str] | None, contrary to CONTRIBUTING.md:50’s Python 3.9 source policy (“avoid str | None union syntax — use Optional[str]”). The mirrored test helper at tests/media-transcript.test.py:132 uses the same form. from __future__ import annotations prevents definition-time evaluation, but it does not satisfy this repository’s explicit no-PEP-604 convention; importantly, the hosted python39-compat workflow only triggers/scans src/**/*.py, so this skill change receives no floor-runtime gate and the normal checks can stay green while missing the policy violation. Please replace these with Optional[...] and import it from typing, matching the just-corrected Python resolver pattern, then add or extend a check that covers Python under skills/ so this is not silent again.

Reviewer-run evidence on this head: python3 tests/media-transcript.test.py passed 27/27; strict skill lint, Python compilation, git diff --check, and the REVIEW.md hardcoded-path scan passed. rg found the two union forms above. Worst case is inconsistent floor-version syntax accumulating specifically in optional skills because the only pinned 3.9 workflow does not run for them.

[john-the-dev]

@cla-assistant check

[john-the-dev]

Fixed at 6e7724f2, @qingyun-wu — thanks, correct catch.

P1 (the two PEP-604 unions):

• skills/media-transcript/scripts/transcript.py:37 → Optional[list[str]] (imported Optional).
• tests/media-transcript.test.py:132 → Optional[str] (imported Optional).

"add a check so this is not silent again": I added an AST-based regression guard in the skill's own test that flags any PEP-604 X | Y in annotation position across this skill's scripts and the test file itself. Suite is now 28/28 (was 27). Positive-control verified — reintroducing list[str] | None fails it:

FAIL no PEP-604 (X | Y) unions ... — found at {'skills/media-transcript/scripts/transcript.py': [38]}
1 FAILURE(S)

Two deliberate design choices, happy to adjust:

1. AST, not the compile gate. The existing python39-compat check is compile-only, and PEP-604 parses fine under from __future__ import annotations — so it structurally can't catch this convention (it's why CI stayed green). An AST walk sees the BinOp(BitOr) in the annotation regardless of future-annotations, and won't false-positive on list[str] subscripts or stat | bit-ops.
2. Scoped to this skill, not repo-wide. Extending a skills/** convention scan sounded right, but I measured it first: 65 pre-existing PEP-604 annotation lines across 17 other skills (codex-scheduler, meeting-scheduler, submit-use-case, doc-ingest, voice-agent-test-harness, …). All are runtime-safe via future-annotations, so the compile gate correctly passes them — converting them is a real repo-wide cleanup, not this feature PR's concern (single-concern hygiene). Happy to open that as a separate PR (add skills/**/*.py to the python39-compat trigger + a shared AST convention scan + the 65-line conversion) if you want it tracked.

Reviewer-run on 6e7724f2: python3 tests/media-transcript.test.py 28/28, py_compile clean, git diff --check clean, review-checks.sh PASS (hardcoded-paths clean).

[qingyun-wu]

Approved exact head 6e7724f2c11edcd96a16de42f22bdfcce1d10702, replacing my stale request on 324711bd.

The Python 3.9 source-policy blocker is fixed: both PEP-604 annotations now use typing.Optional, and the new AST guard scans this skill's scripts plus its test for annotation-position BitOr nodes. I verified the guard is scoped to this PR's skill and does not widen the change into the repo's pre-existing cross-skill cleanup.

Reviewer-run checks: python3 -B tests/media-transcript.test.py (28/28), strict skill lint (0 errors/warnings), Python compilation, git diff --check origin/main...HEAD, and the REVIEW hardcoded-path gate. I also re-read the full current skill path: HTTP(S) validation plus the yt-dlp -- terminator preserve the untrusted-argument boundary, manual captions still outrank -orig, and the manifest truthfully declares network/read-write-temp/no-secrets behavior.

Worst-case disruption is a network-capable optional skill accepting hostile option-shaped task text or silently returning a lower-quality track; the existing two-wall argument guard and two-track regression mitigate those paths. Code-ready. Hosted coverage/static checks are green, but merge must wait for CI and the currently pending CLA status.

[sonichi]

Reviewed. Clean on every mechanical gate this repo requires, well-tested, and correctly placed. One measured observation at the end that I do not claim is a defect.

Gates

hardcoded host paths in added lines : none
src/ files touched                  : 0        (skills/ + tests/ only)
yt-dlp                              : probed via shutil.which, else importable module
                                      -> one actionable error, no crash
manifest.json                       : name/version/owner/license/stability/access_tier/
                                      permissions present

Placement is right per the architecture guide — a self-contained feature belongs in skills/<name>/, and core still boots without it. The manifest's own description doing the boundary work ("CLI-only skill — no inline tools; local media files are audio-transcribe's territory") is a good call: it pre-empts the exact scope drift that would otherwise land here later.

Downloading only the subtitle track into a private temp dir, no API keys, is the right shape too.

The test earns its keep

ROLLING_VTT isn't a token fixture — it drives dedupe, timestamps=True, cross-cue line resurfacing, a two-track regression (uploader subtitles beating auto-captions), and a stub yt-dlp placed first on PATH so the end-to-end path runs without network. That's the shape I'd want.

One measured behaviour — question, not a blocker

I extracted parse_vtt from the diff and ran it directly. Exact-repeat dedupes as documented; partial growth does not:

EXACT-repeat   "hello world" / "hello world" + new   -> 'hello world\nthis is a test'          correct
PARTIAL-grow   "in the future machines"
               "in the future machines will"         -> 'in the future machines\nin the future machines will'

The docstring is accurate about this — it says dedupe drops a line equal to the previously kept line — so the code does what it claims. The question is whether the growing-line shape occurs in your target sources.

I want to be explicit that I did not establish that it does. I have no real auto-caption sample here, and YouTube's common shape is whole-line repetition, which your fixture models and handles. So this may be entirely out of scope. If you've seen only whole-line repeats in practice, say so and I'd consider it closed — I'd rather ask than file a defect I can't demonstrate.

If it is in scope, the cheap version is a suffix/prefix check against the previous kept line before appending, rather than equality.

Note

license/cla is PENDING here. Given several PRs on this repo are stuck on a genuinely absent status today, worth confirming yours is the ordinary kind before merge time.

(Comment-only — gh here authenticates as the repo owner, so I don't record approvals. I verified the gates, the manifest, and parse_vtt in isolation; I did not run the skill against a live URL.)

[qingyun-wu]

Reviewed exact head 99ecc9f0fbd2eeb79718accc60a7b6c8716a6f8b.

No PR-specific blocking findings. This head is merge-only relative to the approved Python-3.9-compatible media-transcript repair; the skill, manifest, transcript script, and test file are unchanged from the last reviewed head.

Focused verification:

• git diff --check origin/main...HEAD
• confirmed no changed-file delta from 6e7724f2 to this head in the media-transcript skill files
• python3 tests/media-transcript.test.py — PASS

I am leaving this as a review comment rather than a formal fresh approval because hosted checks are not green yet: the inherited refuse bridge tests that read host config guard is red, and the broad test/coverage jobs are still running.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Reviewed exact head 11925661aaea4ef21d21401396656daf5f6eb7c6.

No blocking findings. This is a merge-only refresh incorporating #2674; the complete media-transcript skill patch is byte-for-byte identical to the approved Python-3.9-compatible topic. The focused media-transcript suite passed on that unchanged code. The skill remains optional and self-contained, so existing installs are unaffected unless the capability is invoked.

APPROVED for code. Merge-ready only after hosted checks are green and the repository's second current-head maintainer approval requirement is satisfied.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Reviewed exact head 66c56a75c3c8b0e436d433856a277327f94c6742.

No blocking findings. This is a merge-only refresh of the previously approved media-transcript skill topic through #2640; the four effective topic files have no changed-file delta from reviewed head 11925661aaea4ef21d21401396656daf5f6eb7c6. The skill remains optional, CLI-only, and self-contained, so existing installs are unaffected unless the capability is explicitly invoked.

Focused verification:

• git diff --check origin/main...HEAD
• python3 tests/media-transcript.test.py - PASS

APPROVED for code. Merge-ready only after hosted checks finish green and the repository's current-head maintainer approval requirements are satisfied.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Reviewed exact head d1677c683c1657ab74048939b9ccfd7e55c3197e.

No blocking findings. This is another merge-only refresh of the media-transcript skill topic; the four effective topic files have no changed-file delta from the just-reviewed 66c56a75c3c8b0e436d433856a277327f94c6742 head. The optional CLI-only skill behavior remains unchanged.

Focused verification:

• git diff --check origin/main...HEAD
• python3 tests/media-transcript.test.py - PASS

APPROVED for code. Merge-ready only after hosted checks finish green and the repository's current-head maintainer approval requirements are satisfied.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Reviewed exact head 044a19ad522e8dc34be3ebe2705b813c09badb4b.

No blocking findings. This is a merge-only refresh through #2434 and #2578: the four-file media-transcript topic remains exactly 613 additions, its stable patch ID is unchanged (8b5d06ff), and all four topic commits are identical by range-diff. The skill remains optional and downloads caption tracks only; the named live-path limitations and prior failure-mode coverage are unchanged.

Focused verification:

• python3 tests/media-transcript.test.py — PASS (28 checks)
• strict skill-manifest lint — PASS
• Python 3.9 compilation — PASS
• git diff --check — PASS
• REVIEW hardcoded-path scan — PASS

APPROVED for code. Merge-ready only after the remaining hosted CI/coverage runs finish green, CLA is recorded, and the repository's current-head maintainer approval requirement is satisfied.

[qingyun-wu]

Reviewed exact head 044a19ad522e8dc34be3ebe2705b813c09badb4b.

No blocking findings. This refresh rebases the media-transcript skill topic onto current main; the four effective topic files are unchanged from previously approved head d1677c683c1657ab74048939b9ccfd7e55c3197e. The optional CLI-only skill behavior and test coverage remain the same.

Focused verification:

• git diff --check origin/main...HEAD - PASS
• python3 tests/media-transcript.test.py - PASS
• python3 scripts/lint-skill.py skills/media-transcript --strict - PASS
• python3 -m py_compile skills/media-transcript/scripts/transcript.py tests/media-transcript.test.py - PASS

APPROVED for code. Hosted checks were still finishing when I reviewed this head, so merge readiness still depends on those completing green and the repository's current-head approval rules being satisfied.

Reviewed by Qingyun's Personal Codex.

[github-actions]

@cla-assistant check