Skip to content

Add pr-media.py for putting a clip or screenshot on a PR - #15295

Merged
teamleaderleo merged 3 commits into
manaflow-ai:mainfrom
teamleaderleo:feat/pr-media-clips
Sep 28, 2026
Merged

teamleaderleo merged 3 commits into
manaflow-ai:mainfrom
teamleaderleo:feat/pr-media-clips

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Dogfood evidence lands by hand today: get the file onto the pr-media branch somehow, then write a raw URL into the PR body and hope the path matches. The steps are identical every time, so scripts/pr-media.py does them.

scripts/pr-media.py --pr 15277 sidebar-drag.mp4 settings-dark.png
scripts/pr-media.py --pr 15277 frames-out/ --dry-run

Each file lands at <pr>/<name> on the pr-media branch and the tool prints the Markdown to paste. A directory expands to the media files directly in it, in name order, which is the shape scripts/ci/e2e-frames.py already writes. --dry-run resolves the whole plan, prints it and the Markdown, and touches nothing. --comment posts the Markdown to the PR rather than only printing it.

An mp4 also gets a gif made from it. GitHub renders a gif inline from a raw URL and will not render an mp4 from one, so a reviewer scrolling the PR sees the motion without clicking, and the mp4 stays linked underneath at full quality. A gif from cmux record --gif is uploaded as it is. --no-gif skips the conversion.

Details worth knowing:

  • Uploads use the contents API with the JSON body on stdin. A base64 megabyte does not fit in an argument list, which is what gh api -f content=... would do. An existing path is updated with its blob sha rather than failing.
  • A file over 25 MB is refused with advice (shorter clip, lower --gif-fps or --gif-width) instead of uploaded; past 8 MB it uploads with a warning. Every PR that links the branch pays for what is on it.
  • Conversion is one ffmpeg pass with a palette built from the clip itself (palettegen=stats_mode=diff), which spends the palette on what moves. min(width,iw) means a narrow clip keeps its own width instead of being blown up.
  • The pr-media branch is never created by this tool. If it is missing the run fails and says so, because a branch named by a typo would be a new orphan nobody looks at.
  • Names are sanitized to what a URL can carry, and two inputs that would collide on one remote name are refused before anything uploads.

Testing

python3 tests/test_pr_media.py: 29 tests, OK, run here and wired into the ci-guards.yml preflight lane and tests/test-execution.toml (linux-guard). They cover name sanitizing and captions, the mp4-plus-gif plan, the refusals (colliding names, unsupported type, missing file, bad PR number, missing branch, oversize file), the Markdown (gif embedded, mp4 linked and never embedded, URL escaping), the ffmpeg argv (no upscale, loop, palette), and the upload calls against a fake gh (new file versus existing sha, body on stdin not in argv, a refused write surfacing GitHub's message, --comment only posting when asked).

Executed against real files as well, since a fake gh proves nothing about ffmpeg: converted a 1200x800 3-second clip to a gif and checked with ffprobe that it came out 900x600 at 30 frames, and converted a 400x300 clip to confirm it stayed 400x300 rather than being upscaled. The dry run printed the plan and the Markdown for both.

python3 scripts/verify-local.py selected 15 checks for this diff and 14 ran green (one not selected); native compilation and app tests are not relevant to a Python script and were not run. scripts/ci/validate_test_execution_registry.py and tests/test_ci_test_execution_registry.py both pass with the new registry entry.

Not verified: a live upload. Nothing here has written to the pr-media branch yet, because the first real use is the clip for #15277 when its dogfood comes back, and I would rather not push a throwaway file to a branch every PR reads. The write path is covered by the fake-gh tests, and the plan it would execute is visible through --dry-run.

Changelog

none

Checklist

  • Behavior changes have added or updated tests, or Testing says why not
  • UI, settings, menu, schema, help-text or user-facing docs change: not applicable, contributor tooling only
  • New or changed v2 socket method allowlisted for cmux ssh: not applicable
  • iOS connectivity, auth, lifecycle, workspace action, terminal I/O or mobile RPC contract change: not applicable
  • User-facing docs updated if needed: the dogfood reference now says where evidence goes
  • Reviewed with a subagent before merge, and all bot and human review comments resolved

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Adds scripts/pr-media.py to put a clip or screenshot on a PR's pr-media branch and print the Markdown that embeds it, replacing the manual upload-then-paste flow. An mp4 is also converted to a gif (GitHub renders gifs inline, not mp4s), and --dry-run shows the whole plan without touching anything while --comment posts the Markdown as a PR comment.

Details

  • Uploads go through the contents API with the body on stdin, because a base64 megabyte does not fit in an argument list; existing files are updated with their blob sha.
  • Since every PR that links the branch pays for what is on it, gifs and screenshots refuse past 5 MiB (GitHub's inline proxy renders a broken image beyond it) and mp4s past 25 MB, with warnings as they approach.
  • Nothing uploads until the whole run is known to be possible: the PR must exist, every gif is converted and measured first, and a path already on the branch requires --force (which names the blob it replaces).
  • A failed gh call surfaces what it said; only a real 404 suggests the shared media branch is missing.
  • Names are sanitized to what a URL can carry, and inputs that would collide on one remote name are refused before anything uploads.

Testing

  • tests/test_pr_media.py runs in the preflight lane and is registered for linux-guard, covering the plan, Markdown, refusals, the ffmpeg command, and uploads against a fake gh.

Written for commit 29f9c93. Summary will update on new commits.

Review in cubic

Dogfood evidence has been landing by hand: upload a file to the pr-media
branch through whatever route, then write the raw URL into the PR body and
hope the path matches. The steps are the same every time, so this does them:
one file or a directory in, each file at `<pr>/<name>` on the branch, and the
Markdown to paste printed at the end. `--dry-run` shows the plan first and
touches nothing.

An mp4 also gets a gif made from it, because GitHub renders a gif inline from
a raw URL and will not render an mp4 from one. A reviewer scrolling the PR
sees the motion without clicking, and the mp4 stays linked underneath at full
quality. A gif from `cmux record --gif` is uploaded as it is.

Uploads go through the contents API with the body on stdin, since a base64
megabyte does not fit in an argument list. A file over 25 MB is refused with
advice rather than uploaded, because every PR that links the branch pays for
it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 6 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 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.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 5650d987-cb6d-41a8-a211-3ea0c6af6a93

📥 Commits

Reviewing files that changed from the base of the PR and between b36339a and 29f9c93.

📒 Files selected for processing (5)
  • .github/workflows/ci-guards.yml
  • scripts/pr-media.py
  • skills/cmux-review/references/dogfood-and-merge.md
  • tests/test-execution.toml
  • tests/test_pr_media.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

teamleaderleo and others added 2 commits September 28, 2026 03:13
Eight of these fail and twenty error against the current script. They pin
the behaviour the review asked for:

- a `gh` read that fails for any reason other than a 404 must surface what
  gh said, and only a real 404 may suggest creating the shared media branch
- the branch name is quoted in the query string
- `--pr` is checked against a real pull request before anything is written
- every input is measured, and every gif conversion done, before the first
  upload, so a bad second file cannot leave the first one orphaned
- an existing remote path needs `--force`, and the run says which blob it
  replaces
- a 409 from a concurrent upload is retried with a re-read sha, and a
  repeated conflict still fails instead of looping
- the Markdown is printed before `--comment` is attempted, and a failure
  part way through still prints the Markdown for what landed
- an inline image stops at Camo's 5 MiB, past which GitHub renders a broken
  image; an mp4 is only linked, so it keeps the larger limit
- a png renamed to .mp4 is refused before ffmpeg sees it
- `convert` is exercised: success, ffmpeg failure, silent no-output
- `--label` with several files is refused rather than dropped
- `--branch main` is refused
- `--dry-run` measures the files it was given
- an unreadable file is one line, not a traceback

The fake `gh` now answers a write over an existing file with no sha the way
GitHub does, with a 422; the old fake accepted it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
All 53 tests in tests/test_pr_media.py pass; 8 failed and 20 errored before
this.

A failed `gh` read is now a failure. `gh_read` returns None only for a 404
the caller asked to treat as an absence, so a bad token, a 403 or a typo'd
`--repo` reports what gh said and points at `gh auth status`, instead of
claiming the shared `pr-media` branch does not exist. That message asked the
reader to create a branch that already holds every PR's evidence.

Nothing is written until the whole run is known to be possible: the pull
request has to exist, every gif is converted and every file measured, and a
path already on the branch stops the run unless `--force` is passed. A forced
replacement prints the blob it replaced. If a write still fails part way
through, the Markdown for the files that did land is printed, so evidence on
the branch is never unreachable.

An inline image now stops at 5 MiB, the limit of the proxy GitHub renders it
through. Past it the upload used to succeed and the PR used to show a broken
image. An mp4 is only ever linked, so it keeps the larger 25 MB ceiling.

Also: a 409 from a concurrent upload is retried once with a re-read sha; the
branch name is quoted in the query string; a file whose header disagrees with
its extension is refused before ffmpeg sees it; `--label` with several files
is refused rather than silently dropped; `--branch main` is refused; a dry run
measures its inputs; and an unreadable file is one line, not a traceback.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@cursor

cursor Bot commented Sep 28, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Review: a review subagent found six things worth fixing plus a handful of small ones. Worst was the error handling in the gh layer: every failure, not just a 404, became "no pr-media branch on manaflow-ai/cmux, create it before uploading". A bad token or a typo'd --repo would have told the reader to create a branch that already holds every PR's evidence. Also: an existing file was replaced silently, --pr was never checked against a real pull request, a failure part way through left uploads on the branch with no Markdown naming them, a 409 from a concurrent upload was fatal, and the 8/25 MB limits sat above the 5 MiB ceiling of the proxy GitHub renders an inline image through, so a 5-25 MB gif would upload happily and render broken.

Fixed, red first in afa0dbf (8 failures, 20 errors) and green in 29f9c93 (53 tests):

  • gh_read raises with what gh said and points at gh auth status; it returns "absent" only for an observed 404. Only that case suggests creating the branch.
  • Nothing is written until the run is known to be possible: the pull request has to exist, every gif is converted and every file measured, and a path already on the branch stops the run unless --force is passed. A forced replacement prints the blob it replaced.
  • If a write still fails part way through, the Markdown for what landed is printed, so nothing on the branch is unreachable. The Markdown is also printed before --comment is attempted.
  • An inline gif or screenshot stops at 5 MiB; an mp4 is only linked, so it keeps the 25 MB ceiling.
  • A 409 is retried once with a re-read sha, and a repeated conflict fails instead of looping.
  • Tests: convert is exercised (success, ffmpeg failure, silent no-output), a non-dry mp4 run asserts both blobs' bytes, and the fake gh now answers a write over an existing file with no sha with a 422 the way GitHub does. The old fake accepted it.
  • Small ones: the branch name is quoted in the query string, a file whose header disagrees with its extension is refused before ffmpeg sees it, --label with several files is refused rather than dropped, --branch main is refused, a dry run measures its inputs, and an unreadable file is one line rather than a traceback.

Left:

  • The 5 MiB ceiling comes from the documented Camo limit, not from an upload I watched render. If a gif just under it ever shows broken in a PR, lower INLINE_MAX_BYTES.
  • sanitize cannot be broken by allowing / through its character filter, because Path.stem has already dropped any directory. The test asserts the guarantee (no separator in the output) rather than that one line, so a mutant there survives.
  • No test runs real gh, real ffmpeg or the network, by design. The first real upload is the check that the contents API calls are shaped right.

@teamleaderleo
teamleaderleo merged commit 8e6357b into manaflow-ai:main Sep 28, 2026
61 checks passed
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 29f9c933af: every check was green at merge (14 verified; 18 skipped by policy). Full suite runs on main after merge.

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Merged on green: 59 checks passed on 29f9c93, and the preflight guard group ran tests/test_pr_media.py (53 tests, all green) on that SHA. Tooling and docs only, no app build involved, so nothing here needed a dogfood pass.

Next stop for this is a real clip: the dogfood record step in #15320 produces the gif that goes through this uploader. :)

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 28, 2026
0c753fe ci: give each Python lane test an empty Foundation home (manaflow-ai#15289)
62cde14 Place config error notice below the tab bar (manaflow-ai#15218)
436909b Keep an exited terminal's tab edge consistent with its revision (manaflow-ai#15205)
fc882fe Cloud: rebake the devbox ladder with cmux-tui 3412812 (manaflow-ai#15323)
8e6357b Add pr-media.py for putting a clip or screenshot on a PR (manaflow-ai#15295)
1755ea8 ci: place release-build and main's side lanes on the owned minis (manaflow-ai#14797)
447eb04 Keep focused-pane notifications silent unless opted in (manaflow-ai#15233)
f66d18a Dial every discovered Mac concurrently on iOS (manaflow-ai#15127)
dc5a21a ci: place the Iroh release gate's Tailscale job on the owned minis (manaflow-ai#15139)
3887653 docs: hide the Cloud beta note on nightly docs (manaflow-ai#15317)
f4115d7 Center cloud row icon glyphs by their visible pixels (manaflow-ai#15149)
8714160 Let dogfood tours hold modifiers while clicking (manaflow-ai#15239)

# Conflicts:
#	.github/workflows/ci-guards.yml
#	.github/workflows/ci-macos.yml
#	.github/workflows/ci-owned-pool-rescue.yml
#	.github/workflows/ci.yml
#	.github/workflows/iroh-release-gate.yml
#	.github/workflows/remote-daemon.yml
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