Repository navigation
feat: terminal steps - tty/interactive fields and exec step type - #2602
Merged
Merged
Conversation
…suspension Add Interactive/Tty bools to Task and WorkflowStep with converter mappings. Add pkg/signals with a nestable interrupt-exit suspension counter, consulted by the main signal handler so foreground interactive steps own Ctrl-C. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ward The stdin copy goroutine was joined via WaitGroup, but io.Copy from a terminal only returns on the next read after the PTY closes - so ExecWithPTY hung until a keypress after the child exited. Detach the stdin copier (docker-CLI pattern) and drain IO errors non-blockingly. DisableStdinForward supports docker's -t-without-i semantics: the child gets a TTY but host input is not forwarded and the host terminal stays cooked. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Runs a shell command attached to the user's terminal: under a PTY when supported (masking preserved, raw mode routes Ctrl-C to the child for interactive sessions), otherwise via direct fd inheritance with a visible warning that masking is unavailable. Interactive sessions suspend the global SIGINT-exit handler; child exit codes propagate as ExitCodeError. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…l sessions Shell steps with tty: true attach the user's terminal via RunShellSession; interactive: true suspends the Atmos SIGINT-exit handler so the step owns Ctrl-C. Plain steps keep the existing masked shell-interpreter path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Workflow shell steps with tty: true attach the user's terminal via RunShellSession (output modes don't apply); interactive: true suspends the Atmos SIGINT-exit handler so the step owns Ctrl-C. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…step handler Shell tasks with tty: true attach the user's terminal via RunShellSession (no capturable output; exit code recorded in metadata). Interactive tasks attach host stdin, force raw output mode (buffered modes would hide prompts), and suspend the Atmos SIGINT-exit handler while running. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
requested a review
from a team
as a code owner
June 11, 2026 17:03
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request. 
|
Erik Osterman (Cloud Posse) (osterman)
added
the
minor
Dependency Review✅ No vulnerabilities or license issues found.Scanned FilesNone |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
💤 Files with no reviewable changes (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds terminal-aware shell execution (tty, interactive) for full-screen programs and interactive sessions, introduces ChangesTerminal-Aware Steps and Process Replacement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 6
🤖 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/custom_command_shell_step_test.go`:
- Around line 78-87: The test currently shells out to the Unix `sleep` binary
and skips Windows; replace that with a Go-native test helper subprocess: create
a small helper program (invoked via os.Executable()) that reads a duration arg
and calls time.Sleep(duration) then exits, and update the test to invoke that
helper (e.g., exec.Command(exePath, "helper-sleep", "500ms")) instead of "sleep
0.5"; keep the same concurrency pattern using done channel and the same step
(schema.Task with TypeShell and Interactive) and call executeShellStep with the
helper command string so the timing check runs cross-platform without skipping.
Ensure the helper distinguishes mode by args (e.g., "helper-sleep") so normal
test runs unaffected.
In `@pkg/process/shell_session.go`:
- Around line 61-70: RunShellSession currently calls u.GetNextShellLevel()
(which parses ATMOS_SHLVL) before checking spec.DryRun; move the DryRun check
earlier or conditionally call GetNextShellLevel only when not spec.DryRun to
avoid failing on malformed ATMOS_SHLVL for dry-runs—update RunShellSession so
the early return for spec.DryRun happens before invoking u.GetNextShellLevel()
(or wrap the call in if !spec.DryRun { shellLevel, err := u.GetNextShellLevel()
... }) while keeping the rest of the logic intact.
In `@pkg/runner/runner.go`:
- Around line 163-173: The TTY branch is passing Options.Env straight into
ShellSessionSpec.Env which RunShellSession treats as the full environment,
causing system vars (PATH, HOME, AWS creds, etc.) to be lost; import the stdlib
os package and set ShellSessionSpec.Env to a merged environment (start with
os.Environ() and append/override with opts.Env entries) before calling
process.RunShellSession so that Options.Env remains additive rather than
replacing the ambient environment.
In `@pkg/runner/step/shell.go`:
- Around line 72-75: The code only routes terminal-attached steps (step.Tty) to
the new shell session path; update Execute and ExecuteWithWorkflow (and the
other similar checks around executeTtyStep) to also treat interactive steps the
same way by checking step.Interactive (or the struct field name used for
interactive) in addition to step.Tty and calling the
RunShellSession/executeTtyStep path so the Windows-capable fallback in
pkg/process/shell_session.go is used for interactive:true without a tty; locate
checks in Execute, ExecuteWithWorkflow and the other occurrences referenced (the
blocks calling h.executeTtyStep or RunShellSession) and extend their condition
to include interactive.
In `@pkg/terminal/pty/pty_test.go`:
- Around line 335-399: Replace the shell-based child in
TestExecWithPTY_DisableStdinForward (and any other tests using "sh") with a
Go-native test helper subprocess: add a test-only child mode triggered by an env
var (e.g., PTY_TEST_CHILD) in this test file that, when set, performs an
explicit blocking/non-blocking stdin read (with a short timeout) and prints
deterministic markers like "INPUT-RECEIVED" vs "NO-INPUT". Invoke that child by
using os.Executable() and exec.Command(os.Executable(), ...) with the env flag
set, pass Options (e.g., DisableStdinForward) to ExecWithPTY, and assert on
those markers to verify that host stdin is/ is not forwarded; keep
TestExecWithPTY_ReturnsWithBlockedStdin using neverEOFReader but also use
os.Executable() child if it needs to read/exit deterministically. Ensure you
update references to ExecWithPTY, Options.DisableStdinForward, and
neverEOFReader in the tests so they call the new Go-native child helper instead
of "sh".
🪄 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: 6ccf3f67-f3cd-499e-9a1b-4620d5fc67bd
📒 Files selected for processing (28)
cmd/cmd_utils.gocmd/custom_command_shell_step.gocmd/custom_command_shell_step_test.gointernal/exec/workflow_utils.gointernal/exec/workflow_utils_test.gomain.gomain_test.gopkg/datafetcher/schema/atmos/manifest/1.0.jsonpkg/datafetcher/schema/config/global/1.0.jsonpkg/process/shell_session.gopkg/process/shell_session_test.gopkg/runner/runner.gopkg/runner/runner_test.gopkg/runner/step/shell.gopkg/runner/step/shell_test.gopkg/schema/task.gopkg/schema/task_test.gopkg/schema/workflow.gopkg/signals/interrupt.gopkg/signals/interrupt_test.gopkg/terminal/pty/pty.gopkg/terminal/pty/pty_test.gopkg/terminal/pty/setup_unix.gopkg/terminal/pty/setup_windows.gowebsite/blog/2026-06-11-tty-interactive-steps.mdxwebsite/docs/cli/configuration/commands.mdxwebsite/docs/workflows/workflows.mdxwebsite/src/data/roadmap.js
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #2602 +/- ##
==========================================
+ Coverage 79.40% 79.47% +0.06%
==========================================
Files 1347 1355 +8
Lines 127229 127551 +322
==========================================
+ Hits 101031 101373 +342
+ Misses 20549 20523 -26
- Partials 5649 5655 +6
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Steps with type: exec hand the process over entirely (shell exec semantics): execve of the system shell on Unix (inheriting env, working directory, and the terminal natively; ATMOS_SHLVL unchanged), spawn-and-propagate-exit-code emulation on Windows. Exec steps are validated to be the final step and must not set supervisor-only fields (tty, interactive, retry, timeout, output). Wired into custom commands, workflows, and the task runner. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ss-platform test helpers Interactive (non-tty) steps now use the shell session path so suspension and platform-aware shell selection live in one place (pkg/process); dry-run is checked before shell-level validation; the runner env inherits os.Environ. Tests use the test binary as a cross-platform subprocess helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Interactive (non-tty) sessions always attach the real streams, so missing masking is inherent there - debug-log it instead of warning on every step. The visible warning remains for the unexpected case: tty requested but PTY unavailable on the platform. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Move shell-family step routing into pkg/process.RunShellStep (terminal steps to sessions, plain steps via caller fallback) and default masking wiring inside RunShellSession. Delete the cmd/ step helper file and the internal/exec helper - both locations now contain only inline switch-case call sites; the runner and step handler use the same shared routing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When the session command exits, grandchildren that inherited the PTY slave (e.g. aws ssm's session-manager-plugin) can keep it open, so the output copier never gets EIO: atmos hung with the terminal in raw mode (no prompt until a stray keypress, no echo afterwards). Drain output on a 1s deadline after child exit, forcing the pending read to return so the terminal is restored promptly. Reported-by: live SSM session test on PR #2602 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The test binary used as a PTY child links TUI libraries that emit OSC/DSR terminal queries when stdout is a TTY and block ~5s per query waiting for replies no test PTY sends - hanging three pty tests past their deadlines and silently adding ~15s to the process suite. TERM=dumb/NO_COLOR in the child env suppresses the queries (pty suite 25s+fail -> 1.5s; process 16.6s -> 1.6s). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Erik Osterman (Cloud Posse) (osterman)
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/runner/runner.go (1)
163-168:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComment must end with a period.
The multi-line comment for
runShellTaskis missing a period at the end of the last line.📝 Proposed fix
// Tasks with `tty: true` attach to the user's terminal (PTY when supported). // Tasks with `interactive: true` let the child process own Ctrl-C: the Atmos // SIGINT-exit handler is suspended while the task runs. Plain tasks delegate -// to the CommandRunner +// to the CommandRunner. func runShellTask(ctx context.Context, task *Task, runner CommandRunner, opts *Options, dir string) error {🤖 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/runner.go` around lines 163 - 168, The doc comment for runShellTask is missing a final period; update the multi-line comment above the runShellTask function (the line ending with "delegate to the CommandRunner") to end with a period so the comment finishes with proper punctuation.Source: Coding guidelines
🧹 Nitpick comments (1)
pkg/schema/task_validate_test.go (1)
114-157: ⚡ Quick winAdd workflow invalid-field cases for
tty,interactive, andretry.
ValidateExecWorkflowStepsenforces these too, but the workflow table currently pins only timeout/output. Adding these rows will lock the full workflow contract and prevent projection regressions.Suggested test rows.
{ + name: "exec with tty", + steps: []WorkflowStep{ + {Type: TaskTypeExec, Command: "psql", Tty: true}, + }, + wantErr: ErrExecStepInvalidField, + }, + { + name: "exec with interactive", + steps: []WorkflowStep{ + {Type: TaskTypeExec, Command: "psql", Interactive: true}, + }, + wantErr: ErrExecStepInvalidField, + }, + { + name: "exec with retry", + steps: []WorkflowStep{ + {Type: TaskTypeExec, Command: "psql", Retry: &RetryConfig{}}, + }, + wantErr: ErrExecStepInvalidField, + }, + { name: "exec with timeout string", steps: []WorkflowStep{ {Type: TaskTypeExec, Command: "psql", Timeout: "30s"}, },As per coding guidelines, "Every new feature must include comprehensive unit tests targeting >80% code coverage for all packages."
🤖 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_test.go` around lines 114 - 157, Add missing invalid-field test rows to TestValidateExecWorkflowSteps: for WorkflowStep entries with Type: TaskTypeExec set the Tty flag (Tty: true), the Interactive flag (Interactive: true), and a non-zero Retry value (e.g., Retry: 1), and assert wantErr is ErrExecStepInvalidField for each; this ensures ValidateExecWorkflowSteps rejects exec steps that have Tty, Interactive, or Retry set (reference TestValidateExecWorkflowSteps, WorkflowStep, TaskTypeExec, and ErrExecStepInvalidField).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 `@pkg/schema/task.go`:
- Around line 19-21: Update the comment for the Task.Type field to mention the
new TaskTypeExec value in addition to the existing shell/atmos type; locate the
Task.Type field comment (referencing Task.Type) in pkg/schema/task.go and edit
the docstring so it lists both the existing shell/atmos semantics and the new
"exec" semantics (TaskTypeExec) so the comment accurately reflects the supported
task types.
---
Outside diff comments:
In `@pkg/runner/runner.go`:
- Around line 163-168: The doc comment for runShellTask is missing a final
period; update the multi-line comment above the runShellTask function (the line
ending with "delegate to the CommandRunner") to end with a period so the comment
finishes with proper punctuation.
---
Nitpick comments:
In `@pkg/schema/task_validate_test.go`:
- Around line 114-157: Add missing invalid-field test rows to
TestValidateExecWorkflowSteps: for WorkflowStep entries with Type: TaskTypeExec
set the Tty flag (Tty: true), the Interactive flag (Interactive: true), and a
non-zero Retry value (e.g., Retry: 1), and assert wantErr is
ErrExecStepInvalidField for each; this ensures ValidateExecWorkflowSteps rejects
exec steps that have Tty, Interactive, or Retry set (reference
TestValidateExecWorkflowSteps, WorkflowStep, TaskTypeExec, and
ErrExecStepInvalidField).
🪄 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: a460033f-2d27-425b-bfe0-622aa6094578
📒 Files selected for processing (18)
cmd/cmd_utils.gointernal/exec/workflow_utils.gointernal/exec/workflow_utils_test.gopkg/process/exec_replace.gopkg/process/exec_replace_test.gopkg/process/exec_replace_unix.gopkg/process/exec_replace_windows.gopkg/process/shell_session.gopkg/process/shell_session_test.gopkg/process/shell_step_test.gopkg/runner/runner.gopkg/runner/runner_test.gopkg/runner/step/shell.gopkg/runner/step/shell_test.gopkg/schema/task.gopkg/schema/task_validate.gopkg/schema/task_validate_test.gopkg/terminal/pty/pty_test.go
… restore terminal on signal exit
Sessions whose child died from a signal now report 128+signal (e.g. 130)
instead of Go's -1 ('subcommand exited with code -1'). The PTY terminal
restore is also registered as a signal-exit cleanup: os.Exit in the signal
handler skips defers, which could leave the host terminal in raw mode when
atmos was signalled mid-session.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t-session hang) A tty/interactive/exec session that exits non-zero produced a bare ExitCodeError that Atmos rendered through the themed Glamour formatter. That render triggers termenv's terminal queries (OSC 11 background color, DSR cursor) and reads the reply from os.Stdin - but the session's stdin copier steals it, so termenv blocks for its full ~5s timeout before the process exits (Matt's '3-5s then code -1'; Erik's hang on a failed-command exit). Mark session ExitCodeErrors Silent so the code propagates like a shell would, with no themed error box and no terminal query. Honored in CheckErrorPrintAndExit (custom commands), main.run (workflows), and the workflow step-error wrapper. Before: 5.0s on non-zero exit; after: 3ms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Andriy Knysh (aknysh)
approved these changes
Jun 15, 2026
Contributor
|
Tip Atmos Pro
No affected stacks workflow was detected for this pull request.
|
|
These changes were released in v1.222.0-rc.1. |
This was referenced Jun 18, 2026
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
2 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
Terminal steps for custom commands and workflows — three related capabilities:
interactive: true— attach host stdin and let the step own Ctrl-C. Atmos suspends its SIGINT-exit handler while the step runs (newpkg/signalssuspension registry consulted by themain.gosignal handler).tty: true— allocate a pseudo-terminal (reusingpkg/terminal/pty, same engine asatmos devcontainer attach). The command sees a real TTY; secret masking is applied to PTY output. Withinteractive: true, the host terminal switches to raw mode so Ctrl-C flows through the PTY to the child.type: exec— replace the Atmos process entirely (shellexecsemantics):execveof the system shell on Unix (env, working directory, and terminal inherited natively;ATMOS_SHLVLunchanged), spawn-and-propagate-exit-code emulation on Windows. Validated to be the final step;tty/interactive/retry/timeout/outputare rejected on exec steps.Architecture: all logic lives in narrow packages —
pkg/process(RunShellSteprouting,RunShellSession,ReplaceShellSession),pkg/schema(validation),pkg/signals(interrupt suspension).cmd/andinternal/execcontain only inline switch-case call sites;pkg/runnerand the step handler share the same routing.Also fixes in
pkg/terminal/ptyfound along the way:session-manager-plugin) keep the PTY slave open after the child exits, output drains on a 1s deadline instead of hanging with the terminal in raw modeDisableStdinForwardfor-t-without--isemanticswhy
Custom commands had no way to hand the terminal to an interactive process:
ran the SSM session as a piped, masked subprocess: full-screen rendering broke, and Ctrl-C inside the session killed Atmos itself (global SIGINT handler exits 130), killing the orphaned session with SIGPIPE.
With this change:
behaves like
docker run -it(supervised: masking preserved, more steps can follow), and:hands the process over entirely (launcher: native job control, zero proxy overhead, must be the last step).
references
🤖 Generated with Claude Code