Skip to content

Fix lstk aws streaming output hanging until exit by running the aws CLI in a PTY - #417

Merged
anisaoshafi merged 2 commits into
mainfrom
claude/lstk-logs-tail-follow-hang-9ff033
Jul 30, 2026
Merged

Fix lstk aws streaming output hanging until exit by running the aws CLI in a PTY#417
anisaoshafi merged 2 commits into
mainfrom
claude/lstk-logs-tail-follow-hang-9ff033

Conversation

@skyrpex

@skyrpex skyrpex commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What

lstk aws logs tail --follow (and any streaming aws command) showed no output until the process exited when run on a terminal.

Why

The interactive spinner path in cmd/aws.go wraps the child's stdout/stderr in terminal.StopOnWriteWriter. Since that is a plain io.Writer, os/exec hands the aws CLI a pipe instead of the TTY; the Python-based CLI then block-buffers stdout (8 KB), holding all output until the buffer fills or the process exits. The official frozen aws v2 binary also ignores PYTHONUNBUFFERED, so an env-var fix alone doesn't work.

How

  • New proc.RunInPTY (internal/proc/pty.go): runs the child with stdout+stderr on a PTY slave, copies the master to the given writer (the spinner's StopOnWriteWriter, so the spinner still stops on first output). No new session is created, so Ctrl-C still reaches the child via the process group; PTY size is inherited once from lstk's terminal. Falls back to the previous pipe wiring when no PTY can be allocated (Windows).
  • awscli.Exec gains a usePTY parameter; cmd/aws.go enables it only when both stdout and stderr are terminals — piped output (lstk aws ... | grep) keeps seeing a pipe so pipelines never receive colors/CRLF.
  • PYTHONUNBUFFERED=1 is still injected (helps pip-installed, non-frozen CLIs; a user-set value wins).
  • Adds github.com/creack/pty to the root module (previously test-only).

Tests

  • New e2e test TestAWSLogsTailFollowStreamsInPTY (test/integration/aws_e2e_test.go): real LocalStack + real aws CLI in a PTY; asserts a pre-existing log event appears while tail --follow is still running. Failed before the fix (output only after Ctrl-C), passes after. Gated on Docker + aws binary + LOCALSTACK_AUTH_TOKEN, like the terraform/cdk/sam e2e suites.
  • Unit tests for proc.RunInPTY (child sees a TTY, output arrives before exit, exit-code propagation, stderr merge) and for the env construction.
  • All 1171 unit tests and the 23 TestAWSCommand* integration tests pass locally.

Manual testing

With a running LocalStack AWS emulator (lstk start), save this as tail-follow-demo.sh and run LSTK=./bin/lstk sh tail-follow-demo.sh from an interactive terminal. It creates a log group, feeds one event per second in the background, and follows the group; stop with Ctrl-C.

#!/bin/sh
# Demo for DEVX-1026: `lstk aws logs tail --follow` must stream events live.
set -e

LSTK=${LSTK:-./bin/lstk}
GROUP=/lstk-demo/tail-follow
STREAM=demo-$$

$LSTK aws logs create-log-group --log-group-name "$GROUP" 2>/dev/null || true
$LSTK aws logs create-log-stream --log-group-name "$GROUP" --log-stream-name "$STREAM"

# Feed one event per second in the background while tail runs. Ctrl-C signals
# the whole foreground process group, so shield the feeder (and the aws
# processes it spawns, which inherit the ignored disposition) from SIGINT —
# otherwise an aws CLI caught mid-startup dies with a KeyboardInterrupt
# traceback. The EXIT trap still cleans the feeder up.
(
  trap '' INT
  i=0
  while [ "$i" -lt 30 ]; do
    i=$((i + 1))
    $LSTK aws logs put-log-events \
      --log-group-name "$GROUP" --log-stream-name "$STREAM" \
      --log-events "timestamp=$(($(date +%s) * 1000)),message=event #$i" >/dev/null
    sleep 1
  done
) &
FEEDER=$!
trap 'kill "$FEEDER" 2>/dev/null' EXIT

# Fixed: events print about one per second. Broken: silence until Ctrl-C.
$LSTK aws logs tail "$GROUP" --follow --since 1m

Before (built from main) — the spinner runs for the entire session, no events are shown; everything flushes at once only after Ctrl-C:

⣾ Loading service...      <- only this, for the whole run
^C
2026-07-30T07:48:53.000000+00:00 demo-66624 event #1
2026-07-30T07:48:54.000000+00:00 demo-66624 event #2
...
2026-07-30T07:49:02.000000+00:00 demo-66624 event #8   <- all 8 appear only now

After (this branch) — the spinner stops on the first event, then each line appears live, ~1 s apart:

2026-07-30T07:49:06.000000+00:00 demo-66750 event #1
2026-07-30T07:49:08.000000+00:00 demo-66750 event #2
2026-07-30T07:49:09.000000+00:00 demo-66750 event #3
2026-07-30T07:49:10.000000+00:00 demo-66750 event #4
...                                                    <- streaming while it runs
^C

Notes for reviewers

  • Behavior change: with a TTY the aws CLI may now enable colors and its default pager (less) for long output — matching raw aws behavior, but new for lstk aws.
  • In interactive PTY mode the child's stderr is merged into stdout (they share the terminal anyway); redirected/non-interactive runs keep separate streams.
  • lstk az has the identical wrapper pattern and needs the same treatment — follow-up ticket DEVX-1028.

Review

New user-facing behavior (PTY, colors, possible pager) on a heavily used proxy command plus a new production dependency — human review advisable.

Closes DEVX-1026

Co-Authored-By: Claude noreply@anthropic.com

@skyrpex skyrpex added semver: patch docs: skip Pull request does not require documentation changes labels Jul 29, 2026
@skyrpex
skyrpex marked this pull request as ready for review July 30, 2026 08:05
@skyrpex
skyrpex requested a review from a team as a code owner July 30, 2026 08:05

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This runs the wrapped aws CLI on a PTY when both of lstk's streams are terminals, fixing the block-buffered streaming hang (logs tail --follow); the fix is well-scoped and the TTY gating is coherent.

  1. suggestion(non-blocking): on internal/awscli/exec.go — now that the child sees a TTY on stdout, aws v2's default pager (less) will engage for long paged output (e.g. lstk aws s3 ls) where the old pipe wiring suppressed it, and rendered through the copied PTY master it can look janky. Consider defaulting AWS_PAGER="" in execEnv (still user-overridable, same pattern as PYTHONUNBUFFERED) so you keep streaming + colors without an unexpected pager.
  2. thought(non-blocking): on internal/proc/pty.go — the PTY size is captured once and resizes aren't propagated (documented); fine for streaming today, but a SIGWINCH-driven Setsize would be a cheap follow-up if paged/interactive output becomes common.
  3. praise: leaving cmd.Stdin on the real terminal while only stdout/stderr ride the PTY (no new session) is what keeps Ctrl-C and the process-group semantics intact, and the timing-based unit test plus the real-CLI e2e reproduction make the fix genuinely verifiable. @skyrpex

Automated review on behalf of @gtsiolis.


Generated by Claude Code

skyrpex and others added 2 commits July 30, 2026 14:26
…LI in a PTY

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
@anisaoshafi
anisaoshafi force-pushed the claude/lstk-logs-tail-follow-hang-9ff033 branch from 415053f to 1a6ab38 Compare July 30, 2026 12:27

@anisaoshafi anisaoshafi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested before and after, and looks pretty good. Thanks for the clear manual instructions in the description 👏🏼

Pushed a small commit 1a6ab38 to simplify some long comments

@anisaoshafi
anisaoshafi enabled auto-merge (squash) July 30, 2026 12:29
@anisaoshafi
anisaoshafi merged commit 3d5f172 into main Jul 30, 2026
19 checks passed
@anisaoshafi
anisaoshafi deleted the claude/lstk-logs-tail-follow-hang-9ff033 branch July 30, 2026 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs: skip Pull request does not require documentation changes semver: patch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants