Skip to content

download_audio: delegate the link-to-audio policy to yb (#122) - #126

Merged
thorwhalen merged 2 commits into
mainfrom
download-audio-via-yb-bounded
Oct 27, 2026
Merged

thorwhalen merged 2 commits into
mainfrom
download-audio-via-yb-bounded

Conversation

@thorwhalen

@thorwhalen thorwhalen commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #122

download_audio becomes a thin caller of yb.download.download_audio_bounded (reeleehq/yb#48, released in yb 0.1.24):

  • braidio keeps creds.check() (before the network) and _media.as_tool_error (bot gate / expiry / unavailable classification).
  • It passes creds.opts as extra_opts (yb restricts it to DEPLOYMENT_OPT_KEYS), _docs._validate_target as validate_target, and BRAIDIO_AUDIO_MAX_BYTES / BRAIDIO_AUDIO_MAX_DURATION_S as the caps.
  • It ingests result.path, then removes result.path.parent (in a finally, so a failed ingest does not leak the file), and carries result.notice as copyright_notice.
  • yb now owns: the target check (which also re-checks the resolved page URL, something braidio did not do), the playlist/live/duration refusal before any media bytes, the pinned caps, mp3 extraction, and the per-call result directory.
  • Floor bumped to yb[download]>=0.1.24.

Behaviour changes a caller could notice: the over-long-source message is yb's wording (exceeds the 2700s limit), and playlists and live streams are now refused explicitly.

Tests. The download tests now fake yb's two yt-dlp calls rather than the policy, so the guards that credentials reach both calls and that the cap survives the merge run the real download_audio_bounded. The SSRF test goes through the server and matches the refusal's own wording. Called directly, the tool raised no caller identity, which is also a ToolError, so the bare raises(ToolError) would have passed without the URL ever being checked. Mutations checked: dropping validate_target= fails the SSRF test, and dropping the directory cleanup fails two tests.

No deploy coupling beyond the usual: the connector picks this up on its next platform deploy, and its venv must resolve yb >= 0.1.24.

Adversarial review (fresh Opus subagent)

Verdict: MERGE-AFTER-FIXES. The fixes are applied in the second commit.

  • Fixed: an except ValueError branch reported a raw yt-dlp ValueError (bot-gate text included) without classifying it. The branch is removed, so every failure now goes through as_tool_error. Guarded by the bot-gate test, which raises the error as both RuntimeError and ValueError.
  • Fixed: braidio now passes cookies_file=False. Before, yb could fall back to its own default jar, and a bot gate on that jar read as "never provisioned".
  • Fixed: the tests no longer patch yb's private _extract_info / download_youtube_video. yb's own tests pin the caps.
  • Added: a test for an unreadable cookie jar, refused before the network. creds.check() had no tool-level guard, and that gap predates this PR.
  • Added: the cleanup tests assert that only the per-call directory is removed. Removing parent.parent, which is the shared download directory, now fails.
  • Accepted: creds.check() now runs before the address check, so a loopback URL on a server with expired cookies gets the cookie error. Neither path makes a network call.
  • Design: the reviewer called it clean, so nothing was refactored.

🤖 Generated with Claude Code

thorwhalen and others added 2 commits October 27, 2026 16:27
download_audio is now a thin caller of yb.download.download_audio_bounded:
braidio keeps the credential pre-check and the error classification, and
supplies its SSRF check (_docs._validate_target) and its env-tunable caps.
yb owns the target check (now also on the resolved page URL), the
playlist/live/duration refusal, the pinned caps, mp3, the per-call result
directory, and the copyright notice. Floor yb[download]>=0.1.24.

Tests: the download tests fake yb's yt-dlp calls (not the policy), so the
credential-reaches-both-calls and cap-survives-merge guards run the real
policy; the SSRF test goes through the server and matches the refusal's
own words (a direct call raised "no caller identity", a ToolError too).

Closes #122

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…er guards

From the adversarial review of #126: a raw yt-dlp ValueError (bot gate
text) skipped classification, so every failure now goes through
as_tool_error; cookies_file=False so yb's own default jar cannot
masquerade as 'never provisioned'; tests stop patching yb internals,
add a cookie-jar pre-check guard, and assert only the per-call dir is
removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@thorwhalen
thorwhalen merged commit f983bec into main Oct 27, 2026
12 checks passed
@thorwhalen
thorwhalen deleted the download-audio-via-yb-bounded branch October 27, 2026 17: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.

download_audio: delegate the policy to yb.download.download_audio_bounded

1 participant