Show dependency failure tree when command chain fails#296
Merged
Conversation
Reviewer's GuideImplements a new DependencyError abstraction to track command dependency chains on failure, wires executor paths to build this chain, and updates the CLI to render an indented dependency tree (with tests and docs) whenever a command or its depends chain fails. Sequence diagram for dependency failure tree printing on command failuresequenceDiagram
actor User
participant MainCLI
participant RootCmd
participant Executor
participant DependencyHelpers
User->>MainCLI: invoke lets deploy
MainCLI->>RootCmd: ExecuteContext(ctx)
RootCmd->>Executor: execute(ctx)
Executor->>Executor: initCmd(ctx)
alt initCmd fails
Executor->>DependencyHelpers: prependToChain(deploy, err)
DependencyHelpers-->>Executor: *DependencyError
Executor-->>RootCmd: *DependencyError
else initCmd ok
Executor->>Executor: executeDepends(ctx)
Executor->>Executor: runCmd(lint)
Executor-->>Executor: *ExecuteError
Executor->>DependencyHelpers: prependToChain(lint, err)
DependencyHelpers-->>Executor: *DependencyError
Executor-->>RootCmd: *DependencyError (chain deploy→build→lint)
end
RootCmd-->>MainCLI: *DependencyError
MainCLI->>MainCLI: errors.As(err, *DependencyError)
MainCLI->>DependencyHelpers: PrintDependencyTree(depErr, os.Stderr)
DependencyHelpers-->>MainCLI: dependency_tree_rendered
MainCLI->>MainCLI: log.Error(err.Error())
MainCLI->>MainCLI: os.Exit(getExitCode(err, 1))
Class diagram for DependencyError and executor integrationclassDiagram
class Executor {
execute(ctx Context) error
executeParallel(ctx Context) error
}
class DependencyError {
[]string Chain
error Err
Error() string
ExitCode() int
}
class ExecuteError {
ExitCode() int
}
class DependencyHelpers {
prependToChain(name string, err error) error
PrintDependencyTree(e *DependencyError, w io.Writer)
}
class MainCLI {
main()
}
Executor ..> DependencyHelpers : uses
DependencyHelpers ..> DependencyError : constructs
DependencyError ..> ExecuteError : unwraps
MainCLI ..> DependencyHelpers : prints_tree_on_error
MainCLI ..> DependencyError : type_detection
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="tests/dependency_failure_tree.bats" line_range="9-15" />
<code_context>
+ cd ./tests/dependency_failure_tree
+}
+
+@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 2 --partial "failed here"
+}
+
</code_context>
<issue_to_address>
**issue (testing):** These Bats assertions likely only inspect stdout, but the dependency tree is written to stderr
In `main.go` the tree is printed to stderr (`PrintDependencyTree(depErr, os.Stderr)`), but plain bats-core + bats-assert `run` puts stdout in `$output` and stderr in `$error`, and `assert_line` checks `$output` by default. Unless your helpers redirect stderr to stdout, these assertions won’t actually see the tree. You could instead assert on stderr (e.g. `assert_line --stderr ...` with bats-assert ≥ 2, or an equivalent helper), or redirect stderr in the test (e.g. `run env NO_COLOR=1 lets deploy 2>&1`) so the assertions target the correct stream.
</issue_to_address>
### Comment 2
<location path="tests/dependency_failure_tree.bats" line_range="18-22" />
<code_context>
+ assert_line --index 2 --partial "failed here"
+}
+
+@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 "failed here"
+}
+```
</code_context>
<issue_to_address>
**suggestion (testing):** No assertion that NO_COLOR actually suppresses ANSI color codes in the dependency tree output
The tests run with `NO_COLOR=1` but only check for text like `failed here`, so they wouldn’t fail if ANSI escapes were accidentally reintroduced. Add an assertion that the captured output does *not* contain `�[` (or a specific color code), e.g. via `refute_line --regex $'\x1b\['`, so the test explicitly verifies the tree is uncolored when `NO_COLOR` is set.
Suggested implementation:
```shell
@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 2 --partial "failed here"
refute_output --regex $'\x1b\['
}
```
```shell
@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 "failed here"
refute_output --regex $'\x1b\['
}
```
</issue_to_address>
### Comment 3
<location path="tests/dependency_failure_tree.bats" line_range="10-15" />
<code_context>
+}
+
+@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 2 --partial "failed here"
+}
+
</code_context>
<issue_to_address>
**suggestion (testing):** Consider asserting that the dependency tree appears before the error log line to match the documented behavior
The PR description says the tree should appear *above* the error line (e.g., `ERRO[...] failed to run command ...`), but this test only verifies the tree content, not its position relative to the error. To cover the UX requirement, consider asserting that the initial lines are the tree and that any `ERRO` (or command error) line comes after them.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+9
to
+15
| @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 2 --partial "failed here" |
There was a problem hiding this comment.
issue (testing): These Bats assertions likely only inspect stdout, but the dependency tree is written to stderr
In main.go the tree is printed to stderr (PrintDependencyTree(depErr, os.Stderr)), but plain bats-core + bats-assert run puts stdout in $output and stderr in $error, and assert_line checks $output by default. Unless your helpers redirect stderr to stdout, these assertions won’t actually see the tree. You could instead assert on stderr (e.g. assert_line --stderr ... with bats-assert ≥ 2, or an equivalent helper), or redirect stderr in the test (e.g. run env NO_COLOR=1 lets deploy 2>&1) so the assertions target the correct stream.
Comment on lines
+18
to
+22
| @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 "failed here" |
There was a problem hiding this comment.
suggestion (testing): No assertion that NO_COLOR actually suppresses ANSI color codes in the dependency tree output
The tests run with NO_COLOR=1 but only check for text like failed here, so they wouldn’t fail if ANSI escapes were accidentally reintroduced. Add an assertion that the captured output does not contain �[ (or a specific color code), e.g. via refute_line --regex $'\x1b\[', so the test explicitly verifies the tree is uncolored when NO_COLOR is set.
Suggested implementation:
@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 2 --partial "failed here"
refute_output --regex $'\x1b\['
}
@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 "failed here"
refute_output --regex $'\x1b\['
}
Comment on lines
+10
to
+15
| 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 2 --partial "failed here" |
There was a problem hiding this comment.
suggestion (testing): Consider asserting that the dependency tree appears before the error log line to match the documented behavior
The PR description says the tree should appear above the error line (e.g., ERRO[...] failed to run command ...), but this test only verifies the tree content, not its position relative to the error. To cover the UX requirement, consider asserting that the initial lines are the tree and that any ERRO (or command error) line comes after them.
Contributor
Greptile SummaryThis PR adds a Key changes:
The main code-quality gap is that Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant main
participant Executor
participant execute_deploy as execute(deploy)
participant execute_build as execute(build)
participant execute_lint as execute(lint)
main->>Executor: Execute(deploy)
Executor->>execute_deploy: execute(deploy)
execute_deploy->>Executor: executeDepends → Execute(build)
Executor->>execute_build: execute(build)
execute_build->>Executor: executeDepends → Execute(lint)
Executor->>execute_lint: execute(lint)
execute_lint->>execute_lint: runCmd → ExecuteError
execute_lint-->>execute_build: prependToChain("lint", ExecuteError)<br/>→ DependencyError{Chain:["lint"]}
execute_build-->>execute_deploy: prependToChain("build", DependencyError)<br/>→ DependencyError{Chain:["build","lint"]}
execute_deploy-->>main: prependToChain("deploy", DependencyError)<br/>→ DependencyError{Chain:["deploy","build","lint"]}
main->>main: errors.As → PrintDependencyTree(stderr)
Note over main: " deploy\n build\n lint <-- failed here"
main->>main: log.Error + os.Exit(exitCode)
Last reviewed commit: 643262e |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
kindermax
force-pushed
the
feature/dependency-failure-tree
branch
3 times, most recently
from
bdd0fb3 to
31003dc
Compare
Fix spec: clarify chain construction and single-node failure handling Clarify error path coverage, executeParallel, global init, and NO_COLOR in bats test Update spec status to ready for implementation Add dependency failure tree implementation plan
- prependToChain now returns a new DependencyError instead of mutating the original, making it safe for concurrent use - Rename misleading test "wraps ExecuteError exit code" to accurately reflect it tests the fallback-to-1 path - Tighten single-node PrintDependencyTree assertion to use HasPrefix on a split line rather than Contains
Remove stale nolint:wrapcheck comment
kindermax
force-pushed
the
feature/dependency-failure-tree
branch
3 times, most recently
from
df4f145 to
6d752bd
Compare
kindermax
force-pushed
the
feature/dependency-failure-tree
branch
from
6d752bd to
bb3d7e4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DependencyErrortype that carries the fulldependschain when a command failsNO_COLOR)depends) also show a one-node tree for consistencyExample output for
deploy → build → lint(lint fails):```
deploy
build
lint <-- failed here
ERRO[0000] failed to run command 'lint': exit status 1
```
Test Plan
go test ./internal/executor/— covers chain construction, exit code propagation, tree renderinglets test-bats dependency_failure_tree— verifies 3-level chain and single-node cases withNO_COLOR=1dependschain and confirm tree appears above the error lineSummary by Sourcery
Add a dependency-aware error type and reporting so that failing commands display their full depends chain as an indented tree before the error message.
New Features:
Enhancements:
Documentation:
Tests: