[codex] Add native asciicast recording and demo embeds#2672
Merged
Conversation
# Conflicts: # cmd/root.go # pkg/config/load.go
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Dependency Review✅ No vulnerabilities or license issues found.Scanned Files
|
Erik Osterman (Cloud Posse) (osterman)
added
the
minor
Erik Osterman (Cloud Posse) (osterman)
marked this pull request as ready for review
Erik Osterman (Cloud Posse) (osterman)
requested review from
a team
as code owners
|
Warning Release Documentation RequiredThis PR is labeled
|
Contributor
|
Important Review skippedToo many files! This PR contains 1365 files, which is 1215 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1365)
You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR adds end-to-end asciicast recording, replay, and rendering, introduces ChangesCast engine, CLI, and step execution
Estimated code review effort: 5 (Critical) | ~150 minutes Demo cast fixtures, workflows, and static casts
Estimated code review effort: 3 (Moderate) | ~30 minutes Website CastPlayer component
Estimated code review effort: 3 (Moderate) | ~25 minutes PRD, agent skills, and snapshots
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant RootCmd
participant Recorder
participant AsciicastRender
User->>RootCmd: atmos --cast terraform plan
RootCmd->>Recorder: StartRecordingIfRequested
RootCmd->>Recorder: FinalizeRecording
Recorder->>AsciicastRender: Render(out.cast, GIF)
AsciicastRender-->>User: out.gif
sequenceDiagram
participant WorkflowExecutor
participant CastHandler
participant AsciicastRecorder
participant ChildStep
WorkflowExecutor->>CastHandler: Execute(cast step)
CastHandler->>AsciicastRecorder: Start(width,height,command)
loop nested steps
CastHandler->>ChildStep: run(simulate/shell/toast)
ChildStep-->>AsciicastRecorder: Record output
end
CastHandler->>AsciicastRecorder: Close
CastHandler-->>WorkflowExecutor: StepResult(cast path)
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
agent-skills/skills/atmos-asciicast/SKILL.md (1)
1-36:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFix agent skill line limit violation.
The pipeline failed because this SKILL.md exceeds the maximum line limit for agent skills. Move detailed content to
references/to comply with the spec.Suggested structure:
SKILL.md(≤MAX_LINES): Keep front matter, brief description, and links to references.references/atmos-asciicast-authoring.md: Move "Defaults", "Fixture Policy", and "Authoring Checklist" sections there.♻️ Proposed restructuring
--- name: atmos-asciicast -description: Create and update Atmos asciicast demos for command documentation and website publishing. +description: Create and update Atmos asciicast demos. --- # Atmos Asciicast Demo Skill -Use this skill when creating or updating committed Atmos `.cast` recordings. - -## Defaults - -- Use `width: 120` and `height: 36` unless the target docs page needs a narrower recording. -- Write curated casts under `website/static/casts/...`; they are served from `/casts/...`. -- Use `type: cast` with `mode: steps` for deterministic command demos that need exit-code propagation. -- Use `mode: session` only when the demo must show typed input, prompts, key presses, or terminal timing. -- Keep ad hoc local recordings in the XDG cache via `--cast`; do not commit cache recordings. - -## Fixture Policy - -- Use deterministic demo fixtures under `demo/casts`. -- Do not reuse product examples or test fixtures just to make a docs cast easier. -- Keep demo output stable: no local absolute paths, hostnames, real account IDs, secrets, random IDs, or live timestamps. - -## Authoring Checklist - -1. Add or update the workflow/custom command that regenerates the cast. -2. Regenerate the `.cast` into `website/static/casts`. -3. Review the cast as plain text for secrets, local paths, unstable timestamps, and noisy logs. -4. Embed it with the website `CastPlayer` component when the corresponding docs page can show it usefully: - - ```mdx - <CastPlayer src="/casts/cli/describe-component.cast" title="describe component" chrome controls scrubber /> - ``` - -5. Prefer committing only `.cast` files. Commit GIF/MP4/SVG derivatives only when a publishing target cannot consume the player. +Use this skill when creating or updating committed Atmos `.cast` recordings. +See [Authoring Guide](references/atmos-asciicast-authoring.md) for full details.🤖 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-asciicast/SKILL.md` around lines 1 - 36, The atmos-asciicast SKILL.md is over the allowed agent-skill line limit; trim it to a minimal entry point by keeping only the front matter, short description, and a link to a new reference doc. Move the detailed “Defaults”, “Fixture Policy”, and “Authoring Checklist” content into references/atmos-asciicast-authoring.md, and update the SKILL.md text so it points readers to that guide while preserving the skill name atmos-asciicast.pkg/io/context.go (1)
80-84: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRecord partial writes before returning the error.
fmt.Fprintcan returnn > 0witherr != nil. In that case the terminal already received a prefix, but the recorder skips it, so the cast no longer matches what the user saw.Suggested fix
- _, err := fmt.Fprint(writer, masked) - if err != nil { - return fmt.Errorf("%w: %w", errUtils.ErrWriteToStream, err) - } - recordOutput(stream, masked) + n, err := fmt.Fprint(writer, masked) + if n > 0 { + recordOutput(stream, masked[:n]) + } + if err != nil { + return fmt.Errorf("%w: %w", errUtils.ErrWriteToStream, err) + } return 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/io/context.go` around lines 80 - 84, The write path in context output handling is dropping partially written data when fmt.Fprint returns both bytes written and an error. Update the output flow around the writer call and recordOutput so any successfully written prefix is recorded before returning the wrapped error, ensuring the recorder stays in sync with what the terminal actually received.pkg/terminal/pty/pty_test.go (1)
384-422: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAssert the recorded event is masked too.
This test only checks the terminal buffer. It would still pass if
recordingWritermasked stdout correctly but wrote the raw secret into the cast event, which is the regression this path needs to prevent.As per coding guidelines, "`**/*_test.go`: Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages."Suggested test hardening.
func TestRecordingWriterMasksOutput(t *testing.T) { // Create IO context with masking enabled. ioCtx, err := iolib.NewContext() if err != nil { t.Fatalf("Failed to create IO context: %v", err) } + rec := &ptyTestRecorder{} + restore := iolib.SetRecorder(rec) + t.Cleanup(restore) + var buf bytes.Buffer writer := &recordingWriter{ underlying: &buf, masker: ioCtx.Masker(), @@ // Output should contain the mask replacement. if !strings.Contains(output, iolib.MaskReplacement) { t.Errorf("Output does not contain mask replacement: %s", output) } + + if len(rec.events) != 1 { + t.Fatalf("recorded %d events, want 1", len(rec.events)) + } + if strings.Contains(rec.events[0], secretKey) { + t.Fatalf("recorded event contains unmasked secret: %q", rec.events[0]) + } + if !strings.Contains(rec.events[0], iolib.MaskReplacement) { + t.Fatalf("recorded event does not contain mask replacement: %q", rec.events[0]) + } }🤖 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/terminal/pty/pty_test.go` around lines 384 - 422, The masking test only verifies the terminal buffer and misses the cast event path, so it can still pass if recordingWriter writes a raw secret into the recorded event. Harden TestRecordingWriterMasksOutput to also assert the event payload emitted by recordingWriter is masked, using recordingWriter and its write/recording path to verify both stdout and the captured event contain iolib.MaskReplacement and do not contain the secretKey.Source: Coding guidelines
pkg/provisioner/source/vendor.go (1)
44-50: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winReject unsafe target paths before the replace/remove branch.
VendorSourcewill recursively delete any existingtargetDirwhen replacement is enabled. That helper is now fed from workflow-resolved input inpkg/runner/step/workdir.go, so a typo like".","./", or a filesystem root turnsreset: trueinto data loss instead of a validation error. Please hard-fail on obviously unsafe targets before the firstos.Stat/os.RemoveAllpath.Also applies to: 155-177
🤖 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/provisioner/source/vendor.go` around lines 44 - 50, VendorSource currently accepts workflow-resolved targetDir input and can reach the replace/remove branch that recursively deletes existing paths, so add an early validation step in VendorSource before any os.Stat or os.RemoveAll calls to reject obviously unsafe targets like ".", "./", and filesystem roots. Keep the guard near the top of VendorSource, before the replace logic that uses targetDir, and return a hard failure for invalid paths so reset-enabled flows cannot delete unintended directories.
🧹 Nitpick comments (15)
docs/prd/asciicast-recording.md (1)
199-200: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueFix "regexp"/"regex" inconsistency.
Use one form consistently. Prefer "regex" to match the field name
regexused elsewhere in the PRD.- Regex syntax uses Go's `regexp` package. - Invalid regex patterns fail validation before the session starts. + Regex syntax uses Go's `regex` package. + Invalid regex patterns fail validation before the session starts.Wait, Go's package is actually
regexp. So keep "regexp" for the package, change "regex" to "regexp" in the next sentence:- Regex syntax uses Go's `regexp` package. - Invalid regex patterns fail validation before the session starts. + Regexp syntax uses Go's `regexp` package. + Invalid regexp patterns fail validation before the session starts.Or keep both as "regex" since the field is
regex:- Regex syntax uses Go's `regexp` package. + Regexp syntax uses Go's `regexp` package.Actually simplest: just use "regex" consistently since the YAML field is
regex:- Regex syntax uses Go's `regexp` package. + Regex syntax uses Go's `regexp` package. - Invalid regex patterns fail validation before the session starts. + Invalid regex patterns fail validation before the session starts.No change needed to line 200. Just fix line 199 to match:
- Regex syntax uses Go's `regexp` package. + Regexp syntax uses Go's `regexp` package.Hmm, that's worse. Let me just suggest:
- Regex syntax uses Go's `regexp` package. - Invalid regex patterns fail validation before the session starts. + Regexp syntax uses Go's `regexp` package. + Invalid regexp patterns fail validation before the session starts.🤖 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 `@docs/prd/asciicast-recording.md` around lines 199 - 200, The issue is inconsistent use of “regex” vs “regexp” in the asciicast recording PRD bullets. Update the affected sentence in this section to use one term consistently, and keep the Go package reference in the `regexp` mention while aligning the surrounding wording with the `regex` field name used elsewhere. Use the nearby bullets in the same section to verify the final phrasing stays consistent across `regex`/`regexp`.demo/casts/fixtures/demo-vendoring/atmos.yaml (1)
19-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMislabeled comment: "Absolute path" example is actually relative.
Line 27's comment says
# Absolute pathbutvendor.d/vendor1.yamlis a relative path, same as the "Directory" example above it.✏️ Suggested fix
- # Absolute path - `#base_path`: "vendor.d/vendor1.yaml" + # Alternate relative path (single directory) + `#base_path`: "vendor.d/vendor1.yaml"🤖 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 `@demo/casts/fixtures/demo-vendoring/atmos.yaml` around lines 19 - 27, The comment label in the vendor base_path examples is incorrect: the “Absolute path” example in the vendoring YAML fixture points to a relative path. Update the comment near the vendor/base_path examples in atmos.yaml so the label matches the actual path type, or replace it with a truly absolute path; use the vendor section and the base_path examples as the reference points.demo/casts/fixtures/demo-vendoring/vendor/vendor1.yaml (1)
10-16: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPin to a tag/commit instead of
mainfor reproducibility.Sourcing from
version: "main"tracks a moving branch. Since this fixture backs deterministic/reproducible cast recordings, upstream changes tomaincould silently alter the generated demo output over time.♻️ Suggested fix
- version: "main" + version: "1.190.0"🤖 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 `@demo/casts/fixtures/demo-vendoring/vendor/vendor1.yaml` around lines 10 - 16, The vendoring fixture for the ipinfo component is using a მოძრable branch via version: "main", which can make cast output non-deterministic. Update the vendor entry in vendor1.yaml to reference a fixed tag or commit SHA instead, keeping the same source/targets structure and the component/version placeholders so the fixture remains reproducible.website/src/components/FileBrowser/FileViewer.tsx (1)
19-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate
stripFrontmatterimplementation.Identical to
stripFrontmatterinwebsite/src/components/EmbedExample/index.tsx. Extract to a shared utility to avoid divergence.♻️ Suggested extraction
-function stripFrontmatter(content: string): string { - if (!content.startsWith('---')) return content; - - const lines = content.split(/\r?\n/); - if (lines[0].trim() !== '---') return content; - - const endIndex = lines.findIndex((line, index) => index > 0 && line.trim() === '---'); - if (endIndex === -1) return content; - - return lines.slice(endIndex + 1).join('\n').trim(); -} +import { stripFrontmatter } from '`@site/src/utils/frontmatter`';And update
EmbedExample/index.tsxto import from the same shared module instead of defining its own copy.🤖 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/src/components/FileBrowser/FileViewer.tsx` around lines 19 - 30, The stripFrontmatter helper is duplicated in FileViewer and EmbedExample, so move the shared logic into a common utility module and have both FileViewer and EmbedExample import that single implementation. Keep the existing behavior of stripFrontmatter unchanged while removing the local copy from FileViewer and updating the EmbedExample/index.tsx usage to reference the shared helper.pkg/schema/schema.go (1)
142-152: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd doc comments for the exported cast config types.
CastConfigandCastRecordingConfigare exported, so add Go doc comments for lint/doc hygiene. As per coding guidelines,**/*.go: Document all exported functions, types, and methods following Go's documentation conventions.Suggested fix
+// CastConfig configures asciicast recording and playback behavior. type CastConfig struct { Recording CastRecordingConfig `yaml:"recording,omitempty" json:"recording,omitempty" mapstructure:"recording"` } +// CastRecordingConfig configures native asciicast recording defaults. type CastRecordingConfig struct {🤖 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/schema/schema.go` around lines 142 - 152, Add Go doc comments to the exported cast configuration types in schema.go. Document both CastConfig and CastRecordingConfig with comments that begin with the type name and briefly describe their purpose, following standard Go documentation conventions so exported types in the schema package are lint/doc compliant.Source: Coding guidelines
pkg/schema/workflow.go (1)
574-579: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the exported
CastOutputtype.Add a Go doc comment starting with
CastOutput. As per coding guidelines,**/*.go: Document all exported functions, types, and methods following Go's documentation conventions.Suggested fix
+// CastOutput declares cast artifact paths to keep or render. type CastOutput struct { Cast string `yaml:"cast,omitempty" json:"cast,omitempty" mapstructure:"cast"`🤖 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/schema/workflow.go` around lines 574 - 579, Add a Go doc comment for the exported CastOutput type so it starts with “CastOutput” and follows standard Go documentation conventions. Update the type declaration in workflow.go to include a concise comment describing what CastOutput represents, keeping the existing fields Cast, SVG, GIF, and MP4 unchanged.Source: Coding guidelines
pkg/asciicast/recorder_test.go (1)
36-41: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the wrapped sentinel error.
Use
errors.Is(err, ErrCastOutputExists)instead of matching text. As per coding guidelines,**/*.go: Useerrors.Is()for error checking.Suggested fix
import ( + "errors" "os" "path/filepath" "strings" @@ - if !strings.Contains(err.Error(), "already exists") { + if !errors.Is(err, ErrCastOutputExists) { t.Fatalf("unexpected error: %v", err) }🤖 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/recorder_test.go` around lines 36 - 41, The test in recorder_test.go is checking the error by matching its text instead of asserting the sentinel, so update the Start/Options explicit path collision assertion to use errors.Is against ErrCastOutputExists. Keep the existing failure path check, but replace the strings.Contains(err.Error(), ...) validation with a direct wrapped-error check so the test follows the error-handling guideline consistently.Source: Coding guidelines
pkg/asciicast/session.go (1)
25-52: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd doc comments for the exported session API.
ErrUnknownSessionAction,SessionAction,SessionOptions, andRunSessionare exported. As per coding guidelines,**/*.go: Document all exported functions, types, and methods following Go's documentation conventions.🤖 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 25 - 52, Add Go doc comments for every exported session API symbol in this block: ErrUnknownSessionAction, SessionAction, SessionOptions, and RunSession. Place a concise comment immediately above each declaration in pkg/asciicast/session.go, following standard Go documentation conventions so the purpose of each exported error, type, and function is clear.Source: Coding guidelines
pkg/asciicast/recorder.go (1)
32-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd doc comments for the exported recorder API.
This new package exposes multiple public types/functions without Go doc comments. As per coding guidelines,
**/*.go: Document all exported functions, types, and methods following Go's documentation conventions.Also applies to: 72-179, 261-314
🤖 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/recorder.go` around lines 32 - 72, Add Go doc comments for every exported API in the recorder package. Document the public types and functions shown here, including ErrCastOutputExists, Options, Recorder, Header, and Start, and continue through the rest of the package for any exported methods/functions referenced by the review. Keep the comments in standard Go style so each exported symbol clearly describes its purpose and behavior.Source: Coding guidelines
pkg/asciicast/render.go (1)
17-22: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSentinel errors declared locally instead of in
errors/errors.go.
ErrEmptyCastFile,ErrRenderOutputExists,ErrMissingAgg,ErrMissingFFmpegare new package-level sentinel errors defined here rather than in the centralizederrors/errors.go. As per coding guidelines, "All errors MUST be wrapped using static errors defined inerrors/errors.go."🤖 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/render.go` around lines 17 - 22, The sentinel errors in render.go are defined locally instead of using the centralized error definitions, so move ErrEmptyCastFile, ErrRenderOutputExists, ErrMissingAgg, and ErrMissingFFmpeg into the shared errors package and update the render-related code to reference those shared symbols. Keep the render logic in the render package using the imported errors identifiers rather than declaring new package-level error vars.Source: Coding guidelines
cmd/root.go (1)
379-382: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRoute these status lines through the CLI output layer.
Direct
fmt.Fprintf(os.Stderr, ...)bypasses the repo’sui.*/data.*abstractions and makes this output harder to style and test consistently. Emit these messages afterrestore()via the standard helpers instead. As per coding guidelines "Never usefmt.Fprintf(os.Stdout/Stderr, ...)orfmt.Println(...). Usedata.*orui.*functions instead."🤖 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/root.go` around lines 379 - 382, The status messages in the cast recording flow are written directly with fmt.Fprintf to os.Stderr, bypassing the CLI output abstractions. Update the restore()/recording completion path in root.go to emit both the failure and success messages through the existing ui.* or data.* helpers instead of direct stderr writes, keeping the same cast recording context via the rec.Path() and error handling logic.Source: Coding guidelines
cmd/cast/cast.go (1)
46-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRegister
renderflags through the standard parser.These flags are added directly on
renderCmdwhileGetFlagsBuilder()staysnil, which bypasses the repo’s normal flag plumbing and makes this command behave differently from the rest ofcmd/. As per coding guidelines "Commands MUST useflags.NewStandardParser()for command-specific flags." Based on learnings "Define and parse flags with a standard parser (e.g.,flags.NewStandardParser()) and avoid binding flags to Viper."Also applies to: 68-74
🤖 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/cast.go` around lines 46 - 49, The `renderCmd` flags are being registered directly in `init()`, which bypasses the repo’s standard flag plumbing and leaves `GetFlagsBuilder()` unused. Move the `svg`, `gif`, and `mp4` options onto a `flags.NewStandardParser()`-based setup for the `render` command, and wire that parser through the command’s flag builder path so `renderCmd` follows the same pattern as the rest of `cmd/` (including the related code around `GetFlagsBuilder()` and the later flag handling).Sources: Coding guidelines, Learnings
pkg/runner/step/cast.go (2)
85-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer
ui.*/data.*helpers over rawfmt.Fprintffor user-facing output.
fmt.Fprintf(iolib.GetContext().UI(), ...)bypasses the UI formatting layer.As per coding guidelines: "Never use
fmt.Fprintf(os.Stdout/Stderr, ...)orfmt.Println(...). Usedata.*orui.*functions instead."🤖 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.go` around lines 85 - 88, The user-facing cast success message is written directly with fmt.Fprintf via iolib.GetContext().UI(), which bypasses the UI formatting layer. Update the Cast flow in the cast-related codepath to use the existing ui.* or data.* output helper instead of raw formatted I/O, keeping the same message content and routing it through the standard UI abstraction.Source: Coding guidelines
270-305: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap stdlib errors with context instead of returning them raw.
validateKeyAction,validatePauseAction, andvalidateWaitActionreturntime.ParseDuration/regexp.Compileerrors directly (lines 278, 286, 297-298, 304) without identifying which field/action failed, unlikeparseDurationDefaultwhich wraps with%q/%wcontext.♻️ Suggested fix (example for `validatePauseAction`)
func validatePauseAction(action *schema.WorkflowStep) error { if action.Duration == "" { return ErrPauseActionRequiresDuration } - _, err := time.ParseDuration(action.Duration) - return err + if _, err := time.ParseDuration(action.Duration); err != nil { + return fmt.Errorf("invalid pause duration %q: %w", action.Duration, err) + } + return nil }As per coding guidelines: "wrap errors with context using
fmt.Errorf(\"context: %w\", err)."🤖 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.go` around lines 270 - 305, The action validators are returning raw stdlib parse errors without field context, so update validateKeyAction, validatePauseAction, and validateWaitAction to wrap failures from time.ParseDuration and regexp.Compile with fmt.Errorf using the relevant action/field name and %w. Keep the existing validation logic in these helper functions, but make each returned error identify whether the failure came from Key.Interval, Duration, Regex, or Timeout so callers can tell which WorkflowStep field was invalid.Source: Coding guidelines
pkg/runner/step/cast_test.go (1)
1-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for remaining validation and execution branches.
Current tests only cover the
waitaction's text/regex requirement and the headerCommandround trip. Missing coverage:Validatedefault-mode/"steps"-mode required-steps checks,ErrInvalidCastMode,validateWriteAction/validateKeyAction/validatePauseAction, andcastCommandArgs/castModedefaults.As per coding guidelines: "Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages" and "Use table-driven tests for testing multiple scenarios in Go."
🤖 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_test.go` around lines 1 - 83, The current tests only cover session wait validation and recorder header behavior; add table-driven tests around CastHandler.Validate, validateWriteAction, validateKeyAction, validatePauseAction, castCommandArgs, and castMode to exercise the missing branches. Include cases for default mode, steps mode requiring steps, invalid mode returning ErrInvalidCastMode, and the remaining action validators to confirm both valid and invalid inputs. Use the existing CastHandler, startStepRecorder, and related helper symbols to keep the new coverage aligned with the implementation.Source: Coding guidelines
🤖 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 `@cmd/cast/cast.go`:
- Line 20: This introduces a package-local sentinel in cast.go instead of using
the shared static error catalog. Move ErrMissingRenderOutput into
errors/errors.go, then update the cast path to return it via wrapping with
fmt.Errorf("%w", ...) or errors.Join as appropriate. Use the existing
ErrMissingRenderOutput symbol in cast.go only through the shared errors package
so callers can classify it with errors.Is().
In `@cmd/root.go`:
- Around line 705-714: The color-profile setup in Execute leaks forced TrueColor
state into later runs because the non-force branch only updates lipgloss and
never resets the log/theme globals. Update the else branch alongside the
existing forced-color handling in cmd/root.go to mirror the detected terminal
profile for both lipgloss and log, and invalidate the theme cache there as well
so each Execute call starts from the current terminal state.
In `@demo/casts/atmos.yaml`:
- Around line 458-473: The vendor-pull validation in the validate step has an
inconsistent bad-token check because it bans --dry-run without matching the
other cast validations. Update the validation logic in vendor-pull so the bad
list follows the same standard as the other checks, or remove --dry-run from the
forbidden set unless there is a specific vendor-pull-only requirement; use the
validate shell step and its bad list as the location to adjust.
- Around line 526-548: The emulator-aws-lifecycle validation in the shell
snippet is too strict because it treats any "WARN" text as a failure. Update the
validation logic in the cast check to remove the broad "WARN" entry from the bad
list or replace it with a more specific error-only pattern, so legitimate
Terraform/emulator warnings do not fail the check.
In `@demo/casts/fixtures/demo-vendoring/vendor.d/vendor1.yaml`:
- Around line 1-16: The vendor manifest in vendor1.yaml is currently unused and
duplicates the existing ipinfo source, so either wire it into the vendoring flow
by adding it to the main manifest’s spec.imports in vendor.yaml, or remove it if
this fixture is not meant to demonstrate multi-manifest vendoring. Use the
AtmosVendorConfig structure and the spec.imports field to make the intended
relationship explicit.
In `@pkg/asciicast/recorder.go`:
- Line 296: The slug-building logic in recorder.go is stripping any basename
that starts with “atmos”, which incorrectly turns Windows executables like
atmos.exe into “.exe”. Update the slug generation in the relevant recorder path
to skip only when the executable basename exactly matches the Atmos binary name,
using the basename comparison around filepath.Base(arg) instead of TrimPrefix,
so non-exact names remain intact across Linux/macOS/Windows.
In `@pkg/asciicast/render.go`:
- Around line 113-130: The renderTargets helper is wiring the SVG output path to
renderWithAgg, which only generates GIF output, so the opts.SVG branch is
producing the wrong artifact. Update renderTargets and the related
renderTarget/renderWithAgg flow so the SVG option uses a true SVG-capable
renderer (or remove the SVG option until such a renderer exists), while keeping
GIF and MP4 mapped to their correct render functions.
In `@pkg/asciicast/session.go`:
- Around line 151-158: The sessionState.stop method is a no-op, so the goroutine
started in the context watcher around ptmx.Close() can leak after a successful
session. Update sessionState to retain a cancellation mechanism from the context
setup and have stop invoke it, then ensure the watcher goroutine exits when that
cancellation is triggered. Use the sessionState and stop symbols to wire the
cleanup into the existing defer state.stop() path.
- Around line 160-173: finishSession currently kills the child process on
cancel/timeout but returns without reaping it, which can leave a zombie. Update
finishSession in session.go to always call cmd.Wait() after the process is
signaled or when done receives an error, and make sure the timeout and ctx.Done
paths also wait on the child before returning. Use the finishSession and
cmd.Process symbols to locate the cleanup flow.
In `@pkg/io/recorder_test.go`:
- Around line 42-47: The masking check in recorder_test.go is too narrow because
it only rejects one exact unmasked string. Update the assertion around rec.data
in the recorder test to verify the raw secret value is not present anywhere and
that the expected mask token is present, using the existing recorder stream/data
checks to locate the test.
In `@pkg/io/streams.go`:
- Around line 97-105: The Write methods on dynamicWriter, maskedWriter, and
dynamicMaskedWriter currently skip recorder output whenever the underlying
writer returns a partial write with an error or ErrShortWrite. Update each Write
path so it records only the successfully written prefix using p[:n] or
maskedBytes[:n] before returning any error, and keep the existing recordOutput
integration so the recorder matches what actually reached the terminal.
In `@pkg/runner/step/cast.go`:
- Around line 80-91: The error handling in runCastBody/rec.Close drops closeErr
whenever runErr is already set, so the close failure is lost. Update the logic
in cast.go to combine runErr and closeErr with errors.Join instead of choosing
only one, and keep the existing renderCastOutputs flow unchanged. Use the
runCastBody, rec.Close, and renderCastOutputs path to locate the fix.
- Around line 20-29: Move the package-level static error declarations from
cast.go into the centralized errors/errors.go file and update cast.go to
reference those shared errors instead of defining its own errors.New values.
Keep the existing symbol names used by the cast step validation paths (such as
ErrCastStepRequiresSteps, ErrWriteActionRequiresText, and
ErrUnsupportedSessionAction) so call sites can continue to use them
consistently. Ensure any remaining uses in the cast step logic point to the
centralized errors package and that no new dynamic errors are introduced here.
In `@pkg/runner/step/workdir.go`:
- Around line 13-17: The workdir validation errors are introducing package-local
sentinels, which breaks the shared error identity used elsewhere. Move the
workdir path/source/map-key errors into the central error definitions in
errors/errors.go, then update the workdir validation flow in workdir.go to wrap
those shared errors through the standard error builder instead of calling
errors.New locally. Keep the existing validation points for the path/source
checks and key-type handling, but ensure all returned failures use the shared
taxonomy.
In `@website/plugins/file-browser/index.js`:
- Around line 303-321: Tighten the front-matter parsing in the file-browser
plugin by making parseFrontMatter only treat a leading --- block as metadata
when it actually contains valid key: value entries. Update the logic around the
lines/endIndex scan and the loop that fills the data object so that if no valid
pairs are found, the function returns the original content unchanged instead of
stripping the opening section and using body extraction.
In `@website/src/components/CastPlayer/index.tsx`:
- Around line 62-88: The CastPlayer fetch/parsing flow in the effect that loads
`src` should explicitly handle request and JSON parse failures instead of
letting them reject unhandled. In the `fetch(src).then(...).then(...)` chain,
add a `response.ok` check before reading the body, guard the `JSON.parse` calls
for the header and each cast line, and add a `.catch(...)` to reset player state
or render a fallback when the cast is missing or malformed. Use the existing
`setEvents`, `setContent`, `setPosition`, and `renderedEventTime` updates in
`CastPlayer` to keep the UI consistent on failure.
In `@website/src/components/CastPlayer/styles.module.css`:
- Around line 91-107: The styles in CastPlayer/styles.module.css are tripping
stylelint because the quoted SFMono-Regular family and the camelCase cursorBlink
animation/keyframes name violate the current naming/quote rules. Update the
font-family declaration in the CastPlayer stylesheet to use the normalized
family formatting expected by lint, and rename the cursorBlink
animation/keyframes to the approved kebab-case/standard form while keeping the
.cursor rule and `@keyframes` declaration in sync.
In `@website/src/pages/cast-demo.tsx`:
- Around line 43-46: The CastPlayer prompt is using a partial command string, so
the demo reads like a standalone binary instead of the full Atmos CLI
invocation. Update the command passed to CastPlayer in cast-demo.tsx so it
includes the full prefix (for example, the intro should show the Atmos command),
or source the prompt text from the recorded header command used by the cast.
In `@website/static/casts/cli/describe-component.cast`:
- Line 2: The cast fixture includes machine-specific absolute paths in the
recorded payload, which makes the committed asset non-reproducible. Update the
recording used by describe-component.cast so that fields like
component_info.component_path and terraform_config positions (pos.filename) are
normalized to a sandboxed or relative path, and ensure the recorded output is
regenerated from the sanitized source instead of a local developer directory.
In `@website/static/casts/emulator/aws-lifecycle.cast`:
- Around line 59-77: The aws-lifecycle.cast fixture contains non-deterministic
runtime values such as the localhost port, container ID, and resolved provider
patch version, which will keep changing on re-recording. Update the recording
workflow or fixture generation so these values are normalized or masked, and pin
the AWS provider version in the underlying emulator/Terraform setup to keep the
output stable. Use the aws-lifecycle.cast recording and the emulator/Terraform
initialization steps as the place to fix this.
---
Outside diff comments:
In `@agent-skills/skills/atmos-asciicast/SKILL.md`:
- Around line 1-36: The atmos-asciicast SKILL.md is over the allowed agent-skill
line limit; trim it to a minimal entry point by keeping only the front matter,
short description, and a link to a new reference doc. Move the detailed
“Defaults”, “Fixture Policy”, and “Authoring Checklist” content into
references/atmos-asciicast-authoring.md, and update the SKILL.md text so it
points readers to that guide while preserving the skill name atmos-asciicast.
In `@pkg/io/context.go`:
- Around line 80-84: The write path in context output handling is dropping
partially written data when fmt.Fprint returns both bytes written and an error.
Update the output flow around the writer call and recordOutput so any
successfully written prefix is recorded before returning the wrapped error,
ensuring the recorder stays in sync with what the terminal actually received.
In `@pkg/provisioner/source/vendor.go`:
- Around line 44-50: VendorSource currently accepts workflow-resolved targetDir
input and can reach the replace/remove branch that recursively deletes existing
paths, so add an early validation step in VendorSource before any os.Stat or
os.RemoveAll calls to reject obviously unsafe targets like ".", "./", and
filesystem roots. Keep the guard near the top of VendorSource, before the
replace logic that uses targetDir, and return a hard failure for invalid paths
so reset-enabled flows cannot delete unintended directories.
In `@pkg/terminal/pty/pty_test.go`:
- Around line 384-422: The masking test only verifies the terminal buffer and
misses the cast event path, so it can still pass if recordingWriter writes a raw
secret into the recorded event. Harden TestRecordingWriterMasksOutput to also
assert the event payload emitted by recordingWriter is masked, using
recordingWriter and its write/recording path to verify both stdout and the
captured event contain iolib.MaskReplacement and do not contain the secretKey.
---
Nitpick comments:
In `@cmd/cast/cast.go`:
- Around line 46-49: The `renderCmd` flags are being registered directly in
`init()`, which bypasses the repo’s standard flag plumbing and leaves
`GetFlagsBuilder()` unused. Move the `svg`, `gif`, and `mp4` options onto a
`flags.NewStandardParser()`-based setup for the `render` command, and wire that
parser through the command’s flag builder path so `renderCmd` follows the same
pattern as the rest of `cmd/` (including the related code around
`GetFlagsBuilder()` and the later flag handling).
In `@cmd/root.go`:
- Around line 379-382: The status messages in the cast recording flow are
written directly with fmt.Fprintf to os.Stderr, bypassing the CLI output
abstractions. Update the restore()/recording completion path in root.go to emit
both the failure and success messages through the existing ui.* or data.*
helpers instead of direct stderr writes, keeping the same cast recording context
via the rec.Path() and error handling logic.
In `@demo/casts/fixtures/demo-vendoring/atmos.yaml`:
- Around line 19-27: The comment label in the vendor base_path examples is
incorrect: the “Absolute path” example in the vendoring YAML fixture points to a
relative path. Update the comment near the vendor/base_path examples in
atmos.yaml so the label matches the actual path type, or replace it with a truly
absolute path; use the vendor section and the base_path examples as the
reference points.
In `@demo/casts/fixtures/demo-vendoring/vendor/vendor1.yaml`:
- Around line 10-16: The vendoring fixture for the ipinfo component is using a
მოძრable branch via version: "main", which can make cast output
non-deterministic. Update the vendor entry in vendor1.yaml to reference a fixed
tag or commit SHA instead, keeping the same source/targets structure and the
component/version placeholders so the fixture remains reproducible.
In `@docs/prd/asciicast-recording.md`:
- Around line 199-200: The issue is inconsistent use of “regex” vs “regexp” in
the asciicast recording PRD bullets. Update the affected sentence in this
section to use one term consistently, and keep the Go package reference in the
`regexp` mention while aligning the surrounding wording with the `regex` field
name used elsewhere. Use the nearby bullets in the same section to verify the
final phrasing stays consistent across `regex`/`regexp`.
In `@pkg/asciicast/recorder_test.go`:
- Around line 36-41: The test in recorder_test.go is checking the error by
matching its text instead of asserting the sentinel, so update the Start/Options
explicit path collision assertion to use errors.Is against ErrCastOutputExists.
Keep the existing failure path check, but replace the
strings.Contains(err.Error(), ...) validation with a direct wrapped-error check
so the test follows the error-handling guideline consistently.
In `@pkg/asciicast/recorder.go`:
- Around line 32-72: Add Go doc comments for every exported API in the recorder
package. Document the public types and functions shown here, including
ErrCastOutputExists, Options, Recorder, Header, and Start, and continue through
the rest of the package for any exported methods/functions referenced by the
review. Keep the comments in standard Go style so each exported symbol clearly
describes its purpose and behavior.
In `@pkg/asciicast/render.go`:
- Around line 17-22: The sentinel errors in render.go are defined locally
instead of using the centralized error definitions, so move ErrEmptyCastFile,
ErrRenderOutputExists, ErrMissingAgg, and ErrMissingFFmpeg into the shared
errors package and update the render-related code to reference those shared
symbols. Keep the render logic in the render package using the imported errors
identifiers rather than declaring new package-level error vars.
In `@pkg/asciicast/session.go`:
- Around line 25-52: Add Go doc comments for every exported session API symbol
in this block: ErrUnknownSessionAction, SessionAction, SessionOptions, and
RunSession. Place a concise comment immediately above each declaration in
pkg/asciicast/session.go, following standard Go documentation conventions so the
purpose of each exported error, type, and function is clear.
In `@pkg/runner/step/cast_test.go`:
- Around line 1-83: The current tests only cover session wait validation and
recorder header behavior; add table-driven tests around CastHandler.Validate,
validateWriteAction, validateKeyAction, validatePauseAction, castCommandArgs,
and castMode to exercise the missing branches. Include cases for default mode,
steps mode requiring steps, invalid mode returning ErrInvalidCastMode, and the
remaining action validators to confirm both valid and invalid inputs. Use the
existing CastHandler, startStepRecorder, and related helper symbols to keep the
new coverage aligned with the implementation.
In `@pkg/runner/step/cast.go`:
- Around line 85-88: The user-facing cast success message is written directly
with fmt.Fprintf via iolib.GetContext().UI(), which bypasses the UI formatting
layer. Update the Cast flow in the cast-related codepath to use the existing
ui.* or data.* output helper instead of raw formatted I/O, keeping the same
message content and routing it through the standard UI abstraction.
- Around line 270-305: The action validators are returning raw stdlib parse
errors without field context, so update validateKeyAction, validatePauseAction,
and validateWaitAction to wrap failures from time.ParseDuration and
regexp.Compile with fmt.Errorf using the relevant action/field name and %w. Keep
the existing validation logic in these helper functions, but make each returned
error identify whether the failure came from Key.Interval, Duration, Regex, or
Timeout so callers can tell which WorkflowStep field was invalid.
In `@pkg/schema/schema.go`:
- Around line 142-152: Add Go doc comments to the exported cast configuration
types in schema.go. Document both CastConfig and CastRecordingConfig with
comments that begin with the type name and briefly describe their purpose,
following standard Go documentation conventions so exported types in the schema
package are lint/doc compliant.
In `@pkg/schema/workflow.go`:
- Around line 574-579: Add a Go doc comment for the exported CastOutput type so
it starts with “CastOutput” and follows standard Go documentation conventions.
Update the type declaration in workflow.go to include a concise comment
describing what CastOutput represents, keeping the existing fields Cast, SVG,
GIF, and MP4 unchanged.
In `@website/src/components/FileBrowser/FileViewer.tsx`:
- Around line 19-30: The stripFrontmatter helper is duplicated in FileViewer and
EmbedExample, so move the shared logic into a common utility module and have
both FileViewer and EmbedExample import that single implementation. Keep the
existing behavior of stripFrontmatter unchanged while removing the local copy
from FileViewer and updating the EmbedExample/index.tsx usage to reference the
shared helper.
🪄 Autofix (Beta)
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
Run ID: cf9004e3-2149-4d68-8146-337c099732db
📒 Files selected for processing (138)
.gitattributesagent-skills/skills/atmos-asciicast/SKILL.mdcmd/cast/cast.gocmd/cast_flag_test.gocmd/root.godemo/casts/README.mddemo/casts/atmos.yamldemo/casts/fixtures/basic/.gitignoredemo/casts/fixtures/basic/README.mddemo/casts/fixtures/basic/atmos.yamldemo/casts/fixtures/basic/components/terraform/mock/README.mddemo/casts/fixtures/basic/components/terraform/mock/main.tfdemo/casts/fixtures/basic/stacks/catalog/mock.yamldemo/casts/fixtures/basic/stacks/deploy/nonprod.yamldemo/casts/fixtures/basic/stacks/deploy/prod.yamldemo/casts/fixtures/demo-vendoring/README.mddemo/casts/fixtures/demo-vendoring/atmos.yamldemo/casts/fixtures/demo-vendoring/stacks/catalog/myapp.yamldemo/casts/fixtures/demo-vendoring/stacks/deploy/dev.yamldemo/casts/fixtures/demo-vendoring/stacks/deploy/prod.yamldemo/casts/fixtures/demo-vendoring/stacks/deploy/staging.yamldemo/casts/fixtures/demo-vendoring/vendor.d/vendor1.yamldemo/casts/fixtures/demo-vendoring/vendor.yamldemo/casts/fixtures/demo-vendoring/vendor/vendor1.yamldemo/casts/fixtures/demo-vendoring/vendor/vendor2.yamldemo/casts/fixtures/native-terraform/.gitignoredemo/casts/fixtures/native-terraform/README.mddemo/casts/fixtures/native-terraform/atmos.yamldemo/casts/fixtures/native-terraform/components/terraform/vpc/envs/dev.tfvarsdemo/casts/fixtures/native-terraform/components/terraform/vpc/envs/prod.tfvarsdemo/casts/fixtures/native-terraform/components/terraform/vpc/main.tfdemo/casts/fixtures/native-terraform/components/terraform/vpc/outputs.tfdemo/casts/fixtures/native-terraform/components/terraform/vpc/variables.tfdemo/casts/fixtures/native-terraform/stacks/_defaults.yamldemo/casts/fixtures/native-terraform/stacks/dev.yamldemo/casts/fixtures/native-terraform/stacks/prod.yamldocs/prd/asciicast-recording.mdexamples/demo-vendoring/README.mdexamples/emulator-aws/README.mdexamples/emulator-aws/atmos.yamlexamples/native-terraform/README.mdinternal/exec/vendor_model.gopkg/asciicast/recorder.gopkg/asciicast/recorder_test.gopkg/asciicast/render.gopkg/asciicast/render_test.gopkg/asciicast/session.gopkg/config/load.gopkg/io/context.gopkg/io/recorder.gopkg/io/recorder_test.gopkg/io/streams.gopkg/provisioner/source/vendor.gopkg/provisioner/source/vendor_test.gopkg/runner/step/cast.gopkg/runner/step/cast_test.gopkg/runner/step/workdir.gopkg/runner/step/workdir_test.gopkg/schema/schema.gopkg/schema/task.gopkg/schema/workflow.gopkg/terminal/pty/pty.gopkg/terminal/pty/pty_test.gotests/snapshots/TestCLICommands_atmos_--chdir_config_isolation.stdout.goldentests/snapshots/TestCLICommands_atmos_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_--help_config_aliases_section.stdout.goldentests/snapshots/TestCLICommands_atmos_about_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stdout.goldentests/snapshots/TestCLICommands_atmos_atlantis_help.stdout.goldentests/snapshots/TestCLICommands_atmos_auth_env_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_auth_exec_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_auth_login_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_auth_user_configure_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_auth_validate_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_auth_whoami_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_-f_yaml.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_config_imports.stdout.goldentests/snapshots/TestCLICommands_atmos_describe_configuration.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.goldentests/snapshots/TestCLICommands_atmos_helmfile_help.stdout.goldentests/snapshots/TestCLICommands_atmos_non-existent.stderr.goldentests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.goldentests/snapshots/TestCLICommands_atmos_terraform_help.stdout.goldentests/snapshots/TestCLICommands_atmos_toolchain_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_toolchain_info_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_toolchain_install_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.goldentests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.goldentests/snapshots/TestCLICommands_config_alias_tp_--help_shows_terraform_plan_help.stdout.goldentests/snapshots/TestCLICommands_config_alias_tr_--help_shows_terraform_help.stdout.goldentests/snapshots/TestCLICommands_deploy_component_help_shows_stack_flag.stdout.goldentests/snapshots/TestCLICommands_describe_component_help_shows_stack_flag.stdout.goldentests/snapshots/TestCLICommands_echo_info_help_shows_verbose_flag.stdout.goldentests/snapshots/TestCLICommands_help_flag_works.stdout.goldentests/snapshots/TestCLICommands_indentation.stdout.goldentests/snapshots/TestCLICommands_secrets-masking_describe_config.stdout.goldentests/snapshots/TestCLICommands_show_component_help_shows_inherited_stack_flag.stdout.goldentests/snapshots/TestCLICommands_terraform_help_shows_stack_flag.stdout.goldentests/snapshots/TestCLICommands_terraform_provision_help_shows_inherited_stack_flag.stdout.goldentests/snapshots/TestCLICommands_tf_plan_help_shows_inherited_stack_flag.stdout.goldenwebsite/blog/2026-06-30-atmos-demo-workflows.mdxwebsite/docs/cli/commands/describe/describe-component.mdxwebsite/docs/cli/commands/describe/describe-stacks.mdxwebsite/docs/cli/commands/emulator/usage.mdxwebsite/docs/cli/commands/list/list-stacks.mdxwebsite/docs/cli/commands/terraform/terraform-deploy.mdxwebsite/docs/cli/commands/terraform/terraform-output.mdxwebsite/docs/cli/commands/terraform/terraform-plan.mdxwebsite/docs/cli/commands/vendor/vendor-pull.mdxwebsite/plugins/file-browser/index.jswebsite/src/components/CastPlayer/index.tsxwebsite/src/components/CastPlayer/styles.module.csswebsite/src/components/EmbedExample/index.tsxwebsite/src/components/EmbedExample/styles.module.csswebsite/src/components/FileBrowser/DirectoryPage.tsxwebsite/src/components/FileBrowser/FileViewer.tsxwebsite/src/components/FileBrowser/styles.module.csswebsite/src/components/FileBrowser/types.tswebsite/src/pages/cast-demo.module.csswebsite/src/pages/cast-demo.tsxwebsite/static/casts/cli/describe-component.castwebsite/static/casts/cli/describe-stacks.castwebsite/static/casts/cli/list-stacks.castwebsite/static/casts/emulator/aws-lifecycle.castwebsite/static/casts/terraform/deploy.castwebsite/static/casts/terraform/output.castwebsite/static/casts/terraform/plan.castwebsite/static/casts/vendor/pull.cast
|
Warning Release Documentation RequiredThis PR is labeled
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
pkg/schema/task_validate.go (1)
377-384: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd a nested
scriptcase to the concurrent-step tests.validateConcurrentChildnow acceptstype: script, butpkg/schema/workflow_control_test.goonly covers the rejectedinputchild. Add aparallel/concurrentchild test forTaskTypeScriptso this path stays covered.🤖 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/schema/task_validate.go` around lines 377 - 384, Add test coverage for the new allowed concurrent child type in validateConcurrentChild. In pkg/schema/workflow_control_test.go, extend the concurrent/parallel step tests to include a child WorkflowStep with TaskTypeScript, alongside the existing rejected input case, so the TaskTypeScript branch in validateConcurrentChild stays exercised.pkg/workflow/executor.go (1)
360-377: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRoute
type: scriptsteps throughrunShellStepunconditionally.
The container guard blocks plain script steps, so they fall through torunCommandand hit the unsupported-type error even thoughrunShellStepalready handles the host-shell fallback.🐛 Suggested fix
- if commandType == schema.TaskTypeScript && (StepContainerOverride(step) || (params.WorkflowDefinition.Container != nil && params.WorkflowDefinition.Container.IsEnabled() && !StepContainerDisabled(step))) { + if commandType == schema.TaskTypeScript { err = e.runShellStep(params, step, cmdParams, workDir)🤖 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/workflow/executor.go` around lines 360 - 377, The command dispatch in executor.go is still gating `schema.TaskTypeScript` behind the container check in `executeStep`, which lets plain script steps fall through to `runCommand` and fail. Update the `executeStep` branching so `type: script` always goes through `runShellStep` first, relying on its host-shell fallback, and keep the `stepPkg.IsExtendedStepType` path unchanged for extended steps.
🧹 Nitpick comments (2)
internal/exec/workflow_utils.go (1)
661-709: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDuplicated container-dispatch pattern between
"shell"andscriptcases.Lines 663-707 replicate the same "container override → container-enabled → default retry.Do" structure already used for
commandType == "shell"at lines 551-604 (only theCommandstring differs: raw command vs.process.FormatScriptDisplay(...)). Consider extracting a shared helper (e.g.runContainerAwareStep(ctx, workflow, workflowPath, atmosConfig, workflowDefinition, &step, workDir, displayCommand, stepEnv, dryRun, &activeContainer)) to remove this duplication and keep the two paths in sync going forward.🤖 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 `@internal/exec/workflow_utils.go` around lines 661 - 709, The script execution branch in workflow step handling duplicates the same container-dispatch flow already used for the shell path, so refactor the logic around commandType == schema.TaskTypeScript to reuse a shared helper instead of repeating the override/enabled/default retry.Do structure. Extract the common behavior from the step execution code in workflow_utils.go into a helper that takes the step, workflow context, display command, and activeContainer handling, and use it for both the shell and script cases so only the command formatting differs.pkg/schema/task.go (1)
160-170: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd per-field doc comments for cast/session action fields, matching
WorkflowStep.Every other field group in this struct (interactive, log, HTTP, etc.) carries per-field trailing comments; these ten fields are undocumented even though the mirrored
WorkflowStepfields (pkg/schema/workflow.go) already have them. Copying those comments over keeps the two structs in sync and avoids future drift.♻️ Proposed fix
// Cast step and session action fields. - Mode string `yaml:"mode,omitempty" json:"mode,omitempty" mapstructure:"mode"` - Shell string `yaml:"shell,omitempty" json:"shell,omitempty" mapstructure:"shell"` - WriteRate string `yaml:"write_rate,omitempty" json:"write_rate,omitempty" mapstructure:"write_rate"` - KeyInterval string `yaml:"key_interval,omitempty" json:"key_interval,omitempty" mapstructure:"key_interval"` - Text string `yaml:"text,omitempty" json:"text,omitempty" mapstructure:"text"` - Regex string `yaml:"regex,omitempty" json:"regex,omitempty" mapstructure:"regex"` - Key string `yaml:"key,omitempty" json:"key,omitempty" mapstructure:"key"` - Duration string `yaml:"duration,omitempty" json:"duration,omitempty" mapstructure:"duration"` - Interval string `yaml:"interval,omitempty" json:"interval,omitempty" mapstructure:"interval"` - Repeat int `yaml:"repeat,omitempty" json:"repeat,omitempty" mapstructure:"repeat"` + Mode string `yaml:"mode,omitempty" json:"mode,omitempty" mapstructure:"mode"` // Cast mode: steps or session. + Shell string `yaml:"shell,omitempty" json:"shell,omitempty" mapstructure:"shell"` // Shell for session mode. + WriteRate string `yaml:"write_rate,omitempty" json:"write_rate,omitempty" mapstructure:"write_rate"` // Default delay between written bytes. + KeyInterval string `yaml:"key_interval,omitempty" json:"key_interval,omitempty" mapstructure:"key_interval"` // Default delay between repeated keys. + Text string `yaml:"text,omitempty" json:"text,omitempty" mapstructure:"text"` // Text for write/wait actions. + Regex string `yaml:"regex,omitempty" json:"regex,omitempty" mapstructure:"regex"` // Regex for wait actions. + Key string `yaml:"key,omitempty" json:"key,omitempty" mapstructure:"key"` // Key name for key actions. + Duration string `yaml:"duration,omitempty" json:"duration,omitempty" mapstructure:"duration"` // Duration for pause/wait actions. + Interval string `yaml:"interval,omitempty" json:"interval,omitempty" mapstructure:"interval"` // Per-key repeat delay override. + Repeat int `yaml:"repeat,omitempty" json:"repeat,omitempty" mapstructure:"repeat"` // Key repeat count.🤖 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/schema/task.go` around lines 160 - 170, Add per-field doc comments for the cast/session action fields in the task schema struct so they match the corresponding fields in WorkflowStep. Update the field group containing Mode, Shell, WriteRate, KeyInterval, Text, Regex, Key, Duration, Interval, and Repeat with the same trailing descriptions already used in pkg/schema/workflow.go, keeping the names and meanings aligned between the two structs.
🤖 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/process/script.go`:
- Around line 32-41: In the script execution helper in pkg/process/script.go,
the default branch currently falls back to "-c" for any unhandled interpreter,
which causes some runtimes like Ruby to only syntax-check instead of running the
script. Update the fallback in the interpreter selection logic so the generic
path uses stdin-based execution like the Python case, or explicitly rejects
unsupported interpreters, and keep the fix centered in the switch over name that
builds the argv and reader.
In `@pkg/runner/step/script_test.go`:
- Around line 71-73: The test fixture path in script_test.go is built with
string concatenation, which violates the repo’s path rule. Update the fixture
path creation in the test that uses t.TempDir and os.WriteFile to use
filepath.Join instead of dir + "/sops-secrets.cast", keeping the same temporary
directory and filename while relying on the filepath package.
In `@pkg/runner/step/script.go`:
- Around line 124-150: Make the `workDir` and `step.Env` resolution failures
follow the same static-error wrapping convention as `interpreter` and `script`.
In `ScriptHandler.resolveWorkingDirectory` and `ScriptHandler.resolveEnv`,
replace the direct `fmt.Errorf(...)` returns with an
`errUtils.Build(errUtils.ErrTemplateEvaluation)...Err()`-style wrapper using the
existing sentinel from `errors/errors.go`, while preserving the current
contextual message and underlying error. Keep `vars.Resolve`,
`vars.ResolveEnvMap`, and the `step.Name` context intact so callers can use
`errors.Is()` consistently across all template-evaluation failures.
In `@pkg/schema/command.go`:
- Around line 111-133: decodeCommandEnvMapValue currently returns dynamic
fmt.Errorf-wrapped failures for decoder creation and Decode, but the review
requires static sentinel wrapping only. Add or reuse appropriate package-level
errors from errors/errors.go for the decoder initialization and command env
decode failure paths, then update the mapstructure.NewDecoder and decoder.Decode
branches in decodeCommandEnvMapValue to wrap those static errors while keeping
the existing command key context.
In `@pkg/utils/shell_utils.go`:
- Around line 97-98: The comment block in the shell utilities code is missing
terminal punctuation, which will fail the godot lint rule. Update the comment
near the environment-handling logic in the relevant helper so the last line ends
with a period, keeping the wording the same and ensuring the comment style
matches the rest of the Go codebase.
In `@pkg/workflow/control_executor.go`:
- Around line 90-109: The nested control-step execution path in executeScript
currently drops working_directory, so script, shell, and atmos children always
run from the default directory instead of the inherited one. Thread
working_directory through ControlCommandRequest from executeScript and the
related control execution methods, then make the command runner use that value
when launching the child process so it inherits the workflow/step directory
correctly.
In `@pkg/workflow/executor.go`:
- Around line 360-373: The early success return in `executor.go` for the
`commandType == schema.TaskTypeScript` branch skips registering step outputs.
Update `e.runShellStep` success handling in `runStep` to call
`e.stepVars.SetWithOutputs(...)` before returning, matching the fallback success
path so `StepResult` and `finalStack` are preserved and `{{
.steps.<name>.outputs.* }}` works for container-routed script steps. Use the
existing `step.Name`, `cmdParams`, and `stepVars` flow in this branch to wire
outputs consistently.
---
Outside diff comments:
In `@pkg/schema/task_validate.go`:
- Around line 377-384: Add test coverage for the new allowed concurrent child
type in validateConcurrentChild. In pkg/schema/workflow_control_test.go, extend
the concurrent/parallel step tests to include a child WorkflowStep with
TaskTypeScript, alongside the existing rejected input case, so the
TaskTypeScript branch in validateConcurrentChild stays exercised.
In `@pkg/workflow/executor.go`:
- Around line 360-377: The command dispatch in executor.go is still gating
`schema.TaskTypeScript` behind the container check in `executeStep`, which lets
plain script steps fall through to `runCommand` and fail. Update the
`executeStep` branching so `type: script` always goes through `runShellStep`
first, relying on its host-shell fallback, and keep the
`stepPkg.IsExtendedStepType` path unchanged for extended steps.
---
Nitpick comments:
In `@internal/exec/workflow_utils.go`:
- Around line 661-709: The script execution branch in workflow step handling
duplicates the same container-dispatch flow already used for the shell path, so
refactor the logic around commandType == schema.TaskTypeScript to reuse a shared
helper instead of repeating the override/enabled/default retry.Do structure.
Extract the common behavior from the step execution code in workflow_utils.go
into a helper that takes the step, workflow context, display command, and
activeContainer handling, and use it for both the shell and script cases so only
the command formatting differs.
In `@pkg/schema/task.go`:
- Around line 160-170: Add per-field doc comments for the cast/session action
fields in the task schema struct so they match the corresponding fields in
WorkflowStep. Update the field group containing Mode, Shell, WriteRate,
KeyInterval, Text, Regex, Key, Duration, Interval, and Repeat with the same
trailing descriptions already used in pkg/schema/workflow.go, keeping the names
and meanings aligned between the two structs.
🪄 Autofix (Beta)
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
Run ID: 974392f6-9743-4039-b78b-1a15185e462b
📒 Files selected for processing (41)
cmd/cmd_utils.gocmd/custom_command_integration_test.godemo/casts/README.mddemo/casts/atmos.yamldemo/casts/fixtures/native-terraform/atmos.yamlexamples/custom-commands/README.mdexamples/quick-start-advanced/README.mdexamples/quick-start-simple/README.mdexamples/sops-secrets/README.mdexamples/toolchain/README.mdinternal/exec/shell_utils.gointernal/exec/workflow_utils.gopkg/config/load.gopkg/config/load_command_env_test.gopkg/container/ephemeral.gopkg/process/script.gopkg/process/script_test.gopkg/runner/step/command_handlers_test.gopkg/runner/step/script.gopkg/runner/step/script_test.gopkg/runner/step/variables.gopkg/runner/step/variables_test.gopkg/schema/command.gopkg/schema/command_test.gopkg/schema/task.gopkg/schema/task_test.gopkg/schema/task_validate.gopkg/schema/task_validate_test.gopkg/schema/workflow.gopkg/utils/shell_utils.gopkg/workflow/container.gopkg/workflow/control_executor.gopkg/workflow/control_executor_test.gopkg/workflow/executor.gopkg/workflow/executor_test.gowebsite/docs/cli/commands/workflow.mdxwebsite/static/casts/examples/custom-commands.castwebsite/static/casts/examples/quick-start-advanced.castwebsite/static/casts/examples/quick-start-simple.castwebsite/static/casts/examples/sops-secrets.castwebsite/static/casts/examples/toolchain.cast
✅ Files skipped from review due to trivial changes (12)
- examples/sops-secrets/README.md
- website/static/casts/examples/custom-commands.cast
- examples/toolchain/README.md
- examples/custom-commands/README.md
- website/static/casts/examples/quick-start-simple.cast
- website/static/casts/examples/quick-start-advanced.cast
- demo/casts/README.md
- website/static/casts/examples/sops-secrets.cast
- examples/quick-start-advanced/README.md
- website/static/casts/examples/toolchain.cast
- examples/quick-start-simple/README.md
- website/docs/cli/commands/workflow.mdx
🚧 Files skipped from review as they are similar to previous changes (2)
- demo/casts/fixtures/native-terraform/atmos.yaml
- pkg/schema/workflow.go
|
Warning Release Documentation RequiredThis PR is labeled
|
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
# Conflicts: # pkg/utils/yaml_utils.go # website/src/data/roadmap.js
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
# Conflicts: # docs/prd/flag-handling/unified-flag-parsing.md # tests/test-cases/atmos-cli-imports.yaml # tests/test-cases/auth-validation.yaml
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
# Conflicts: # cmd/cmd_utils.go # cmd/custom_command_integration_test.go # docs/prd/flag-handling/unified-flag-parsing.md # pkg/utils/yaml_utils.go
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
# Conflicts: # cmd/cmd_utils.go # cmd/root.go # internal/exec/testmain_test.go # internal/exec/workflow_utils.go # internal/exec/workflow_utils_test.go # scripts/generate-notice.sh
Resource Changes Found for
|
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
# Conflicts: # cmd/cast/cast.go # cmd/cast/recording.go # cmd/cast/recording_test.go # cmd/cmd_utils.go # cmd/help_template.go # cmd/root.go # cmd/root_helpers_test.go # errors/errors.go # pkg/asciicast/cellgrid.go # pkg/asciicast/cellgrid_test.go # pkg/asciicast/exec.go # pkg/asciicast/exec_test.go # pkg/asciicast/recorder.go # pkg/asciicast/recorder_test.go # pkg/asciicast/render_static_test.go # pkg/asciicast/render_test.go # pkg/asciicast/session.go # pkg/asciicast/session_test.go # pkg/asciicast/session_unix.go # pkg/asciicast/session_windows.go # pkg/asciicast/testmain_test.go # pkg/config/command_merge_core_test.go # pkg/datafetcher/schema/config/global/1.0.json # pkg/process/script.go # pkg/process/script_test.go # pkg/provisioner/source/vendor.go # pkg/provisioner/source/vendor_test.go # pkg/runner/step/cast.go # pkg/runner/step/cast_simulate.go # pkg/runner/step/cast_test.go # pkg/runner/step/script_test.go # pkg/runner/step/workdir_test.go # pkg/schema/command_test.go # pkg/schema/schema.go # pkg/schema/workflow.go # pkg/schema/workflow_control_test.go # pkg/terminal/pty/pty_test.go # pkg/terminal/terminal.go # pkg/terminal/terminal_test.go # pkg/ui/formatter_test.go # tests/cli_auth_login_provider_test.go # tests/snapshots/TestCLICommands_atmos_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_--help_config_aliases_section.stdout.golden # tests/snapshots/TestCLICommands_atmos_about_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_atlantis_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_atlantis_generate_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_atlantis_generate_help.stdout.golden # tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_atlantis_generate_repo-config_help.stdout.golden # tests/snapshots/TestCLICommands_atmos_atlantis_help.stdout.golden # tests/snapshots/TestCLICommands_atmos_auth_env_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_auth_exec_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_auth_login_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_auth_user_configure_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_auth_validate_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_auth_whoami_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_helmfile_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_helmfile_apply_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_helmfile_apply_help.stdout.golden # tests/snapshots/TestCLICommands_atmos_helmfile_help.stdout.golden # tests/snapshots/TestCLICommands_atmos_terraform_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_terraform_--help_alias_subcommand_check.stdout.golden # tests/snapshots/TestCLICommands_atmos_terraform_apply_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_terraform_apply_help.stdout.golden # tests/snapshots/TestCLICommands_atmos_terraform_help.stdout.golden # tests/snapshots/TestCLICommands_atmos_toolchain_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_toolchain_info_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_toolchain_install_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_validate_editorconfig_--help.stdout.golden # tests/snapshots/TestCLICommands_atmos_validate_editorconfig_help.stdout.golden # tests/snapshots/TestCLICommands_config_alias_tp_--help_shows_terraform_plan_help.stdout.golden # tests/snapshots/TestCLICommands_config_alias_tr_--help_shows_terraform_help.stdout.golden # tests/snapshots/TestCLICommands_deploy_component_help_shows_stack_flag.stdout.golden # tests/snapshots/TestCLICommands_describe_component_help_shows_stack_flag.stdout.golden # tests/snapshots/TestCLICommands_echo_info_help_shows_verbose_flag.stdout.golden # tests/snapshots/TestCLICommands_help_flag_works.stdout.golden # tests/snapshots/TestCLICommands_show_component_help_shows_inherited_stack_flag.stdout.golden # tests/snapshots/TestCLICommands_terraform_help_shows_stack_flag.stdout.golden # tests/snapshots/TestCLICommands_terraform_provision_help_shows_inherited_stack_flag.stdout.golden # tests/snapshots/TestCLICommands_tf_plan_help_shows_inherited_stack_flag.stdout.golden
Prevents "Incomplete lock file information" warnings when the recorded secrets-masking cast (or anyone running the example fresh) plans the demo-dev-test workspace, matching the per-instance lock convention already used in examples/quick-start-advanced.
|
💥 This pull request now has conflicts. Could you fix it Erik Osterman (Cloud Posse) (@osterman)? 🙏 |
# Conflicts: # demo/screengrabs/demo-stacks.txt # internal/exec/docs_generate_test.go
…ndor commands The new `atmos config`, `atmos stack`, and `atmos vendor get/set/update/diff` docs pages (from the format-preserving YAML edits merge) still imported the `Screengrab` component, which this branch already removed in favor of the static-cast `CastPlayer` system. That broke the website build with 25 "Module not found" errors. Converts all 25 pages to `CastPlayer` and records their `--help` screengrabs via the existing `casts generate screengrabs cli` pipeline. Recording those screengrabs surfaced a real regression: a prior merge decision made non-TTY width detection ignore COLUMNS entirely (to keep CI snapshots stable), which silently broke the docs screengrab pipeline's 90-column recording width, causing --help output to wrap at the 120-column default instead. Fixed by: - Wiring ATMOS_CAST_RECORDING_WIDTH/HEIGHT to cast.recording.width/height (pkg/config/load.go), so the recorder's own declared dimensions honor it. - Letting --force-tty mode honor ATMOS_CAST_RECORDING_WIDTH specifically (pkg/terminal/terminal.go), since that variable is only ever set deliberately by recording pipelines, never ambiently like COLUMNS.
Andriy Knysh (aknysh)
approved these changes
Jul 9, 2026
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
These changes were released in v1.223.0-rc.7. |
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
3 participants
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
why
references
Summary by CodeRabbit
--cast(supports generated.castoutput and rendering to.gif/.mp4and more formats).atmos cast playandatmos cast render.scriptandworkdirstep capabilities.CastPlayercomponent and an embedded cast demo gallery.