feat(actions): add a record composite action - #28
Merged
Merged
Conversation
Every consumer that wants clips in CI re-implements the same recipe. twd-vue-example's record.yml is 13 steps, five of which plus two files exist only to work around twd-cli defects. Those defects are now fixed, which is what makes this action worth building: packaging the workarounds would have distributed them as public input surface that then has to be deprecated rather than deleted. The preconditions the spec set are all met — a destroyed ffmpeg stream aborts the run with its stderr surfaced (#24), the output is playable outside Chrome (#24), the preflight checks capability rather than existence (#24), and --changed-since exists (#25). So the action is the four steps it was supposed to be: - uses: BRIKEV/twd-cli/.github/actions/record@v1 with: changed-since: ${{ github.event.pull_request.base.sha }} Notes on the two decisions that are not obvious from the spec: It installs ffmpeg 8.x from BtbN's n8.1 build, not the distro package and not "the obvious static build". Puppeteer passes -movflags hybrid_fragmented, which arrived after ffmpeg 7: 6.1.1 (ubuntu-24.04) no, 7.0.2 (johnvansickle release) no, 8.1.2 yes. The gpl variant also carries libx264 for the H.264 conversion, so one download covers both requirements. The URL is rolling and unchecksummable, which is acceptable because the CLI now probes the binary's real capability before launching a browser — an unusable build fails fast and says why. The action deliberately does not verify that capability itself. That belongs in the CLI preflight so every user gets it, not only Actions users. clip-count 0 is a success. A branch that changed no tests has nothing to record, so the upload step is skipped at zero rather than running with if-no-files-found: error and failing the job on a normal outcome. Workflow policy — the trigger, the PR comment, the dev server, timeout-minutes and continue-on-error — stays with the caller, exactly as it does for the `run` action. The reference workflow in the README shows all of it. Verified by extracting each step body from the YAML and running it against fixtures: argument construction (titles with spaces, quotes and apostrophes each arrive as one argument; a shell-injection attempt arrives as literal text and does not execute; CRLF input is stripped; --record-pace 0 survives), clip counting (nested and non-video files excluded, missing and unresolved directories give 0), directory resolution (absent, partial and unparseable configs all fall back), and input validation. Not verified: the assembled action has never run on a runner. test-example-app does not depend on twd-cli — the e2e workflow calls node ../bin/twd-cli.js — so an e2e job here would exercise the published CLI rather than this branch. The real proof is rewriting twd-vue-example's record.yml onto the action, which is a separate repo and a follow-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
TWD Contract Validation
23 passed · 41 failed · 3 warnings · 1 skipped Failed validations./contracts/users-3.0.json
./contracts/posts-3.1.json
./contracts/products-3.0.json
./contracts/events-3.1.json
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements
docs/spec-record-action.md.The preconditions are met
The spec said do not build this first — packaging the workarounds would distribute them as public input surface that has to be deprecated rather than deleted. All four are now done:
--changed-since <ref>exists — feat(run): add --changed-since <ref> to run only the tests a branch touched #25So this is the four-step action it was meant to be, not the ten-step one.
Two decisions worth reviewing
It installs ffmpeg 8.x from BtbN's
n8.1build. Not apt, and not "the obvious static build" — my #24 measurements killed that assumption. Puppeteer passes-movflags hybrid_fragmented, which arrived after ffmpeg 7:The
gplvariant also carrieslibx264for the H.264 conversion, so one download covers both requirements. The URL is rolling and therefore unchecksummable — acceptable here because the CLI now probes the binary's real capability before launching a browser, so a bad build fails fast with an actionable message instead of producing a broken clip. Linux-only; other runners get a warning and skip.The action deliberately does not verify that capability itself — that stays in the CLI preflight so every user gets it, not only Actions users.
clip-count: 0is a success. A branch that changed no tests has nothing to record, so the upload step is skipped at zero rather than running withif-no-files-found: errorand failing the job on a normal outcome.Verification
There is nothing here for vitest — it's YAML. So I extracted each step body from the file itself and ran it against fixtures:
"quotes"and an apostrophe$(touch /tmp/pwned); rm -rf /testsinput\rstrippedpace: 0tests.txt+ a nested mp4.webmand.gifrecord.dir/ unparseable./twd-artifactstests+changed-sincetogether::error::and exit 1actionlintcan only parse workflow files, not composite actions, so it gave no signal here — theif:expressions mirror the shapes already proven in.github/actions/run.What is not verified, and what I did not do
The assembled action has never run on a runner. I considered adding an
e2e-recordjob, buttest-example-appdoes not depend ontwd-cli— the e2e workflow callsnode ../bin/twd-cli.js— so the action'snpx twd-cliwould exercise the published CLI rather than this branch. Making that meaningful needs npm-link plumbing, and putting a real recording on every PR adds cost and a flake source to this repo's CI.The honest end-to-end proof is the spec's last acceptance criterion: rewriting
twd-vue-example'srecord.ymlonto the action so it loses the five workaround steps and both scripts. That is a different repo and needs a merged ref to point at, so it is the natural follow-up — happy to do it next.Also
timeout-minutes,continue-on-error— stays with the caller, as it does forrun. The reference workflow in the README carries all of it, including the timeout andcontinue-on-erroryour notes asked to keep as belt.clip-countwill be 0 or 1 until one-clip-per-test lands; it counts files rather than assuming, so it becomes correct for free when that ships.CLAUDE.mdgains a section on the conventions both actions share.561 tests passing(unchanged — nosrc/code in this PR).🤖 Generated with Claude Code