feat(core): structured execution outcomes and configurable exit codes - #85
feat(core): structured execution outcomes and configurable exit codes#85carldebilly wants to merge 1 commit into
Conversation
Every run that reaches the core pipeline now ends in a ReplExecutionOutcome (ReplExecutionOutcomeKind: Success, Help, UsageError, BindingError, HandlerError, HandlerExitCode, HandlerException, Cancelled, Interrupted, FrameworkError). The kind is mapped to an integer once, at the top of CoreReplApp.ExecuteCoreAsync, through the new ReplOptions.ExitCodes table (ExitCodeOptions) and then handed to the optional ExitCodes.Resolver hook. Default codes change: framework refusals (unknown command, ambiguous prefix, invalid or colliding option, context validation, unknown output format, ambient misuse) and binding failures exit 2 instead of 1; handler failures stay 1, help and success stay 0; Results.Exit keeps its code verbatim. ExitCodes.Cancelled (int?, unset by default) turns a caller-token cancellation into an exit code instead of letting the exception escape. ReplExecutionContext.Result exposes the handler return value to middleware, readable and replaceable after next(); ReplNext and Use are unchanged. The interactive loop resolves shell-integration mark codes through the same table and resolver, including Ctrl+C (conventional 130 unless remapped). MCP sub-invocations keep the built-in defaults and skip the resolver. Repl.Testing still raises TimeoutException when the app under test maps Cancelled. Interrupted is reserved for the process-signal bridge (#79/#80). Closes #81.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71c0042168
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var ambientExecution = ambientOutcome == AmbientCommandOutcome.HandledError | ||
| ? ExecutionOutcome.Usage() | ||
| : ExecutionOutcome.Success; |
There was a problem hiding this comment.
When an interactive user enters the built-in help or ? ambient command, a successful render reaches this blanket Success branch rather than ReplExecutionOutcomeKind.Help. Consequently, applications that configure ExitCodes.Help differently from Success, or whose resolver switches on Kind, emit the wrong shell-integration command-end code for interactive help; distinguish the help ambient entry and resolve it as ExecutionOutcome.Help.
Useful? React with 👍 / 👎.
| cancellationToken.ThrowIfCancellationRequested(); | ||
| var outcome = await ExecuteCoreOutcomeAsync(args, serviceProvider, isSubInvocation, cancellationToken) | ||
| .ConfigureAwait(false); | ||
| return ResolveExitCode(outcome, isSubInvocation); |
There was a problem hiding this comment.
When a configured resolver throws OperationCanceledException—for example, because it closes over a cancellation token—this call is still inside the catch's protected region, so the exception is reclassified as a cancelled run and ResolveExitCode invokes the resolver a second time from the catch block. This violates the documented once-per-run contract, can duplicate resolver side effects, and may replace a resolver failure with the configured cancellation code; narrow the cancellation catch to execution of the pipeline and invoke the final resolver afterward.
Useful? React with 👍 / 👎.
1 participant
Summary
ReplExecutionOutcome(ReplExecutionOutcomeKind:Success,Help,UsageError,BindingError,HandlerError,HandlerExitCode,HandlerException,Cancelled,Interrupted,FrameworkError)CoreReplApp.ExecuteCoreAsync, through the newReplOptions.ExitCodestable (ExitCodeOptions) and then through the optionalExitCodes.Resolverhook2; handler failures stay1; help and success stay0;Results.Exit(n)keeps its code verbatimExitCodes.Cancelled(int?, unset by default) to turn a caller-token cancellation into an exit code instead of lettingOperationCanceledExceptionescapeRunAsyncReplExecutionContext.Result(readable and replaceable afterawait next(), settable by a short-circuiting middleware);ReplNextandUseare unchangedRepl.Testing's per-commandTimeoutExceptionworking when the app under test mapsCancelledintreturn value is data, never an exit codeFixes #81.
Design notes
Repl.Core;src/Repl.Defaults/ReplApp.cs,ReplRunOptions.csandReplAppProfileExtensions.csare intentionally untouched so this does not collide with fix(runtime): cancel standalone runs on process signals #80.ReplExecutionOutcomeKind.InterruptedandCoreReplApp.ResolveExitCode(ExecutionOutcome, bool)are the seam for the process-signal bridge to route SIGINT/SIGTERM through the same resolver.Cancelleddefaults tonull(rethrow) for the same reason: fix(runtime): cancel standalone runs on process signals #80 catches theOperationCanceledExceptioninReplAppto produce 130/143, and a core default would make that catch unreachable. Consumers opt in witho.ExitCodes.Cancelled = 130.boundflag aroundHandlerArgumentBinder.Bind, so a handler-thrownInvalidOperationExceptionisHandlerExceptionwhile conversion/[FromServices]failures areBindingError.ReplApp.RunAsyncstill return1directly (same file as fix(runtime): cancel standalone runs on process signals #80; to be routed through the policy afterwards).Breaking changes
See
CHANGELOG.md→ Changed — breaking: framework exit codes:--outputformat, ambient misuse in one-shot mode, unrenderable help →2(was1); unbindable/unconvertible arguments →2(was1)Run/RunAsyncoverload now checks the caller's token before doing any work (previously onlyCoreReplApp.RunAsyncdid)Repl.Testing.CommandExecution.ExitCodefollows the policy; MCP failure text reads "exit code 2" for those casesValidation
dotnet build src/Repl.slnx -c Release -warnaserror— 0 warnings, 0 errorsGiven_ExitCodes(34 tests) written first and observed red (29/30) before the implementation; 53 integration assertions flippedBe(1)→Be(2), each verified to be a usage/binding refusalCancelled; MCP ignores both table and resolver;Repl.Testingtimeout withCancelledmapped; enum exhaustiveness guard onExitCodeOptions.Mapsamples/01-core-basics: unknown command →2,--help→0git diff --checkcleanskeptic,quality,contractrun before commit; all findings addressed