Repository navigation
feat(ytdlp)!: drop the bundled yt-dlp download, resolve from PATH - #50
Merged
Merged
Conversation
youtube-dl-exec was a dependency veta never imported, declared purely so its postinstall would drop a yt-dlp copy that binary.ts used as a third resolution source. Resolution is now VETA_YTDLP_PATH then PATH, and bundledPath() plus the "bundled" BinarySource are gone. The bundled copy cost more than it was worth. The postinstall fetched whatever releases/latest served on install day, unpinned and without a checksum, so every user ran a different unauditable binary; it failed the whole install when the GitHub API was unreachable or rate-limited, which is why both workflows had to set YOUTUBE_DL_SKIP_DOWNLOAD to survive the 60/hour anonymous limit on shared runner egress; its preinstall refused to install without Python >= 3.9 and the asset it dropped was a Python zipapp, so the fallback needed Python at run time too; and roughly 40 of the 55 production packages existed to support it. npm has also started warning that the package has install scripts not yet covered by allowScripts, a warning no package-side change can suppress, since only the installing project's own package.json counts. The YTDLP_NOT_FOUND message named pnpm as the way out, the one package manager that skips dependency build scripts by default, and a ytDlpPath config key that has never existed. It now names brew, pipx and VETA_YTDLP_PATH. Its test only reached that error by accident: with an empty PATH the zipapp's #!/usr/bin/env python3 shebang failed, so on Windows or any setup where the bundled probe succeeded it would have resolved source "bundled" instead of rejecting. With no bundled source the case is deterministic, and two missing cases are covered: plain PATH resolution, and falling back to PATH when the configured path is unusable. A clean npm install goes from 59 packages to 16. Install now means installing yt-dlp yourself, which the README already led with and which keeps it current -- and staying current matters, because yt-dlp breaks and gets fixed on YouTube's schedule, not veta's. PRD goal 4, NFR-1, FR-24 and the technology table said the binary arrived automatically with no Python needed; all four were wrong on both counts and are corrected. docs/06 records why it must not come back. BREAKING CHANGE: veta no longer ships or downloads a yt-dlp binary. Users without yt-dlp on PATH must install it (brew install yt-dlp, pipx install yt-dlp) or set VETA_YTDLP_PATH.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
youtube-dl-execwas a dependency veta never imported, declared purely so itspostinstallwould drop ayt-dlpcopy thatbinary.tsused as a third resolution source. It's gone. Resolution is nowVETA_YTDLP_PATH→PATH, andbundledPath()plus the"bundled"BinarySourceare deleted.This implements the preferred fix from
review/HIGH/R-012.Why
The bundled copy cost far more than it was worth:
releases/latestserved on install day, with no version pin and no checksum. Every user ran a different, unauditable binary.npm install -g @cmglezpdev/vetafailed outright when the GitHub API was unreachable or rate-limited. Both workflows had to setYOUTUBE_DL_SKIP_DOWNLOADto survive the 60-requests-per-hour anonymous limit on shared runner egress — it took out macOS the first time CI ran.preinstallrefused to install without Python >= 3.9, and the asset it dropped was a Python zipapp, so the bundled fallback needed a Python interpreter at run time too. PRD NFR-1 claimed the opposite on both counts.1 package has install scripts not yet covered by allowScripts. No package-side change can suppress it — I verified that a paste ofallowScriptsinto veta's ownpackage.jsondoes not propagate to consumers; only the installing project'spackage.jsoncounts. That warning was headed for every veta user, and eventually becomes a block.Error message
YTDLP_NOT_FOUNDtold users to "reinstall with pnpm" — the one package manager that skips dependency build scripts by default, so the advice could never work — and to set aytDlpPathconfig key that has never existed. It now namesbrew install yt-dlp,pipx install yt-dlpandVETA_YTDLP_PATH.Tests
The
YTDLP_NOT_FOUNDtest only reached that error by accident: with an emptyPATH, the zipapp's#!/usr/bin/env python3shebang failed. On Windows (yt-dlp.exe), or any setup where the bundled probe succeeded, it would have resolvedsource: "bundled"instead of rejecting. With no bundled source the case is deterministic. Two missing cases are now covered: plainPATHresolution, and falling back toPATHwhen the configured path is unusable.Verified
pnpm typecheckclean; 603 tests pass, 1 skipped.Docs
PRD goal 4, NFR-1, FR-24 and the technology table all described the automatic-download, no-Python story — wrong on both counts, all four corrected.
docs/06-development.mdnow records why the binary must not come back, so nobody reintroduces it.pnpm-workspace.yamlis deleted: its only content was theallowBuildsapproval for the removed package.Breaking
veta no longer ships or downloads a yt-dlp binary. Users without
yt-dlponPATHmust install it or setVETA_YTDLP_PATH. The README's quick path already led with that install.