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: 1 addition & 1 deletion pkg/cli/add_interactive_orchestrator.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func RunAddInteractive(ctx context.Context, workflowSpecs []string, verbose bool
}

// Clear the screen for a fresh interactive experience
fmt.Fprint(os.Stderr, "\033[H\033[2J")
console.ClearScreen()

// Step 1: Welcome message
fmt.Fprintln(os.Stderr, "")
Expand Down
11 changes: 4 additions & 7 deletions pkg/console/console.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ type CompilerError struct {
Hint string // Optional hint for fixing the error
}

// ANSI escape sequences for terminal control
var clearScreenSequence = "\033[2J\033[H" // Clear screen and move cursor to home position

// isTTY checks if stdout is a terminal
func isTTY() bool {
return tty.IsStdoutTerminal()
Expand Down Expand Up @@ -483,19 +480,19 @@ func RenderTableAsJSON(config TableConfig) (string, error) {
return string(jsonBytes), nil
}

// ClearScreen clears the terminal screen if stdout is a TTY
// ClearScreen clears the terminal screen if stderr is a TTY
// Uses ANSI escape codes for cross-platform compatibility
func ClearScreen() {
if isTTY() {
fmt.Print(clearScreenSequence)
if tty.IsStderrTerminal() {
fmt.Fprint(os.Stderr, ansiClearScreen)
}
}

// ClearLine clears the current line in the terminal if stderr is a TTY
// Uses ANSI escape codes: \r moves cursor to start, \033[K clears to end of line
func ClearLine() {
if tty.IsStderrTerminal() {
fmt.Fprint(os.Stderr, "\r\033[K")
fmt.Fprintf(os.Stderr, "%s%s", ansiCarriageReturn, ansiClearLine)
}
}

Expand Down
6 changes: 3 additions & 3 deletions pkg/console/spinner.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (m spinnerModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
// render manually prints the spinner frame (required when using WithoutRenderer)
func (m spinnerModel) render() {
if m.output != nil {
fmt.Fprintf(m.output, "\r\033[K%s %s", m.spinner.View(), m.message)
fmt.Fprintf(m.output, "%s%s%s %s", ansiCarriageReturn, ansiClearLine, m.spinner.View(), m.message)
}
}

Expand Down Expand Up @@ -138,7 +138,7 @@ func (s *SpinnerWrapper) Stop() {
s.mu.Unlock()
s.program.Quit()
s.wg.Wait() // Wait for the goroutine to complete
fmt.Fprint(os.Stderr, "\r\033[K")
fmt.Fprintf(os.Stderr, "%s%s", ansiCarriageReturn, ansiClearLine)
} else {
s.mu.Unlock()
}
Expand All @@ -153,7 +153,7 @@ func (s *SpinnerWrapper) StopWithMessage(msg string) {
s.mu.Unlock()
s.program.Quit()
s.wg.Wait() // Wait for the goroutine to complete
fmt.Fprintf(os.Stderr, "\r\033[K%s\n", msg)
fmt.Fprintf(os.Stderr, "%s%s%s\n", ansiCarriageReturn, ansiClearLine, msg)
} else {
s.mu.Unlock()
// Still print the message even if spinner wasn't running
Expand Down
36 changes: 36 additions & 0 deletions pkg/console/terminal.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package console

import (
"fmt"
"os"

"github.com/github/gh-aw/pkg/tty"
)

// ANSI escape sequences for terminal control
const (
// ansiClearScreen clears the screen and moves cursor to home position
ansiClearScreen = "\033[H\033[2J"

// ansiClearLine clears from cursor to end of line
ansiClearLine = "\033[K"

// ansiCarriageReturn moves cursor to start of current line
ansiCarriageReturn = "\r"
)

// MoveCursorUp moves cursor up n lines if stderr is a TTY.
// Uses ANSI escape code: \033[nA where n is the number of lines.
func MoveCursorUp(n int) {
if tty.IsStderrTerminal() {
fmt.Fprintf(os.Stderr, "\033[%dA", n)
}
}

// MoveCursorDown moves cursor down n lines if stderr is a TTY.
// Uses ANSI escape code: \033[nB where n is the number of lines.
func MoveCursorDown(n int) {
if tty.IsStderrTerminal() {
fmt.Fprintf(os.Stderr, "\033[%dB", n)
}
Comment on lines +24 to +35
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

MoveCursorUp/MoveCursorDown will emit escape sequences for n==0 and will format negative values into the ANSI sequence (e.g., "\033[-1A"), which is not meaningful and may behave inconsistently across terminals. Consider validating n > 0 and returning early otherwise (or clamping at 0) to make the API safer.

Copilot uses AI. Check for mistakes.
}
173 changes: 173 additions & 0 deletions pkg/console/terminal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
//go:build !integration

package console

import (
"bytes"
"io"
"os"
"testing"

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

// captureStderr captures stderr output during function execution
func captureStderr(t *testing.T, fn func()) string {
t.Helper()

// Save original stderr
oldStderr := os.Stderr

// Create a pipe to capture stderr
r, w, err := os.Pipe()
if err != nil {
t.Fatalf("Failed to create pipe: %v", err)
}

// Replace stderr with the write end of the pipe
os.Stderr = w

// Create a channel to receive the captured output
outputChan := make(chan string, 1)

// Read from the pipe in a goroutine
go func() {
var buf bytes.Buffer
io.Copy(&buf, r)
outputChan <- buf.String()
}()

// Execute the function
fn()

// Close the write end and restore stderr
w.Close()
os.Stderr = oldStderr

// Get the captured output
output := <-outputChan
r.Close()

Comment on lines +18 to +50
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

captureStderr mutates global os.Stderr but doesn’t guarantee restoration/pipe closure if fn panics, which can leak the redirected stderr into later tests and can also deadlock the reader goroutine (io.Copy won’t finish until w is closed). Use t.Cleanup/defer to restore os.Stderr and close w/r, and consider recovering panic to ensure cleanup always runs.

Copilot uses AI. Check for mistakes.
return output
}

func TestMoveCursorUp(t *testing.T) {
tests := []struct {
name string
lines int
}{
{
name: "move up 1 line",
lines: 1,
},
{
name: "move up 5 lines",
lines: 5,
},
{
name: "move up 0 lines",
lines: 0,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
output := captureStderr(t, func() {
MoveCursorUp(tt.lines)
})

// In non-TTY environments, output should be empty
// We just ensure no panic occurs
assert.NotNil(t, output, "MoveCursorUp should not panic")
})
Comment on lines +73 to +82
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

These assertions are ineffective: output is a string, so it can never be nil. If the intent is “does not panic”, use assert.NotPanics around MoveCursorUp, and/or assert the captured output is empty given captureStderr forces stderr to be non-TTY.

This issue also appears on line 105 of the same file.

Copilot uses AI. Check for mistakes.
}
}

func TestMoveCursorDown(t *testing.T) {
tests := []struct {
name string
lines int
}{
{
name: "move down 1 line",
lines: 1,
},
{
name: "move down 5 lines",
lines: 5,
},
{
name: "move down 0 lines",
lines: 0,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
output := captureStderr(t, func() {
MoveCursorDown(tt.lines)
})

// In non-TTY environments, output should be empty
// We just ensure no panic occurs
assert.NotNil(t, output, "MoveCursorDown should not panic")
})
}
}

func TestTerminalCursorFunctionsNoTTY(t *testing.T) {
// This test verifies that in non-TTY environments (like CI/tests),
// no ANSI codes are emitted for cursor movement functions

tests := []struct {
name string
fn func()
}{
{
name: "MoveCursorUp",
fn: func() { MoveCursorUp(5) },
},
{
name: "MoveCursorDown",
fn: func() { MoveCursorDown(3) },
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
output := captureStderr(t, tt.fn)

// Since tests typically run in non-TTY, verify output is empty
// This ensures we properly respect TTY detection
if os.Getenv("CI") != "" || !isRealTerminal() {
assert.Empty(t, output, "%s should not output ANSI codes in non-TTY", tt.name)
}
})
Comment on lines +136 to +145
Copy link

Copilot AI Feb 7, 2026

Choose a reason for hiding this comment

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

This test can become a no-op locally because the assertion is gated on CI env / isRealTerminal(). Since captureStderr always redirects stderr to a pipe (non-TTY), you can assert output is empty unconditionally here to actually validate the TTY guard behavior.

Copilot uses AI. Check for mistakes.
}
}

// isRealTerminal checks if we're actually running in a terminal
// This is a helper to distinguish between test environments and real terminals
func isRealTerminal() bool {
// In test environments, stderr is typically redirected
fileInfo, err := os.Stderr.Stat()
if err != nil {
return false
}
// Check if stderr is a character device (terminal)
return (fileInfo.Mode() & os.ModeCharDevice) != 0
}

func TestTerminalCursorFunctionsDoNotPanic(t *testing.T) {
// Ensure all cursor movement functions can be called safely without panicking
// even in edge cases

t.Run("all cursor functions", func(t *testing.T) {
assert.NotPanics(t, func() {
MoveCursorUp(0)
MoveCursorUp(100)
MoveCursorDown(0)
MoveCursorDown(100)
}, "Cursor movement functions should never panic")
})
}
Loading