Unify command output line splitting across executors

pkg/executor/dry.go

@@ -27,12 +27,7 @@ func (ex *Dry) Run(_ context.Context, cmd string, _ *RunOpts) (out []string, err
 	var stdoutBuf bytes.Buffer
 	mwr := io.MultiWriter(ex.logs.Out, &stdoutBuf)
 	mwr.Write([]byte(cmd)) // nolint
-	for line := range strings.SplitSeq(stdoutBuf.String(), "\n") {
-		if line != "" {
-			out = append(out, line)
-		}
-	}
-	return out, nil
+	return splitOutputLines(stdoutBuf.String()), nil
 }
 
 // Upload doesn't actually upload, just prints the command

pkg/executor/dry_test.go

@@ -14,10 +14,19 @@ import (
 func TestDry_Run(t *testing.T) {
 	ctx := context.Background()
 	dry := NewDry(MakeLogs(true, false, nil))
-	res, err := dry.Run(ctx, "ls -la /srv", &RunOpts{Verbose: true})
-	require.NoError(t, err)
-	require.Len(t, res, 1)
-	require.Equal(t, "ls -la /srv", res[0])
+
+	t.Run("single line", func(t *testing.T) {
+		res, err := dry.Run(ctx, "ls -la /srv", &RunOpts{Verbose: true})
+		require.NoError(t, err)
+		require.Len(t, res, 1)
+		require.Equal(t, "ls -la /srv", res[0])
+	})
+
+	t.Run("multi line with blank line", func(t *testing.T) {
+		res, err := dry.Run(ctx, "ls -la /srv\n\ndf -h\n", &RunOpts{Verbose: true})
+		require.NoError(t, err)
+		require.Equal(t, []string{"ls -la /srv", "", "df -h"}, res)
+	})
 }
 
 func TestDryUpload(t *testing.T) {

pkg/executor/executor.go

@@ -48,6 +48,22 @@ type DeleteOpts struct {
 // exclude patterns and remote paths always use forward slashes.
 func normalizeSlashes(s string) string { return strings.ReplaceAll(s, `\`, "/") }
 
+// splitOutputLines splits captured command output into lines the same way bufio.ScanLines does, i.e. dropping
+// a trailing \r and the empty element after the final newline, but without the scanner's token size limit.
+// All executors share it, so the same stdout produces the same lines regardless of the implementation.
+// The output is already fully buffered, so a single line of any length is returned as is.
+func splitOutputLines(s string) []string {
+	if s == "" {
+		return nil
+	}
+	lines := strings.Split(strings.TrimSuffix(s, "\n"), "\n")
+	res := make([]string, 0, len(lines))
+	for _, line := range lines {
+		res = append(res, strings.TrimSuffix(line, "\r"))
+	}
+	return res
+}
+
 // isExcluded reports whether fpath matches any of the exclude patterns. A pattern ending in "/*"
 // also matches the directory it names, so the whole subtree is protected, but this directory match
 // only applies when fpath is itself a directory. This prevents a pattern like "dir*/*" from

pkg/executor/executor_test.go

@@ -123,3 +123,25 @@ func captureStdOut(t *testing.T, f func()) string {
 	io.Copy(&buf, r)
 	return buf.String()
 }
+
+func TestSplitOutputLines(t *testing.T) {
+	tbl := []struct {
+		name string
+		in   string
+		res  []string
+	}{
+		{"empty", "", nil},
+		{"single line, no trailing newline", "hello", []string{"hello"}},
+		{"single line with trailing newline", "hello\n", []string{"hello"}},
+		{"multiple lines", "line1\nline2\nline3\n", []string{"line1", "line2", "line3"}},
+		{"blank line in the middle", "line1\n\nline2\n", []string{"line1", "", "line2"}},
+		{"single newline", "\n", []string{""}},
+		{"trailing blank line", "line1\n\n", []string{"line1", ""}},
+		{"crlf line endings", "line1\r\nline2\r\n", []string{"line1", "line2"}},
+	}
+	for _, tt := range tbl {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.res, splitOutputLines(tt.in))
+		})
+	}
+}

pkg/executor/local.go

@@ -62,21 +62,6 @@ func (l *Local) Run(ctx context.Context, cmd string, _ *RunOpts) (out []string,
 	return splitOutputLines(stdoutBuf.String()), nil
 }
 
-// splitOutputLines splits captured command output into lines the same way bufio.ScanLines does, i.e. dropping
-// a trailing \r and the empty element after the final newline, but without the scanner's token size limit.
-// The output is already fully buffered, so a single line of any length is returned as is.
-func splitOutputLines(s string) []string {
-	if s == "" {
-		return nil
-	}
-	lines := strings.Split(strings.TrimSuffix(s, "\n"), "\n")
-	res := make([]string, 0, len(lines))
-	for _, line := range lines {
-		res = append(res, strings.TrimSuffix(line, "\r"))
-	}
-	return res
-}
-
 // Upload just copy file from one place to another
 func (l *Local) Upload(ctx context.Context, src, dst string, opts *UpDownOpts) (err error) {
 

pkg/executor/local_test.go

@@ -98,28 +98,6 @@ func TestRun(t *testing.T) {
 	})
 }
 
-func TestSplitOutputLines(t *testing.T) {
-	tbl := []struct {
-		name string
-		in   string
-		res  []string
-	}{
-		{"empty", "", nil},
-		{"single line, no trailing newline", "hello", []string{"hello"}},
-		{"single line with trailing newline", "hello\n", []string{"hello"}},
-		{"multiple lines", "line1\nline2\nline3\n", []string{"line1", "line2", "line3"}},
-		{"blank line in the middle", "line1\n\nline2\n", []string{"line1", "", "line2"}},
-		{"single newline", "\n", []string{""}},
-		{"trailing blank line", "line1\n\n", []string{"line1", ""}},
-		{"crlf line endings", "line1\r\nline2\r\n", []string{"line1", "line2"}},
-	}
-	for _, tt := range tbl {
-		t.Run(tt.name, func(t *testing.T) {
-			assert.Equal(t, tt.res, splitOutputLines(tt.in))
-		})
-	}
-}
-
 func TestUploadAndDownload(t *testing.T) {
 	testCases := []struct {
 		name        string

pkg/executor/remote.go

@@ -11,7 +11,6 @@ import (
 	"os"
 	"path/filepath"
 	"slices"
-	"strings"
 	"time"
 
 	"github.com/pkg/sftp"
@@ -357,12 +356,7 @@ func (ex *Remote) sshRun(ctx context.Context, client *ssh.Client, command string
 		return nil, fmt.Errorf("canceled: %w", ctx.Err())
 	}
 
-	for line := range strings.SplitSeq(stdoutBuf.String(), "\n") {
-		if line != "" {
-			out = append(out, line)
-		}
-	}
-	return out, nil
+	return splitOutputLines(stdoutBuf.String()), nil
 }
 
 type sftpReq struct {

pkg/executor/remote_test.go

@@ -431,6 +431,12 @@ func TestExecuter_Run(t *testing.T) {
 		assert.Equal(t, "data2.txt", out[1])
 	})
 
+	t.Run("blank lines and crlf preserved as local does", func(t *testing.T) {
+		out, e := sess.Run(ctx, "sh -c 'printf \"line1\\n\\nline2\\r\\n\"'", nil)
+		require.NoError(t, e)
+		assert.Equal(t, []string{"line1", "", "line2"}, out)
+	})
+
 	t.Run("find out", func(t *testing.T) {
 		cmd := fmt.Sprintf("find %s -type f -exec stat -c '%%n:%%s' {} \\;", "/tmp/")
 		out, e := sess.Run(ctx, cmd, &RunOpts{Verbose: true})

pkg/runner/commands.go

@@ -561,7 +561,13 @@ func (ec *execCmd) Echo(ctx context.Context) (resp execCmdResp, err error) {
 	if err != nil {
 		return resp, ec.errorFmt("can't run echo command on %s: %w", ec.hostAddr, err)
 	}
-	resp.details = fmt.Sprintf(" {echo: %s}", strings.Join(out, "; "))
+	printed := make([]string, 0, len(out))
+	for _, line := range out { // empty lines carry nothing for the report and would show up as empty segments
+		if line != "" {
+			printed = append(printed, line)
+		}
+	}
+	resp.details = fmt.Sprintf(" {echo: %s}", strings.Join(printed, "; "))
 	return resp, nil
 }
 

pkg/runner/commands_test.go

@@ -794,6 +794,20 @@ func Test_execEcho(t *testing.T) {
 		assert.Equal(t, " {echo: foo welcome back}", resp.details)
 	})
 
+	t.Run("echo command with blank line in output", func(t *testing.T) {
+		ec := execCmd{exec: sess, tsk: &config.Task{Name: "test"}, cmd: config.Cmd{Echo: "$(echo first; echo; echo second)", Name: "test"}}
+		resp, err := ec.Echo(ctx)
+		require.NoError(t, err)
+		assert.Equal(t, " {echo: first; second}", resp.details)
+	})
+
+	t.Run("echo command with whitespace-only line in output", func(t *testing.T) {
+		ec := execCmd{exec: sess, tsk: &config.Task{Name: "test"}, cmd: config.Cmd{Echo: "$(echo first; echo '   '; echo second)", Name: "test"}}
+		resp, err := ec.Echo(ctx)
+		require.NoError(t, err)
+		assert.Equal(t, " {echo: first;    ; second}", resp.details)
+	})
+
 	t.Run("echo command with condition true", func(t *testing.T) {
 		defer os.Remove("/tmp/test.condition")
 		_, err := sess.Run(ctx, "touch /tmp/test.condition", nil)