fix(io): emit valid GitHub log groups, and only on GitHub

io/ci.go

@@ -1,8 +1,33 @@
 package io
 
-import "os"
+import (
+	"os"
+	"strings"
+)
 
-// isCI returns true if running in a CI environment.
-func isCI() bool {
-	return os.Getenv("GITHUB_ACTIONS") == "true" || os.Getenv("CI") == "true"
+// envTrue is the value CI providers use for their boolean environment flags.
+const envTrue = "true"
+
+// isGitHubActions reports whether the runner is GitHub Actions specifically.
+//
+// Workflow commands like ::group:: are GitHub syntax, not a CI convention. Any
+// other provider renders them as literal text, so they must be gated on this
+// rather than on a general "is this CI" check.
+func isGitHubActions() bool {
+	return os.Getenv("GITHUB_ACTIONS") == envTrue
+}
+
+// workflowCommandEscaper escapes data in a GitHub workflow command. An
+// unescaped newline would end the command early and emit the remainder as
+// ordinary log lines; percent is escaped first so it cannot double-encode the
+// replacements that follow.
+var workflowCommandEscaper = strings.NewReplacer(
+	"%", "%25",
+	"\r", "%0D",
+	"\n", "%0A",
+)
+
+// escapeWorkflowData makes a value safe to embed in a workflow command.
+func escapeWorkflowData(s string) string {
+	return workflowCommandEscaper.Replace(s)
 }

io/ci_group_test.go

@@ -0,0 +1,119 @@
+package io_test
+
+import (
+	"os"
+	"strings"
+	"testing"
+
+	"github.com/flowexec/tuikit/io"
+)
+
+// groupOutput runs fn against a logger writing to a temp file and returns what
+// was written, with the given CI-related environment applied.
+func groupOutput(t *testing.T, env map[string]string, fn func(l *io.StandardLogger)) string {
+	t.Helper()
+	for k, v := range env {
+		t.Setenv(k, v)
+	}
+
+	f, err := os.CreateTemp(t.TempDir(), "tuikit-group")
+	if err != nil {
+		t.Fatalf("temp file: %v", err)
+	}
+	defer f.Close()
+
+	logger := io.NewLogger(io.WithMode(io.Text), io.WithOutput(f))
+	fn(logger)
+
+	if _, err := f.Seek(0, 0); err != nil {
+		t.Fatalf("seek: %v", err)
+	}
+	buf := make([]byte, 8192)
+	n, _ := f.Read(buf)
+	return string(buf[:n])
+}
+
+func TestBeginGroupEmitsWorkflowCommandOnGitHubActions(t *testing.T) {
+	out := groupOutput(t, map[string]string{"GITHUB_ACTIONS": "true", "CI": "true"},
+		func(l *io.StandardLogger) {
+			l.BeginGroup("build app")
+			l.EndGroup()
+		})
+
+	if !strings.Contains(out, "::group::build app") {
+		t.Errorf("output = %q, want a ::group:: command", out)
+	}
+	if !strings.Contains(out, "::endgroup::") {
+		t.Errorf("output = %q, want an ::endgroup:: command", out)
+	}
+}
+
+// ::group:: is GitHub syntax, not a CI convention. Emitting it on another
+// provider prints literal noise into the log rather than collapsing anything.
+func TestBeginGroupIsPlainOnNonGitHubCI(t *testing.T) {
+	out := groupOutput(t, map[string]string{"GITHUB_ACTIONS": "", "CI": "true"},
+		func(l *io.StandardLogger) {
+			l.BeginGroup("build app")
+			l.EndGroup()
+		})
+
+	if strings.Contains(out, "::group::") || strings.Contains(out, "::endgroup::") {
+		t.Errorf("output = %q, want no GitHub workflow commands outside GitHub Actions", out)
+	}
+	if !strings.Contains(out, "build app") {
+		t.Errorf("output = %q, want the group name in the plain header", out)
+	}
+}
+
+// GitHub does not support nested groups: a second ::group:: before the first
+// closes swallows the remainder of the log.
+func TestNestedGroupsEmitOnlyTheOutermost(t *testing.T) {
+	out := groupOutput(t, map[string]string{"GITHUB_ACTIONS": "true"},
+		func(l *io.StandardLogger) {
+			l.BeginGroup("outer")
+			l.BeginGroup("inner")
+			l.EndGroup()
+			l.EndGroup()
+		})
+
+	if got := strings.Count(out, "::group::"); got != 1 {
+		t.Errorf("::group:: count = %d, want 1; output = %q", got, out)
+	}
+	if got := strings.Count(out, "::endgroup::"); got != 1 {
+		t.Errorf("::endgroup:: count = %d, want 1; output = %q", got, out)
+	}
+	if !strings.Contains(out, "::group::outer") {
+		t.Errorf("output = %q, want the outermost group to be the one emitted", out)
+	}
+}
+
+// A stray ::endgroup:: would close whatever GitHub had open around it, so an
+// unmatched EndGroup must emit nothing. This is what lets a caller skip
+// BeginGroup conditionally without having to mirror that condition on the way out.
+func TestUnmatchedEndGroupEmitsNothing(t *testing.T) {
+	out := groupOutput(t, map[string]string{"GITHUB_ACTIONS": "true"},
+		func(l *io.StandardLogger) {
+			l.EndGroup()
+		})
+
+	if strings.Contains(out, "::endgroup::") {
+		t.Errorf("output = %q, want no ::endgroup:: without a matching BeginGroup", out)
+	}
+}
+
+// An unescaped newline ends the workflow command early and leaks the remainder
+// as ordinary log lines.
+func TestGroupNameIsEscaped(t *testing.T) {
+	out := groupOutput(t, map[string]string{"GITHUB_ACTIONS": "true"},
+		func(l *io.StandardLogger) {
+			l.BeginGroup("build\nrm -rf / 100%")
+			l.EndGroup()
+		})
+
+	if !strings.Contains(out, "::group::build%0Arm -rf / 100%25") {
+		t.Errorf("output = %q, want newline and percent escaped", out)
+	}
+	if strings.Count(out, "\n::group::") > 0 && strings.Count(out, "::group::") != 1 {
+		t.Errorf("output = %q, want a single well-formed group command", out)
+	}
+}

io/logger.go

@@ -39,6 +39,10 @@ type StandardLogger struct {
 	// writeMu serializes the Std{Out,Err}Writer flip/render/restore sequence so concurrent
 	// writers cannot interleave and render each other's output in the wrong mode.
 	writeMu sync.Mutex
+	// groupMu guards groupDepth, which tracks open GitHub Actions log groups so
+	// that nested or unbalanced Begin/EndGroup calls cannot emit invalid output.
+	groupMu    sync.Mutex
+	groupDepth int
 }
 
 type LoggerOptions func(*StandardLogger)
@@ -609,19 +613,41 @@ func (l *StandardLogger) archiveTaskSummary(tasks []*TaskContext) {
 }
 
 func (l *StandardLogger) BeginGroup(name string) {
-	if isCI() {
-		_, _ = fmt.Fprintf(l.outFile, "::group::%s\n", name)
-	} else {
-		palette := l.theme.ColorPalette()
-		style := lipgloss.NewStyle().
-			Foreground(lipgloss.Color(palette.Secondary)).
-			Bold(true)
-		_, _ = fmt.Fprintf(l.outWriter, "\n%s\n", style.Render("--- "+name+" ---"))
+	if isGitHubActions() {
+		l.groupMu.Lock()
+		defer l.groupMu.Unlock()
+		// GitHub does not support nested groups: a second ::group:: before the
+		// first closes swallows the rest of the log. Track depth and emit only
+		// the outermost, so a nested caller degrades to no group rather than a
+		// broken one.
+		l.groupDepth++
+		if l.groupDepth == 1 {
+			_, _ = fmt.Fprintf(l.outFile, "::group::%s\n", escapeWorkflowData(name))
+		}
+		return
 	}
+
+	palette := l.theme.ColorPalette()
+	style := lipgloss.NewStyle().
+		Foreground(lipgloss.Color(palette.Secondary)).
+		Bold(true)
+	_, _ = fmt.Fprintf(l.outWriter, "\n%s\n", style.Render("--- "+name+" ---"))
 }
 
 func (l *StandardLogger) EndGroup() {
-	if isCI() {
+	if !isGitHubActions() {
+		return
+	}
+
+	l.groupMu.Lock()
+	defer l.groupMu.Unlock()
+	// Without a matching BeginGroup there is nothing to close, and a stray
+	// ::endgroup:: would collapse whatever GitHub had open around it.
+	if l.groupDepth == 0 {
+		return
+	}
+	l.groupDepth--
+	if l.groupDepth == 0 {
 		_, _ = fmt.Fprintln(l.outFile, "::endgroup::")
 	}
 }