Repository navigation
fix(ci): stop installing ffmpeg in jobs that never use it - #1360
Conversation
`release` is red right now, and every open pull request with it, because `apt-get update` sat for fourteen minutes with no output while azure.archive.ubuntu.com returned `Ign:` for every index. That is the third distinct way this one install has broken CI in a single day: first a corrupt published asset, then a version pin that stopped resolving, then a step deadline too tight for a slow mirror — and now the mirror itself. None of it was ever needed. No package script invokes ffmpeg. Nothing installs it as a dependency. `src/` shells out to it only at runtime — frame extraction, video merging, audio playback — never during install, lint, typecheck, build or pack, which is all these jobs do. The proof was already in the file: provider-safety-net has never installed ffmpeg, and it builds the package and runs three suites without trouble. So five jobs were paying for a tool none of them invoke, on the single most failure-prone step in the pipeline, reached over a network that has now failed three different ways. Since build-check became a required check, each of those failures blocked every open pull request rather than one job. Removed from all five: the three CI jobs and both release jobs. This takes the last unnecessary network fetch out of the critical path rather than tuning its timeouts again — the previous two fixes each replaced one failure mode with another, because the dependency itself was the problem. A comment in each workflow records why it is absent and what to do if a job ever genuinely needs it: install it in that job alone, and bound every wait.
|
Warning Review limit reached
Next review available in: 41 minutes Limit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
There was a problem hiding this comment.
Pull request overview
This PR removes ffmpeg installation from GitHub Actions workflows where it is not used, to reduce CI flakiness and unblock required checks that were timing out due to apt-get/mirror instability.
Changes:
- Removed
apt-get-basedffmpeginstall/verify steps from CI and release workflows. - Added in-workflow commentary documenting why
ffmpegis intentionally not installed and how to add it back only for jobs that truly need it.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/ci.yml | Removes ffmpeg install steps from CI jobs and replaces them with explanatory comments. |
| .github/workflows/release.yml | Removes ffmpeg install steps from release workflow jobs and replaces them with explanatory comments. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Because build-check is a required check, each of those blocked every | ||
| # open pull request on a dependency none of these jobs use. |
There was a problem hiding this comment.
Fixed in #1895: the release.yml comment no longer says build-check gates that workflow; it separates the pull-request blocking in ci.yml from a stall in a release run.
|
💬 MINOR: Remove unnecessary ffmpeg installation from CI - This change removes problematic ffmpeg installation from CI workflows that never actually need it. The ffmpeg dependency is bundled via ffmpeg-static/ffprobe-static as optional dependencies and is used at runtime only (frame extraction, video merging). All CI jobs either run static analysis (lint, format, typecheck), mock tests, or build verification - none of which require runtime media processing. This change addresses real CI failures where apt-get install ffmpeg caused 90+ minute lock waits and was blocked by corrupt upstream assets. |
|
💬 MINOR: Remove unnecessary ffmpeg installation from release workflow - Removing ffmpeg from release.yml prevents the same apt lock issues from affecting release pipelines. The release workflow builds and publishes packages - it does not run any media processing suites that would require ffmpeg at runtime. |
Review Summary for PR #1360Decision: ✅ APPROVEDSummary: This is a clean, well-justified CI/CD optimization that removes problematic ffmpeg installation from workflow files. No code changes - purely configuration cleanup. Findings (2 MINOR)
Impact on Existing Code
Review Scope & NotesThis PR addresses a real pain point: the apt-get install ffmpeg step was causing:
The change correctly identifies that:
Recommendation: If No other review needed - this is a straightforward, well-documented improvement. |
|
💬 MINOR: Remove unnecessary ffmpeg installation from release workflow - Removing ffmpeg from release.yml prevents the same apt lock issues from affecting release pipelines. The release workflow builds and publishes packages - it does not run any media processing suites that would require ffmpeg at runtime. This is consistent with ci.yml changes - release jobs don't need ffmpeg and were vulnerable to the same apt lock issues. |
Review SummaryDecision: APPROVED ✅This PR removes problematic ffmpeg installation from CI workflows (ci.yml and release.yml) that never actually need it during their execution. The change improves CI reliability by eliminating apt lock contention issues that have previously blocked all open PRs. Findings Summary
Total: 2 minor findings, both related to CI/CD optimization and reliability. Detailed Findings
Impact on Existing Code
Reviewed ScopeThis review covered:
ConclusionThis is a safe, low-risk improvement that addresses real CI reliability issues without impacting functionality. The changes are well-documented with clear explanations of why ffmpeg isn't needed and guidance for when it might be required in the future. Recommendation: APPROVE ✅ |
| exit 1 | ||
| } | ||
| echo "✅ ffmpeg installed successfully" | ||
| # ffmpeg is deliberately NOT installed in this workflow. Nothing these |
There was a problem hiding this comment.
💡 MINOR: Remove unnecessary ffmpeg installation from CI - The ffmpeg dependency is bundled via ffmpeg-static/ffprobe-static as optional dependencies and is used at runtime only (frame extraction, video merging). All CI jobs either run static analysis (lint, format, typecheck), mock tests, or build verification - none require runtime media processing. This change addresses real CI failures where apt-get install ffmpeg caused 90+ minute lock waits and was blocked by corrupt upstream assets. Jobs that genuinely need ffmpeg (e.g., test:media suite) should be added separately with proper apt timeout guards.
| exit 1 | ||
| } | ||
| echo "✅ ffmpeg installed successfully" | ||
| # ffmpeg is deliberately NOT installed in this workflow. Nothing these |
There was a problem hiding this comment.
💡 MINOR: Remove unnecessary ffmpeg installation from release workflow - Removing ffmpeg from release.yml prevents the same apt lock issues from affecting release pipelines. The release workflow builds and publishes packages - it does not run any media processing suites that would require ffmpeg at runtime. Consistent with ci.yml changes - release jobs don't need ffmpeg and were vulnerable to the same apt lock issues.
Review SummaryDecision: APPROVED ✅ This PR removes problematic ffmpeg installation from CI workflows that never actually need it during their execution. The change improves CI reliability by eliminating apt lock contention issues that have previously blocked all open PRs. Findings (2 total, both MINOR):
Impact on existing code:
Review scope:Focused on CI/CD optimization and reliability. No CRITICAL or MAJOR issues found. The change is self-contained and safe to merge. |
|
🎉 This PR is included in version 11.2.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes the review threads left open on merged PRs against the CI workflows, the repo's gate scripts and a few dev tools. Each change is the smallest one that closes its finding. Behaviour changes are covered by a new suite (test/continuous-test-suite-tooling-scripts.ts, `pnpm run test:tooling-scripts`) and by additions to the provider-structure and provider-descriptors suites; every new test was run red against the unfixed source first. Workflows - ci.yml: persist-credentials: false on the seven checkouts that never push, semantic-release-validation keeps its token (T3790294038, #1335). The permissions comment names that job as the one contents: write exception (T3858986829-a, #1552). The pinned suite counts and the 373-assertion figure are gone (T3869180755-f1, #1580). The new tooling-scripts suite is added to extended-suites; its weight (100) is an estimate, not a CI median. - release.yml: the ffmpeg note no longer says build-check gates this workflow (T3810295618-a, #1360). - single-commit-enforcement.yml: one SKIP_RE shared by both greps, printf instead of echo, and the guidance names the push/pull_request workflows rather than "every workflow" (T3813387872-printf-regex, T3813416696-overstated-guidance, #1364). Config and lint docs - config/models.json: Opus 4.5 uses the real snapshot id 20251101 for anthropic, bedrock and vertex instead of the 20251124 launch date (T3816077440, #1375). provider-structure now checks every Claude id in the file against the model enums. - eslint-rules/index.cjs: header lists e2e-tests-only, no-inline-secret-regex, provider-typed-errors and provider-base-class (T3801758166-1, #1344). Scripts - build-validations.ts: fails when typedoc.json carries an unanchored `**/<dir>/**` exclude, which drops every file under a checkout whose path contains that directory (T4042344752-guard, #1723). - check-banned-deps.ts: scans each file as a whole, so import(), require() and `from` followed by a specifier on the next line are found, and a `//` inside a string no longer hides the rest of the line (T3956062753, #1662). Files in the repo root and .mts/.cts are scanned too (T3956062775, #1662). - check-shipped-types.ts: the declarations under dist/ must equal the set the source tree emits, so a partial or stale build above the 100-file floor fails (PF-T3927528338, #1627). A wildcard export is matched against the whole pattern, including a `*` in a directory component (T3931686738-wildcard-match, #1632). - codex-replay-listener.ts: the tool-call script names `replay_tool` instead of `exec`, which Codex declares as a custom tool and which raised a Fatal "incompatible payload" error (F1-T4087477953-custom-tool-shape, #1783); reproduced and cleared against codex-cli 0.160.0. --requests counts served /responses turns, so a 404 probe cannot shut the listener down first (F2-T4087477985-requests-limit-counts-404s, #1783). - commit-validation.ts: execFileSync("git", [...]) instead of a shell string; behaviour unchanged (T3838161513-b, #1499). - migration-symbol-diff.mjs: this/super-rooted paths keep their full name, and tagged templates, obj["name"](), super() and import() are tracked; the header says it follows calls (T3835058026-residual, PF-T3833252257, #1448). - tools/automation/environmentManager.ts: credential-free providers count as configured only when the .env sets one of their variables, the score no longer divides by the size of the catalog, and the report lists the configured providers plus one count instead of every missing one (T3792794348, T3792807279, #1337). Not done, on purpose - The skip-checks trailer in the single-commit grep (optional in the finding). - Checkouts in workflows other than ci.yml: the findings named only ci.yml. - migration-symbol-diff still does not record a function passed by reference (`items.forEach(handler)`); the header now says so. Pre-existing, not touched: test:dynamic fails its five live cases without provider credentials, identically with config/models.json reverted.
releaseis red right now, and every open PR with it. This removes the cause rather than tuning it again.What's happening
apt-get updatesat for fourteen minutes with no output whileazure.archive.ubuntu.comreturnedIgn:for every index, then hit the step deadline:That is the third distinct way this one install has broken CI in a single day:
Each of my last two fixes replaced one failure mode with another, because the dependency itself was the problem.
None of it was ever needed
scripts.src/shells out to ffmpeg only at runtime — frame extraction, video merging, audio playback — never during install, lint, typecheck, build or pack, which is all these jobs do.The proof was already sitting in the same file:
provider-safety-nethas never installed ffmpeg, and it runspnpm run buildplus three test suites without trouble.So five jobs were paying for a tool none of them invoke, on the most failure-prone step in the pipeline, reached over a network that has now failed three different ways. And since
build-checkbecame a required check, each of those failures blocks every open PR rather than one job.Change
Removed from all five:
test,build-check,quality-gate, and bothrelease.ymljobs. Net −204 / +30 lines.A comment in each workflow records why it's absent and what to do if a job ever genuinely needs it — install it in that job alone, and bound every wait.
Verification
test10,provider-safety-net9,build-check7,proxy-performance5,quality-gate12,semantic-release-validation5; releasetest5,release10).ffmpegstep remains in any job — only the explanatory comments.