fix(cli): propagate handler exit code to process exit - #97
Merged
Conversation
Top-level `await builder.RunAsync()` in Program.cs discarded the int the CLI host returned, so the program compiled as `Task Main` and the OS always saw exit code 0 — even when a handler returned non-zero after the API responded with 400/409/422. Scripts and E2E tests silently passed when they should have failed. Surgical fix: return the int from RunAsync. Defensive fix in CliHost so future consumers can't reintroduce the same mistake: every code path now mirrors the exit code into Environment.ExitCode before returning, which is honored even when the caller's Main returns void/Task. Regression tests in CliExitCodeTests launch the real CLI binary and assert the process exits non-zero for: missing required option, HTTP 400 HttpValidationProblemDetails, HTTP 409 ProblemDetails, HTTP 422 ProblemDetails. The test project gets a RepositoryRoot AssemblyMetadata attribute so the tests can locate the CLI csproj — same pattern E2E uses.
CI builds Release while `dotnet run --no-build` defaults to Debug, so the
test was launching `dotnet run` against a missing Debug binary and failing
with "An error occurred trying to start process". Locally everyone's
already built Debug, so the test passed.
Fix: derive the CLI dll path from the test assembly's location (sibling
output directories under artifacts/bin/{Project}/{config}/) and invoke
`dotnet <CLI>.dll`. Config-agnostic and avoids the dotnet-run startup
overhead. Drops the now-unneeded RepositoryRoot AssemblyMetadata.
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
The CLI was exiting 0 even when a handler returned a non-zero exit code (e.g., after the API responded with 400
HttpValidationProblemDetails, 409ProblemDetails, or 422ProblemDetails). Scripts and E2E tests silently passed when they should have failed; the symptom in the variable interpolation E2E suite was that Step08/Step09 collisions over the sharedtierscope were masked until Step11 choked on non-JSON stdout.Root cause was in
src/GroundControl.Cli/Program.cs— top-levelawait builder.RunAsync();discarded the int return value, so the program compiled asTask Main(void-returning) and the OS got exit code 0 regardless of whatRunAsyncproduced. The System.CommandLine action plumbing (SetAction(ExecuteAsync),ParseResult.InvokeAsync, the chain throughCommand<,>andCliHost.RunAsync) was already returning the right int.Changes
src/GroundControl.Cli/Program.cs— surgical fix:await builder.RunAsync();→return await builder.RunAsync();.src/GroundControl.Host.Cli/CliHost.cs— defense-in-depth in the framework: every return path inRunAsyncnow goes through aSetExitCodehelper that mirrors the value intoEnvironment.ExitCode. Guarantees the OS gets the right exit code even if a future consumer ofCliHostBuilderdiscards the int again.tests/GroundControl.Cli.Tests/CliExitCodeTests.cs— fourProcess.Start-based regression tests that launch the real CLI and assertExitCode == 1for missing required option, HTTP 400, HTTP 409, and HTTP 422 paths.tests/GroundControl.Cli.Tests/GroundControl.Cli.Tests.csproj— adds theRepositoryRootAssemblyMetadataAttribute(same pattern used byGroundControl.E2E.Tests) so the tests can locateGroundControl.Cli.csprojfrom the test assembly.