Skip to content

Production-grade hardening: lint/security/ffmpeg-observability/process-leak/tests/CI - #18

Open
itsPremkumar wants to merge 9 commits into
mainfrom
prod-grade
Open

Production-grade hardening: lint/security/ffmpeg-observability/process-leak/tests/CI#18
itsPremkumar wants to merge 9 commits into
mainfrom
prod-grade

Conversation

@itsPremkumar

@itsPremkumar itsPremkumar commented Jul 24, 2026

Copy link
Copy Markdown
Owner

Summary

Drives the Automated-Video-Generator through the gstack /health/review → fix-loop methodology to production grade. All changes verified locally: typecheck clean, ESLint 0 errors, full suite 560 pass / 0 fail / 14 env-skip, line coverage 82.05% ≥ 80% floor.

Changes (atomic commits)

  • fix(lint): clear 4 ESLint errors (prefer-const, no-useless-escape, no-unused-expressions, no-empty-object-type) that were blocking the CI lint gate.
  • fix(security): route ffmpeg drawtext captions through the canonical ffmpegDrawtextEscape() (handles \ : ' " ,) in export.ts/brand.ts, closing a filter-injection / caption-breakage vector. Added regression test.
  • fix(ffmpeg): surface swallowed stderr in previously-silent catch blocks (sfx/visual-fx/watermark) for observability, preserving the "never hard-fail offline" fallback contract.
  • fix(process): killBackend() now tears down the full backend process tree on Windows via taskkill /T /F /PID — a detached Python backend is not reaped by a bare SIGTERM (real RAM leak between jobs).
  • fix(logic): candidatesPerAsset default corrected 4→2 to match its own documented default; buildDuckExpression now emits correct raw-comma ffmpeg between(t,s,e) wrapped in gt() (was String.raw with stray backslashes that broke the duck curve).
  • test: aligned buildDuckExpression assertions to the corrected output; env-dependent bundled-music tests skip when input/bgm/__bundled__ is empty (same convention as network-image skips).
  • ci: enforce a line-coverage floor (80%) as a regression gate via scripts/check-coverage.mjs.
  • fix(deps): declare @remotion/shapes, @remotion/paths, @remotion/motion-blur, @remotion/transitions as explicit dependencies (matching ^4.0.487). They were imported by remotion/*.tsx only via transitive hoisting, which a clean npm ci (e.g. CI) did not guarantee — causing TS2307: Cannot find module '@remotion/...' and breaking main's typecheck. This is what made CI red on every recent main push; this PR fixes that root cause.

CI note

The repo's CI workflow currently only triggers on push to main and pull_request; in practice no pull_request run has ever been recorded for this repo, so CI does not run on this PR branch. The fix above targets the exact TS2307 error seen in main's CI logs (run 30057719212, step "Typecheck"). Merging this PR to main will let the push trigger re-run CI, which should now pass.

Test plan

  • npm run typecheck — clean (0 errors)
  • npx eslint src/ remotion/ — 0 errors
  • npm run test:unit — 560 pass / 0 fail / 14 skip (network + absent bundled assets)
  • npm run test:coverage through gate — 82.05% lines

Note

The 14 skipped tests pass only when their infra is present — the input/bgm/__bundled__ music tracks aren't committed in this checkout. Committing those assets would flip them from skip→run.

…s-escape, no-unused-expressions, no-empty-object-type)
…o close filter-injection/breakage; add regression test
…ual-fx/watermark) for observability, keep offline-fallback contract
…lt; buildDuckExpression emits raw ffmpeg between()+gt() (was String.raw with stray backslashes)
…put; skip bundled-music tests when input/bgm/__bundled__ is empty (env, like network skips)
…ts/check-coverage.mjs to parse V8 coverage summary
…n-blur, @remotion/transitions as explicit dependencies

remotion/*.tsx imported these subpackages without declaring them, so a clean
npm ci (e.g. in CI) failed typecheck with TS2307 'Cannot find module
@remotion/...'. They were only present via transitive hoisting, which is
non-deterministic. Declaring them explicitly (pinned to ^4.0.487, matching
the other @Remotion deps) makes the install reproducible and unblocks CI.
…en infra is absent; give restitch test 240s timeout (flaky under load, not a logic failure)
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.

1 participant