Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,8 @@ lstk supports Git-style extensions: when `lstk <name>` is not a built-in command

Wrapped external tools (`aws`, `terraform`, `cdk`, `sam`, `az`, and extensions) are run through `proc.Run(cmd)` (in `internal/proc/run.go`) rather than `cmd.Run()`. These execs are created with `exec.CommandContext` using lstk's root context, which is cancelled on `SIGINT`/`SIGTERM`; `exec.CommandContext`'s default `Cancel` would then SIGKILL the child immediately, denying tools like `terraform apply` the chance to clean up (e.g. release the state lock). `proc.Run` disarms that (its `Cancel` returns `os.ErrProcessDone`, which both suppresses the kill and avoids injecting `context.Canceled` into the wait result, preserving the tool's real exit code) and instead lets the tool terminate from the signal it receives, waiting for it to finish its own shutdown. Forwarding is per-signal: `SIGTERM` is always relayed to the child (a terminal never generates it, so `kill <lstk-pid>` / `timeout` / an IDE stop button would otherwise never reach the tool), while `SIGINT` is relayed only when none of lstk's std streams is a terminal — an attached terminal already delivers Ctrl-C to the child via the foreground process group, and a second near-simultaneous SIGINT makes tools like terraform abort immediately instead of cleaning up. The any-stream check matters: with only stdin redirected (`yes | lstk terraform apply`) lstk still sits in the terminal's foreground process group. This differs from `npm/launcher.js`, which forwards unconditionally — safe there because its child is lstk itself, which tolerates duplicate signals; wrapped tools do not. Short internal captured-output execs (version checks, schema discovery, backend provisioning) still use `cmd.Run()` directly. End-to-end signal tests live in `test/integration/signal_forwarding_test.go`, backed by the reference extension's `signal-wait` mode.

When lstk's stdout and stderr are both terminals, `lstk aws` runs the child via `proc.RunInPTY` (in `internal/proc/pty.go`) instead: the spinner path wraps the child's output in an `io.Writer`, which makes os/exec hand the child a pipe, and the frozen Python aws CLI then block-buffers stdout (8 KB, ignoring `PYTHONUNBUFFERED`) — streaming commands like `aws logs tail --follow` showed nothing until exit (DEVX-1026). The PTY makes the child see a terminal (line-buffered, colored output; stdout/stderr merged, no new session so Ctrl-C still reaches it via the process group) while the master side is copied through the spinner's `StopOnWriteWriter`. Falls back to plain `proc.Run` when no PTY can be allocated (Windows), and stays on pipes whenever stdout is redirected so pipelines never receive colors/CRLF. `lstk az` has the same wrapper pattern and needs the same treatment (pending follow-up).


# Snapshots

Expand Down
10 changes: 8 additions & 2 deletions cmd/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ Examples:
// --help/-h never contacts LocalStack, so it runs directly without
// requiring Docker or a running emulator (DEVX-1002).
if awscli.IsHelp(passthrough) {
return awscli.Exec(cmd.Context(), "", false, os.Stdout, os.Stderr, passthrough)
return awscli.Exec(cmd.Context(), "", false, false, os.Stdout, os.Stderr, passthrough)
}

rt, err := runtime.NewDockerRuntime(cfg.DockerHost)
Expand Down Expand Up @@ -119,7 +119,13 @@ Examples:
stderr = &terminal.StopOnWriteWriter{W: os.Stderr, Spinner: s}
}

return awscli.Exec(cmd.Context(), "http://"+host, profileExists, stdout, stderr, passthrough)
// Only hand the aws CLI a PTY when both of lstk's output streams
// are the terminal: with stdout piped (`lstk aws ... | grep`) the
// child must keep seeing a pipe, or it would emit colors and CRLF
// into the pipeline.
usePTY := !cfg.NonInteractive && terminal.IsTerminal(os.Stdout) && terminal.IsTerminal(os.Stderr)

return awscli.Exec(cmd.Context(), "http://"+host, profileExists, usePTY, stdout, stderr, passthrough)
},
}
}
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ require (
github.com/charmbracelet/lipgloss v1.1.0
github.com/charmbracelet/x/exp/teatest v0.0.0-20260216111343-536eb63c1f4c
github.com/containerd/errdefs v1.0.0
github.com/creack/pty v1.1.24
github.com/docker/go-units v0.5.0
github.com/google/uuid v1.6.0
github.com/hashicorp/hcl/v2 v2.24.0
Expand Down
2 changes: 2 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ github.com/containerd/errdefs/pkg v0.3.0 h1:9IKJ06FvyNlexW690DXuQNx2KA2cUJXx151X
github.com/containerd/errdefs/pkg v0.3.0/go.mod h1:NJw6s9HwNuRhnjJhM7pylWwMyAkmCQvQ4GpJHEqRLVk=
github.com/cpuguy83/go-md2man/v2 v2.0.6 h1:XJtiaUW6dEEqVuZiMTn1ldk455QWwEIsMIJlo5vtkx0=
github.com/cpuguy83/go-md2man/v2 v2.0.6/go.mod h1:oOW0eioCTA6cOiMLiUPZOpcVxMig6NIQQ7OS05n1F4g=
github.com/creack/pty v1.1.24 h1:bJrF4RRfyJnbTJqzRLHzcGaZK1NeM5kTC9jGgovnR1s=
github.com/creack/pty v1.1.24/go.mod h1:08sCNb52WyoAwi2QDyzUCTgcvVFhUzewun7wtTfvcwE=
github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ=
github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
Expand Down
42 changes: 36 additions & 6 deletions internal/awscli/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ func IsHelp(args []string) bool {
return false
}

func Exec(ctx context.Context, endpointURL string, useProfile bool, stdout, stderr io.Writer, args []string) error {
// Exec runs `aws <args...>` against endpointURL. When usePTY is true (lstk's
// stdout and stderr are both terminals), the child's output goes through a
// pseudo-terminal merged into stdout — see proc.RunInPTY for why; otherwise
// stdout/stderr are wired as given.
func Exec(ctx context.Context, endpointURL string, useProfile, usePTY bool, stdout, stderr io.Writer, args []string) error {
ctx, span := otel.Tracer("github.com/localstack/lstk/internal/awscli").Start(ctx, "aws cli")
defer span.End()

Expand Down Expand Up @@ -75,13 +79,22 @@ func Exec(ctx context.Context, endpointURL string, useProfile bool, stdout, stde

cmd := exec.CommandContext(ctx, awsBin, cmdArgs...)
cmd.Stdin = os.Stdin
cmd.Stdout = stdout
cmd.Stderr = stderr
if !useProfile {
cmd.Env = BuildEnv(os.Environ())
cmd.Env = execEnv(os.Environ(), useProfile)

var runErr error
started := false
if usePTY {
started, runErr = proc.RunInPTY(cmd, stdout)
}
if !started {
// No PTY requested or none could be allocated (e.g. on Windows): plain
// writer wiring, as before.
cmd.Stdout = stdout
cmd.Stderr = stderr
runErr = proc.Run(cmd)
}

if err := proc.Run(cmd); err != nil {
if err := runErr; err != nil {
var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
span.SetAttributes(attribute.Int("aws.exit_code", exitErr.ExitCode()))
Expand All @@ -95,6 +108,23 @@ func Exec(ctx context.Context, endpointURL string, useProfile bool, stdout, stde
return nil
}

// execEnv builds the child environment for the aws CLI. PYTHONUNBUFFERED stops
// a pip-installed (non-frozen) aws CLI from block-buffering stdout when it gets
// a pipe instead of a terminal (DEVX-1026); the official frozen v2 binary
// ignores it — that build's buffering is what the PTY in Exec fixes. (Python
// treats any non-empty value as enabled, so a user-set value is left alone.)
func execEnv(base []string, useProfile bool) []string {
var env []string
if useProfile {
env = make([]string, len(base), len(base)+1)
copy(env, base)
} else {
env = BuildEnv(base)
}
setIfAbsent(&env, "PYTHONUNBUFFERED", "1")
return env
}

func BuildEnv(base []string) []string {
env := make([]string, len(base), len(base)+3)
copy(env, base)
Expand Down
31 changes: 31 additions & 0 deletions internal/awscli/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,34 @@ func TestBuildEnvPartialOverride(t *testing.T) {
assert.Contains(t, env, "AWS_SECRET_ACCESS_KEY=test")
assert.Contains(t, env, "AWS_DEFAULT_REGION=us-east-1")
}

func TestExecEnvForcesUnbufferedPython(t *testing.T) {
base := []string{"PATH=/usr/bin"}

withProfile := execEnv(base, true)
assert.Contains(t, withProfile, "PYTHONUNBUFFERED=1")
assert.NotContains(t, withProfile, "AWS_ACCESS_KEY_ID=test")

withoutProfile := execEnv(base, false)
assert.Contains(t, withoutProfile, "PYTHONUNBUFFERED=1")
assert.Contains(t, withoutProfile, "AWS_ACCESS_KEY_ID=test")
}

func TestExecEnvPreservesUserPythonUnbuffered(t *testing.T) {
base := []string{"PYTHONUNBUFFERED=x"}
env := execEnv(base, true)

assert.Contains(t, env, "PYTHONUNBUFFERED=x")
assert.NotContains(t, env, "PYTHONUNBUFFERED=1")
}

func TestExecEnvDoesNotMutateInput(t *testing.T) {
base := []string{"PATH=/usr/bin"}
original := make([]string, len(base))
copy(original, base)

execEnv(base, true)
execEnv(base, false)

assert.Equal(t, original, base)
}
46 changes: 46 additions & 0 deletions internal/proc/pty.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
package proc

import (
"io"
"os"
"os/exec"

"github.com/creack/pty"
)

// RunInPTY runs cmd like Run, but gives the child a pseudo-terminal instead of
// a plain pipe for stdout/stderr, copying its output to out. Over a pipe,
// e.g. the Python aws CLI block-buffers stdout (ignoring PYTHONUNBUFFERED),
// holding back streaming commands like `logs tail --follow` until exit; a PTY
// makes it detect a terminal and stay line-buffered (DEVX-1026).
//
// No new session is created, so the child stays in lstk's process group and
// still gets Ctrl-C directly. started is false if no PTY could be allocated
// (e.g. Windows); the caller should fall back to Run.
func RunInPTY(cmd *exec.Cmd, out io.Writer) (started bool, err error) {
ptmx, tty, err := pty.Open()
if err != nil {
return false, err
}
defer func() { _ = ptmx.Close() }()

if size, sizeErr := pty.GetsizeFull(os.Stdout); sizeErr == nil {
_ = pty.Setsize(ptmx, size)
}

cmd.Stdout = tty
cmd.Stderr = tty

copied := make(chan struct{})
go func() {
_, _ = io.Copy(out, ptmx)
close(copied)
}()

runErr := Run(cmd)

// Wait for the copy goroutine to drain remaining output and hit EOF/EIO.
_ = tty.Close()
<-copied
return true, runErr
}
123 changes: 123 additions & 0 deletions internal/proc/pty_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
package proc

import (
"os/exec"
"runtime"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func skipWithoutPTY(t *testing.T) {
t.Helper()
if runtime.GOOS == "windows" {
t.Skip("PTY not supported on Windows")
}
}

// syncWriter collects writes with timestamps so tests can assert on when
// output arrived, not just what arrived.
type syncWriter struct {
mu chan struct{}
chunks []timedChunk
}

type timedChunk struct {
at time.Time
data string
}

func newSyncWriter() *syncWriter {
w := &syncWriter{mu: make(chan struct{}, 1)}
w.mu <- struct{}{}
return w
}

func (w *syncWriter) Write(p []byte) (int, error) {
<-w.mu
defer func() { w.mu <- struct{}{} }()
w.chunks = append(w.chunks, timedChunk{at: time.Now(), data: string(p)})
return len(p), nil
}

func (w *syncWriter) String() string {
<-w.mu
defer func() { w.mu <- struct{}{} }()
var b strings.Builder
for _, c := range w.chunks {
b.WriteString(c.data)
}
return b.String()
}

func (w *syncWriter) firstWriteContaining(s string) (time.Time, bool) {
<-w.mu
defer func() { w.mu <- struct{}{} }()
var seen strings.Builder
for _, c := range w.chunks {
seen.WriteString(c.data)
if strings.Contains(seen.String(), s) {
return c.at, true
}
}
return time.Time{}, false
}

func TestRunInPTYChildSeesTerminal(t *testing.T) {
skipWithoutPTY(t)
out := newSyncWriter()

cmd := exec.Command("sh", "-c", "test -t 1 && test -t 2 && echo is-a-tty")
started, err := RunInPTY(cmd, out)

require.True(t, started)
require.NoError(t, err)
assert.Contains(t, out.String(), "is-a-tty")
}

func TestRunInPTYStreamsBeforeExit(t *testing.T) {
skipWithoutPTY(t)
out := newSyncWriter()

start := time.Now()
cmd := exec.Command("sh", "-c", "echo early-line; sleep 1")
started, err := RunInPTY(cmd, out)
elapsed := time.Since(start)

require.True(t, started)
require.NoError(t, err)
require.GreaterOrEqual(t, elapsed, time.Second, "sanity: child slept after writing")
at, ok := out.firstWriteContaining("early-line")
require.True(t, ok, "output missing early-line: %q", out.String())
assert.Less(t, at.Sub(start), 900*time.Millisecond,
"output written before exit should reach the writer while the child still runs")
}

func TestRunInPTYPropagatesExitCode(t *testing.T) {
skipWithoutPTY(t)
out := newSyncWriter()

cmd := exec.Command("sh", "-c", "exit 7")
started, err := RunInPTY(cmd, out)

require.True(t, started)
var exitErr *exec.ExitError
require.ErrorAs(t, err, &exitErr)
assert.Equal(t, 7, exitErr.ExitCode())
}

func TestRunInPTYMergesStderrIntoOut(t *testing.T) {
skipWithoutPTY(t)
out := newSyncWriter()

cmd := exec.Command("sh", "-c", "echo to-stdout; echo to-stderr >&2")
started, err := RunInPTY(cmd, out)

require.True(t, started)
require.NoError(t, err)
assert.Contains(t, out.String(), "to-stdout")
assert.Contains(t, out.String(), "to-stderr")
}
Loading
Loading