feat(record): video recording for twd-cli runs - #13
Merged
Conversation
Adds the design for `npx twd-cli run --record`, producing a video of a scoped run via page.screencast(). Entirely within twd-cli, no twd-js changes. Covers the ffmpeg prerequisite and fast-fail probe, recorder teardown ordering against browser.close(), sidebar framing (including the !important html margin reset that beats useLayout's inline style), a recording-only viewport, the nested `record` config block and the shallow-merge trap in loadConfig, content-based artifact naming, and the one-clip-per-run constraint forced by the single page.evaluate() chunk. Notes explicitly that a recorded run is a demo artifact, not a substitute for a CI run, since the viewport change can alter outcomes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Six TDD tasks: record config defaults with nested merge, CLI flag parsing, artifact filename resolution, the recorder module (ffmpeg probe, framing CSS, screencast wrapper), lifecycle wiring with teardown, and docs. Also corrects two things in the spec that only surfaced while working out the implementation: - Recording must start after filter resolution, not before handler enumeration. screencast() needs the output path up front and the filename depends on the resolved test count. Side benefit: the clip starts at the first test, and the no-match early return can no longer leave a recorder running. - The config merge needs two levels, not one, since record.viewport is itself nested. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…le nits Corrects the fallback video filename claim (run.<ext> per the configured format, not unconditionally run.mp4), aligns the --record usage line with its siblings, and matches the ffmpeg Requirements bullet's plain-text style.
Three defects found in the final whole-branch review, all verified empirically against real ffmpeg and Chrome before fixing. startRecording built its screencast options without ffmpegPath, so the value reached only the pre-flight probe. A user who set record.ffmpegPath because ffmpeg is not on PATH got a passing probe followed by Puppeteer's raw "spawnSync ffmpeg ENOENT" after launch and navigation, which is exactly the late failure the probe exists to prevent. Puppeteer's ScreencastOptions does support ffmpegPath and defaults it to a bare 'ffmpeg' when absent. "Recorded N test(s) to <path>" was printed over a file that is routinely 0 bytes. Puppeteer's frame pipeline uses bufferCount(2, 1), so nothing is written until a second CDP screencast frame arrives, and Chrome only emits frames on a compositor update. A 1s recording of a static page produced 0 bytes with stop() resolving cleanly. An assertion-only suite, or an empty run, got a confident success line next to an unplayable file. The success line is now gated on the file actually having bytes, which also covers a stop() that rejected. record.viewport.deviceScaleFactor defaulted to 2 for a crispness benefit that does not exist: Puppeteer's #getNativePixelDimensions() forces the factor to 0 before measuring, so the recorder is always built from CSS-pixel dimensions. Recording at 2 and at 1 produced byte-identical files. The factor is live on the page during the run, though, so it changed the environment under test (srcset picks 2x assets, dpr-branching code takes another path) for nothing. Default is now 1. Closes the test blind spots that let these ship: - The probe call was never asserted; deleting it left all 278 tests green. Now covered for enabled, disabled, and fail-before-launch. - The teardown ordering test's stop mock was synchronous, so dropping the await still passed while causing the un-awaited ffmpeg finalize that truncates output. The mock now resolves on a later tick. - The stop-at-most-once invariant was untested. A new test throws in the window between the success-path stop and browser.close() and asserts stop ran once.
The README and the design spec both claimed video dimensions are viewport times deviceScaleFactor, "so the scale factor is what makes the clip crisp rather than soft". Measured false: Puppeteer forces the factor to 0 before measuring, and recording at 2 and at 1 produced byte-identical files. Both now say what is actually true, that the viewport sets the video dimensions while the scale factor only changes the page environment under test. The spec also notes that Puppeteer's real output-size knob is `scale`, which this feature does not expose, as a possible follow-up. Document the empty-artifact warning in the README and the spec's Output section, since it changes what a recorded run prints. CLAUDE.md had drifted: its config key list omitted `record`, its runTests() step list omitted the whole recording lifecycle, and its "two core source files" line stopped being true several files ago. Adds src/parseArgs.js and src/recorder.js, expands the orchestrator steps from 9 to 15, and adds a recording gotchas section for the four invariants that are easy to undo by accident.
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
|
Member
Author
|
Bumped to
To publish: cut a GitHub Release from this branch tagged Then, in a product repo: npm i -D twd-cli@beta
npx twd-cli run --record --test "<a real test name>"
open twd-artifacts/*.mp4That also covers the manual verification the test suite cannot do: that the file actually plays, the sidebar is gone, and there is no leftover gutter where it used to be. Note ffmpeg has to be on |
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
|
The last thing a test did never made it into the video. Chrome only emits a screencast frame when the page repaints, and Puppeteer's pipeline holds each frame until the next one arrives, since the next timestamp is what says how long to show the current one. The newest frame is therefore never written and stop() pads the tail by repeating the one before it. Waiting does not fix it, because a settled page produces no more repaints. Measured against real Chrome: stopping immediately ended two states early, a 400ms plain wait still ended one state early, and an 800ms wait with a 1px animated probe produced no new frames at all, because a 1px element gets its own composited layer and changes no visible surface. holdFinalFrame toggles a viewport-sized overlay between two near-identical alpha values, which repaints the whole surface and forces real frames while staying invisible in the output. That flushes the true final state through and then holds it. Adds record.postRoll (default 500ms, the fix) and record.preRoll (default 0, cosmetic). Both run only when recording is enabled, so normal runs are unaffected.
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
|
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
|
The release is handled separately, after --record-pace and other work land, so this branch should not carry a version. Back to 1.3.1, matching main. 1.4.0-beta.1 stays published on npm under the beta dist-tag; only the in-repo version is reverted.
Member
Author
|
Dropped the version bump from this branch ( The release happens later, once Two things unchanged by this:
|
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
1 participant
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.
Adds an opt-in
--recordflag that captures a video of a test run via Puppeteer'spage.screencast().npx twd-cli run --record --test "checkout flow"The goal is a shareable clip of a single flow: a PR attachment, a docs clip, a demo of your own app. Recording is always explicitly requested, so the normal CI path is untouched.
Entirely within
twd-cli. No twd-js changes.What's in it
src/config.jsrecordblock with a two-level merge, so a partialrecordorrecord.viewportno longer wipes the other defaultssrc/parseArgs.js--record,--record-dir <path>,--record-speed <n>, both space and=formssrc/recordFilename.jssrc/recorder.jssrc/index.js.gitignoreTwo design points worth knowing while reviewing
Recording starts after filter resolution, not before enumeration.
screencast()needs the output path up front, and the filename depends on how many tests will run, which is only known oncebaseIdsexists. Two things fall out of that: the clip begins at the first test rather than at page load, and the "no tests matched" early return is structurally incapable of leaking a recorder.The framing CSS needs
!importanton the html margins, not justdisplay: noneon the sidebar. twd-js'suseLayoutwritesmarginLeft: 280pxas an inline style, so hiding the sidebar alone would leave a 280px gutter in frame.Recording is not deterministic
A recorded run sets its own viewport (1280x720 versus the implicit 800x600 a normal run gets) and reflows the app to full width, so it can pass or fail differently. The README says this plainly. A recorded run is a demo artifact, not a substitute for a CI run.
Prerequisite
ffmpeg. Puppeteer's screencast spawns it. There is a pre-flight probe that fails fast with install instructions rather than letting Puppeteer fail late, after launch and navigation.
Testing
287 passing, up from 246 on main. The 26 pre-existing
runTeststests are unchanged, which is the evidence that non-recording runs behave exactly as before.Three bugs were caught late by testing against real ffmpeg and Chrome rather than mocks, and all three are fixed here. Recording them because they show where this feature's risk actually lives:
record.ffmpegPathwas documented but never reachedpage.screencast(). It was consumed only by the probe, so setting it made the probe pass and then hit Puppeteer's rawENOENTafter launch. The existing test pinned the bug with an exact-match assertion.Recorded N test(s)printed over a 0-byte file. Puppeteer buffers withbufferCount(2, 1)and CDP emits frames only on compositor updates, so a page that never repaints writes nothing whilestop()resolves cleanly. There is now a byte check on the artifact.deviceScaleFactornever reaches the encoder, because Puppeteer forces it to 0 before measuring. Recording at 2 and at 1 produced byte-identical files. The default is now 1, since the factor is still live on the page and changes the environment under test for zero video benefit.Known follow-ups, deliberately not in this PR
recordedFileSizemaps everystatSyncerror to 0, so a permissions error would report as an empty recording. Should narrow toENOENT.=== 0, so a header-only unplayable container would still report success.stop()path has no test, and a rejected stop leaving a partial file prints the success line beneath the finalize warning.explainErrorreturns null for it. Fix belongs insrc/diagnostics.js.stop(); a wedged ffmpeg would hang the CLI beforebrowser.close().Not covered by any test
Nothing automated proves a real
.mp4plays, that the sidebar is actually absent, or that there is no gutter. Worth one manual run against a real app before merge:Docs on the branch
docs/superpowers/specs/2026-07-27-cli-video-recording-design.mdis the design.docs/superpowers/plans/2026-07-27-cli-video-recording.mdis the execution plan and is a frozen record: it still carries the disprovendeviceScaleFactorclaim that the spec and README were corrected for.