Repository navigation
feat(workflows): VHS-dialect tape interpretation for type: cast steps - #2889
Open
Erik Osterman (Cloud Posse) (osterman) wants to merge 18 commits into
Open
Erik Osterman (Cloud Posse) (osterman) wants to merge 18 commits into
Erik Osterman (Cloud Posse) (osterman) wants to merge 18 commits into
Conversation
A type: cast workflow/custom-command step can now interpret VHS-dialect .tape scripts directly, in memory, via tape: (inline) or tape_file: (path) -- no vhs binary required, nothing written to disk. Directives translate into the step's existing mode: steps (real, exit-code-tracked children) or mode: session (PTY-driven keypress replay) machinery, with Hide/Show, Screenshot, Source, and Require all supported. A new `atmos cast record <input.tape> --output=<path>` CLI subcommand records a tape ad hoc with no workflow YAML at all. Adds the distributed atmos-vhs skill covering tape interpretation and hand-migration to native steps:. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
August 6, 2026 13:52
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Erik Osterman (Cloud Posse) (osterman)
added
the
minor
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds VHS ChangesVHS cast workflow
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CastStep
participant TapeParser
participant TapeExpander
participant SessionOrSteps
participant Recorder
participant ScreenshotRenderer
CastStep->>TapeParser: parse inline or external tape
TapeParser->>TapeExpander: provide directives
TapeExpander->>SessionOrSteps: create session actions or child steps
SessionOrSteps->>Recorder: record output and markers
Recorder->>ScreenshotRenderer: render marker timestamps
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
Contributor
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (11)
pkg/runner/step/cast_tape.go (1)
195-205: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueA malformed
Set Width/Set Heightvalue is dropped without a signal.
applyTapeSetIntignores an unparsable value. Every other unsupportedSetkey logs a warning throughlogUnsupportedTapeSet. A warning here keeps the behavior consistent and helps a tape author find the typo.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/runner/step/cast_tape.go` around lines 195 - 205, The applyTapeSetInt function currently silently ignores malformed integer values; update it to log a warning through logUnsupportedTapeSet when strconv.Atoi fails, while preserving the existing behavior for already-set values and valid integers.pkg/runner/step/cast_tape_test.go (2)
16-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
captureTapeLogOutputrestoresos.Stderrrather than the previous writer.If another test or the package init points the logger somewhere else, this cleanup overwrites that destination. Capturing and restoring the prior writer is safer, if the logger exposes a getter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/runner/step/cast_tape_test.go` around lines 16 - 25, Update captureTapeLogOutput to save the logger’s current output writer before redirecting it to buf, then restore that saved writer in t.Cleanup instead of unconditionally using os.Stderr. Preserve the existing capture and cleanup timing around fn.
239-250: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWrap the matrix in
t.Runsubtests.The nested loop over tapes and modes reports a single test name. A
t.Run(tape+"/"+mode, ...)subtest names the failing combination directly and matches the table-driven style used inTestExpandCastTapeStepsModeErrorsOnSessionOnlyDirectives.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/runner/step/cast_tape_test.go` around lines 239 - 250, Update TestExpandCastTapeCopyPasteEnvAlwaysError to wrap each tape/mode combination in a t.Run subtest named with the tape and mode, while keeping the existing step construction and error assertion inside the subtest.cmd/cast/record_test.go (2)
128-140: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTests mutate shared
recordCmdand global Viper state.Each test calls
resetRecordCommandandclearRecordViperOverridesfirst, so the current set passes. The safety depends on every future test in this file doing the same. Moving both helpers into a singlesetupRecordTest(t)helper makes the precondition hard to skip.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/cast/record_test.go` around lines 128 - 140, Introduce a shared setupRecordTest(t) helper that performs both resetRecordCommand(t) and clearRecordViperOverrides(t), then update TestRecordCmdRunEReturnsErrorForMissingOutput and the other record command tests to call it instead of invoking the helpers separately. Keep the existing test behavior unchanged.
167-180: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd coverage for
--formatandmode: steps.The current tests cover the raw
.castpath and the two error paths. No test drives--formatthroughrunRecordCommand, and none drivesmode: steps. Amode: stepstest with aType "..." Entertape would also coverplanRecordOutput's render branch end to end without a live PTY.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/cast/record_test.go` around lines 167 - 180, Add tests in record_test.go that exercise runRecordCommand with the --format flag and with a steps-mode tape so the render path is covered end to end. Reuse the existing recordCmd, resetRecordCommand, and clearRecordViperOverrides setup, then add a case that passes a .cast output format through runRecordCommand and a case that uses mode: steps with a Type "..." Enter tape to drive planRecordOutput’s render branch without a live PTY.pkg/asciicast/session_actions.go (2)
53-71: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPacing loops in
session_actions.goignore context cancellation. Both action handlers pace their output with a baretime.Sleepand neither accepts acontext.Context, so a cancelled or timed-out session keeps writing to the PTY until the loop finishes.runPauseActionalready selects onctx.Done(), so the file is inconsistent with itself. Add onesleepCtx(ctx, d)helper and threadctxfromrunActioninto both handlers.
pkg/asciicast/session_actions.go#L53-L71: add actx context.Contextfirst parameter torunWriteActionand replace the per-runetime.Sleep(rate)withsleepCtx.pkg/asciicast/session_actions.go#L73-L95: add actx context.Contextfirst parameter torunKeyActionand replace the inter-keytime.Sleep(interval)withsleepCtx.As per coding guidelines: "Use
context.Contextonly for cancellation, deadlines/timeouts, ... and place it first in function parameters."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asciicast/session_actions.go` around lines 53 - 71, Pacing loops ignore context cancellation. In pkg/asciicast/session_actions.go lines 53-71, add ctx context.Context first to runWriteAction and replace time.Sleep(rate) with the shared sleepCtx helper; in lines 73-95, add ctx context.Context first to runKeyAction and replace time.Sleep(interval) with sleepCtx. Thread ctx from runAction into both handlers and implement sleepCtx to return promptly when ctx.Done() is signaled.Source: Coding guidelines
108-121: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse static sentinel errors instead of dynamic ones.
This file builds errors dynamically in several places:
"invalid pause duration %q"here, plus"invalid write rate %q"(line 58),"invalid key interval %q"(line 103), and the bareregexp.Compileerror (line 137). The coding guidelines require a static error fromerrors/errors.gowrapped with%w. Callers cannot match any of these witherrors.Is.♻️ Example for the pause duration
duration, err := time.ParseDuration(action.Duration) if err != nil { - return fmt.Errorf("invalid pause duration %q: %w", action.Duration, err) + return fmt.Errorf("%w: invalid pause duration %q: %w", errUtils.ErrInvalidSessionAction, action.Duration, err) }Reuse an existing sentinel if one fits, otherwise add one alongside
ErrScreenshotActionRequiresPath.As per coding guidelines: "Wrap all errors with static errors from
errors/errors.go; ... never use dynamic errors directly."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asciicast/session_actions.go` around lines 108 - 121, The session action validation errors in runPauseAction and the related write-rate, key-interval, and regexp compilation paths must use static sentinels from errors/errors.go so callers can match them with errors.Is. Reuse an existing sentinel where appropriate; otherwise define sentinels alongside ErrScreenshotActionRequiresPath, wrap them with %w, and preserve the existing contextual details in each error message.Source: Coding guidelines
pkg/asciicast/tape_parser.go (1)
143-150: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winValidate the Sleep duration while parsing.
consumeTapeSleepstores the raw token. An invalid value only fails later inrunPauseAction, after the PTY session has started, and the resulting error carries no file or line. Parsing it here letstapeErrreport the offending directive before anything is recorded.Two related notes:
- VHS accepts a bare number as seconds, for example
Sleep 1.time.ParseDuration("1")rejects that, so such a tape fails at run time.- A parse-time check also covers the
Setkeys that hold durations, if you want the same treatment there.♻️ Proposed change
func consumeTapeSleep(tokens []tapeToken, pos int, file string) (TapeDirective, int, error) { line := tokens[pos].Line duration, next, err := nextTapeArg(tokens, pos+1, file, "Sleep duration") if err != nil { return TapeDirective{}, 0, err } + if _, err := time.ParseDuration(duration); err != nil { + return TapeDirective{}, 0, tapeErr(file, line, fmt.Errorf("%w: invalid Sleep duration %q", ErrTapeParseFailed, duration)) + } return TapeDirective{Kind: TapeSleep, File: file, Line: line, Duration: duration}, next, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asciicast/tape_parser.go` around lines 143 - 150, Update consumeTapeSleep to validate the returned duration token during parsing, accepting bare numeric values as seconds before applying duration parsing. On invalid input, return a tapeErr associated with the directive’s file and line so parsing fails before the PTY session starts; apply the same duration validation to Set keys that store durations if they share the relevant parsing path.pkg/asciicast/tape_test.go (1)
429-432: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBuild the fixture path with
filepath.Join.The test hardcodes
"testdata/legacy-demo.tape"with a forward slash. The coding guidelines requirefilepath.Joinfor paths in tests.♻️ Proposed change
func TestParseTapeLegacyDemoFixture(t *testing.T) { - tape, err := ParseTapeFile("testdata/legacy-demo.tape", "testdata", osTapeFileReader{}) + tape, err := ParseTapeFile(filepath.Join("testdata", "legacy-demo.tape"), "testdata", osTapeFileReader{}) require.NoError(t, err)Add
"path/filepath"to the imports.As per coding guidelines: "Use
filepath.Joinfor paths, avoid slash concatenation and Unix-specific expected paths".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asciicast/tape_test.go` around lines 429 - 432, Update TestParseTapeLegacyDemoFixture to import path/filepath and construct the fixture argument with filepath.Join("testdata", "legacy-demo.tape"), preserving the existing base directory argument and test assertions.Source: Coding guidelines
pkg/asciicast/tape.go (2)
75-88: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueDrop
perf.TrackfromError()andUnwrap().Both methods are trivial accessors. The coding guidelines exclude trivial accessors from
perf.Track.Error()also runs on every error formatting, so tracking adds noise to the perf report for no signal.♻️ Proposed cleanup
func (e *TapeError) Error() string { - defer perf.Track(nil, "asciicast.TapeError.Error")() - if e.File != "" { return fmt.Sprintf("%s:%d: %v", e.File, e.Line, e.Err) } return fmt.Sprintf("tape line %d: %v", e.Line, e.Err) } func (e *TapeError) Unwrap() error { - defer perf.Track(nil, "asciicast.TapeError.Unwrap")() - return e.Err }As per coding guidelines: "Add
defer perf.Track(atmosConfig, "pkg.FuncName")()plus a blank line to public functions, except trivial accessors, command constructors, simple factories, delegators, and pure validation/lookups."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asciicast/tape.go` around lines 75 - 88, Remove the defer perf.Track calls from TapeError.Error and TapeError.Unwrap, leaving both methods’ existing error formatting and unwrapping behavior unchanged.Source: Coding guidelines
325-332: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAlign quoted-string escape behavior with VHS or document the divergence.
scanTapeQuotedStringcurrently turns"\n"and"\t"into literalnandt, soType "a\nb"typesanb. VHS string tokens do not interpret standard backslash escapes, so either change this parser to match that behavior, or document that Atmos intentionally supports limited escapes such as control characters while raw strings keep backslashes literal.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asciicast/tape.go` around lines 325 - 332, Update scanTapeQuotedString’s backslash handling to match VHS string-token behavior: do not discard the backslash for unsupported escapes such as \n and \t, while preserving any intentionally supported control-character escapes; alternatively, explicitly document the intentional divergence and its limited-escape semantics near the parser.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent-skills/skills/atmos-vhs/SKILL.md`:
- Around line 255-263: Document that cosmetic Set directives emit warnings and
are not applied, rather than being silently ignored. Update the category title
and explanation in agent-skills/skills/atmos-vhs/SKILL.md (lines 255-263) and
agent-skills/skills/atmos-vhs/references/vhs-directive-support.md (lines 43-67),
and revise the Set directive table in
website/docs/workflows/workflows/workflow/steps/type/cast.mdx (lines 121-131) to
describe the warning behavior consistently.
- Around line 85-88: Correct the mode: steps documentation: in
agent-skills/skills/atmos-vhs/SKILL.md lines 85-88, use session mode or a tape
without Sleep and Wait; in lines 96-112, include Sleep and Wait among parse-time
rejection criteria; in lines 222-226, describe removing them as manual migration
work rather than interpreter behavior; and in
website/docs/workflows/workflows/workflow/steps/type/cast.mdx lines 116-119,
state that Sleep and Wait fail in mode: steps.
In `@cmd/cast/record_test.go`:
- Around line 142-165: Update TestRecordCmdRunEExecutesTapeAndRecordsCast to
avoid the default session mode and its PTY-backed shell by configuring recordCmd
to use the steps mode before invoking RunE. Keep the test focused on verifying
the non-empty cast output, without introducing platform-specific binaries, shell
commands, or live shell dependencies.
In `@cmd/cast/record.go`:
- Around line 37-47: Update recordCmd to add comprehensive Long help and an
embedded usage example, following the sibling cast subcommands’ go:embed, Long,
and utils.PrintfMarkdown() pattern. Add the corresponding cast_record_usage.md
markdown resource and wire it into the command’s help output while preserving
the existing argument validation and runRecordCommand flow.
In `@pkg/asciicast/screenshot.go`:
- Around line 19-45: Update writeScreenshotPNG to write the PNG to a temporary
file in the target directory, close it successfully, then atomically rename it
to path; remove the temporary file on any failure. Wrap creation, encoding,
flushing/closing, and renaming failures with the static screenshot-render error
from errors/errors.go, adding operation context with %w.
In `@pkg/asciicast/tape.go`:
- Around line 244-263: Update step() so any '#' character starts skipComment(),
including after non-space tokens on the same line, matching tokenizeTape’s
documented behavior. Remove the lx.atLineStart condition while preserving the
existing handling for line-start whitespace and other token types.
- Around line 123-131: Update ParseTapeFile’s fr.ReadFile error handling to wrap
the failure with the existing ErrTapeSourceNotFound sentinel and the requested
path context using %w, preserving the original error for matching. Keep
successful reads flowing unchanged into ParseTape.
- Around line 257-258: Update tapeLexer regex handling around scanRegexToken so
a leading slash is lexed as a regex only when the preceding tokens permit it:
after Wait, Wait+Screen, Wait+Line, or a Set key; otherwise scan it as a bare
word for absolute paths. Add an Output /tmp/demo.gif case to TestTokenizeTape.
In `@pkg/runner/step/cast_tape_test.go`:
- Around line 188-190: Update the second shell-child assertion in the
tapeResolveCd test to construct the expected WorkingDirectory with
filepath.Join("examples", "quick-start", "nested") instead of a slash-delimited
literal, and add the filepath import if needed.
In `@pkg/runner/step/cast_tape.go`:
- Around line 484-487: Update the TapeHide branch in the tape-processing switch
so a repeated Hide preserves the existing hiddenBuffer instead of clearing it.
Keep setting hidden to true and returning the same result, ensuring actions
buffered before a second Hide remain available until Show.
In `@website/blog/2026-07-11-vhs-tape-interpretation.mdx`:
- Line 36: The mode comparison in
website/blog/2026-07-11-vhs-tape-interpretation.mdx#L36 must state that only
non-liftable Hide/Show blocks require mode: session; setup-only blocks
containing exclusively export, unset, cd, and clear work in mode: steps. Update
website/docs/workflows/workflows/workflow/steps/type/cast.mdx#L118-L130 to
remove the claim that every Hide/Show block fails in mode: steps, while
preserving the restriction for other unsupported commands.
In `@website/docs/cli/commands/cast/record.mdx`:
- Around line 8-12: Add a static CastPlayer component immediately after the
Intro in the record command documentation, using the page’s appropriate
committed terminal cast example. Do not add the legacy Screengrab component, and
preserve the existing Intro content.
---
Nitpick comments:
In `@cmd/cast/record_test.go`:
- Around line 128-140: Introduce a shared setupRecordTest(t) helper that
performs both resetRecordCommand(t) and clearRecordViperOverrides(t), then
update TestRecordCmdRunEReturnsErrorForMissingOutput and the other record
command tests to call it instead of invoking the helpers separately. Keep the
existing test behavior unchanged.
- Around line 167-180: Add tests in record_test.go that exercise
runRecordCommand with the --format flag and with a steps-mode tape so the render
path is covered end to end. Reuse the existing recordCmd, resetRecordCommand,
and clearRecordViperOverrides setup, then add a case that passes a .cast output
format through runRecordCommand and a case that uses mode: steps with a Type
"..." Enter tape to drive planRecordOutput’s render branch without a live PTY.
In `@pkg/asciicast/session_actions.go`:
- Around line 53-71: Pacing loops ignore context cancellation. In
pkg/asciicast/session_actions.go lines 53-71, add ctx context.Context first to
runWriteAction and replace time.Sleep(rate) with the shared sleepCtx helper; in
lines 73-95, add ctx context.Context first to runKeyAction and replace
time.Sleep(interval) with sleepCtx. Thread ctx from runAction into both handlers
and implement sleepCtx to return promptly when ctx.Done() is signaled.
- Around line 108-121: The session action validation errors in runPauseAction
and the related write-rate, key-interval, and regexp compilation paths must use
static sentinels from errors/errors.go so callers can match them with errors.Is.
Reuse an existing sentinel where appropriate; otherwise define sentinels
alongside ErrScreenshotActionRequiresPath, wrap them with %w, and preserve the
existing contextual details in each error message.
In `@pkg/asciicast/tape_parser.go`:
- Around line 143-150: Update consumeTapeSleep to validate the returned duration
token during parsing, accepting bare numeric values as seconds before applying
duration parsing. On invalid input, return a tapeErr associated with the
directive’s file and line so parsing fails before the PTY session starts; apply
the same duration validation to Set keys that store durations if they share the
relevant parsing path.
In `@pkg/asciicast/tape_test.go`:
- Around line 429-432: Update TestParseTapeLegacyDemoFixture to import
path/filepath and construct the fixture argument with filepath.Join("testdata",
"legacy-demo.tape"), preserving the existing base directory argument and test
assertions.
In `@pkg/asciicast/tape.go`:
- Around line 75-88: Remove the defer perf.Track calls from TapeError.Error and
TapeError.Unwrap, leaving both methods’ existing error formatting and unwrapping
behavior unchanged.
- Around line 325-332: Update scanTapeQuotedString’s backslash handling to match
VHS string-token behavior: do not discard the backslash for unsupported escapes
such as \n and \t, while preserving any intentionally supported
control-character escapes; alternatively, explicitly document the intentional
divergence and its limited-escape semantics near the parser.
In `@pkg/runner/step/cast_tape_test.go`:
- Around line 16-25: Update captureTapeLogOutput to save the logger’s current
output writer before redirecting it to buf, then restore that saved writer in
t.Cleanup instead of unconditionally using os.Stderr. Preserve the existing
capture and cleanup timing around fn.
- Around line 239-250: Update TestExpandCastTapeCopyPasteEnvAlwaysError to wrap
each tape/mode combination in a t.Run subtest named with the tape and mode,
while keeping the existing step construction and error assertion inside the
subtest.
In `@pkg/runner/step/cast_tape.go`:
- Around line 195-205: The applyTapeSetInt function currently silently ignores
malformed integer values; update it to log a warning through
logUnsupportedTapeSet when strconv.Atoi fails, while preserving the existing
behavior for already-set values and valid integers.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c7661bcc-8ee3-4003-9272-3cb9c9868535
📒 Files selected for processing (33)
.claude/skills/atmos-vhsagent-skills/skills/atmos-vhs/SKILL.mdagent-skills/skills/atmos-vhs/references/vhs-directive-support.mdcmd/cast/record.gocmd/cast/record_test.goerrors/errors.gopkg/asciicast/cellgrid.gopkg/asciicast/cellgrid_test.gopkg/asciicast/recorder.gopkg/asciicast/screenshot.gopkg/asciicast/screenshot_test.gopkg/asciicast/session.gopkg/asciicast/session_actions.gopkg/asciicast/session_test.gopkg/asciicast/tape.gopkg/asciicast/tape_parser.gopkg/asciicast/tape_test.gopkg/asciicast/testdata/legacy-demo.tapepkg/datafetcher/schema/atmos/manifest/1.0.jsonpkg/datafetcher/schema/config/global/1.0.jsonpkg/io/recorder.gopkg/runner/step/cast.gopkg/runner/step/cast_tape.gopkg/runner/step/cast_tape_test.gopkg/schema/task.gopkg/schema/workflow.gowebsite/blog/2026-07-11-vhs-tape-interpretation.mdxwebsite/docs/cli/commands/cast/play.mdxwebsite/docs/cli/commands/cast/record.mdxwebsite/docs/cli/commands/cast/render.mdxwebsite/docs/workflows/workflows/workflow/steps/type/cast.mdxwebsite/src/data/roadmap.jswebsite/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json
Comment on lines
+85
to
+88
| ```shell | ||
| atmos cast record demo/landing/hero.tape --output=hero.mp4 # mode: session (default) | ||
| atmos cast record demo/landing/hero.tape --output=hero.cast --mode=steps # real exit codes | ||
| ``` |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Correct the mode: steps contract for Sleep and Wait.
Sleep and Wait are parse-time errors in mode: steps. They are not ignored or dropped.
agent-skills/skills/atmos-vhs/SKILL.md#L85-L88: use a session-mode example or a tape withoutSleepandWait.agent-skills/skills/atmos-vhs/SKILL.md#L96-L112: includeSleepandWaitin the rejection criteria.agent-skills/skills/atmos-vhs/SKILL.md#L222-L226: describe removal as manual migration work, not interpreter behavior.website/docs/workflows/workflows/workflow/steps/type/cast.mdx#L116-L119: state that these directives fail inmode: steps.
📍 Affects 2 files
agent-skills/skills/atmos-vhs/SKILL.md#L85-L88(this comment)agent-skills/skills/atmos-vhs/SKILL.md#L96-L112agent-skills/skills/atmos-vhs/SKILL.md#L222-L226website/docs/workflows/workflows/workflow/steps/type/cast.mdx#L116-L119
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@agent-skills/skills/atmos-vhs/SKILL.md` around lines 85 - 88, Correct the
mode: steps documentation: in agent-skills/skills/atmos-vhs/SKILL.md lines
85-88, use session mode or a tape without Sleep and Wait; in lines 96-112,
include Sleep and Wait among parse-time rejection criteria; in lines 222-226,
describe removing them as manual migration work rather than interpreter
behavior; and in website/docs/workflows/workflows/workflow/steps/type/cast.mdx
lines 116-119, state that Sleep and Wait fail in mode: steps.
Comment on lines
+37
to
+47
| var recordCmd = &cobra.Command{ | ||
| Use: "record <input.tape>", | ||
| Short: "Record a VHS-dialect tape into an asciicast recording", | ||
| Args: cobra.ExactArgs(1), | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| if err := recordParser.BindFlagsToViper(cmd, viper.GetViper()); err != nil { | ||
| return err | ||
| } | ||
| return runRecordCommand(cmd, args[0]) | ||
| }, | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add Long help and an embedded usage example.
recordCmd defines only Short. The coding guidelines require an embedded usage example for command help. Add a cmd/markdown/cast_record_usage.md file, embed it with //go:embed, and render it with utils.PrintfMarkdown(), matching the sibling cast subcommands.
As per coding guidelines: "Embed command usage examples from cmd/markdown/*_usage.md with //go:embed and render them with utils.PrintfMarkdown()" and "Provide comprehensive help text for all commands and flags, include examples in command help".
Run this to copy the pattern from a sibling command:
#!/bin/bash
# Show how other cast subcommands wire Long help and embedded usage markdown.
fd -e go . cmd/cast --exec rg -n 'go:embed|Long:|PrintfMarkdown|Example:' {}
fd -g '*cast*_usage.md' cmd/markdown🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/cast/record.go` around lines 37 - 47, Update recordCmd to add
comprehensive Long help and an embedded usage example, following the sibling
cast subcommands’ go:embed, Long, and utils.PrintfMarkdown() pattern. Add the
corresponding cast_record_usage.md markdown resource and wire it into the
command’s help output while preserving the existing argument validation and
runRecordCommand flow.
Source: Coding guidelines
| atmos cast record demo/hero.tape --output=hero.mp4 | ||
| ``` | ||
|
|
||
| The step's existing `mode` still decides how the tape runs: `mode: session` replays raw keypresses faithfully (the same trade-off session mode already has — no per-command exit code, since it's one continuous PTY session), while `mode: steps` turns each typed command into a real, executed step with a real exit code, and errors up front if the tape uses anything `mode: steps` can't express (a raw keypress, `Hide`/`Show`, or `Copy`/`Paste`/`Env`). |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Document the setup-only Hide/Show exception consistently.
A Hide/Show block containing only export, unset, cd, and clear is lifted into cast configuration and works in mode: steps. Only other hidden commands require mode: session.
website/blog/2026-07-11-vhs-tape-interpretation.mdx#L36-L36: limit the session-only warning to non-liftableHide/Showblocks.website/docs/workflows/workflows/workflow/steps/type/cast.mdx#L118-L130: remove the claim that everyHide/Showblock errors inmode: steps.
📍 Affects 2 files
website/blog/2026-07-11-vhs-tape-interpretation.mdx#L36-L36(this comment)website/docs/workflows/workflows/workflow/steps/type/cast.mdx#L118-L130
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@website/blog/2026-07-11-vhs-tape-interpretation.mdx` at line 36, The mode
comparison in website/blog/2026-07-11-vhs-tape-interpretation.mdx#L36 must state
that only non-liftable Hide/Show blocks require mode: session; setup-only blocks
containing exclusively export, unset, cd, and clear work in mode: steps. Update
website/docs/workflows/workflows/workflow/steps/type/cast.mdx#L118-L130 to
remove the claim that every Hide/Show block fails in mode: steps, while
preserving the restriction for other unsupported commands.
…istry Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Viper treats any registered flag as shadowing every dotted key nested under its own key, so the global --cast flag (bound to the bare "cast" key) silently zeroed out unrelated keys like cast.record.mode and cast.recording.width whenever --cast itself was left unset. This broke `atmos cast record`'s documented session-mode default and made ATMOS_CAST_RECORDING_WIDTH/HEIGHT a silent no-op. Add WithViperKey to pin a flag's Viper storage key independently of its CLI name, and move the global --cast flag to "cast.target" so "cast" stays free to be the map root the CastConfig schema and other cast.* commands need. Also fix BindFlagsToViper's inherited-flags loop, which was rebinding every persistent flag (including --cast) under its bare name regardless of this override, via a pflag annotation that survives across commands.
…ssion recording Two independent bugs corrupted mode: session cast recordings: 1. answerTerminalQueries (driven by the background output-reader goroutine) and scripted write/key actions (driven by the foreground action loop) both wrote to the same PTY input with no synchronization, so a terminal-capability-query response could interleave with a concurrently in-flight typed command and corrupt it. Fixed with a mutex-guarded syncWriter whose Locked method makes a whole scripted action, or a whole query-response burst, atomic as a unit. 2. answerTerminalQueries only scanned one PTY read chunk at a time via bytes.Contains, so a query sequence split across two reads was silently never answered, and the querying shell would retry indefinitely. Fixed by carrying a small trailing fragment forward across chunks (pendingQueryTail) so a boundary-split query is still recognized. Also bump finishSession's hardcoded 2s teardown safety-valve to 5s (defaultSessionExitMaxWait): real shell startup/profile-loading overhead was legitimately exceeding it, forcibly killing a shell that was about to exit cleanly and discarding an otherwise-successful recording. This is very likely the root cause of the previously flaky TestRecordCmdRunEExecutesTapeAndRecordsCast (now green 10/10, was failing consistently before this fix). Verified against real zsh and sh sessions (100% clean over repeated runs, previously corrupted). Real bash 3.2 (macOS's frozen system bash) still shows intermittent issues under Set Shell bash and is tracked as a known follow-up, not fixed by this change.
…-migration # Conflicts: # pkg/asciicast/session.go # pkg/datafetcher/schema/atmos/manifest/1.0.json # pkg/datafetcher/schema/config/global/1.0.json # pkg/runner/step/cast.go # website/src/data/roadmap.js # website/static/schemas/atmos/atmos-manifest/1.0/atmos-manifest.json
Write marker screenshots atomically (temp file + rename) instead of truncating in place, so a failed PNG encode never destroys an existing screenshot. Add Long/Example help and a committed --help screengrab to `atmos cast record`, pinning its test to mode: steps so it no longer spawns a real PTY-backed shell. Make the Windows-cd-tracking test build its expected path with filepath.Join instead of a hardcoded separator. Fix three tape lexer/parser bugs: wrap ParseTapeFile's top-level read error with the same sentinel + path context loadSource already uses, skip '#' comments that follow a directive on the same line (not just at line start), and stop absolute paths (e.g. `Output /tmp/demo.gif`) from being mis-lexed as regex literals -- only Wait*/Set now start a regex. Preserve buffered Hide actions across a second Hide before the matching Show instead of discarding them. Correct the atmos-vhs skill docs: a Hide/Show block of only export/unset/cd/clear lifts into native env:/working_directory: config under mode: session only, never mode: steps (any Hide/Show token is an unconditional parse-time error there); cosmetic Set directives warn before being ignored rather than doing so silently. Two related CodeRabbit threads asked for the opposite change to cast.mdx and the changelog post -- left both untouched since they already state the correct contract. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
pkg/asciicast/session_actions.go (1)
227-254: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHoist the key-sequence map to package scope.
sequencesis rebuilt on everykeySequencecall, which happens once perkeyaction and per repeat resolution. A package-levelvaravoids the repeated allocation and makes the table easier to find.Separately,
len(key) == 1at line 250 counts bytes. A single non-ASCII rune such as"é"has length 2, so it falls through toErrUnsupportedCastKey. Useutf8.RuneCountInString(key) == 1if single non-ASCII keypresses should work.♻️ Proposed change
+var keySequences = map[string]string{ + "enter": "\r", + "return": "\r", + "tab": "\t", + "esc": "\x1b", + "escape": "\x1b", + "backspace": "\x7f", + "space": " ", + "up": "\x1b[A", + "down": "\x1b[B", + "right": "\x1b[C", + "left": "\x1b[D", + "pageup": "\x1b[5~", + "pagedown": "\x1b[6~", +} + func keySequence(key string) (string, error) { normalized := strings.ToLower(strings.TrimSpace(key)) - sequences := map[string]string{ - ... - } - if seq, ok := sequences[normalized]; ok { + if seq, ok := keySequences[normalized]; ok { return seq, nil } if seq, ok := ctrlKeySequence(normalized); ok { return seq, nil } - if len(key) == 1 { + if utf8.RuneCountInString(key) == 1 { return key, nil } return "", fmt.Errorf("%w: %q", ErrUnsupportedCastKey, key) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asciicast/session_actions.go` around lines 227 - 254, Move the sequences map out of keySequence into a package-level variable and reuse it for lookups. In keySequence, replace the byte-based len(key) == 1 check with utf8.RuneCountInString(key) == 1 so single non-ASCII keypresses are accepted, adding the required utf8 import.pkg/asciicast/session.go (2)
151-157: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider dropping
perf.Trackfrom thesyncWriterhot path.
runWriteActioncallsw.Writeonce per rune insideLocked. Each call adds aperf.Trackdefer plus a map/stat update.WriteandLockedare trivial synchronization wrappers, so the tracking adds cost on the typing path without giving useful profile signal. The repository guideline exempts trivial accessors fromperf.Track.♻️ Proposed change
func (s *syncWriter) Write(p []byte) (int, error) { - defer perf.Track(nil, "asciicast.syncWriter.Write")() - s.mu.Lock() defer s.mu.Unlock() return s.w.Write(p) }As per coding guidelines, "Add
defer perf.Track(atmosConfig, "pkg.FuncName")()... except trivial accessors".Also applies to: 177-183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asciicast/session.go` around lines 151 - 157, Remove the perf.Track defer from the trivial synchronization wrappers syncWriter.Write and the additionally referenced method around lines 177–183, while preserving their locking and underlying writer behavior. Do not alter tracking elsewhere unless it belongs to these hot-path wrappers.Source: Coding guidelines
385-393: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPattern list duplicates the literals in
answerTerminalQueries.
terminalQueryPatternsandanswerTerminalQueries(lines 428-441) each hardcode the same five query sequences. If someone adds a query toanswerTerminalQueriesonly, split-read carry-over silently stops covering it, which is exactly the hang this code fixes. Consider driving both from one table that pairs each query with its response.♻️ Sketch
var terminalQueries = []struct { query []byte response []byte }{ {[]byte("\x1b]11;?\x07"), []byte("\x1b]11;rgb:0000/0000/0000\x1b\\")}, {[]byte("\x1b]11;?\x1b\\"), []byte("\x1b]11;rgb:0000/0000/0000\x1b\\")}, {[]byte("\x1b]10;?\x07"), []byte("\x1b]10;rgb:ffff/ffff/ffff\x1b\\")}, {[]byte("\x1b]10;?\x1b\\"), []byte("\x1b]10;rgb:ffff/ffff/ffff\x1b\\")}, {[]byte("\x1b[6n"), []byte("\x1b[1;1R")}, }
answerTerminalQueriesthen iterates the table, andterminalQueryTailreadsqueryfrom the same source.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asciicast/session.go` around lines 385 - 393, Replace the separate terminalQueryPatterns literals and response cases in answerTerminalQueries with one shared terminalQueries table pairing each query with its response. Update answerTerminalQueries to iterate that table and update terminalQueryTail to derive matching query bytes from the same table, ensuring newly added queries automatically support both response handling and split-read carry-over.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@agent-skills/skills/atmos-vhs/SKILL.md`:
- Around line 321-323: Update the portability guidance near the Hide block
documentation to remove any claim that Hide blocks work across both mode: steps
and mode: session. State that Hide/Show requires mode: session, or explicitly
note that mode: steps requires manually lifting those commands outside the
block.
In `@pkg/asciicast/screenshot.go`:
- Around line 52-57: Update writeScreenshotPNG to create the parent directory
returned by filepath.Dir(path) with os.MkdirAll(dir, 0o755) before calling
os.CreateTemp. Handle and return any directory-creation error using the existing
screenshot render error format, while preserving the current temporary-file
flow.
In `@pkg/asciicast/session_actions.go`:
- Around line 78-97: Update runWriteAction and runKeyAction so pacing delays
occur outside syncWriter.Locked, while preserving atomicity for each individual
writer operation and terminal-query response. Avoid holding syncWriter.mu across
time.Sleep, and keep the action’s existing write/error behavior unchanged.
In `@pkg/asciicast/tape_parser.go`:
- Around line 102-107: Update tapeCtrlKey to validate that the one-character
suffix after "Ctrl+" is an ASCII letter before lowercasing and returning it;
reject digits, symbols, and other invalid suffixes while preserving valid letter
handling. Add test cases covering rejected suffixes such as Ctrl+1 and Ctrl++.
In `@pkg/datafetcher/schema/atmos/config/1.0.json`:
- Around line 15001-15022: Add the existing tape and tape_file field definitions
to the $defs.Task schema, matching the types, nullability, and descriptions used
by the workflow step schema. Place them alongside the mode and shell fields so
Command.steps, which resolves through $defs.Tasks to $defs.Task, exposes tape
configuration for custom-command steps.
In `@pkg/flags/options.go`:
- Line 25: Terminate the inline field comments with periods in
pkg/flags/options.go lines 25-25 and pkg/flags/standard.go lines 43-43; update
both comments without changing their content or surrounding code.
In `@pkg/flags/standard_test.go`:
- Around line 239-267: Extend TestStandardFlagParser_WithViperKeyAvoidsShadowing
to use a parent-child Cobra command hierarchy with the root --cast flag
registered as persistent, then bind the child through the inherited-flags path.
Set the inherited flag and assert its value resolves to cast.target while
cast.record.mode remains "session".
In `@pkg/runner/step/cast.go`:
- Around line 91-93: Update ExecuteWithWorkflow to expand cast tape before
execution, ensuring step.Steps and session actions are populated even when
Validate was not called; reuse applyCastRecordingDefaults or call Validate as
appropriate, and make the expansion safe when the same workflowStep is processed
more than once.
In `@website/static/casts/screengrabs/atmos-cast-record--help.cast`:
- Line 10: Mark the --output flag as required in the cast record command
definition near the existing missing-output validation, using required-flag
metadata or explicit help text. Then regenerate the atmos-cast-record--help
recording so both affected help entries show --output as required.
- Around line 35-36: Remove the inline “default: session” text from the cast
mode flag description in the flag definition within record.go, allowing the flag
renderer to provide the sole default suffix. Regenerate the
atmos-cast-record--help.cast asset and verify the help output contains only one
default value.
---
Nitpick comments:
In `@pkg/asciicast/session_actions.go`:
- Around line 227-254: Move the sequences map out of keySequence into a
package-level variable and reuse it for lookups. In keySequence, replace the
byte-based len(key) == 1 check with utf8.RuneCountInString(key) == 1 so single
non-ASCII keypresses are accepted, adding the required utf8 import.
In `@pkg/asciicast/session.go`:
- Around line 151-157: Remove the perf.Track defer from the trivial
synchronization wrappers syncWriter.Write and the additionally referenced method
around lines 177–183, while preserving their locking and underlying writer
behavior. Do not alter tracking elsewhere unless it belongs to these hot-path
wrappers.
- Around line 385-393: Replace the separate terminalQueryPatterns literals and
response cases in answerTerminalQueries with one shared terminalQueries table
pairing each query with its response. Update answerTerminalQueries to iterate
that table and update terminalQueryTail to derive matching query bytes from the
same table, ensuring newly added queries automatically support both response
handling and split-read carry-over.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b13ff920-fad4-4b20-b6c0-6608c35aebd9
📒 Files selected for processing (40)
.claude/skills/atmos-vhsagent-skills/skills/atmos-vhs/SKILL.mdagent-skills/skills/atmos-vhs/references/vhs-directive-support.mdcmd/cast/record.gocmd/cast/record_test.gocmd/markdown/atmos_cast_record_usage.mdcmd/markdown/content.godemo/casts/atmos.d/screengrabs/cli.yamlerrors/errors.gopkg/asciicast/cellgrid.gopkg/asciicast/cellgrid_test.gopkg/asciicast/recorder.gopkg/asciicast/screenshot.gopkg/asciicast/screenshot_test.gopkg/asciicast/session.gopkg/asciicast/session_actions.gopkg/asciicast/session_test.gopkg/asciicast/tape.gopkg/asciicast/tape_parser.gopkg/asciicast/tape_test.gopkg/asciicast/testdata/legacy-demo.tapepkg/datafetcher/schema/atmos/config/1.0.jsonpkg/datafetcher/schema/atmos/manifest/1.0.jsonpkg/flags/global_builder.gopkg/flags/options.gopkg/flags/standard.gopkg/flags/standard_test.gopkg/io/recorder.gopkg/runner/step/cast.gopkg/runner/step/cast_tape.gopkg/runner/step/cast_tape_test.gopkg/schema/task.gopkg/schema/workflow.gowebsite/blog/2026-07-11-vhs-tape-interpretation.mdxwebsite/docs/cli/commands/cast/play.mdxwebsite/docs/cli/commands/cast/record.mdxwebsite/docs/cli/commands/cast/render.mdxwebsite/docs/workflows/workflows/workflow/steps/type/cast.mdxwebsite/src/data/roadmap.jswebsite/static/casts/screengrabs/atmos-cast-record--help.cast
🚧 Files skipped from review as they are similar to previous changes (22)
- pkg/datafetcher/schema/atmos/manifest/1.0.json
- .claude/skills/atmos-vhs
- website/blog/2026-07-11-vhs-tape-interpretation.mdx
- pkg/asciicast/cellgrid_test.go
- website/docs/cli/commands/cast/play.mdx
- website/docs/cli/commands/cast/record.mdx
- website/src/data/roadmap.js
- pkg/asciicast/recorder.go
- pkg/asciicast/cellgrid.go
- pkg/io/recorder.go
- website/docs/cli/commands/cast/render.mdx
- errors/errors.go
- pkg/schema/workflow.go
- pkg/asciicast/screenshot_test.go
- pkg/asciicast/testdata/legacy-demo.tape
- pkg/asciicast/session_test.go
- cmd/cast/record_test.go
- pkg/schema/task.go
- pkg/asciicast/tape.go
- cmd/cast/record.go
- pkg/runner/step/cast_tape_test.go
- pkg/runner/step/cast_tape.go
…-migration # Conflicts: # website/src/data/roadmap.js
Contributor
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Resource Changes Found for
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
pkg/asciicast/tape.go (1)
75-88: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider dropping
perf.TrackfromErrorandUnwrap.Both methods are trivial accessors.
Unwrapruns on everyerrors.Isanderrors.Astraversal, so the tracking call adds overhead on each hop of the error chain. The coding guidelines exclude trivial accessors from theperf.Trackrequirement.♻️ Proposed change
func (e *TapeError) Error() string { - defer perf.Track(nil, "asciicast.TapeError.Error")() - if e.File != "" { return fmt.Sprintf("%s:%d: %v", e.File, e.Line, e.Err) } return fmt.Sprintf("tape line %d: %v", e.Line, e.Err) } func (e *TapeError) Unwrap() error { - defer perf.Track(nil, "asciicast.TapeError.Unwrap")() - return e.Err }As per coding guidelines: "Add
defer perf.Track(atmosConfig, "pkg.FuncName")()plus a blank line to public functions, except trivial accessors, command constructors, simple factories, delegators, and pure validation/lookups."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/asciicast/tape.go` around lines 75 - 88, Remove the perf.Track defers from the trivial accessor methods TapeError.Error and TapeError.Unwrap, leaving their existing error formatting and Err return behavior unchanged.Source: Coding guidelines
pkg/runner/step/cast_tape.go (2)
1-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winThe file exceeds the 600-line limit.
This file is 672 lines. The parsing/settings half (
applyTapeSettings,applyTapeSetDirective,tapeOutputExtensions,matchTapeSetupCommand,tapeTrackedState) and the translation half (tapeSessionTranslator,translateTapeSteps) split cleanly, for example intocast_tape.goandcast_tape_translate.go.As per coding guidelines: "Keep files under 600 lines, use one command implementation per file, co-locate tests, and never disable the file-length linter."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/runner/step/cast_tape.go` around lines 1 - 15, The cast_tape.go file exceeds the 600-line limit; split it into focused files without changing behavior. Keep the parsing/settings symbols applyTapeSettings, applyTapeSetDirective, tapeOutputExtensions, matchTapeSetupCommand, and tapeTrackedState together, and move tapeSessionTranslator and translateTapeSteps into a separate translation file while preserving package visibility and existing references.Source: Coding guidelines
216-261: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCollapse the eight identical output-extension closures.
Every entry repeats the same "assign if empty" shape. A field-selector map removes the duplication and makes adding a format a one-line change.
♻️ Proposed refactor
-var tapeOutputExtensions = map[string]func(*schema.CastOutput, string){ - ".cast": func(o *schema.CastOutput, p string) { - if o.Cast == "" { - o.Cast = p - } - }, - ".gif": func(o *schema.CastOutput, p string) { - if o.GIF == "" { - o.GIF = p - } - }, - ".mp4": func(o *schema.CastOutput, p string) { - if o.MP4 == "" { - o.MP4 = p - } - }, - ".html": func(o *schema.CastOutput, p string) { - if o.HTML == "" { - o.HTML = p - } - }, - ".ascii": func(o *schema.CastOutput, p string) { - if o.ASCII == "" { - o.ASCII = p - } - }, - ".png": func(o *schema.CastOutput, p string) { - if o.PNG == "" { - o.PNG = p - } - }, - ".jpg": func(o *schema.CastOutput, p string) { - if o.JPG == "" { - o.JPG = p - } - }, - ".jpeg": func(o *schema.CastOutput, p string) { - if o.JPG == "" { - o.JPG = p - } - }, -} +var tapeOutputExtensions = map[string]func(*schema.CastOutput) *string{ + ".cast": func(o *schema.CastOutput) *string { return &o.Cast }, + ".gif": func(o *schema.CastOutput) *string { return &o.GIF }, + ".mp4": func(o *schema.CastOutput) *string { return &o.MP4 }, + ".html": func(o *schema.CastOutput) *string { return &o.HTML }, + ".ascii": func(o *schema.CastOutput) *string { return &o.ASCII }, + ".png": func(o *schema.CastOutput) *string { return &o.PNG }, + ".jpg": func(o *schema.CastOutput) *string { return &o.JPG }, + ".jpeg": func(o *schema.CastOutput) *string { return &o.JPG }, +}Then in
applyTapeOutputDirective:field := apply(step.CastOutput) if *field == "" { *field = d.Value }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/runner/step/cast_tape.go` around lines 216 - 261, Refactor tapeOutputExtensions to map each supported lowercase extension to a pointer-producing field selector on schema.CastOutput instead of a per-extension assignment closure. Update applyTapeOutputDirective to obtain the selected field, assign d.Value only when the pointed-to field is empty, and preserve the existing extension-to-field mapping and first-value-wins behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/asciicast/tape.go`:
- Around line 290-308: Restrict the Set branch in regexAllowed to return true
only when the preceding key token is WaitPattern, rather than allowing any Set
value to start a regex. Preserve the existing Wait, Wait+Screen, and Wait+Line
handling, and add a TestTokenizeTape table case covering Set Shell /bin/bash to
ensure it remains a word value.
In `@pkg/runner/step/cast_tape.go`:
- Around line 375-389: Update tapeTrackedState.apply to handle "export" and
"unset" separately: retain the existing assignment behavior for exports, but
delete each environment key for unsets unless it is explicitly protected by
explicitEnvKeys. Ensure unsetting an absent variable does not create it.
- Around line 321-332: Update tapeResolveCd to explicitly detect targets
containing shell command substitution such as $(...) and return them unchanged
before joining with current, so substitution targets remain literal after prior
cd operations; keep absolute, empty, and ordinary relative-path behavior
unchanged.
- Around line 99-121: Update loadTape to resolve step.TapeFile relative to
baseDir before calling asciicast.ParseTapeFile: join non-absolute paths with
baseDir while preserving absolute paths unchanged. Add a regression test
covering a relative tape_file with working_directory and verify the tape is
loaded from that directory.
---
Nitpick comments:
In `@pkg/asciicast/tape.go`:
- Around line 75-88: Remove the perf.Track defers from the trivial accessor
methods TapeError.Error and TapeError.Unwrap, leaving their existing error
formatting and Err return behavior unchanged.
In `@pkg/runner/step/cast_tape.go`:
- Around line 1-15: The cast_tape.go file exceeds the 600-line limit; split it
into focused files without changing behavior. Keep the parsing/settings symbols
applyTapeSettings, applyTapeSetDirective, tapeOutputExtensions,
matchTapeSetupCommand, and tapeTrackedState together, and move
tapeSessionTranslator and translateTapeSteps into a separate translation file
while preserving package visibility and existing references.
- Around line 216-261: Refactor tapeOutputExtensions to map each supported
lowercase extension to a pointer-producing field selector on schema.CastOutput
instead of a per-extension assignment closure. Update applyTapeOutputDirective
to obtain the selected field, assign d.Value only when the pointed-to field is
empty, and preserve the existing extension-to-field mapping and first-value-wins
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1df0a835-0693-4c22-99b2-e8e86b0444e1
📒 Files selected for processing (40)
.claude/skills/atmos-vhsagent-skills/skills/atmos-vhs/SKILL.mdagent-skills/skills/atmos-vhs/references/vhs-directive-support.mdcmd/cast/record.gocmd/cast/record_test.gocmd/markdown/atmos_cast_record_usage.mdcmd/markdown/content.godemo/casts/atmos.d/screengrabs/cli.yamlerrors/errors.gopkg/asciicast/cellgrid.gopkg/asciicast/cellgrid_test.gopkg/asciicast/recorder.gopkg/asciicast/screenshot.gopkg/asciicast/screenshot_test.gopkg/asciicast/session.gopkg/asciicast/session_actions.gopkg/asciicast/session_test.gopkg/asciicast/tape.gopkg/asciicast/tape_parser.gopkg/asciicast/tape_test.gopkg/asciicast/testdata/legacy-demo.tapepkg/datafetcher/schema/atmos/config/1.0.jsonpkg/datafetcher/schema/atmos/manifest/1.0.jsonpkg/flags/global_builder.gopkg/flags/options.gopkg/flags/standard.gopkg/flags/standard_test.gopkg/io/recorder.gopkg/runner/step/cast.gopkg/runner/step/cast_tape.gopkg/runner/step/cast_tape_test.gopkg/schema/task.gopkg/schema/workflow.gowebsite/blog/2026-07-11-vhs-tape-interpretation.mdxwebsite/docs/cli/commands/cast/play.mdxwebsite/docs/cli/commands/cast/record.mdxwebsite/docs/cli/commands/cast/render.mdxwebsite/docs/workflows/workflows/workflow/steps/type/cast.mdxwebsite/src/data/roadmap.jswebsite/static/casts/screengrabs/atmos-cast-record--help.cast
🚧 Files skipped from review as they are similar to previous changes (34)
- pkg/schema/task.go
- cmd/markdown/content.go
- website/src/data/roadmap.js
- website/docs/cli/commands/cast/play.mdx
- errors/errors.go
- pkg/asciicast/testdata/legacy-demo.tape
- .claude/skills/atmos-vhs
- website/docs/cli/commands/cast/render.mdx
- pkg/schema/workflow.go
- demo/casts/atmos.d/screengrabs/cli.yaml
- pkg/io/recorder.go
- pkg/flags/global_builder.go
- pkg/flags/standard.go
- pkg/asciicast/session_actions.go
- cmd/cast/record.go
- pkg/asciicast/screenshot_test.go
- pkg/asciicast/screenshot.go
- pkg/datafetcher/schema/atmos/config/1.0.json
- website/blog/2026-07-11-vhs-tape-interpretation.mdx
- pkg/flags/options.go
- pkg/flags/standard_test.go
- pkg/runner/step/cast.go
- pkg/asciicast/tape_parser.go
- pkg/asciicast/cellgrid_test.go
- pkg/asciicast/cellgrid.go
- website/static/casts/screengrabs/atmos-cast-record--help.cast
- website/docs/cli/commands/cast/record.mdx
- pkg/datafetcher/schema/atmos/manifest/1.0.json
- pkg/asciicast/recorder.go
- cmd/cast/record_test.go
- pkg/runner/step/cast_tape_test.go
- pkg/asciicast/session.go
- pkg/asciicast/session_test.go
- pkg/asciicast/tape_test.go
…2889 Create the marker screenshot's parent directory before writing, so a Screenshot targeting a not-yet-created directory doesn't fail. Restrict tape "Ctrl+<X>" directives to a letter suffix (rejecting Ctrl+1, Ctrl++, etc.) instead of accepting any one-byte suffix. Stop the session's output-recording goroutine from stalling behind the writer lock a paced "write"/"key" action holds for its whole multi- character duration: answering a terminal capability query still needs that same lock for atomicity, but doing it synchronously inside recordOutputChunk blocked recording of every later chunk (and reading the next one) until the action finished, bunching recorded timestamps at the end of the action. Dispatch the query answer in the background instead -- each known response is a fixed, self-contained reply, so answering slightly out of arrival order is harmless. Fix another missed "Hide is portable across both modes" claim in the atmos-vhs skill's Common Patterns section (mode: steps still hard-errors on any Hide/Show regardless of contents). Add trailing periods to two merged-in pkg/flags field comments, and cover the cmd.InheritedFlags() path in BindFlagsToViper with a real parent-child command test (the existing shadowing tests only ever bind unrelated commands). Mark `--output` as required and drop the duplicated "(default: session)" text from `--mode`'s description on `atmos cast record` (the flag renderer already appends the default once); regenerate the --help screengrab to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The "Get modified .tape files" step ran the same `cut -d/ -f2-` on both branches of its if/else: correct for find's "./pkg/..." output (strips the literal "./"), but wrong for `git diff --name-only`'s output, which is already repo-root-relative with no such prefix -- so it silently chopped off the real leading directory instead, turning "pkg/asciicast/testdata/legacy-demo.tape" into "asciicast/testdata/legacy-demo.tape" and failing the vhs-action step with ENOENT once this PR's diff touched that fixture. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
pkg/asciicast/testdata/legacy-demo.tape is a Go parser/translator smoke-test fixture (see its own header comment), not a real tape meant for the real vhs binary -- but nothing excluded it from vhs.yaml's matrix once this PR's diff touched it, so the workflow tried to render it for real and failed downstream in vhs-action's own font-download step (an unrelated symptom of the fixture being in the matrix at all: this retired tape references a since-removed examples/quick-start layout and scripts an interactive atmos TUI navigation sequence that was never meant to run through the real tool). Exclude any/testdata/** tape, mirroring the existing demo/landing/** exclusion, in both matrix-building branches (find and git diff). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Narrow the tape lexer's regexAllowed() to only Set WaitPattern's value -- every other Set key's value must stay a bare word, or e.g. Set Shell /bin/bash mis-lexes as regex "bin" plus word "bash" instead of failing loudly, silently corrupting the Shell value. Fix two setup-command bugs in the tape translator's tracked cwd/env state: a cd target containing shell command substitution (e.g. `$(git rev-parse --show-toplevel)`, as hero.tape's Hide block uses) only stayed literal when no prior cd had run yet -- after an earlier cd it got joined onto the tracked directory instead, producing a nonsense path. And unset treated an environment key exactly like export (storing an empty string) instead of deleting it, so unsetting a variable that was never set fabricated it as "", and unsetting one this tape itself exported left it exported as empty instead of removed. Resolve a relative tape_file: against the step's working_directory before reading it -- it was passed straight to ParseTapeFile unjoined, so a tape_file: paired with working_directory read from the process CWD instead, unlike Source directives inside the tape, which already resolved against the same base correctly. Split cast_tape.go's mode: session translator (tapeSessionTranslator and its dispatch methods) into a new cast_tape_session.go: the loadTape fix pushed the file over the 500-line limit, and this is the same file-per- mode split cast.go/cast_simulate.go already follow. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
github-actions
Bot
added
size/xl
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/runner/step/cast_tape.go (1)
379-400: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve
unsetsemantics for inherited variables.
Variables.Envincludes the process environment. Generated shell steps overlaystep.Env, so deleting a key fromtapeTrackedState.envdoes not remove it from the child environment. Track removed keys and filter them during environment construction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/runner/step/cast_tape.go` around lines 379 - 400, Update tapeTrackedState.apply, specifically the "unset" branch, to record inherited environment keys that were removed rather than only deleting them from env. Add or reuse removal tracking in tapeTrackedState, skip explicit environment keys, and ensure the child environment construction filters recorded removals after applying step.Env overlays.
🧹 Nitpick comments (3)
pkg/runner/step/cast_tape_session.go (2)
80-87: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider closing the unterminated Hide region with a
show.The tail flush emits
hideand the buffered actions, but nevershow. The generated action list then ends unbalanced. A consumer that validates or restores hide/show state sees a dangling region.♻️ Proposed balance
if len(t.hiddenBuffer) > 0 { t.steps = append(t.steps, schema.WorkflowStep{Type: "hide"}) t.steps = append(t.steps, t.hiddenBuffer...) + t.steps = append(t.steps, schema.WorkflowStep{Type: "show"}) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/runner/step/cast_tape_session.go` around lines 80 - 87, Update the tail-flush logic in the tape session step conversion method to append a closing show step after emitting the buffered actions for an unterminated Hide region. Preserve the existing hide and buffered-step ordering, ensuring the generated action list is balanced.
122-132: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMixed type constants and string literals.
Line 125 uses
schema.TaskTypeScreenshot, while"key","write","pause","wait","hide", and"show"stay as literals in this file. If constants exist for those action types, use them everywhere for consistency and rename safety.#!/bin/bash # Description: Check whether schema defines constants for the remaining session action types. set -eu rg -n 'TaskType[A-Za-z]+\s*=' pkg/schema🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/runner/step/cast_tape_session.go` around lines 122 - 132, The dispatchSimple method mixes schema.TaskTypeScreenshot with a raw "key" task type. Check pkg/schema for constants representing the remaining session action types, then replace the corresponding literals throughout this tape session translator, including key, write, pause, wait, hide, and show, with their schema constants while preserving existing behavior.pkg/runner/step/cast_tape.go (1)
331-336: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider covering backtick and
${...}forms too.The new
$(guard fixes the common case. Two sibling forms still slip through: backtick substitution and parameter expansion.cd \git rev-parse --show-toplevel`after an earliercd` still joins onto the tracked directory.♻️ Proposed hardening
+// tapeShellExpansion reports whether target contains shell substitution or +// expansion, which must not be joined onto a tracked directory. +func tapeShellExpansion(target string) bool { + return strings.Contains(target, "$(") || strings.Contains(target, "${") || strings.Contains(target, "`") +} + func tapeResolveCd(current, target string) string { - if target == "" || filepath.IsAbs(target) || current == "" || strings.Contains(target, "$(") { + if target == "" || filepath.IsAbs(target) || current == "" || tapeShellExpansion(target) { return target } return filepath.Join(current, target) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/runner/step/cast_tape.go` around lines 331 - 336, Update tapeResolveCd to treat backtick command substitutions and ${...} parameter expansions like the existing $(...) form: return target unchanged when either syntax is present, while preserving the current handling for empty, absolute, and current-less paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/vhs.yaml:
- Line 58: Update the changed-files filter in the workflow’s files assignment to
use a root-aware testdata exclusion matching both testdata/foo.tape and nested
testdata directories. Replace the current slash-only testdata pattern while
preserving the existing style/defaults and demo/landing exclusions.
- Around line 56-58: Update the tape-file matrix construction around the files
variable so an empty result after excluding style/defaults, demo/landing, and
testdata produces matrix=[] rather than [""] and skips the VHS action. Ensure
both the find-based and git-diff-based branches preserve valid tape paths while
filtering empty entries before the matrix is consumed.
---
Outside diff comments:
In `@pkg/runner/step/cast_tape.go`:
- Around line 379-400: Update tapeTrackedState.apply, specifically the "unset"
branch, to record inherited environment keys that were removed rather than only
deleting them from env. Add or reuse removal tracking in tapeTrackedState, skip
explicit environment keys, and ensure the child environment construction filters
recorded removals after applying step.Env overlays.
---
Nitpick comments:
In `@pkg/runner/step/cast_tape_session.go`:
- Around line 80-87: Update the tail-flush logic in the tape session step
conversion method to append a closing show step after emitting the buffered
actions for an unterminated Hide region. Preserve the existing hide and
buffered-step ordering, ensuring the generated action list is balanced.
- Around line 122-132: The dispatchSimple method mixes schema.TaskTypeScreenshot
with a raw "key" task type. Check pkg/schema for constants representing the
remaining session action types, then replace the corresponding literals
throughout this tape session translator, including key, write, pause, wait,
hide, and show, with their schema constants while preserving existing behavior.
In `@pkg/runner/step/cast_tape.go`:
- Around line 331-336: Update tapeResolveCd to treat backtick command
substitutions and ${...} parameter expansions like the existing $(...) form:
return target unchanged when either syntax is present, while preserving the
current handling for empty, absolute, and current-less paths.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 143df407-ccbf-492c-a812-bad8253ba95f
📒 Files selected for processing (6)
.github/workflows/vhs.yamlpkg/asciicast/tape.gopkg/asciicast/tape_test.gopkg/runner/step/cast_tape.gopkg/runner/step/cast_tape_session.gopkg/runner/step/cast_tape_test.go
🚧 Files skipped from review as they are similar to previous changes (3)
- pkg/asciicast/tape_test.go
- pkg/asciicast/tape.go
- pkg/runner/step/cast_tape_test.go
fakeTapeFileReader's map keys are forward-slash literals, but the real
loadSource/ParseTapeFile canonicalize via filepath.Join/Clean, which is
backslash-separated on Windows -- so every Source-inlining test failed
there with "file does not exist" despite the fixture existing in the
map. Normalize the lookup path with filepath.ToSlash instead of
requiring every test author to hand-maintain two path spellings.
TestTapeResolveCd's "absolute target" case used a Unix-style "/srv/app"
literal, which filepath.IsAbs correctly reports as *not* absolute on
Windows (no drive letter or UNC prefix) -- tapeResolveCd then joined it
onto "current" instead of returning it unchanged. Build a real absolute
path with filepath.Abs instead of hardcoding one.
Also fix two gaps CodeRabbit found in the still-open vhs.yaml testdata
exclusion: the git-diff branch's `grep -v '/testdata/'` didn't match a
root-level `testdata/foo.tape` (no leading slash), unlike the find
branch's already root-aware `-path '*/testdata' -prune` -- switch to
`grep -vE '(^|/)testdata/'`. And when every changed tape gets excluded,
`"" | jq -R -s -c 'split(" ")'` produces `[""]`, not `[]`, so
`needs.prepare.outputs.matrix != '[]'` doesn't skip the vhs job and
vhs-action runs once with an empty path -- filter empty entries so an
all-excluded result actually produces `[]`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2889 +/- ##
==========================================
- Coverage 83.95% 83.93% -0.02%
==========================================
Files 1993 2000 +7
Lines 195879 196751 +872
==========================================
+ Hits 164444 165152 +708
- Misses 23399 23529 +130
- Partials 8036 8070 +34
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Warning SHA Pin Verification Passed — with documented exceptionsAll 233 third-party action reference(s) are covered, but 2 rely on a documented allowlist entry in
See the action run for full details. |
… colord) Bump pnpm.overrides for four already-overridden transitive deps to their patched versions, and add a new override for colord (previously unpinned): js-yaml 3.15.1->3.15.2 and 4.3.1->4.3.2, svgo 3.3.4->3.3.5, joi 17.13.4->^17.13.6 (resolves 17.13.7), colord 2.9.3->^2.9.4 (resolves 2.10.0). All are patch/minor bumps within a single major version, so none are blocked by dependabot.yml's semver-major ignore policy. Fixes GHSA alerts #295, #294 (js-yaml), #293, #292 (svgo), #291, #289 (joi), #290 (colord). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
…-migration # Conflicts: # website/package.json # website/pnpm-lock.yaml
Contributor
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
This branch was successfully deployed
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.
what
tape:/tape_file:fields to thetype: castworkflow/custom-command step: a VHS-dialect.tapescript (inline or from a file) is interpreted directly, in memory, at execution time — novhsbinary, no generated YAML, nothing written to disk.mode: steps(real, exit-code-tracked child steps) ormode: session(PTY-driven keypress replay) machinery, includingHide/Show,Screenshot,Source, andRequire; unsupported directives log a warning (cosmeticSetkeys) or error out with the offending line (Sleep/Waitdropped undermode: steps, bare keypresses requiringmode: session).atmos cast record <input.tape> --output=<path>CLI subcommand so a tape can be recorded ad hoc with no workflow YAML at all, completing therecord/play/renderverb set.Screenshotdirective → a rasterized PNG at that point in the recording) topkg/asciicast.atmos-vhsskill covering tape interpretation and hand-migration to nativesteps:.minorroadmap entry + blog post for the new capability.why
.tapefiles previously had to choose between keeping a second recording toolchain installed or hand-rewriting every directive into Atmos's own cast step YAML..tapescripts run as Atmos casts unmodified, whilemode: stepsupgrades typed commands to real, individually-executed steps with real exit codes when that's wanted instead of raw PTY replay.references
/workflows/workflows/workflow/steps/type/cast#tape-interpretation,/cli/commands/cast/recordSummary by CodeRabbit
.tapefiles.atmos cast recordwith.cast, GIF, and other supported output formats.