Skip to content

feat: recipes — saved, shareable session scripts - #14

Merged
sanketsudake merged 30 commits into
mainfrom
feat/recipes-recording
Jul 27, 2026
Merged

feat: recipes — saved, shareable session scripts#14
sanketsudake merged 30 commits into
mainfrom
feat/recipes-recording

Conversation

@sanketsudake

Copy link
Copy Markdown
Owner

Implements RFC-0009 (recipes). Stacked on #13#12#11#10.

RFC-0011 (session recording) is being added to this same branch and will appear here shortly.

What this is for

session already does the hard part — many commands over one held connection, with refs that stay valid across the batch. What it lacked was a way to keep a batch.

Today, a user who works out the exact eleven commands that submit their timesheet has that knowledge in shell history, or a bespoke wrapper, or an agent's context that's gone tomorrow. There's no artifact — so nothing accumulates, nothing is reviewable, and nothing is shareable. The most valuable thing a user of this tool produces is a working automation for a specific internal app, and there was no unit in which to hand that to a colleague.

name: submit-timesheet
description: Fill and submit the weekly timesheet.
inputs:
  week:  { required: true, description: "Monday of the week, YYYY-MM-DD" }
  hours: { default: "8" }
target: url:workday
steps:
  - label: open the timesheet
    run: ["nav", "https://workday.internal/time/{{week}}"]
  - run: ["wait", "--idle"]
  - label: save
    run: ["click", "--by", "name", "Save and Close", "--role", "button", "--wait-text", "saved"]
chrome-cdp recipe run submit-timesheet --set week=2026-07-20

It is deliberately not a language

The design constraint is that a recipe is session with a header. run is an argv array — identical to a session stdin line, so anything valid in one is valid in the other. on_error is abort or continue and nothing else: no retries, no conditionals, no loops. If a recipe needs control flow it should be a program that calls session, and the docs say so.

VS-10 is the structural guard for that claim, and it's implemented: dry-run output piped into session executes identically to recipe run, asserted by reflect.DeepEqual on the recorded browser call sequences. It holds because the runner's only execution primitive is the same Execute(argv...) that session uses per line.

For the pipe to be real, human-mode --dry-run puts only argv lines on stdout (the note goes to stderr).

There is no shell, and there must never be one

A recipe is a file that gets shared and run against your authenticated browser. A shell: step type would make every shared recipe a code-execution vector, so substitution goes into an argv element and the schema is decoded with KnownFields(true) — an unknown key, including shell:, is exit 2.

VS-15 pins it with 11 hostile values (; rm -rf /, backticks, $(...), newlines, quotes), asserted byte-for-byte at the browser boundary.

Validation is a separate pass from execution

Parse → validate → resolve inputs → check every placeholder → then connect. Everything a recipe can get wrong statically is exit 2 with Chrome never contacted, proven with the noCall(t) helper across 12 cases.

An unknown --set key is rejected, not ignored — silently dropping a typo would let a run proceed with a default the user didn't intend.

Rules added beyond the RFC text, each closing a hole

  • name must equal the filename stem — otherwise recipe list advertises one name and recipe run uses another.
  • session is a reserved step verb — a step invoking it would read the process's stdin.
  • argv[0] must be a literal — without this, run: ["{{cmd}}"] plus --set cmd=recipe smuggles recursion past the static recursion check.
  • run elements must carry the !!str tag, so ["--nth", 2] says "quote it" rather than silently coercing.

Policy interaction (from #12)

recipe run is classified Exempt for the same reason session is: it touches no tab itself and re-enters the command tree per step, so each step is classified and checked on its own. Classifying the wrapper Mutating would refuse a read-only recipe on a read_only origin; Reading would be a lie.

That's a security claim, so it has a test: TestRecipeStepsAreCheckedIndividually runs a recipe whose steps target a refused origin against a browser double that fails the test if any action method is reached. If per-step enforcement regressed, a shared recipe would become a way to drive exactly the origins the policy was configured to refuse.

Testing

All 15 verification scenarios. The bulk is pure internal/recipe tests over t.TempDir() fixtures — no browser, -short clean. gofmt, go vet, go test -race ./... all green.

Ships a worked example and guide in docs/scenarios/.

Base automatically changed from feat/observability to main July 27, 2026 08:22
sanketsudake and others added 27 commits July 27, 2026 14:10
A recipe is a YAML file whose steps are argv arrays — exactly the lines
`session` reads on stdin — with declared inputs substituted into argv
elements. It is the unit in which a working automation becomes something
you can name, re-run, review, and commit for a team.

The design constraint is that recipes are a thin layer over `session`,
not a new language:

- `run` is an argv array; `recipe run` feeds the resolved lines through
  the same command tree `session` re-enters, so there is one execution
  path. TestRecipeRunEqualsDryRunThroughSession asserts it structurally:
  `recipe run` and `recipe run --dry-run | session` drive the same
  recording stub with an identical call sequence.
- `{{name}}` substitutes into ONE argv element. There is no shell here
  and no `shell:` step type — a shared recipe drives an authenticated
  browser, so a shell escape hatch would make every one a code-execution
  vector. Substitution is a single pass, so a value is never re-expanded.
- `on_error` is abort (default) or continue; no retries, conditionals, or
  loops. Recipes cannot invoke recipes, and 200 steps is the cap.

Validation is a separate pass from execution: parse → validate schema →
resolve inputs → check every placeholder → then connect. Everything a
recipe can get wrong statically is exit 2 with Chrome never contacted,
proved against the noCall stub. An unknown --set key is rejected rather
than ignored — dropping `--set hurs=9` would run with the default the
user was trying to override.

`recipe run` emits one NDJSON envelope per step (with `step` and `label`
so a caller correlates without counting lines) and a summary; on failure
the summary carries failed:{index,label,code} and the process exits with
the failing step's code. --quiet emits the summary alone.

Names resolve ./.chrome-cdp/recipes → $XDG_CONFIG_HOME/chrome-cdp/recipes
→ --dir, first match winning, and `recipe list` marks each source: a
recipe committed to a repo beats a teammate's personal copy, which is
what makes sharing work.

Adds gopkg.in/yaml.v3.
recipe run is Exempt for the same reason session is: it touches no tab
itself, it re-enters the command tree per step, and each step is
classified and checked on its own. Classifying the wrapper as Mutating
would refuse a recipe made entirely of reads on a read_only origin;
Reading would be a lie.

That is a security claim, so it gets a test. A recipe is a file someone
else wrote running against your authenticated browser — if per-step
enforcement regressed, a shared recipe would become a way to drive
exactly the origins the policy was configured to refuse.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RFC-0011 splits encoding out of the driver so the parts that must be
exactly right — the shared palette, the frame delays, the annotation
compositing, the --max-size reduction — are testable without a browser,
where frame timing is inherently variable.

The two load-bearing properties, both covered:

  - annotation composites at EXPORT, never at capture, so without
    --annotate the exported frames are pixel-identical to what Chrome
    produced (VS-13, and with it the README-asset use case);
  - GIF goes through the standard library alone, because a single static
    binary must be able to produce its default format. mp4/webm need
    ffmpeg, and its absence is a named error rather than a silent
    fallback (VS-10).

--max-size terminates on three independent bounds: each step strictly
shrinks the plan, the plan is refused below a minimum canvas and two
frames, and the iteration count is capped. A ceiling that cannot be met
is reported as unmet rather than quietly missed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The driver half of RFC-0011: RecordStart/Stop/Status/Cancel on the
Browser seam, a stub default, both halves of the daemon RPC, and the
record_buffer / record_max_bytes config keys.

Frames live on the object that holds the connection — the daemon —
because a recording spans many CLI invocations, and because the process
that dies with a failed automation was then never the one holding them
(US-7).

Capture is Page.startScreencast rather than a screenshot loop: Chrome
pushes a frame only when the page changes. That comes with one hard
obligation — every frame must be acknowledged or the stream stops after
the first — and the ack is a CDP call, which cannot run on the event
loop. Frames are therefore handed to a pump goroutine that acks first
and stores second; a handoff overflow drops the payload but never the
acknowledgement.

Three bounds, and every one of them reports what it did: a ring of
--max-frames, a byte ceiling (a frame count alone does not bound memory
when the viewport can be 4K), and --max-duration, which stops the
capture without ending the recording. Eviction increments
dropped_frames and sets truncated with a reason, so a partial recording
can never be presented as a complete one (US-6). The cadence throttle
deliberately does NOT count as loss — the caller asked for 4fps.

Marks come from the pointer verbs' existing centre resolution and are
recorded unconditionally, so annotation stays a decision made at export.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The command surface for RFC-0011, plus the export path through
internal/encode.

Every flag is validated before a target is resolved, and one of those
checks is load-bearing rather than tidy: whether this machine can
produce the requested format is answered BEFORE the recording is
drained. `record stop --format mp4` with no ffmpeg is exit 2 with the
frames still in the daemon, so the user exports them as a GIF instead of
losing them to a missing dependency (VS-10).

The two lifecycle mistakes — already recording, nothing to stop — are
`usage`, not target errors: both are statements about the caller's
sequence of commands, and an agent has to tell "fix your script" from
"retry".

A recording whose tab has closed is still exported, which is the sharp
end of US-7: the frames were never held by the tab. That path takes the
same no-origin policy check `raw --browser` does, so the escape hatch is
not also a way around the boundary, and it refuses an ephemeral target
spec rather than re-interpreting it against a changed tab list.

`session --record` brackets a whole batch, starting as soon as a line
resolves a tab (the first is usually `use`) and stopping after the last
line — so a batch that failed half way still has the failure on film,
reported as one extra NDJSON line.

The record verbs are classified Reading: a recording is a stream of the
page's own pixels, so allow/deny covers it, while read_only does not —
nothing here modifies the page.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Documents `record` where it is read, including the part the feature owes
the user: this records their real, logged-in browser, and a recording
attached to a public issue may carry their own data.

*.gif, *.mp4 and *.webm join *.png and *.pdf in .gitignore — a recording
test that left files in the repo would be an easy and annoying
regression.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…answers with

Found by the live test, and it would have made the feature useless in
normal use rather than only in CI: a tab that is not the frontmost one
produces no compositor frames, so a screencast of it goes silent even
while the page is animating. Measured headless: three frames at startup,
then nothing for twenty seconds. Most tabs are backgrounded for a tool
that drives the user's real browser.

The recorder now nudges the page the same way capture.go's pokeFrame
already nudges it for element geometry — a 1x1 screenshot — but only
when no frame has arrived for a whole cadence gap, so an actively
rendering page is never poked. A page that genuinely did not change
answers the nudge with an identical frame, which store() drops as a
duplicate: nudging does not cost a static page a ring full of identical
frames, and it is not counted as loss because nothing was lost.

The live test drives the page's changes itself instead of trusting the
fixture's setInterval, which the same power management throttles, and
asserts --scale against a scale-1 capture of the same page rather than
against an emulated viewport the headless surface does not match.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
VS-14 at the CLI boundary: a command failing mid-recording leaves the
recording alone, and `record stop` afterwards still exports — the case
the feature is most useful for, and the one a design holding frames in
the commanding process could not serve.

The daemon tests cover what no stub-backed test can see: the frames
really cross the socket (base64 inside an array of objects marshals
fine and arrives empty when the forwarder is wrong), and RecordOpts
survives its arg decoder field for field.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bringing a window to the front and reading its layout are visible to the
user, and an invocation that was always going to be refused should cost
them nothing. The authoritative check stays the one under the lock.

The start result now reports max_width/max_height rather than
width/height: they are the caps requested of Chrome, and a screencast
frame is never upscaled to fill them, so the real dimensions are the
per-frame ones reported at `record stop`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
image/gif's LoopCount is not a play count: the stdlib plays LoopCount+1
times, and a negative LoopCount writes no NETSCAPE block at all, which
every viewer renders as exactly one play. Assigning Options.Loop straight
through gave `--loop 3` a GIF that plays four times.

The old round-trip assertion could not see it — both ends agreed on the
same wrong number — so the regression test decodes the artifact and
asserts the play count the flag promises, including the one-play case.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The canvas is derived from the first frame and every later frame was
forced onto it, so a window resized mid-recording silently distorted the
rest of the export — a recording of a page at an aspect ratio it never
had. Frames are now scaled to fit and padded.

Marks are in page coordinates, so the marker mapping moves onto the
content rectangle rather than the canvas; a frame whose shape already
matches fits exactly, pads nothing, and stays pixel-identical (VS-13).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The stride kept every n'th frame and let the dropped frames take their
Marks with them, while Annotated was set from the flag regardless — so
`record start --annotate` plus `record stop --annotate --max-size 500KB`
reported an annotated GIF with no markers anywhere on it.

Marks now move to the nearest kept frame, and Annotated reports whether a
marker actually put pixels on the canvas. That also covers a mark
resolving outside the frame, which drew nothing and claimed otherwise.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…tself

The reserved-verb check read argv[0] and assumed it was the verb. Cobra
does not: it strips leading flags before resolving the command, and
nothing set DisableFlagParsing. So a step could name one command to the
validator and another to the runner.

  run: ["--json", "recipe", "run", "selfloop"]

loaded clean and recursed runPlan -> execStep -> Execute -> cmdRecipeRun
-> runPlan without bound: 8.4 GB of RSS in 8 seconds, no Chrome contact,
MaxSteps never consulted. And

  run: ["--quiet", "session"]

consumed the process's own stdin, executing commands the recipe never
contained and reporting ok: true.

Both halves are fixed, because neither is sufficient alone:

  - Load refuses a step whose argv[0] starts with "-". A step must name
    its command literally, which is the same rule that already blocks
    run: ["{{cmd}}"] -- the validator and the runner have to be looking
    at the same verb.
  - runPlan refuses to run a plan while another plan is running.
    Recursion through the exec path is a property of the runner; no
    validator reading one file can see it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two numbers in Result described the pre-ffmpeg canvas rather than the
file. yuv420p needs even dimensions, so -vf scale=trunc(iw/2)*2 turned a
101x61 canvas into a 100x60 file while the envelope kept saying 101x61 —
and --max-size lands on an odd canvas about half the time. -framerate is
floored at 1, so five frames five seconds apart were reported as
fps 0.25 / 20250ms for a file that is 1fps and 5.0s, which is every
recording containing a pause.

The clamp moves out of encodeVideo into a shared videoGeometry so the
file and the envelope are computed once, from the same values. Verified
against ffprobe where ffmpeg is installed, and as a table where it is not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…s too

Check() returned allowed("exempt: ") before ever consulting verbsDenied.
The recipe and session verbs are Exempt (they touch no tab themselves;
each step is checked on its own), so an operator who wrote

  verbs_denied = ["recipe run"]

got silence: the config validator accepted the name, and the checker
ignored it. Denying "run files other people wrote against my logged-in
browser" is the most obvious thing to want from that list, and it was
the one entry that did nothing.

verbs_denied names a verb rather than an origin, so there is no class it
should not reach. It now runs ahead of the Exempt short-circuit in the
checker, and the CLI's enforcement hook no longer short-circuits past a
verb the policy explicitly denies. `recipe run` consults the hook before
executing a plan; `recipe show` and `--dry-run` still work, which is how
you review an untrusted recipe.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
recipe.Resolve substituted inputs and handed the result to cobra, which
parses flags at any position. So an input whose VALUE looked like a flag
became one. With a recipe pinned to `target: url:payroll.corp` and a
step `run: ["text", "{{sel}}"]`:

  --set sel=--target=@2

resolved to ["text", "--target=@2"], which suppressed the recipe's own
target injection (hasTargetFlag scanned the argv AFTER substitution) and
read a different tab. Same exposure for html, snap, grid, raw, key and
upload. RFC-0009 promises an input substitutes into ONE argv element and
never into a command line; that held for word splitting but not for flag
parsing.

Both halves are now decided from the argv AS WRITTEN, the only text the
recipe's author wrote:

  - The step's data elements are emitted after a `--` terminator, so a
    substituted value is always one argv element of data. A step that
    wrote its own `--` gets exactly one, in the right place -- the
    injected `--target X` used to land after it and parse as two
    positionals.
  - The pinned target is injected unless the AUTHOR wrote a --target
    among the elements the command tree will parse as flags.

Classifying an argv needs flag arity, which lives in the command tree,
so the CLI supplies a Splitter built from cobra's own flag definitions
(on a scratch App, since newRoot binds flags to its receiver's fields).
Resolve without one keeps the argv in written order.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
RecordStop drains the buffer before anything encodes or writes, so every
failure after it lost the whole capture with no retry — the next `record
stop` answers "no recording is active on this tab". Three reachable
triggers, three fixes:

- decodeAll aborted on the first undecodable frame, and the capture path
  deliberately RETAINS frames whose header it could not read, so frame
  342 cost the other 599. Undecodable frames are now skipped and counted
  in Result.DecodeFailures / the envelope's decode_failures; only a
  recording with nothing decodable at all is still an error.
- `-o /nonexistent/dir/demo.gif`, an output path that is a directory, and
  an unwritable directory are now caught before RecordStop, the way the
  encoder probe already is (VS-10). Writability is probed by creating a
  file, since the mode bits do not answer the question everywhere.
- Anything left — a full disk, an ffmpeg that dies — hands the frames
  back through a new Browser.RecordRestore rather than dropping them, and
  the error message says the retry is available.

RecordRestore was chosen over a non-destructive RecordStop because
retaining frames past a successful stop would make `record status` report
a recording that is over, contradicting VS-2. The re-seat only pays its
RPC cost on the failure path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… argv

checkPlaceholders only walked step argv. A recipe with

  target: "url:{{env}}.corp.test"

loaded clean, emitted the placeholder verbatim as each step's --target,
and failed at runtime with target_not_found -- after connecting. An
UNDECLARED placeholder there escaped load-time validation entirely,
which is exactly the ordering this package exists to guarantee: parse,
validate, resolve, check every placeholder, and only then connect.

`target:` is now placeholder-checked at load and substituted at resolve,
like any other argv element. A --target given on the command line is
already a value and is used as given.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`chrome-cdp --json record stop -o - > demo.gif` wrote the GIF and then an
envelope onto the same stream, producing a GIF with a JSON line stuck on
the end. screenshot and pdf already answer this by emitting no envelope
for `-o -`; record now agrees with them.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…error

runPlan never set inSession, so `console --follow` (and `net --follow`)
was accepted inside a step. It then blocked for the whole --timeout,
buffered every streamed envelope into execStep's in-memory buffer,
failed parseStepEnvelope on dec.More(), and dumped the raw stream with
no step or label on it.

A recipe run makes the same one-envelope-per-line promise `session`
does, so it now marks itself as a batch for the duration of the run and
restores the previous value afterwards (a recipe can be a `session`
line). The two --follow messages name a recipe alongside `session`,
since either is now the reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
within_max_size was only emitted inside `if res.Reduced`, but the
reduction ladder refuses at step 0 for a small canvas with few frames —
so the case where the ceiling was missed by the widest margin was
precisely the one that said nothing about it. It is now gated on
--max-size having been asked for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…bering

yaml.v3 drops a null sequence element into []fileStep without a word, so

  steps:
    -
    #  run: ["nav", "https://app.internal/"]
    - run: ["fill", "#a", "1"]

loaded as a two-step recipe. The author reads three. `--from-step 2`
then runs what they read as step 3 -- and --from-step is documented as
sharp, assuming every earlier step's effect is already done, so the step
it skipped is typically the navigation that put the page where the rest
expects it. `failed.index` was off by the same amount. `- {}` is NOT
dropped, so the behaviour was not even consistent.

An empty entry is now refused by the number the author reads. The raw
sequence is re-read rather than decoding `steps` as a yaml.Node, because
Node.Decode does not honour KnownFields and an unknown key inside a step
would stop being an error.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
exportRecording took no context, so the ladder — up to nine full
re-encodes, each resizing every pixel and rebuilding the palette from
every pixel of every frame — could run for minutes past --timeout with no
output at all.

Encode now takes a context. When it expires the last COMPLETE attempt is
returned with max_size_timed_out in the envelope (a finished larger
artifact beats nothing, and it tells the user a longer --timeout is the
next move); an expiry before any attempt finished is an error, and the
frames are re-seated by the caller either way.

The palette histogram also stops inserting one map entry per pixel above
a budget, sampling instead — with a full confirming pass when the sample
says the frame set is flat, so the exact-palette regime that VS-16 rests
on cannot be broken by a missed colour.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…code matches the exit

Two failures of the same path, where a step's stdout is not exactly one
envelope.

`screenshot -o -` (and `pdf -o -`) writes the file to stdout and emits
no envelope at all. Those bytes were passed through into the middle of
the recipe's NDJSON -- the PNG header, then the summary glued onto the
end of an IDAT chunk -- so every reader downstream lost the rest of the
run. And because the step "succeeded", it was counted as completed.
A step whose output is not one envelope now fails with a message saying
so, and the stream stays parseable.

Separately, envCode fell back to `generic` whenever no envelope could be
read, while failExit carried the step's real exit: the summary could say
`generic` (exit 1) while the process exited 4. The exit is now derived
from the code the summary reports, so the envelope -- which is the
public API -- decides both. Where only an exit is known, codeForExit
names a code that maps back to it, pinned by a round-trip test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
runPlan folded the run's --timeout, --port, --no-daemon, --profile-dir,
--no-launch and --json into a.defaults so they would survive into each
step's Execute, and never put them back. A `recipe run` line inside a
`session` therefore handed its connection flags and its NDJSON mode to
every later line in that session -- the exact "do not cache flag-derived
state on App across invocations" the RFC guidance calls out.

They are now saved and restored alongside inSession and inRecipe.

--timeout stays per STEP, and is now documented as such rather than left
to be discovered: each step is one command and gets the whole budget,
the same way each line of a `session` does. A whole-run budget would
make `recipe run` and `recipe run --dry-run | session` behave
differently, and that equivalence is the structural guard on "a recipe
is a `session` script with a header". A slow step can carry its own
--timeout in its run array.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- A `record start` that failed after Chrome had already enabled the
  screencast only forgot the recorder, leaving the tab pushing frames
  nobody acknowledges for the life of the connection. It now stops the
  capture too (best-effort: if the enable is what failed, there is
  nothing to turn off).
- store() advanced the 1/fps cadence clock before deciding to keep the
  frame, so a byte-identical frame — exactly what a static page answers
  the stall nudge with — spent the budget and throttled away the genuine
  change arriving in the next gap. The clock now advances on retention.
- Nothing removed a recording when its tab closed, so an abandoned one
  held up to record_max_bytes (96MB) for the daemon's life. The frames
  still outlive the tab (US-7), now for a ten-minute grace period.
- `--format frames` into a populated directory left the previous, longer
  export's PNGs behind, so the directory held more frames than the
  envelope reported. Only the names this command writes are removed, and
  only if they are regular files.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…splitting

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e-free

The step splitter closed over one cobra tree. cobra's Command.Commands()
sorts its child slice IN PLACE, so any two concurrent callers of the
splitter race on that sort — and a Splitter is a plain func value with
nothing stopping two callers holding it at once.

It surfaced as a flaky -race failure that only appeared once both halves
of this branch were in the tree together: two in three runs, and never
when either package was tested alone, which is exactly the shape that
gets dismissed as noise.

Build the tree per call instead. It costs microseconds against a step
that is about to drive a browser, and it removes the shared mutable
state rather than guarding it.

TestSplitStepArgv had the same bug in its own harness — one root shared
across parallel subtests — so it is now a tree per subtest, with the
reason recorded next to it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sanketsudake
sanketsudake force-pushed the feat/recipes-recording branch from b9f1926 to 731df26 Compare July 27, 2026 09:23
sanketsudake and others added 2 commits July 27, 2026 15:19
macOS CI failed the --scale assertion: a 0.5 recording of a 756x413
surface came back a square 207x207. Reproduced locally at about one run
in three once I looked for it.

startScreencast takes a maximum size in PIXELS, so RecordStart derives
the box from the viewport — and it was reading it once, immediately.
A recording started right after a nav, or right after a viewport change,
can therefore size itself from a viewport the page has not laid out yet,
and every frame of the whole recording then has the wrong aspect. That is
the same class as the element-capture stale-rect bug: geometry read
before the page settled.

RecordStart now polls until three consecutive reads agree, poking a frame
each time for the same reason element capture does — a backgrounded tab
runs no rendering steps on its own, so waiting alone would never converge
there. Three rather than two because a viewport mid-resize was observed
holding a transient value across a single 60ms gap.

The test was also comparing two captures taken seconds apart, so a
headless window that resized between them (observed flipping 413x413 to
756x413) failed an assertion that is supposed to be about --scale. It now
pins the viewport, waits for the override to actually take effect —
setDeviceMetricsOverride returns before the visual viewport reports the
new size — and takes both references back to back.

Verified: 8 consecutive TestRecordLive runs green, where the original
failed 1 in 3.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI failed again, differently: the SCALED capture was correct at 400x300
while the unscaled reference read 600x600.

The wait polled innerWidth/innerHeight — the layout viewport — but the
recorder sizes the screencast from the VISUAL viewport that
page.getLayoutMetrics reports. Those settle independently after a
setDeviceMetricsOverride, so waiting on one left the other mid-resize.
The test now requires both to report the pinned size.

Also asserts the precondition before the claim: if the scale-1 reference
is not the pinned 800x600, the window moved under the test and the run
says nothing about --scale. A future failure now names that rather than
being read as a --scale defect, which is how the last two rounds were
misdiagnosed at first glance.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@sanketsudake
sanketsudake merged commit 3cd4c2e into main Jul 27, 2026
4 checks passed
@sanketsudake
sanketsudake deleted the feat/recipes-recording branch July 27, 2026 10:31
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