Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 24, 2026

Manual ANSI escape sequence in logs_orchestrator.go bypassed TTY detection, risking escape codes in piped output or logs.

Changes

  • Added console.ClearLine() with proper tty.IsStderrTerminal() detection
  • Replaced manual escape in logs_orchestrator.go:778 with console helper
  • Added test coverage following existing TestClearScreen pattern

Example

Before:

if progressBar != nil {
    fmt.Fprint(os.Stderr, "\r\033[K") // No TTY check
}

After:

if progressBar != nil {
    console.ClearLine() // TTY-aware
}

The new helper follows the same pattern as ClearScreen() and centralizes ANSI escape management in the console package.

Original prompt

This section details on the original issue you should resolve

<issue_title>[plan] Abstract manual ANSI escape sequences to console package</issue_title>
<issue_description>## Objective

Move hardcoded ANSI escape sequence (\033[K) from logs_orchestrator.go into the console package as a proper helper function.

Context

From discussion #11611: The codebase has 1 instance of manual ANSI escape codes that should be abstracted through the console package for consistency and TTY detection.

Current Issue: logs_orchestrator.go:778 uses fmt.Fprint(os.Stderr, "\r\033[K") directly.

Approach

  1. Create console.ClearLine() helper function:

    • Add new function in pkg/console/ (suggest output.go or new terminal.go)
    • Implement TTY detection using tty.IsStderrTerminal()
    • Only output ANSI codes when in TTY mode
  2. Update logs_orchestrator.go:

    • Replace manual ANSI escape with console.ClearLine() call
    • Import console package if not already imported
  3. Add tests:

    • Test TTY mode outputs ANSI sequence
    • Test non-TTY mode outputs nothing
    • Add to existing console package tests

Files to Modify

  • Create/Update: pkg/console/terminal.go (or appropriate file)
  • Update: pkg/cli/logs_orchestrator.go (line ~778)
  • Update: pkg/console/*_test.go (add test coverage)

Implementation Example

// pkg/console/terminal.go
func ClearLine() {
    if tty.IsStderrTerminal() {
        fmt.Fprint(os.Stderr, "\r\033[K")
    }
}
// logs_orchestrator.go (replace line ~778)
// Before:
fmt.Fprint(os.Stderr, "\r\033[K")

// After:
console.ClearLine()

Acceptance Criteria

  • console.ClearLine() function created with TTY detection
  • Manual ANSI escape in logs_orchestrator.go replaced with console helper
  • Tests verify TTY and non-TTY behavior
  • Code passes make fmt and make lint
  • make agent-finish completes successfully

Priority

Phase 1: Critical Fix (1-2 hours estimated)

AI generated by Plan Command for discussion #11611

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

- Add console.ClearLine() helper with TTY detection using tty.IsStderrTerminal()
- Replace manual ANSI escape in logs_orchestrator.go with console.ClearLine()
- Add test coverage for ClearLine() function

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title [WIP] Abstract manual ANSI escape sequences to console package Abstract manual ANSI escape sequences to console package Jan 24, 2026
Copilot AI requested a review from pelikhan January 24, 2026 14:31
@pelikhan pelikhan marked this pull request as ready for review January 24, 2026 15:43
@pelikhan pelikhan merged commit 79a1b8a into main Jan 24, 2026
145 checks passed
@pelikhan pelikhan deleted the copilot/abstract-ansi-sequences-console branch January 24, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[plan] Abstract manual ANSI escape sequences to console package

2 participants