diff --git a/pkg/console/console.go b/pkg/console/console.go index 021a2639ef7..9d92857aef6 100644 --- a/pkg/console/console.go +++ b/pkg/console/console.go @@ -42,22 +42,23 @@ func applyStyle(style lipgloss.Style, text string) string { return applyStdoutStyleWithTTY(style, text, isTTY, stdoutEnviron()) } -func applyStdoutStyleWithTTY(style lipgloss.Style, text string, ttyCheck func() bool, environ []string) string { +func applyStyleWithTTYAndEnviron(style lipgloss.Style, text string, ttyCheck func() bool, environ []string) string { if !ttyCheck() { return text } return colorwriter.Degrade(style.Render(text), environ) } +func applyStdoutStyleWithTTY(style lipgloss.Style, text string, ttyCheck func() bool, environ []string) string { + return applyStyleWithTTYAndEnviron(style, text, ttyCheck, environ) +} + func applyStderrStyle(style lipgloss.Style, text string) string { return applyStderrStyleWithTTY(style, text, isStderrTTY, stderrEnviron()) } func applyStderrStyleWithTTY(style lipgloss.Style, text string, ttyCheck func() bool, environ []string) string { - if !ttyCheck() { - return text - } - return colorwriter.Degrade(style.Render(text), environ) + return applyStyleWithTTYAndEnviron(style, text, ttyCheck, environ) } // applyStyleWithTTY conditionally renders raw ANSI based on a provided TTY check. diff --git a/pkg/console/console_formatting_test.go b/pkg/console/console_formatting_test.go index 44f7b629e6d..012ca0b1fbc 100644 --- a/pkg/console/console_formatting_test.go +++ b/pkg/console/console_formatting_test.go @@ -309,6 +309,29 @@ func TestApplyStderrStyleWithTTY(t *testing.T) { }) } +func TestApplyStyleWithTTYAndEnviron(t *testing.T) { + t.Run("plain text when not tty", func(t *testing.T) { + result := applyStyleWithTTYAndEnviron(styles.Warning, "warning", func() bool { return false }, []string{"TERM=xterm-256color"}) + if result != "warning" { + t.Fatalf("applyStyleWithTTYAndEnviron() = %q, want plain text", result) + } + }) + + t.Run("styled text when tty", func(t *testing.T) { + result := applyStyleWithTTYAndEnviron(styles.Warning, "warning", func() bool { return true }, []string{"TERM=xterm-256color"}) + if !strings.Contains(result, "\x1b[") { + t.Fatalf("applyStyleWithTTYAndEnviron() = %q, want ANSI styling", result) + } + }) + + t.Run("no color disables styling", func(t *testing.T) { + result := applyStyleWithTTYAndEnviron(styles.Warning, "warning", func() bool { return true }, []string{"TERM=xterm-256color", "NO_COLOR=1"}) + if strings.Contains(result, "\x1b[") { + t.Fatalf("applyStyleWithTTYAndEnviron() = %q, want ANSI-free text", result) + } + }) +} + func TestFormatErrorStderrWithTTY(t *testing.T) { err := CompilerError{ Position: ErrorPosition{File: "workflow.md", Line: 2, Column: 3}, diff --git a/pkg/console/render.go b/pkg/console/render.go index 31c6e1a4d27..ed6cac44334 100644 --- a/pkg/console/render.go +++ b/pkg/console/render.go @@ -586,39 +586,48 @@ func applyTagFormat(val reflect.Value, format, baseValue string) string { return baseValue } -// applyNumberFormat formats a value as a human-readable number (e.g., "1k", "1.2M"). -func applyNumberFormat(val reflect.Value, baseValue string) string { +// applyIntegerFormat formats integer values via a shared dispatcher used by +// number/filesize tag formatters. +func applyIntegerFormat(val reflect.Value, baseValue string, format func(int64) string) string { if val.CanInterface() { switch v := val.Interface().(type) { case int: - return FormatNumber(v) + return format(int64(v)) case int64: - // #nosec G115 -- Converting int64 to int for display formatting - return FormatNumber(int(v)) + return format(v) case int32: - return FormatNumber(int(v)) + return format(int64(v)) case uint: - // #nosec G115 -- Converting uint to int for display formatting - return FormatNumber(int(v)) + // #nosec G115 -- Converting uint to int64 for display formatting + return format(int64(v)) case uint64: - // #nosec G115 -- Converting uint64 to int for display formatting - return FormatNumber(int(v)) + // #nosec G115 -- Converting uint64 to int64 for display formatting + return format(int64(v)) case uint32: - return FormatNumber(int(v)) + return format(int64(v)) } } + // Fallback: use integer kind directly, keeping signed and unsigned separate // to avoid calling Int() on an unsigned kind (which panics). switch { case val.Kind() >= reflect.Int && val.Kind() <= reflect.Int64: - return FormatNumber(int(val.Int())) + return format(val.Int()) case val.Kind() >= reflect.Uint && val.Kind() <= reflect.Uint64: - // #nosec G115 -- Converting uint to int for display formatting - return FormatNumber(int(val.Uint())) + // #nosec G115 -- Converting uint to int64 for display formatting + return format(int64(val.Uint())) } return baseValue } +// applyNumberFormat formats a value as a human-readable number (e.g., "1k", "1.2M"). +func applyNumberFormat(val reflect.Value, baseValue string) string { + return applyIntegerFormat(val, baseValue, func(v int64) string { + // #nosec G115 -- Converting int64 to int for display formatting + return FormatNumber(int(v)) + }) +} + // applyCostFormat formats a value as currency with $ prefix. func applyCostFormat(val reflect.Value, baseValue string) string { if val.CanInterface() { @@ -643,33 +652,7 @@ func applyCostFormat(val reflect.Value, baseValue string) string { // applyFilesizeFormat formats a value as a human-readable file size (e.g., "1.2 MB"). func applyFilesizeFormat(val reflect.Value, baseValue string) string { - if val.CanInterface() { - switch v := val.Interface().(type) { - case int: - return FormatFileSize(int64(v)) - case int64: - return FormatFileSize(v) - case int32: - return FormatFileSize(int64(v)) - case uint: - // #nosec G115 -- Converting uint to int64 for file size display - return FormatFileSize(int64(v)) - case uint64: - // #nosec G115 -- Converting uint64 to int64 for file size display - return FormatFileSize(int64(v)) - case uint32: - return FormatFileSize(int64(v)) - } - } - // Fallback for integer kinds - if val.Kind() >= reflect.Int && val.Kind() <= reflect.Int64 { - return FormatFileSize(val.Int()) - } - if val.Kind() >= reflect.Uint && val.Kind() <= reflect.Uint64 { - // #nosec G115 -- Converting uint to int64 for file size display - return FormatFileSize(int64(val.Uint())) - } - return baseValue + return applyIntegerFormat(val, baseValue, FormatFileSize) } // FormatNumber formats large numbers in a human-readable way (e.g., "1k", "1.2k", "1.12M") diff --git a/pkg/constants/constants_test.go b/pkg/constants/constants_test.go index 5a3d006dd34..c2f66fd6a7e 100644 --- a/pkg/constants/constants_test.go +++ b/pkg/constants/constants_test.go @@ -353,6 +353,7 @@ func TestHelperMethods(t *testing.T) { {"Version", Version("1.0.0"), Version(""), "1.0.0"}, {"JobName", JobName("agent"), JobName(""), "agent"}, {"StepID", StepID("check_membership"), StepID(""), "check_membership"}, + {"MCPServerID", MCPServerID("github"), MCPServerID(""), "github"}, {"CommandPrefix", CommandPrefix("gh aw"), CommandPrefix(""), "gh aw"}, } diff --git a/pkg/constants/job_constants.go b/pkg/constants/job_constants.go index 27dac756d50..016636cda5f 100644 --- a/pkg/constants/job_constants.go +++ b/pkg/constants/job_constants.go @@ -55,6 +55,11 @@ func (m MCPServerID) String() string { return string(m) } +// IsValid returns true if the MCP server ID is non-empty +func (m MCPServerID) IsValid() bool { + return m != "" +} + // Job name constants for GitHub Actions workflow jobs const AgentJobName JobName = "agent" const ActivationJobName JobName = "activation" diff --git a/pkg/constants/spec_test.go b/pkg/constants/spec_test.go index f7250b40a9c..cdcf02eb125 100644 --- a/pkg/constants/spec_test.go +++ b/pkg/constants/spec_test.go @@ -165,6 +165,19 @@ func TestSpec_SemanticTypes_StringAndIsValid(t *testing.T) { "empty StepID.IsValid() should return false") }) + t.Run("MCPServerID implements String and IsValid", func(t *testing.T) { + t.Parallel() + m := constants.GitHubMCPServerID + assert.Equal(t, "github", m.String(), + "GitHubMCPServerID.String() should return 'github' as documented") + assert.True(t, m.IsValid(), + "non-empty MCPServerID.IsValid() should return true") + + empty := constants.MCPServerID("") + assert.False(t, empty.IsValid(), + "empty MCPServerID.IsValid() should return false") + }) + t.Run("CommandPrefix implements String and IsValid", func(t *testing.T) { t.Parallel() // From spec: CLIExtensionPrefix // "gh aw" — user-facing CLI prefix