Skip to content

ci(eval): install ffmpeg for eval scaffolds that render frames - #15482

Merged
kyle-sexton merged 2 commits into
mainfrom
ci/eval-browser-ffmpeg
Nov 12, 2026
Merged

kyle-sexton merged 2 commits into
mainfrom
ci/eval-browser-ffmpeg

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Nov 12, 2026 •

Copy link
Copy Markdown
Member

No related issue: owner decision Q14 (failing workflows)

Summary

eval-browser-dispatch.yml runs eval cases on a hosted runner that has no ffmpeg. Four cases name ffmpeg as a host tool for their scaffold: three in explainer-video (pr-exit-zero-reads-every-frame, pr-light-skips-frame-judge, pr-standard-silent-judge-gets-beats) and knowledge's vd-light-skips-vision-spot-check. Run 42563549712 reported NOT COMPARABLE: vd-light-skips-vision-spot-check (with-arm run 1 errored: scaffold failed (exit 127): ... scaffold.sh: line 13: ffmpeg: command not found).

Fix

Each eval job (eval, eval-subscription, eval-fallback) gets an "Install ffmpeg for eval scaffolds" step after "Isolate the runner", beside the pinned shfmt step, so the downloaded binary never runs with sudo. It runs only when the staged plugin list holds explainer-video or knowledge. It uses the BtbN static build pr-require-checks.yml's demo-video leg already pins, with the same release tag, build and SHA-256. It checks the archive's checksum and each binary's version, and installs ffmpeg and ffprobe into /opt/tools/bin, which the eval sandbox can read.

Done when

  • An eval job that stages explainer-video or knowledge has ffmpeg and ffprobe on PATH at the pinned build, installed after isolation.
  • Evals for other plugins skip the download.

Verification

  • The 7 node suites that read this workflow (plugin-eval/workflow, credential-legs, eval-handoff, resolve-config/callers, check-kill-switch, check-upstream-status, work-unblock/label-unblocked) all pass.
  • scripts/check-affected.sh: 10 passed, 6 skipped.
  • actionlint: no new findings. The local binary flags the existing cache-mode job key at the same 3 places on main, so those predate this change.
  • The eval is deliberately not dispatched after merge, because a dispatch spends credit. The next eval dispatch that includes these cases is the run-time check.

Related

🤖 Generated with Claude Code

https://claude.ai/code/session_01VQb9Q2ceZLXhTR4d1vkTCe

Four eval cases name ffmpeg as a host tool (three in explainer-video,
knowledge's vd-light-skips-vision-spot-check), and the hosted image has
none: run 42563549712 reported vd-light-skips-vision-spot-check NOT
COMPARABLE with "scaffold.sh: line 13: ffmpeg: command not found". Each
eval job (key, subscription, fallback) now installs the BtbN static
build pr-require-checks.yml's demo-video leg pins, with the same tag
and SHA-256, into /opt/tools/bin before the runner is isolated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQb9Q2ceZLXhTR4d1vkTCe
@kyle-sexton
kyle-sexton marked this pull request as ready for review November 12, 2026 01:03
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-12T01:05:12.451164Z 69d895f Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-security-review: 1 inline finding(s) on 69d895f30083e0270f5b664180eb08723ec0c58a; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed aae9e6daf25d105fe50b99fca5207cc8dbd01cab: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Security review: PR #15482, incremental (69d895f3..aae9e6da)

Scope. This was an incremental review covering the one listed file, .github/workflows/eval-browser-dispatch.yml. I read every hunk in .claude-lane/incremental.diff. Because the file is on a trusted path, I also read the steps around the change at head: tool directory creation, the managed-settings deny policy, runner isolation, the ffmpeg and shfmt installs, and the policy probe. This lane already reviewed an earlier head, so this review reports blocking findings only.

What changed. In each of the three eval jobs, "Isolate the runner" and "Install the locked root toolchain" now run before "Install ffmpeg for eval scaffolds". The ffmpeg step also runs only when the staged plugin list includes plugins/explainer-video or plugins/knowledge. All three copies are identical. The jobs are at L509-L558, L1372-L1421 and L2188-L2237.

What I checked

  • Earlier finding fixed. My SUGGESTION from the last review (discussion_r4545009572) was that the pinned BtbN binary ran before isolation, while sudo still worked. The step now runs after setup-runner-isolation, so the archive is extracted and ffmpeg -version / ffprobe -version run without sudo or docker. That matches the job's own rule at L453-L455. The step never calls sudo. It writes only to /opt/tools, which L458 makes owned by the runner user. So it works after isolation without regaining any privilege.
  • Isolation moved up. The deny-policy step (L472-L507) still runs before isolation, while sudo is available, so the root-owned managed settings file is still written. The locked toolchain still installs after sudo and docker are removed. No step that needs sudo now comes after isolation.
  • New if: condition. It reads steps.plugins.outputs.list (step id: plugins, L370), and only inside an if: expression. It never reaches a run: shell, so there is no injection surface. The check is a substring match, so a similarly named plugin path would trigger the same pinned, checksummed install. That costs some extra download time and nothing else.
  • Trust controls. The change does not touch the trusted-trigger check, the lane token request, the kill switch, setup-runner-isolation's inputs or any merge hold. The pins (FFMPEG_RELEASE, FFMPEG_BUILD, FFMPEG_SHA256) did not change.

Spend. No new charge. Evals for other plugins now skip the ffmpeg download, so those jobs run slightly shorter.

No embedded instructions were found in the pull request context. No security issues found.

critical-findings: 0

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-review: 2 inline finding(s) on 69d895f30083e0270f5b664180eb08723ec0c58a; the summary is in this lane's pull request comment.

claude-lane: claude-review

@github-actions

github-actions Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed aae9e6daf25d105fe50b99fca5207cc8dbd01cab: read 1 of 1 changed file(s); 0 finding(s), 0 as inline comments.

Code-review lane: PR #15482, incremental review (69d895f3..aae9e6da)

No new findings. This round reported blocking findings only, because the lane reviewed an earlier head. I read every hunk of the one file in scope, .github/workflows/eval-browser-dispatch.yml, which is on the trusted-path list.

Both earlier findings are fixed in all three jobs (eval, eval-subscription, eval-fallback):

  1. Placement (discussion_r4545010340): this push moved "Isolate the runner" and "Install the locked root toolchain" above "Install ffmpeg for eval scaffolds" (L512-L537, and the matching blocks at L1375 and L2191).
    • The downloaded ffmpeg/ffprobe now run only after sudo and docker access are gone.
    • The rule in "Create the tool directories", "no third-party code runs before isolation" (L453-L455), is true again.
    • The "Deny the eval's reads" step, which uses sudo, still runs before isolation, as it did before this PR.
    • The ffmpeg step needs no sudo: /opt/tools belongs to the runner user, and the shfmt step already downloads over the network after isolation.
  2. Gating (discussion_r4545010344): the step now runs only when the staged plugin list includes explainer-video or knowledge. The check is contains(steps.plugins.outputs.list, 'plugins/explainer-video' | 'plugins/knowledge'), the same pattern as the shfmt step.
    • steps.plugins (id: plugins, L370) is set before this step, and its list output holds one plugins/<name> per line.
    • No other plugin's name starts with knowledge or explainer-video, so the substring check cannot match the wrong plugin.

PR "Done when" items: both are met by the workflow as written. They are only checked at run time: nothing exercises the step until an eval dispatch stages explainer-video or knowledge, as the PR says.

Verification: the PR's test-suite, check-affected.sh and actionlint results are what the author reports, and I could not check them because this lane has no shell.

Closing issue: none. The PR cites owner decision Q14, so there are no issue acceptance criteria to check against.

Spend (not a hold): the ffmpeg download now runs only on evals that stage those two plugins, so it uses fewer hosted-runner minutes than the previous head. Nothing about billing changes.

The step ran the downloaded ffmpeg while sudo still worked, which breaks
the job's rule that no third-party code runs before isolation. It now
runs after "Isolate the runner", beside the pinned shfmt step. It is
also gated on the staged plugin list holding explainer-video or
knowledge, so other evals do not download the archive.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VQb9Q2ceZLXhTR4d1vkTCe
@kyle-sexton
kyle-sexton added this pull request to the merge queue Nov 12, 2026
Merged via the queue into main with commit a84c815 Nov 12, 2026
83 checks passed
@kyle-sexton
kyle-sexton deleted the ci/eval-browser-ffmpeg branch November 12, 2026 02:12
@github-actions github-actions Bot added the merge-path: owner-agent Post-merge accounting record (README.md, Merge paths) label Nov 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-path: owner-agent Post-merge accounting record (README.md, Merge paths)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant