Merge pull request #321 from lets-cli/codex/update-failure-log-format

Fix command failure output formatting

Author: kindermax

docs/docs/changelog.md

@@ -11,6 +11,7 @@ title: Changelog
 * `[Added]` Expose `LETS_OS` and `LETS_ARCH` environment variables at command runtime.
 * `[Removed]` Drop deprecated `eval_env` directive. Use `env` with `sh` execution mode instead.
 * `[Added]` When a command or its `depends` chain fails, print an indented tree to stderr showing the full chain with the failing command highlighted
+* `[Changed]` Format command failure output as a `lets:`-prefixed tree plus a separate final status line such as `lets: exit status 1`.
 * `[Added]` Support `env_file` in global config and commands. File names are expanded after `env` is resolved, and values loaded from env files override values from `env`.
 * `[Changed]` Migrate the LSP YAML parser from the CGO-based tree-sitter bindings to pure-Go [`gotreesitter`](https://github.com/odvcencio/gotreesitter), removing the C toolchain requirement from normal builds and release packaging.
 * `[Refactoring]` Move CLI startup flow from `cmd/lets/main.go` into `internal/cli/cli.go`, keeping `main.go` as a thin launcher.

internal/cli/cli.go

@@ -135,6 +135,8 @@ func Main(version string, buildDate string) int {
 		var depErr *executor.DependencyError
 		if errors.As(err, &depErr) {
 			executor.PrintDependencyTree(depErr, os.Stderr)
+			log.Errorf("%s", depErr.FailureMessage())
+			return getExitCode(err, 1)
 		}
 
 		log.Errorf("%s", err.Error())

internal/executor/dependency_error.go

@@ -16,6 +16,8 @@ type DependencyError struct {
 	Err   error
 }
 
+const treePrefix = "lets: "
+
 func (e *DependencyError) Error() string { return e.Err.Error() }
 func (e *DependencyError) Unwrap() error { return e.Err }
 
@@ -29,6 +31,15 @@ func (e *DependencyError) ExitCode() int {
 	return 1
 }
 
+func (e *DependencyError) FailureMessage() string {
+	var executeErr *ExecuteError
+	if errors.As(e.Err, &executeErr) {
+		return executeErr.Cause().Error()
+	}
+
+	return e.Err.Error()
+}
+
 // prependToChain prepends name to the chain in err if err is already a *DependencyError,
 // otherwise wraps err in a new single-element DependencyError.
 func prependToChain(name string, err error) error {
@@ -45,9 +56,14 @@ func prependToChain(name string, err error) error {
 // Respects NO_COLOR automatically via fatih/color.
 func PrintDependencyTree(e *DependencyError, w io.Writer) {
 	red := color.New(color.FgRed).SprintFunc()
+	treeIndent := strings.Repeat(" ", len(treePrefix))
 
 	for i, name := range e.Chain {
-		indent := strings.Repeat("  ", i+1)
+		indent := treeIndent + strings.Repeat("  ", i+1)
+		if i == 0 {
+			indent = treePrefix
+		}
+
 		if i == len(e.Chain)-1 {
 			fmt.Fprintf(w, "%s%s  %s\n", indent, name, red("<-- failed here"))
 		} else {

internal/executor/dependency_error_test.go

@@ -3,8 +3,10 @@ package executor
 import (
 	"bytes"
 	"fmt"
+	"os"
 	"os/exec"
 	"strings"
+	"syscall"
 	"testing"
 )
 
@@ -101,6 +103,46 @@ func TestDependencyErrorError(t *testing.T) {
 	}
 }
 
+func TestDependencyErrorFailureMessage(t *testing.T) {
+	t.Run("returns root cause for execute errors", func(t *testing.T) {
+		depErr := &DependencyError{
+			Chain: []string{"lint"},
+			Err:   &ExecuteError{err: fmt.Errorf("failed to run command 'lint': %w", fmt.Errorf("exit status 1"))},
+		}
+
+		if got := depErr.FailureMessage(); got != "exit status 1" {
+			t.Fatalf("expected root cause message, got %q", got)
+		}
+	})
+
+	t.Run("keeps non execute errors intact", func(t *testing.T) {
+		depErr := &DependencyError{
+			Chain: []string{"lint"},
+			Err:   fmt.Errorf("failed to calculate checksum for command 'lint': missing file"),
+		}
+
+		if got := depErr.FailureMessage(); got != "failed to calculate checksum for command 'lint': missing file" {
+			t.Fatalf("expected original message, got %q", got)
+		}
+	})
+
+	t.Run("keeps path context for execute errors", func(t *testing.T) {
+		depErr := &DependencyError{
+			Chain: []string{"lint"},
+			Err: &ExecuteError{
+				err: fmt.Errorf(
+					"failed to run command 'lint': %w",
+					&os.PathError{Op: "chdir", Path: "/tmp/missing", Err: syscall.ENOENT},
+				),
+			},
+		}
+
+		if got := depErr.FailureMessage(); got != "chdir /tmp/missing: no such file or directory" {
+			t.Fatalf("expected path-aware message, got %q", got)
+		}
+	})
+}
+
 func TestPrintDependencyTree(t *testing.T) {
 	t.Run("single node", func(t *testing.T) {
 		depErr := &DependencyError{Chain: []string{"lint"}, Err: fmt.Errorf("fail")}
@@ -111,8 +153,8 @@ func TestPrintDependencyTree(t *testing.T) {
 		if len(lines) != 1 {
 			t.Fatalf("expected 1 line, got %d: %v", len(lines), lines)
 		}
-		if !strings.HasPrefix(lines[0], "  lint") {
-			t.Errorf("expected line to start with '  lint', got: %q", lines[0])
+		if !strings.HasPrefix(lines[0], "lets: lint") {
+			t.Errorf("expected line to start with 'lets: lint', got: %q", lines[0])
 		}
 		if !strings.Contains(out, "failed here") {
 			t.Errorf("expected 'failed here' annotation on lint line, got: %q", out)
@@ -137,9 +179,9 @@ func TestPrintDependencyTree(t *testing.T) {
 			name      string
 			hasFailed bool
 		}{
-			{"  ", "deploy", false},
-			{"    ", "build", false},
-			{"      ", "lint", true},
+			{"lets: ", "deploy", false},
+			{"          ", "build", false},
+			{"            ", "lint", true},
 		}
 		for i, c := range checks {
 			if !strings.HasPrefix(lines[i], c.prefix+c.name) {

internal/executor/executor.go

@@ -25,6 +25,18 @@ func (e *ExecuteError) Error() string {
 	return e.err.Error()
 }
 
+func (e *ExecuteError) Unwrap() error {
+	return e.err
+}
+
+func (e *ExecuteError) Cause() error {
+	if err := errors.Unwrap(e.err); err != nil {
+		return err
+	}
+
+	return e.err
+}
+
 // ExitCode will return exit code from underlying ExitError or returns default error code.
 func (e *ExecuteError) ExitCode() int {
 	var exitErr *exec.ExitError

tests/command_cmd.bats

@@ -43,7 +43,8 @@ setup() {
     # as there is no guarantee in which order cmds runs
     # we can not guarantee that all commands will run and complete.
     # But error message must be in the output.
-    assert_output --partial "failed to run command 'cmd-as-map-error': exit status 2"
+    assert_output --partial "lets: cmd-as-map-error"
+    assert_output --partial "lets: exit status 2"
 }
 
 @test "command_cmd: cmd-as-map must propagate env" {
@@ -83,4 +84,4 @@ setup() {
 
     assert_success
     assert_line --index 0 "Hello from short"
-}
+}

tests/default_env.bats

@@ -61,5 +61,7 @@ setup() {
     LETS_CONFIG_DIR=./a run lets print-workdir
 
     assert_failure
-    assert_line "lets: failed to run command 'print-workdir': chdir ${TEST_DIR}/b: no such file or directory"
+    assert_line --index 0 --partial "lets: print-workdir"
+    assert_line --index 0 --partial "failed here"
+    assert_line --index 1 "lets: chdir ${TEST_DIR}/b: no such file or directory"
 }

tests/dependency_failure_tree.bats

@@ -9,15 +9,17 @@ setup() {
 @test "dependency_failure_tree: shows full 3-level chain on failure" {
     run env NO_COLOR=1 lets deploy
     assert_failure
-    assert_line --index 0 "  deploy"
-    assert_line --index 1 "    build"
-    assert_line --index 2 --partial "      lint"
+    assert_line --index 0 "lets: deploy"
+    assert_line --index 1 "          build"
+    assert_line --index 2 --partial "            lint"
     assert_line --index 2 --partial "failed here"
+    assert_line --index 3 "lets: exit status 1"
 }
 
 @test "dependency_failure_tree: single node when no depends" {
     run env NO_COLOR=1 lets lint
     assert_failure
-    assert_line --index 0 --partial "  lint"
+    assert_line --index 0 --partial "lets: lint"
     assert_line --index 0 --partial "failed here"
+    assert_line --index 1 "lets: exit status 1"
 }