Skip to content

Commit 66929bc

Browse files
Copilotwbrezahemarina
authored
Fix hyperlink ANSI escape codes appearing in non-terminal output (#6015)
* Initial plan * Fix hyperlink functions to detect terminal state and return plain URLs in non-terminal environments Co-authored-by: wbreza <[email protected]> * Use input.IsTerminal logic for hyperlink terminal detection and revert unrelated extension changes Co-authored-by: hemarina <[email protected]> * Refactor: Move IsTerminal to internal/terminal package to eliminate code duplication Co-authored-by: wbreza <[email protected]> * Simplify hyperlink output in non-terminal mode and remove input.IsTerminal wrapper Co-authored-by: hemarina <[email protected]> --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: wbreza <[email protected]> Co-authored-by: hemarina <[email protected]>
1 parent 2d1fb1e commit 66929bc

File tree

8 files changed

+186
-26
lines changed

8 files changed

+186
-26
lines changed

cli/azd/.vscode/cspell.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ words:
3030
- protoimpl
3131
- Retryable
3232
- runcontext
33+
- surveyterm
3334
- unmarshals
3435
- unmarshaling
3536
- unsetting

cli/azd/cmd/container.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import (
2727
"github.com/azure/azure-dev/cli/azd/internal/cmd"
2828
"github.com/azure/azure-dev/cli/azd/internal/grpcserver"
2929
"github.com/azure/azure-dev/cli/azd/internal/repository"
30+
"github.com/azure/azure-dev/cli/azd/internal/terminal"
3031
"github.com/azure/azure-dev/cli/azd/pkg/account"
3132
"github.com/azure/azure-dev/cli/azd/pkg/ai"
3233
"github.com/azure/azure-dev/cli/azd/pkg/alpha"
@@ -132,7 +133,7 @@ func registerCommonDependencies(container *ioc.NestedContainer) {
132133
}
133134

134135
isTerminal := cmd.OutOrStdout() == os.Stdout &&
135-
cmd.InOrStdin() == os.Stdin && input.IsTerminal(os.Stdout.Fd(), os.Stdin.Fd())
136+
cmd.InOrStdin() == os.Stdin && terminal.IsTerminal(os.Stdout.Fd(), os.Stdin.Fd())
136137

137138
return input.NewConsole(rootOptions.NoPrompt, isTerminal, input.Writers{Output: writer}, input.ConsoleHandles{
138139
Stdin: cmd.InOrStdin(),
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package terminal
5+
6+
import (
7+
"os"
8+
"strconv"
9+
10+
"github.com/azure/azure-dev/cli/azd/internal/tracing/resource"
11+
"github.com/mattn/go-isatty"
12+
)
13+
14+
// IsTerminal returns true if the given file descriptors are attached to a terminal,
15+
// taking into account of environment variables that force TTY behavior.
16+
func IsTerminal(stdoutFd uintptr, stdinFd uintptr) bool {
17+
// User override to force TTY behavior
18+
if forceTty, err := strconv.ParseBool(os.Getenv("AZD_FORCE_TTY")); err == nil {
19+
return forceTty
20+
}
21+
22+
// By default, detect if we are running on CI and force no TTY mode if we are.
23+
// If this is affecting you locally while debugging on a CI machine,
24+
// use the override AZD_FORCE_TTY=true.
25+
if resource.IsRunningOnCI() {
26+
return false
27+
}
28+
29+
return isatty.IsTerminal(stdoutFd) && isatty.IsTerminal(stdinFd)
30+
}

cli/azd/pkg/input/console.go

Lines changed: 5 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,14 @@ import (
2121
"time"
2222

2323
"github.com/AlecAivazis/survey/v2"
24-
"github.com/AlecAivazis/survey/v2/terminal"
24+
surveyterm "github.com/AlecAivazis/survey/v2/terminal"
2525
"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
2626
"github.com/Azure/azure-sdk-for-go/sdk/azcore/to"
2727
"github.com/azure/azure-dev/cli/azd/internal/tracing"
28-
"github.com/azure/azure-dev/cli/azd/internal/tracing/resource"
2928
"github.com/azure/azure-dev/cli/azd/pkg/alpha"
3029
"github.com/azure/azure-dev/cli/azd/pkg/output"
3130
"github.com/azure/azure-dev/cli/azd/pkg/output/ux"
3231
tm "github.com/buger/goterm"
33-
"github.com/mattn/go-isatty"
3432
"github.com/nathan-fiscaletti/consolesize-go"
3533
"github.com/theckman/yacspin"
3634
"go.uber.org/atomic"
@@ -600,7 +598,7 @@ func (c *AskerConsole) Prompt(ctx context.Context, options ConsoleOptions) (stri
600598

601599
result, err := c.promptClient.Prompt(ctx, opts)
602600
if errors.Is(err, promptCancelledErr) {
603-
return "", terminal.InterruptErr
601+
return "", surveyterm.InterruptErr
604602
} else if err != nil {
605603
return "", err
606604
}
@@ -655,7 +653,7 @@ func (c *AskerConsole) Select(ctx context.Context, options ConsoleOptions) (int,
655653

656654
result, err := c.promptClient.Prompt(ctx, opts)
657655
if errors.Is(err, promptCancelledErr) {
658-
return -1, terminal.InterruptErr
656+
return -1, surveyterm.InterruptErr
659657
} else if err != nil {
660658
return -1, err
661659
}
@@ -735,7 +733,7 @@ func (c *AskerConsole) MultiSelect(ctx context.Context, options ConsoleOptions)
735733

736734
result, err := c.promptClient.Prompt(ctx, opts)
737735
if errors.Is(err, promptCancelledErr) {
738-
return nil, terminal.InterruptErr
736+
return nil, surveyterm.InterruptErr
739737
} else if err != nil {
740738
return nil, err
741739
}
@@ -802,7 +800,7 @@ func (c *AskerConsole) Confirm(ctx context.Context, options ConsoleOptions) (boo
802800

803801
result, err := c.promptClient.Prompt(ctx, opts)
804802
if errors.Is(err, promptCancelledErr) {
805-
return false, terminal.InterruptErr
803+
return false, surveyterm.InterruptErr
806804
} else if err != nil {
807805
return false, err
808806
}
@@ -1023,24 +1021,6 @@ func NewConsole(
10231021
return c
10241022
}
10251023

1026-
// IsTerminal returns true if the given file descriptors are attached to a terminal,
1027-
// taking into account of environment variables that force TTY behavior.
1028-
func IsTerminal(stdoutFd uintptr, stdinFd uintptr) bool {
1029-
// User override to force TTY behavior
1030-
if forceTty, err := strconv.ParseBool(os.Getenv("AZD_FORCE_TTY")); err == nil {
1031-
return forceTty
1032-
}
1033-
1034-
// By default, detect if we are running on CI and force no TTY mode if we are.
1035-
// If this is affecting you locally while debugging on a CI machine,
1036-
// use the override AZD_FORCE_TTY=true.
1037-
if resource.IsRunningOnCI() {
1038-
return false
1039-
}
1040-
1041-
return isatty.IsTerminal(stdoutFd) && isatty.IsTerminal(stdinFd)
1042-
}
1043-
10441024
func GetStepResultFormat(result error) SpinnerUxType {
10451025
formatResult := StepDone
10461026
if result != nil {

cli/azd/pkg/output/colors.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strconv"
1010
"strings"
1111

12+
"github.com/azure/azure-dev/cli/azd/internal/terminal"
1213
"github.com/charmbracelet/glamour"
1314
"github.com/fatih/color"
1415
"github.com/nathan-fiscaletti/consolesize-go"
@@ -95,7 +96,15 @@ func WithMarkdown(markdownText string) string {
9596
}
9697

9798
// WithHyperlink wraps text with the colored hyperlink format escape sequence.
99+
// When stdout is not a terminal (e.g., in CI/CD pipelines like GitHub Actions),
100+
// it returns the plain URL without escape codes to avoid displaying raw ANSI sequences.
98101
func WithHyperlink(url string, text string) string {
102+
// Check if stdout is a terminal
103+
if !terminal.IsTerminal(os.Stdout.Fd(), os.Stdin.Fd()) {
104+
// Not a terminal - return plain URL without escape codes
105+
return url
106+
}
107+
// Terminal - use hyperlink escape codes
99108
return WithLinkFormat(fmt.Sprintf("\033]8;;%s\007%s\033]8;;\007", url, text))
100109
}
101110

cli/azd/pkg/output/colors_test.go

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License.
3+
4+
package output
5+
6+
import (
7+
"os"
8+
"testing"
9+
10+
"github.com/stretchr/testify/require"
11+
)
12+
13+
func TestWithHyperlink(t *testing.T) {
14+
tests := []struct {
15+
name string
16+
url string
17+
text string
18+
expectEscape bool
19+
expectedPlain string
20+
}{
21+
{
22+
name: "URL and text are the same",
23+
url: "https://example.com",
24+
text: "https://example.com",
25+
expectEscape: true,
26+
expectedPlain: "https://example.com",
27+
},
28+
{
29+
name: "URL and text are different",
30+
url: "https://example.com",
31+
text: "Example Site",
32+
expectEscape: true,
33+
expectedPlain: "https://example.com",
34+
},
35+
{
36+
name: "Text is empty",
37+
url: "https://example.com",
38+
text: "",
39+
expectEscape: true,
40+
expectedPlain: "https://example.com",
41+
},
42+
}
43+
44+
for _, tt := range tests {
45+
t.Run(tt.name, func(t *testing.T) {
46+
// Test non-terminal mode by checking the actual output
47+
// Since we can't reliably set os.Stdout.Fd() to non-terminal in tests,
48+
// we'll just verify the function doesn't panic and returns a string
49+
result := WithHyperlink(tt.url, tt.text)
50+
require.NotEmpty(t, result)
51+
52+
// When running in a non-TTY environment (like most CI systems),
53+
// the result should be the plain text version
54+
if !isTTY() {
55+
require.Equal(t, tt.expectedPlain, result)
56+
}
57+
})
58+
}
59+
}
60+
61+
// isTTY checks if stdout is a TTY (for testing purposes)
62+
func isTTY() bool {
63+
fileInfo, _ := os.Stdout.Stat()
64+
return (fileInfo.Mode() & os.ModeCharDevice) != 0
65+
}

cli/azd/pkg/ux/ux.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"strings"
1212
"unicode/utf8"
1313

14+
"github.com/azure/azure-dev/cli/azd/internal/terminal"
1415
"github.com/azure/azure-dev/cli/azd/pkg/ux/internal"
1516
"github.com/fatih/color"
1617
"github.com/nathan-fiscaletti/consolesize-go"
@@ -26,11 +27,20 @@ func init() {
2627
}
2728

2829
// Hyperlink returns a hyperlink formatted string.
30+
// When stdout is not a terminal (e.g., in CI/CD pipelines like GitHub Actions),
31+
// it returns the plain URL without escape codes to avoid displaying raw ANSI sequences.
2932
func Hyperlink(url string, text ...string) string {
3033
if len(text) == 0 {
3134
text = []string{url}
3235
}
3336

37+
// Check if stdout is a terminal
38+
if !terminal.IsTerminal(os.Stdout.Fd(), os.Stdin.Fd()) {
39+
// Not a terminal - return plain URL without escape codes
40+
return url
41+
}
42+
43+
// Terminal - use hyperlink escape codes
3444
return fmt.Sprintf("\033]8;;%s\007%s\033]8;;\007", url, text[0])
3545
}
3646

cli/azd/pkg/ux/ux_test.go

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,73 @@
44
package ux
55

66
import (
7+
"os"
78
"testing"
89
)
910

11+
func TestHyperlink(t *testing.T) {
12+
tests := []struct {
13+
name string
14+
url string
15+
text []string
16+
expectEscape bool
17+
expectedPlain string
18+
}{
19+
{
20+
name: "URL only",
21+
url: "https://example.com",
22+
text: nil,
23+
expectEscape: true,
24+
expectedPlain: "https://example.com",
25+
},
26+
{
27+
name: "URL and text are the same",
28+
url: "https://example.com",
29+
text: []string{"https://example.com"},
30+
expectEscape: true,
31+
expectedPlain: "https://example.com",
32+
},
33+
{
34+
name: "URL and text are different",
35+
url: "https://example.com",
36+
text: []string{"Example Site"},
37+
expectEscape: true,
38+
expectedPlain: "https://example.com",
39+
},
40+
{
41+
name: "Text is empty string",
42+
url: "https://example.com",
43+
text: []string{""},
44+
expectEscape: true,
45+
expectedPlain: "https://example.com",
46+
},
47+
}
48+
49+
for _, tt := range tests {
50+
t.Run(tt.name, func(t *testing.T) {
51+
// Test non-terminal mode by checking the actual output
52+
result := Hyperlink(tt.url, tt.text...)
53+
if len(result) == 0 {
54+
t.Errorf("expected non-empty result")
55+
}
56+
57+
// When running in a non-TTY environment (like most CI systems),
58+
// the result should be the plain text version
59+
if !isTTY() {
60+
if result != tt.expectedPlain {
61+
t.Errorf("expected %q, got %q", tt.expectedPlain, result)
62+
}
63+
}
64+
})
65+
}
66+
}
67+
68+
// isTTY checks if stdout is a TTY (for testing purposes)
69+
func isTTY() bool {
70+
fileInfo, _ := os.Stdout.Stat()
71+
return (fileInfo.Mode() & os.ModeCharDevice) != 0
72+
}
73+
1074
func Test_CountLineBreaks(t *testing.T) {
1175
testCases := []struct {
1276
name string

0 commit comments

Comments
 (0)