InvocationPipeline improvements based on PR feedback

Jon Sequeira · Jon Sequeira · commit 6de04f7c18f0 · 2026-04-02T12:44:18.000-07:00
diff --git a/src/System.CommandLine/Invocation/InvocationPipeline.cs b/src/System.CommandLine/Invocation/InvocationPipeline.cs
@@ -15,42 +15,39 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
 
             try
             {
-                int exitCode = 0;
+                int actionResult = 0;
+                int preActionResult = 0;
 
                 if (parseResult.PreActions is not null)
                 {
                     for (int i = 0; i < parseResult.PreActions.Count; i++)
                     {
                         var action = parseResult.PreActions[i];
-                        int preActionResult;
+                        var result = 0;
 
                         switch (action)
                         {
                             case SynchronousCommandLineAction syncAction:
-                                preActionResult = syncAction.Invoke(parseResult);
+                                result = syncAction.Invoke(parseResult);
                                 break;
                             case AsynchronousCommandLineAction asyncAction:
-                                preActionResult = await asyncAction.InvokeAsync(parseResult, cts.Token);
-                                break;
-                            default:
-                                preActionResult = 0;
+                                result = await asyncAction.InvokeAsync(parseResult, cts.Token);
                                 break;
+                           
                         }
 
-                        if (exitCode == 0)
+                        if (result != 0)
                         {
-                            exitCode = preActionResult;
+                            preActionResult = result;
                         }
                     }
                 }
 
                 if (parseResult.Action is null)
                 {
-                    return exitCode != 0 ? exitCode : ReturnCodeForMissingAction(parseResult);
+                    return preActionResult != 0 ? preActionResult : ReturnCodeForMissingAction(parseResult);
                 }
 
-                int actionResult;
-
                 switch (parseResult.Action)
                 {
                     case SynchronousCommandLineAction syncAction:
@@ -85,7 +82,7 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio
                         throw new ArgumentOutOfRangeException(nameof(parseResult.Action));
                 }
 
-                return exitCode != 0 ? exitCode : actionResult;
+                return preActionResult != 0 ? preActionResult : actionResult;
             }
             catch (Exception ex) when (parseResult.InvocationConfiguration.EnableDefaultExceptionHandler)
             {
@@ -101,32 +98,19 @@ internal static int Invoke(ParseResult parseResult)
         {
             try
             {
-                int exitCode = 0;
+                int preActionResult = 0;
 
                 if (parseResult.PreActions is not null)
                 {
-#if DEBUG
-                    for (var i = 0; i < parseResult.PreActions.Count; i++)
-                    {
-                        var action = parseResult.PreActions[i];
-
-                        if (action is not SynchronousCommandLineAction)
-                        {
-                            parseResult.InvocationConfiguration.EnableDefaultExceptionHandler = false;
-                            throw new Exception(
-                                $"This should not happen. An instance of {nameof(AsynchronousCommandLineAction)} ({action}) was called within {nameof(InvocationPipeline)}.{nameof(Invoke)}. This is supposed to be detected earlier resulting in a call to {nameof(InvocationPipeline)}{nameof(InvokeAsync)}");
-                        }
-                    }
-#endif
-
                     for (var i = 0; i < parseResult.PreActions.Count; i++)
                     {
                         if (parseResult.PreActions[i] is SynchronousCommandLineAction syncPreAction)
                         {
-                            int preActionResult = syncPreAction.Invoke(parseResult);
-                            if (exitCode == 0)
+                            int result = syncPreAction.Invoke(parseResult);
+                            
+                            if (result != 0)
                             {
-                                exitCode = preActionResult;
+                                preActionResult = result;
                             }
                         }
                     }
@@ -135,11 +119,11 @@ internal static int Invoke(ParseResult parseResult)
                 switch (parseResult.Action)
                 {
                     case null:
-                        return exitCode != 0 ? exitCode : ReturnCodeForMissingAction(parseResult);
+                        return preActionResult != 0 ? preActionResult : ReturnCodeForMissingAction(parseResult);
 
                     case SynchronousCommandLineAction syncAction:
                         int actionResult = syncAction.Invoke(parseResult);
-                        return exitCode != 0 ? exitCode : actionResult;
+                        return preActionResult != 0 ? preActionResult : actionResult;
 
                     default:
                         throw new InvalidOperationException($"{nameof(AsynchronousCommandLineAction)} called within non-async invocation.");

Original file line number	Diff line number	Diff line change
`@@ -15,42 +15,39 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio`
`15`	`15`
`16`	`16`	`try`
`17`	`17`	`{`
`18`		`- int exitCode = 0;`
	`18`	`+ int actionResult = 0;`
	`19`	`+ int preActionResult = 0;`
`19`	`20`
`20`	`21`	`if (parseResult.PreActions is not null)`
`21`	`22`	`{`
`22`	`23`	`for (int i = 0; i < parseResult.PreActions.Count; i++)`
`23`	`24`	`{`
`24`	`25`	`var action = parseResult.PreActions[i];`
`25`		`- int preActionResult;`
	`26`	`+ var result = 0;`
`26`	`27`
`27`	`28`	`switch (action)`
`28`	`29`	`{`
`29`	`30`	`case SynchronousCommandLineAction syncAction:`
`30`		`- preActionResult = syncAction.Invoke(parseResult);`
	`31`	`+ result = syncAction.Invoke(parseResult);`
`31`	`32`	`break;`
`32`	`33`	`case AsynchronousCommandLineAction asyncAction:`
`33`		`- preActionResult = await asyncAction.InvokeAsync(parseResult, cts.Token);`
`34`		`- break;`
`35`		`- default:`
`36`		`- preActionResult = 0;`
	`34`	`+ result = await asyncAction.InvokeAsync(parseResult, cts.Token);`
`37`	`35`	`break;`
	`36`	`+`
`38`	`37`	`}`
`39`	`38`
`40`		`- if (exitCode == 0)`
	`39`	`+ if (result != 0)`
`41`	`40`	`{`
`42`		`- exitCode = preActionResult;`
	`41`	`+ preActionResult = result;`
`43`	`42`	`}`
`44`	`43`	`}`
`45`	`44`	`}`
`46`	`45`
`47`	`46`	`if (parseResult.Action is null)`
`48`	`47`	`{`
`49`		`- return exitCode != 0 ? exitCode : ReturnCodeForMissingAction(parseResult);`
	`48`	`+ return preActionResult != 0 ? preActionResult : ReturnCodeForMissingAction(parseResult);`
`50`	`49`	`}`
`51`	`50`
`52`		`- int actionResult;`
`53`		`-`
`54`	`51`	`switch (parseResult.Action)`
`55`	`52`	`{`
`56`	`53`	`case SynchronousCommandLineAction syncAction:`
`@@ -85,7 +82,7 @@ internal static async Task<int> InvokeAsync(ParseResult parseResult, Cancellatio`
`85`	`82`	`throw new ArgumentOutOfRangeException(nameof(parseResult.Action));`
`86`	`83`	`}`
`87`	`84`
`88`		`- return exitCode != 0 ? exitCode : actionResult;`
	`85`	`+ return preActionResult != 0 ? preActionResult : actionResult;`
`89`	`86`	`}`
`90`	`87`	`catch (Exception ex) when (parseResult.InvocationConfiguration.EnableDefaultExceptionHandler)`
`91`	`88`	`{`
`@@ -101,32 +98,19 @@ internal static int Invoke(ParseResult parseResult)`
`101`	`98`	`{`
`102`	`99`	`try`
`103`	`100`	`{`
`104`		`- int exitCode = 0;`
	`101`	`+ int preActionResult = 0;`
`105`	`102`
`106`	`103`	`if (parseResult.PreActions is not null)`
`107`	`104`	`{`
`108`		`-#if DEBUG`
`109`		`- for (var i = 0; i < parseResult.PreActions.Count; i++)`
`110`		`- {`
`111`		`- var action = parseResult.PreActions[i];`
`112`		`-`
`113`		`- if (action is not SynchronousCommandLineAction)`
`114`		`- {`
`115`		`- parseResult.InvocationConfiguration.EnableDefaultExceptionHandler = false;`
`116`		`- throw new Exception(`
`117`		`- $"This should not happen. An instance of {nameof(AsynchronousCommandLineAction)} ({action}) was called within {nameof(InvocationPipeline)}.{nameof(Invoke)}. This is supposed to be detected earlier resulting in a call to {nameof(InvocationPipeline)}{nameof(InvokeAsync)}");`
`118`		`- }`
`119`		`- }`
`120`		`-#endif`
`121`		`-`
`122`	`105`	`for (var i = 0; i < parseResult.PreActions.Count; i++)`
`123`	`106`	`{`
`124`	`107`	`if (parseResult.PreActions[i] is SynchronousCommandLineAction syncPreAction)`
`125`	`108`	`{`
`126`		`- int preActionResult = syncPreAction.Invoke(parseResult);`
`127`		`- if (exitCode == 0)`
	`109`	`+ int result = syncPreAction.Invoke(parseResult);`
	`110`	`+`
	`111`	`+ if (result != 0)`
`128`	`112`	`{`
`129`		`- exitCode = preActionResult;`
	`113`	`+ preActionResult = result;`
`130`	`114`	`}`
`131`	`115`	`}`
`132`	`116`	`}`
`@@ -135,11 +119,11 @@ internal static int Invoke(ParseResult parseResult)`
`135`	`119`	`switch (parseResult.Action)`
`136`	`120`	`{`
`137`	`121`	`case null:`
`138`		`- return exitCode != 0 ? exitCode : ReturnCodeForMissingAction(parseResult);`
	`122`	`+ return preActionResult != 0 ? preActionResult : ReturnCodeForMissingAction(parseResult);`
`139`	`123`
`140`	`124`	`case SynchronousCommandLineAction syncAction:`
`141`	`125`	`int actionResult = syncAction.Invoke(parseResult);`
`142`		`- return exitCode != 0 ? exitCode : actionResult;`
	`126`	`+ return preActionResult != 0 ? preActionResult : actionResult;`
`143`	`127`
`144`	`128`	`default:`
`145`	`129`	`throw new InvalidOperationException($"{nameof(AsynchronousCommandLineAction)} called within non-async invocation.");`