Skip to content

fix: PublishCommand returns non-zero exit code on error paths#266

Merged
sellakumaran merged 2 commits intomicrosoft:mainfrom
pratapladhani:fix/publish-command-exit-codes
Feb 17, 2026
Merged

fix: PublishCommand returns non-zero exit code on error paths#266
sellakumaran merged 2 commits intomicrosoft:mainfrom
pratapladhani:fix/publish-command-exit-codes

Conversation

@pratapladhani
Copy link
Contributor

Summary

Fixes #264 - PublishCommand now correctly returns exit code 1 on all error paths.

Problem

All ~27 error
eturn;\ paths in \PublishCommand.cs\ silently returned exit code 0 because the \SetHandler\ used a typed-parameter signature (\�sync (bool, bool, string, string?, bool) => { ... }) which returns \Task\ (not \Task). System.CommandLine always interprets this as exit code 0.

This caused calling scripts (e.g., PowerShell automation) to incorrectly report success when publish operations failed.

Fix

Switch to the \InvocationContext\ pattern (matching \ConfigCommand):

  • Handler signature: \SetHandler(async (InvocationContext context) => { ... })\
  • Option extraction: \context.ParseResult.GetValueForOption(...)\
  • Exit code: \context.ExitCode = 1\ in a \ inally\ block for non-normal exits
  • Uses \isNormalExit\ flag to distinguish 4 normal exit paths (dry-run, skip-graph, missing-tenantId, success) from ~24 error paths

Why context.ExitCode over Environment.Exit(1)?

Environment.Exit(1) context.ExitCode = 1
Terminates process? Yes, immediately No - natural exit
Output flushing? May truncate Completes fully
Cleanup? Partial Full

Validation

Tested with real publish error (HTTP 400 'Must upload a newer version'):

BEFORE (v1.1.62) AFTER (patched)
Exit code 0 (bug) 1 (fixed)
Output Complete Complete
Truncation None None
  • Full solution build: 0 errors, 0 warnings
  • Full test suite: 900 passed, 17 skipped, 0 failed

Related

Switch SetHandler from typed parameters to InvocationContext pattern
(matching ConfigCommand) to enable context.ExitCode = 1 on error paths.

Previously, all ~27 error return paths in PublishCommand silently
returned exit code 0 because the typed-parameter SetHandler signature
returns Task (not Task<int>), so System.CommandLine always interprets
the result as success.

Changes:
- Use InvocationContext-based SetHandler (same pattern as ConfigCommand)
- Add isNormalExit flag tracking (default false)
- Mark 4 normal exits: dry-run, skip-graph, missing-tenantId, success
- Add finally block setting context.ExitCode = 1 when !isNormalExit
- Uses context.ExitCode (graceful) instead of Environment.Exit (abrupt)

Validated with real publish error (HTTP 400 'Must upload a newer version'):
- BEFORE: exit code 0 (bug), full output
- AFTER: exit code 1 (fixed), full output, no truncation

Fixes microsoft#264
@pratapladhani pratapladhani requested review from a team as code owners February 16, 2026 16:53
Copilot AI review requested due to automatic review settings February 16, 2026 16:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical bug where the publish command always returned exit code 0, even when errors occurred. The root cause was that the SetHandler used a typed-parameter signature returning Task instead of Task<int>, which System.CommandLine interprets as always successful. This prevented calling scripts and CI/CD pipelines from detecting publish failures.

Changes:

  • Refactored SetHandler to use InvocationContext pattern for exit code control
  • Added isNormalExit flag to track successful completion paths
  • Implemented finally block to set context.ExitCode = 1 for all error paths

…de documentation

This commit addresses code review feedback on PR microsoft#266 by adding:

1. **Comprehensive Test Coverage** (BLOCKING issue resolved):
   - Created PublishCommandTests.cs with 7 tests covering exit code behavior
   - Tests verify error paths return exit code 1
   - Tests verify normal paths (dry-run, skip-graph, success) return exit code 0
   - Tests validate exception handling returns exit code 1
   - All tests passing (7/7)

2. **Improved Code Documentation** (MEDIUM issue resolved):
   - Added explanatory comment in finally block about exit code pattern choice
   - Documented why pattern differs from ConfigCommand approach
   - Added rationale for defensive programming with isNormalExit flag

3. **Design Decision Documentation** (LOW issue resolved):
   - Added comment explaining missing tenantId treatment as normal exit
   - Clarifies that MOS publish success with optional Graph operations is exit code 0

Tests validate the fix for issue microsoft#264 where ~27 error paths incorrectly
returned exit code 0, causing automation scripts to misreport failures as success.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@sellakumaran sellakumaran merged commit 93f15a1 into microsoft:main Feb 17, 2026
4 checks passed
@pratapladhani pratapladhani deleted the fix/publish-command-exit-codes branch February 18, 2026 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PublishCommand: All error paths return exit code 0 instead of non-zero

3 participants