fix: route every transcript client through the centralized proxy (#1087) - #1129
fix: route every transcript client through the centralized proxy (#1087)#1129groupthinking wants to merge 3 commits into
Conversation
youtube-transcript-api >=1.0 only honours a proxy when a ``proxy_config`` is passed to the constructor. Six call sites across four modules built a bare ``YouTubeTranscriptApi()``, so they egressed from the host's own IP and bypassed WEBSHARE_PROXY_URL entirely — including ``_extract_transcript_with_rotation``, whose name implies the opposite. - src/integration/youtube_api.py:99 - src/agents/process_video_with_mcp.py:222,233 - src/agents/interactive_metadata_extractor.py:87 - src/mcp/mcp_video_processor.py:694,718 Each now resolves its config from the canonical ``youtube_extension.utils.proxy.get_transcript_proxy_config`` helper, guarded by the import fallback this repo already uses in gemini_video_master_agent.py so modules stay importable outside the package. The helper returns None when no proxy is configured, so direct connections keep working unchanged. Adds tests/unit/test_transcript_proxy_coverage.py: an AST walk over src/ and shared/ that fails if any construction omits proxy_config, plus a canary that fails if the scan ever matches nothing (the vacuity mode that has bitten this repo repeatedly). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change centralizes proxy configuration for YouTube transcript clients. Four transcript modules support standalone imports, fail fast when the helper is unavailable, and pass the proxy configuration to direct, routed, and transcript-list requests. ChangesTranscript proxy enforcement
Sequence Diagram(s)sequenceDiagram
participant TranscriptProcessor
participant ProxyConfig
participant YouTubeTranscriptApi
TranscriptProcessor->>ProxyConfig: load get_transcript_proxy_config()
ProxyConfig-->>TranscriptProcessor: return proxy settings
TranscriptProcessor->>YouTubeTranscriptApi: construct client with proxy settings
YouTubeTranscriptApi-->>TranscriptProcessor: fetch transcript data
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsThese MCP integrations need to be re-authenticated in the Integrations settings: Sentry Linked repositories: Public OSS repositories can only analyze public repositories installed in this organization. No linked repositories were analyzed; skipped Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Agent Completion Truth Gate: NOT_APPLICABLEEvidence agrees. Machine-readable verdict{
"details": {},
"reasons": [],
"verdict": "not_applicable"
} |
There was a problem hiding this comment.
🟡 Not ready to approve
Standalone CLIs can still bypass configured proxies, and the regression guard accepts several noncompliant constructions.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Routes remaining transcript clients through the centralized outbound proxy and adds an AST-based regression guard.
Changes:
- Passes canonical proxy configuration to six transcript client constructions.
- Adds repository-wide proxy coverage tests and helper validation.
File summaries
| File | Description |
|---|---|
src/integration/youtube_api.py |
Proxies transcript fetching. |
src/agents/process_video_with_mcp.py |
Proxies primary and fallback extraction. |
src/agents/interactive_metadata_extractor.py |
Proxies metadata transcript extraction. |
src/mcp/mcp_video_processor.py |
Proxies direct and routed extraction. |
tests/unit/test_transcript_proxy_coverage.py |
Adds structural regression guards. |
Review details
Suppressed comments (2)
tests/unit/test_transcript_proxy_coverage.py:159
- This only proves that the helper's name occurs somewhere in the module and rejects literal
None; a constructor usingproxy_config=custom_config,object(), or another noncanonical value still passes. That contradicts this test's stated guarantee that each caller sources the argument from the canonical helper. Inspect eachproxy_configAST value and require a call toget_transcript_proxy_config().
assert PROXY_HELPER in source, (
f"{module_path} constructs {CLIENT_NAME} but never references "
f"{PROXY_HELPER}; proxy_config is likely hardcoded."
)
tests/unit/test_transcript_proxy_coverage.py:73
- Treating any
**kwargsas compliant leaves the new guard open to exactly the silent bypass it is meant to prevent:YouTubeTranscriptApi(**{})(or a mapping withoutproxy_config) passes this check while egressing directly. Require an explicitproxy_config=keyword; callers using a mapping can still spell that argument out alongside**kwargs.
# ``**kwargs`` — the keyword may be supplied dynamically; treat the
# call as opaque rather than reporting a false positive.
if keyword.arg is None:
return True
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
|
Automated remediation pass — adjudication of the open Copilot review I verified Copilot's two "🟡 not ready to approve" findings against the guard source at
Other state at
Recommendation: the production change is sound and complete; merge is a maintainer call (protected Generated by Claude Code |
|
Verified Copilot's review — all five findings are real (not false positives), with severity bounded: Source — fail-open proxy bypass ( Severity Copilot didn't bound: the production path is unaffected — uvicorn runs with Tests —
Suggest requiring an explicit Recommendation: hold merge until at least the three test-guard holes are closed; the standalone fail-open is lower priority (prod unaffected) but worth the few-line change while here. CI is otherwise green (test/build/lint/security/CodeQL/coverage all pass; the Generated by Claude Code |
…roxy Review on #1129 found the guarded-import fallback introduced by this branch was itself a silent bypass: when a module is executed by path (the documented CLI entry point) youtube_extension is not importable, so the stub returned None and proxy_config=get_transcript_proxy_config() egressed directly from the host IP -- the exact failure this issue exists to prevent. Replace the stub in all four standalone-executable modules with a sys.path bootstrap to the repository's src directory, falling back to a hard ImportError. The proxy is now either honoured or the module refuses to load; it is never silently skipped. Harden the guard suite against three ways it could pass while protecting less than it claims: - a missing configured source root was skipped silently - SOURCE_ROOTS could be narrowed by hand with every other test still green; the required roots are now rediscovered from the tree - referencing the helper was satisfied by the call site alone, so a broken import passed; every import from the proxy module must now bind the helper All 9 mutations kill their tests, including one that revealed a newly added test was itself vacuous (it asserted parents[1] from a test-side constant rather than the index the source actually uses). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
🔍 PR Validation |
|
PR remediation run — ready-for-review pass at head
Not done, by design: no push (branch Staged next step once the trusted publication lands (or the gate is waived) and you approve: Generated by Claude Code |
|
@coderabbitai review Please review exact head |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/agents/interactive_metadata_extractor.py`:
- Around line 106-109: Remove fabricated transcript fallbacks and ensure only
validated real transcript data is returned. In
src/agents/interactive_metadata_extractor.py lines 106-109, stop invoking the
placeholder _generate_transcript_whisper fallback after the YouTube request
fails. In src/agents/process_video_with_mcp.py lines 241-246, propagate the
direct extraction failure when no real fallback succeeds. In
src/agents/process_video_with_mcp.py lines 257-260, replace the fixed result in
RealVideoProcessor._extract_transcript_with_rotation with actual subtitle
extraction, or raise a typed extraction error when unavailable.
- Around line 106-109: Introduce one shared transcript HTTP-client factory that
returns a requests.Session enforcing finite connect and read timeouts, then pass
its result via http_client= to every YouTubeTranscriptApi call. Update
src/agents/interactive_metadata_extractor.py lines 106-109,
src/agents/process_video_with_mcp.py lines 241-246 and 257-260,
src/integration/youtube_api.py lines 118-120, and src/mcp/mcp_video_processor.py
lines 713 and 737; apply the same bounded-session pattern to all transcript
fetches under src/youtube_extension/backend. Preserve existing proxy
configuration and fetch behavior while ensuring every transcript client uses the
shared timeout-enabled session.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 759ea240-3ded-4491-8ef1-955c0d0b8eeb
⛔ Files ignored due to path filters (1)
tests/unit/test_transcript_proxy_coverage.pyis excluded by!tests/**
📒 Files selected for processing (4)
src/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/integration/youtube_api.pysrc/mcp/mcp_video_processor.py
📜 Review details
⚠️ CI failures not shown inline (2)
GitHub Actions: Agent completion enforcement / 0_Agent completion enforcement.txt: fix: route every transcript client through the centralized proxy (#1087)
Conclusion: failure
##[group]Run actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3
with:
script: const fs = require('fs');
const pull = await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: Number(process.env.PR)
});
let verdict = {
conclusion: 'failure',
reason: 'verifier_did_not_publish',
details: {}
};
try {
verdict = JSON.parse(fs.readFileSync(
'enforcement-verdict.json', 'utf8'
));
} catch (error) {
core.warning(error.message);
}
const conclusion = verdict.conclusion === 'success'
? 'success'
: 'failure';
const summary = JSON.stringify(verdict);
await github.rest.checks.create({
owner: context.repo.owner,
repo: context.repo.repo,
name: 'Agent completion enforcement',
head_sha: pull.data.head.sha,
status: 'completed',
conclusion,
output: {
title: conclusion === 'success'
? 'Trusted evidence verified'
: 'Trusted evidence blocked',
summary: summary.slice(0, 60000)
}
});
if (conclusion !== 'success') {
core.setFailed(verdict.reason || 'trusted evidence blocked');
}
github-***REDACTED_SECRET_ASSIGNMENT***
debug: false
user-agent: actions/github-script
result-encoding: json
retries: 0
retry-exempt-status-codes: 400,401,403,404,422
env:
PR: 1129
##[endgroup]
##[error]missing_trusted_publication
GitHub Actions: Agent completion enforcement / Agent completion enforcement: fix: route every transcript client through the centralized proxy (#1087)
Conclusion: failure
##[group]Run actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3
with:
script: const fs = require('fs');
const pull = await github.rest.pulls.get({
owner: context.repo.owner,
repo: context.repo.repo,
pull_number: Number(process.env.PR)
});
let verdict = {
conclusion: 'failure',
reason: 'verifier_did_not_publish',
details: {}
};
try {
verdict = JSON.parse(fs.readFileSync(
'enforcement-verdict.json', 'utf8'
));
} catch (error) {
core.warning(error.message);
}
const conclusion = verdict.conclusion === 'success'
? 'success'
: 'failure';
const summary = JSON.stringify(verdict);
await github.rest.checks.create({
owner: context.repo.owner,
repo: context.repo.repo,
name: 'Agent completion enforcement',
head_sha: pull.data.head.sha,
status: 'completed',
conclusion,
output: {
title: conclusion === 'success'
? 'Trusted evidence verified'
: 'Trusted evidence blocked',
summary: summary.slice(0, 60000)
}
});
if (conclusion !== 'success') {
core.setFailed(verdict.reason || 'trusted evidence blocked');
}
github-***REDACTED_SECRET_ASSIGNMENT***
debug: false
user-agent: actions/github-script
result-encoding: json
retries: 0
retry-exempt-status-codes: 400,401,403,404,422
env:
PR: 1129
##[endgroup]
##[error]missing_trusted_publication
🧰 Additional context used
📓 Path-based instructions (12)
**/*.{py,js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{py,js,jsx,ts,tsx}: Use Python 3.9+ and Node 18+ for development
Never hardcode API keys, database URLs, or secrets in code
Make minimal, surgical changes and avoid deleting working code unless fixing security issues
Files:
src/integration/youtube_api.pysrc/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/mcp/mcp_video_processor.py
**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.py: Always use type hints for Python functions
Use Black formatter with 88 character line length for Python code
Follow PEP 8 conventions for Python code
Use descriptive variable names and add docstrings to all public functions in Python
Group imports in Python: standard library, third-party, local
Use SQLAlchemy ORM and never write raw SQL queries
Use environment variables via os.getenv() or pydantic-settings to access configuration
Wrap database operations in try-except blocks and use context managers or FastAPI dependencies for connection cleanup
Implement comprehensive error handling with proper logging in all functions
Version APIs using /api/v1/ prefix for stability
Use JSON-RPC 2.0 protocol for all MCP communication
Follow the single-flow workflow: YouTube link → context extraction → agent dispatch → outputs
Use context managers for resource management in Python code
Implement comprehensive input validation and sanitize outputs for security
Use parameterized queries and SQLAlchemy ORM to prevent SQL injection
Define API request/response models using Pydantic for FastAPI endpoints
Store the single unified workflow as the only workflow; never introduce alternate flows or manual triggers
Use SQLAlchemy with connection pooling for database connections and manage sessions with context managers
Provide sensible defaults for non-sensitive configuration in Python settings
Maintain backward compatibility and do not break existing API endpoints
Include comprehensive logging for debugging in MCP implementations
Files:
src/integration/youtube_api.pysrc/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/mcp/mcp_video_processor.py
⚙️ CodeRabbit configuration file
Python backend code. Check for type hints, proper exception handling, async context manager usage, and potential blocking calls in async functions. Flag any bare except clauses or missing timeout parameters on network calls. CRITICAL: Flag any file that contains placeholder/stub implementations — especially in unified_ai_sdk. Any class or function that says "TODO: Replace with production implementation" or returns mock/fake data must be flagged as a blocking issue. Flag any code generation output that reaches users without AST validation or syntax checking.
Files:
src/integration/youtube_api.pysrc/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/mcp/mcp_video_processor.py
**/*.{py,js,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain >80% code coverage for new features
Files:
src/integration/youtube_api.pysrc/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/mcp/mcp_video_processor.py
**/*.{py,ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{py,ts,tsx}: Keep frontend and backend data models synchronized using matching Pydantic (backend) and TypeScript (frontend) interfaces
Use type-safe interfaces for backend-frontend data exchange
**/*.{py,ts,tsx}: Production code must use real behavior only: no mock delays, fake data, or simulated responses.
Maintain strict type safety: mypy strict mode for Python and TypeScript strict mode for the frontend.
Name events using the<domain>.<entity>.<action>format.
Files:
src/integration/youtube_api.pysrc/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/mcp/mcp_video_processor.py
**/*
📄 CodeRabbit inference engine (Custom checks)
**/*: Strictly verify that GitHub Copilot has explicitly reviewed and approved the pull request; human approvals alone must not satisfy this check.
Before allowing a merge, require thecopilot-rabbitlabel and AI-generated unit tests committed alongside the code changes; fail the check if either is missing.For Vercel-specific work, include
https://vercel.com/docs/llms-full.txtin the AI assistant context set.
Files:
src/integration/youtube_api.pysrc/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/mcp/mcp_video_processor.py
**/*.{py,pyw}
📄 CodeRabbit inference engine (AGENTS.md)
Write Python code to remain compatible with Linux and Windows where possible, including correct handling of
asyncioevent loops.
Files:
src/integration/youtube_api.pysrc/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/mcp/mcp_video_processor.py
src/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.py: Format Python code with Black using an 88-character line length.
Sort Python imports with isort using the Black profile.
Run Ruff with E, W, F, I, B, C4, and UP rules; E501 is ignored.
Use mypy strict mode; untyped function definitions are disallowed.
Target Python 3.9 or newer.
Validate backend inputs with Pydantic and sanitize subprocess arguments.
Use Anthropic SDK featuresthinking={"type": "adaptive"}andoutput_config={"effort": "..."}withanthropic>=0.105.0; do not addTypeErrorfallbacks for these parameters.Use the service container dependency-injection pattern in
backend/containers/.
Files:
src/integration/youtube_api.pysrc/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/mcp/mcp_video_processor.py
**/*.{py,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not commit secrets; store keys and credentials in gitignored
.envfiles.
Files:
src/integration/youtube_api.pysrc/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/mcp/mcp_video_processor.py
**/*.{py,pyi}
📄 CodeRabbit inference engine (GEMINI.md)
**/*.{py,pyi}: Use Black formatting with an 88-character line length for Python code.
Use Ruff with rules E, W, F, I, B, C4, and UP for Python linting.
Use strict mypy type checking; do not define untyped functions.
Target Python 3.9 or newer.
Validate Python inputs with Pydantic.
Sanitize subprocess arguments before execution.
Do not add mock delays or fake data to production Python code; production runs in REAL_MODE_ONLY.
Keep secrets out of Python source code; load keys and credentials from environment variables instead.
Use absolute imports that resolve withPYTHONPATH=srcin the Python backend.
Files:
src/integration/youtube_api.pysrc/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/mcp/mcp_video_processor.py
**/*.{py,pyi,ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
**/*.{py,pyi,ts,tsx}: Preserve the single workflow: YouTube link → transcript → events → agents → outputs; do not introduce alternative flows or manual triggers that bypass it.
Use event names following<domain>.<entity>.<action>, such asyoutube.video.captured.
Make surgical, precise changes and do not delete working code without justification.
Files:
src/integration/youtube_api.pysrc/agents/interactive_metadata_extractor.pysrc/agents/process_video_with_mcp.pysrc/mcp/mcp_video_processor.py
src/mcp/**/*.py
📄 CodeRabbit inference engine (.cursorrules)
In
src/mcp/, MCP tool definitions and the registry must be maintained as real tool implementations.
Files:
src/mcp/mcp_video_processor.py
**/mcp*/**
⚙️ CodeRabbit configuration file
MCP (Model Context Protocol) integration code. Verify tools follow proper MCP protocol structure — correct tool definitions, input schemas, and response formats. Flag any MCP tool that doesn't handle errors gracefully or lacks proper input validation.
Files:
src/mcp/mcp_video_processor.py
🔍 Remote MCP GitHub Copilot
Additional review context
-
PR
#1129is open at head5659f2b; latest CodeRabbit review approved it. Core checks—including tests, build, lint, CodeQL, security scans, coverage, and dependency review—passed.Agent completion enforcementremains failed, whilePR Governancepassed. -
Issue
#1087remains open and is directly linked. PR#496previously completed theyoutube-transcript-api >=1.0instance-API migration; this PR addresses the remaining proxy-routing gap. -
pyproject.tomldeclaresyoutube-transcript-api>=1.0.0only under the optionalyoutubeextra. The canonical helper validatesWEBSHARE_PROXY_URL, createsGenericProxyConfig(http_url=url, https_url=url), and returnsNonewhen unset, malformed, or when proxy support is unavailable. -
A separate local
_get_transcript_proxy_config()still exists inshared/libs/youtube_proxy.py; its two client constructions use that local helper rather than the canonicalyoutube_extension.utils.proxy.get_transcript_proxy_config(). The new canonical-helper test lists only sevensrc/modules, so it does not enforce canonical-helper provenance forshared/. -
Copilot’s remaining guard concerns are real in the current test source: any
**kwargsis accepted as compliant, and the canonical-helper check verifies helper-name presence/import binding but not that eachproxy_configAST value is specifically a call toget_transcript_proxy_config(). No current call site uses**kwargs, and CodeRabbit approved the exact head.
| None, | ||
| lambda: YouTubeTranscriptApi(proxy_config=get_transcript_proxy_config()) | ||
| .fetch(video_id) | ||
| .to_raw_data(), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
Stop returning fabricated transcript results.
Both paths report successful extraction after real extraction fails. InteractiveMetadataExtractor.extract_transcript falls back to _generate_transcript_whisper, which returns a hard-coded segment. RealVideoProcessor._extract_transcript_with_rotation returns a fixed segment after yt_dlp.extract_info() without extracting subtitle content. This sends false video data to downstream agents.
Use a real transcriber and validate its output. If no real transcript is available, raise a typed extraction error.
src/agents/interactive_metadata_extractor.py#L106-L109: Do not call the placeholder transcript fallback after this request fails.src/agents/process_video_with_mcp.py#L241-L246: Propagate the direct extraction failure when no real fallback succeeds.src/agents/process_video_with_mcp.py#L257-L260: Replace the fixedyt_dlpfallback result with actual subtitle extraction or an error.
As per coding guidelines, “Production code must use real behavior only: no mock delays, fake data, or simulated responses.” As per path instructions, “Flag any file that contains placeholder/stub implementations … or returns mock/fake data.”
📍 Affects 2 files
src/agents/interactive_metadata_extractor.py#L106-L109(this comment)src/agents/process_video_with_mcp.py#L241-L246src/agents/process_video_with_mcp.py#L257-L260
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agents/interactive_metadata_extractor.py` around lines 106 - 109, Remove
fabricated transcript fallbacks and ensure only validated real transcript data
is returned. In src/agents/interactive_metadata_extractor.py lines 106-109, stop
invoking the placeholder _generate_transcript_whisper fallback after the YouTube
request fails. In src/agents/process_video_with_mcp.py lines 241-246, propagate
the direct extraction failure when no real fallback succeeds. In
src/agents/process_video_with_mcp.py lines 257-260, replace the fixed result in
RealVideoProcessor._extract_transcript_with_rotation with actual subtitle
extraction, or raise a typed extraction error when unavailable.
Sources: Coding guidelines, Path instructions
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Locate every transcript client construction and any existing bounded Session wrapper.
rg -n -C 3 --glob='*.py' 'YouTubeTranscriptApi\s*\(' src
rg -n -C 3 --glob='*.py' 'http_client\s*=|class .*Session|def request\(' src
# Confirm the declared youtube-transcript-api constraint.
sed -n '1,220p' pyproject.tomlRepository: groupthinking/EventRelay
Length of output: 16595
🌐 Web query:
youtube-transcript-api http_client YouTubeTranscriptApi __init__ _http_client requests Session timeout
💡 Result:
In the python library youtube-transcript-api, you can control the HTTP configuration by passing a custom requests.Session object to the YouTubeTranscriptApi constructor via the http_client parameter [1][2]. While the library does not have a native timeout parameter, you can manage request timeouts by configuring the Session object before passing it to the API [1][2][3]. To set a timeout for all requests made by the library: 1. Initialize a requests.Session object. 2. Manually set a default timeout for the session's request methods or use a transport adapter. A common pattern in the requests library is to monkeypatch the session or override the request method, as requests.Session does not have a built-in default timeout attribute [3]. Example of passing a custom session: from requests import Session from youtube_transcript_api import YouTubeTranscriptApi # Create a session http_client = Session # Customize the session (e.g., custom headers) http_client.headers.update({"Accept-Encoding": "gzip, deflate"}) # Inject the session into the API ytt_api = YouTubeTranscriptApi(http_client=http_client) ytt_api.fetch(video_id) Note that YouTubeTranscriptApi is not thread-safe because it uses a requests.Session object internally [2][4]. If you are working in a multi-threaded environment, you must initialize a new instance of YouTubeTranscriptApi (and ideally a new requests.Session) per thread [2][4]. Regarding the timeout specifically, users have noted that the library lacks a direct timeout argument, and requests for this feature have been discussed in the project's issue tracker [3]. Users are often advised to handle timeouts by wrapping the API calls or customizing the underlying requests session [3].
Citations:
- 1: http://github.com/jdepoix/youtube-transcript-api
- 2: https://github.com/jdepoix/youtube-transcript-api/blob/master/youtube_transcript_api/_api.py
- 3: Timeout Option for Proxy Requests jdepoix/youtube-transcript-api#324
- 4: Cannot retrieve transcripts using concurrent.future jdepoix/youtube-transcript-api#403
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the executor call sites and whether any youtube-transcript-api call
# passes http_client or configures timeouts.
rg -n -C 4 'http_client|timeout' src/agents src/integration/youtube_api.py src/mcp/mcp_video_processor.py src/youtube_extension/backend/enhanced_video_processor.py src/youtube_extension/backend/services/youtube/adapters 2>/dev/null || true
# Static model of YouTubeTranscriptApi construction semantics by reading the installed package
# if available, without network access or repo execution.
python3 - <<'PY'
import inspect, importlib.util
mods = ["youtube_transcript_api._api", "youtube_transcript_api"]
for mod_name in mods:
spec = importlib.util.find_spec(mod_name)
print(mod_name, "FOUND" if spec else "NOT_FOUND", getattr(spec, "origin", None))
if spec:
mod = importlib.util.module_from_spec(spec)
try:
spec.loader.exec_module(mod)
except Exception as exc:
print("LOAD_ERROR", type(exc).__name__, str(exc))
continue
if hasattr(mod, "YouTubeTranscriptApi"):
print("YouTubeTranscriptApi file:", getattr(mod.YouTubeTranscriptApi, "__module__", None))
print(inspect.signature(mod.YouTubeTranscriptApi.__init__))
src = inspect.getsource(mod.YouTubeTranscriptApi.__init__)
print(src)
PYRepository: groupthinking/EventRelay
Length of output: 40895
Set a finite HTTP timeout for every transcript client.
youtube-transcript-api accepts a custom HTTP client, but these calls pass only proxy_config. Without an enforced connect/read timeout, a blocked transcript/SRT fetch can hold an executor worker and stall the surrounding timeout or circuit-breaker path.
Add one shared transcript HTTP-client factory that creates a requests.Session with bounded connect and read timeouts, and pass it through http_client= at every transcript API call, including:
src/agents/interactive_metadata_extractor.py#L106-L109src/agents/process_video_with_mcp.py#L241-L246src/agents/process_video_with_mcp.py#L257-L260src/integration/youtube_api.py#L118-C20src/mcp/mcp_video_processor.py#L713src/mcp/mcp_video_processor.py#L737
Apply the same bounded-session pattern to the other YouTube transcript fetches under src/youtube_extension/backend, since they create the same unbounded client.
📍 Affects 4 files
src/agents/interactive_metadata_extractor.py#L106-L109(this comment)src/agents/process_video_with_mcp.py#L241-L246src/agents/process_video_with_mcp.py#L257-L260src/integration/youtube_api.py#L118-L120src/mcp/mcp_video_processor.py#L713-L713src/mcp/mcp_video_processor.py#L737-L737
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agents/interactive_metadata_extractor.py` around lines 106 - 109,
Introduce one shared transcript HTTP-client factory that returns a
requests.Session enforcing finite connect and read timeouts, then pass its
result via http_client= to every YouTubeTranscriptApi call. Update
src/agents/interactive_metadata_extractor.py lines 106-109,
src/agents/process_video_with_mcp.py lines 241-246 and 257-260,
src/integration/youtube_api.py lines 118-120, and src/mcp/mcp_video_processor.py
lines 713 and 737; apply the same bounded-session pattern to all transcript
fetches under src/youtube_extension/backend. Preserve existing proxy
configuration and fetch behavior while ensuring every transcript client uses the
shared timeout-enabled session.
Source: Path instructions
|
Exact-head triage for CodeRabbit thread
The consolidated thread remains unresolved. Neither finding is being folded speculatively into #1129; both require a focused child issue and canonical repair unit. #1129 remains draft. The test-only guard hardening from duplicate #1131 has been transferred to canonical head |
|
@coderabbitai review Exact-head review requested for |
|
I will assess the transferred guard hardening and the focused proxy-routing change. I will not treat ✅ Action performedReview finished.
|
Closing — branch orphaned by the secret-purge force-pushNo common ancestor with current The work is unlanded — routing every transcript client through the centralized proxy, plus rejecting opaque and foreign proxy configs. Two pre-existing gaps were identified on this PR's review thread and are worth carrying forward as their own focused issues rather than being lost here: fabricated transcript fallbacks, and unbounded HTTP clients. Tracked in #1378. Branch retained for archive-tagging. Generated by Claude Code |
Canonical issue
Closes #1087
#1087 asks for three things: (a) migrate transcript calls to the
youtube-transcript-api v1.x instance API, (b) add centralized outbound proxy
handling, and (c) redact credentials from logs.
Only (b) was outstanding, and only partially — the helper existed but six
call sites never used it. Evidence for the other two on
origin/main:grep -rn "YouTubeTranscriptApi\.\(get_transcript|get_transcripts|list_transcripts\)"→ 0 hits. Live surface is instance-only:.fetch/.list/.to_raw_data(). Floor pinnedyoutube-transcript-api>=1.0.0inpyproject.toml:109andrequirements.txt:63.proxy_config(table below).redact_proxy_credentialslives insrc/youtube_extension/utils/proxy.py:70; #1118 (which targets issue 1113) generalises it. Deliberately not touched here to keep the PRs disjoint.Outcome
youtube-transcript-api>= 1.0 only honours a proxy when aproxy_configispassed to the constructor. A bare
YouTubeTranscriptApi()egresses from thehost's own IP regardless of
WEBSHARE_PROXY_URL. Six call sites did exactlythat, so the "centralized" proxy was silently bypassed on those paths — and
because the fallback paths are the ones that run after YouTube starts blocking,
these were the calls that most needed the proxy.
src/integration/youtube_api.py:99.fetch(video_id, languages=…)src/agents/process_video_with_mcp.py:222.fetch(video_id)_extract_transcript_with_rotationsrc/agents/process_video_with_mcp.py:233.list(video_id)src/agents/interactive_metadata_extractor.py:87.fetch(video_id)src/mcp/mcp_video_processor.py:694.fetch(…)_direct_extractionsrc/mcp/mcp_video_processor.py:718.list(video_id)_routed_extractionThe method name
_extract_transcript_with_rotationstates the intent that thecode did not implement.
Each site now resolves its config from the canonical helper
youtube_extension.utils.proxy.get_transcript_proxy_config(), wrapped in theguarded-import pattern this repo already uses in
src/agents/gemini_video_master_agent.py:59-66, so these modules stay importablewhen executed outside the package. No behaviour change when no proxy is
configured — the helper returns
None, which is what the constructordefaulted to before.
Regression guard
tests/unit/test_transcript_proxy_coverage.py(11 tests) walks the AST of every.pyundersrc/andshared/and fails if anyYouTubeTranscriptApi(...)construction omits
proxy_config. Reviewing this by eye does not scale to 14call sites across 6 modules, and the failure mode is silent.
It also carries an explicit anti-vacuity canary
(
test_guard_finds_the_client_at_all): if the client is renamed upstream or thescan roots stop resolving, the guard fails instead of passing over an empty
set. This repo has repeatedly shipped checks that never actually check
(#1116, #1121, #1091); this one is built not to.
Risk
WEBSHARE_PROXY_URLis unset the helper returnsNone— byte-identicalbehaviour to the previous bare constructor.
ImportErrorit defines alocal stub returning
None.src/andshared/only. Tests may still construct bareclients against stubs — intentional.
fix(security): stop proxy credentials leaking from subprocess errors #1118). Kept disjoint so neither PR blocks the other.
Verification
All commands run at
5659f2b9c.Suite:
pytest tests/unit/test_transcript_proxy_coverage.py→ 21 passed.Coverage of the fix:
grep -rn "YouTubeTranscriptApi(" src/ shared/→ 14constructions, 14 pass
proxy_config, 0 bare.Mutation testing — the guard is worthless if it cannot fail. Each mutation
was applied, the suite run, then reverted; every one is killed:
mcp_video_processor.py:701)proxy_configfrom a pre-existing correct site (robust.py:454)proxy_config=NoneSOURCE_ROOTSso the scan matches nothingCLIENT_NAME(simulates upstream rename)GenericProxyConfig("",""))None(the exact production bug)parents[1]→parents[0])Baseline and post-revert state: 21 passed each time.
M7/M8 target the helper rather than the call sites, which closes the loop: the
structural test proves every site passes
proxy_config=<helper>(), andtest_proxy_helper_returns_config_carrying_the_url_when_setproves the helperreturns a config actually carrying the URL. Together they establish that every
site is genuinely proxied, not merely syntactically compliant. That test stubs
youtube_transcript_api.proxies, so it holds without the optional extrainstalled.
Unplanned real-world validation: mid-review a stray
git checkout --revertedall four source files. The guard caught it immediately and named every site:
No regressions. Test collection over
tests/unit, measured against a cleanorigin/mainworktree with the same interpreter:origin/main+21is exactly this PR's new tests; the 75 errors are pre-existing(missing optional deps) and unchanged. The two failures in files that import
the changed modules (
tests/utilities/process_video_with_mcp.py, missingaiohttp;test_security_agent_distinguishes_eval_from_literal_eval) reproducebyte-identically on the clean
origin/mainworktree.Lint held at baseline.
ruff checkon the four changed files: 25 before,25 after — no new findings.
black --check --diffper file, this branch vsorigin/main:youtube_api.pyinteractive_metadata_extractor.pyprocess_video_with_mcp.pymcp_video_processor.pyMeasured as changed lines (
grep -c '^[+-][^+-]'), not hunks — hunk counts are anunstable metric here, since inserting a line can split one existing mega-hunk into
two and report a regression with zero new unformatted code.
Those files were never black-formatted; this PR does not add drift and does not
reformat unrelated lines. The new test file is clean under both
blackandruff.Compile check:
python -m py_compilepasses on all four modified modules.Review remediation
Review found the guarded import this branch introduced was itself a silent
bypass, and it was right. When these modules are executed by path — the
documented CLI entry point —
youtube_extensionis not importable, so thefallback stub returned
Noneandproxy_config=get_transcript_proxy_config()egressed from the host IP. The call site still read as proxied, which is
exactly the failure mode this PR exists to eliminate.
All four modules now bootstrap
srcontosys.pathand re-attempt thecanonical helper, raising
ImportErrorif it is still unreachable. The proxy iseither honoured or the module refuses to load. Bootstrapping is safe:
proxy.pyis stdlib-only apart from a guarded
youtube_transcript_api.proxiesimport, andits package
__init__chain pulls onlyreandtyping— verified by importingit from
/tmpwith noPYTHONPATH.The guard suite was hardened against three ways it could pass while protecting
less than it claimed:
SOURCE_ROOTScould be narrowed by hand with every other test still green, sothe required roots are now rediscovered by AST-scanning the repository for
YouTubeTranscriptApiconstructionspassed; every
ImportFromtargeting the proxy module must now bind the helperTwo of the tests added in this round were themselves vacuous and were caught only
by mutation, which is the honest headline here:
parents[1]from a test-side constant, somutating the source's index changed nothing it observed. It now parses the
index out of the source AST, scoped to the
sys.path.insertcall node —necessary because
process_video_with_mcp.pycontains an unrelatedpre-existing
parents[2].PROXY_HELPER in source, which the callsite satisfies on its own.
The repo-wide scan is AST-based rather than grep-based for a concrete reason:
scripts/skill_builder.pycontainsYouTubeTranscriptApi(...)inside a stringliteral, which grep would report as an unproxied egress path. It is fail-loud on
unparseable files rather than best-effort — all 662 non-excluded files parse today.
Production evidence
WEBSHARE_PROXY_URLset, the six listed callsites connected to YouTube from the pod's own IP. On the paths that matter
most — the
.list()fallbacks reached only after a primary fetch fails, i.e.when YouTube is already rate-limiting — this produced
IpBlocked/TooManyRequeststhat the proxy existed specifically to prevent.src/andshared/route throughget_transcript_proxy_config().NonewhenWEBSHARE_PROXY_URLis unset, asserted bytest_proxy_helper_returns_none_without_configuration.this class of regression cannot silently return.