Address Copilot review comments on PR #440

- `--messaging-endpoint`: fail fast when supplied without its precondition
  (`--m365` on `setup all`; `--endpoint-only` on `setup blueprint`) instead
  of silently ignoring it.
- Distinguish "option omitted" from "explicitly passed empty" — an empty
  `--messaging-endpoint ""` now errors with exit 1 rather than being treated
  as omitted (which would prompt/defer/no-op).
- `setup blueprint --endpoint-only` now captures the registration result and
  exits non-zero when the endpoint did not register, so scripting is reliable.
- Add handler-invocation tests for the three guards.
- review-staged skill: add generic Rules W (CLI option precondition
  enforcement), X (Option<string?> empty-vs-omitted conflation), and Y
  (ignored result-bearing call / missing exit code) so future reviews catch
  this class of issue.

Author: sellakumaran

.claude/skills/review-staged/SKILL.md

@@ -134,7 +134,19 @@ Nine checks that static per-file analysis tends to miss — each requires tracin
 
   Implementation: at the start of the review, emit the list of branch-level files and treat them as the review surface. At the end, verify each file was read or explicitly justified as "no rule applies."
 
-Full detection rules and real examples are in `.claude/agents/pr-code-reviewer.md` Step 9, Rules N through V.
+- **CLI option precondition enforcement (Rule W)**: **Added 2026-06-02 after PR #440's Copilot review.** When a new or modified `Option<...>` has a description that constrains its applicability — phrases like `(--X only)`, `use with --Y`, `Only meaningful with --Z`, `requires an existing ...` — the handler **must validate that precondition and fail fast** (`logger.LogError(...) + context.ExitCode = 1 + return`) when the option is supplied outside that context. A flag that is accepted and then silently ignored (because the code path it feeds is skipped) is a bug: the user gets exit 0 and assumes it took effect.
+
+  Detection: for every option whose description contains a constraint keyword, grep the handler for a guard that errors when the option is present but the precondition is false. Use `context.ParseResult.CommandResult.FindResultFor(<option>) != null` to detect "supplied" (not the value, which can't tell omitted from empty). If no such guard exists, flag it. **Severity: MEDIUM.** PR #440 example: `--messaging-endpoint` was documented `(--m365 only)` and `with --endpoint-only`, but both handlers accepted it unconditionally and dropped it when the messaging step / endpoint-only path was skipped.
+
+- **`Option<string?>` empty-vs-omitted conflation (Rule X)**: **Added 2026-06-02 after PR #440.** When a string option is read as `GetValueForOption(opt)?.Trim()` and then gated only with `string.IsNullOrWhiteSpace(...)`, an **explicitly-passed empty value** (`--opt ""` or `--opt "   "`) is indistinguishable from **omitted**. This is a defect whenever "omitted" triggers a *different* behavior than "empty should" — e.g. omitted falls back to config / prompts / defers, so `--opt ""` silently does the fallback instead of erroring.
+
+  Detection: for each `Option<string?>` whose omitted-path does something non-trivial (prompt, default, defer, config fallback), verify the handler distinguishes "specified" via `FindResultFor(opt) != null` and emits a targeted error + `ExitCode = 1` when specified-but-whitespace. This is the CLAUDE.md "Input Validation" rule applied to the explicit-empty case. **Severity: MEDIUM.** PR #440 example: `--messaging-endpoint ""` was `.Trim()`ed to `""`, read as omitted, and triggered prompting/deferral instead of a clear error.
+
+- **Ignored result-bearing call / missing exit code (Rule Y)**: **Added 2026-06-02 after PR #440.** Extends the exit-code-completeness check (CLAUDE.md item 8) to *discarded return values*. When a command handler `await`s an operation that returns a success/failure-bearing result (an enum like `*Result`, a `bool`, a tuple with an outcome) and **does not capture or branch on it**, any failure that result encodes will still exit 0 — breaking scripting/CI.
+
+  Detection: in every command handler, find `await SomeOp(...)` calls whose return type is non-`void`/non-`Task` and whose result is not assigned or checked. For each, confirm the failure values map to `context.ExitCode = 1`. **Severity: MEDIUM** (HIGH if the command is commonly scripted). PR #440 example: the `--endpoint-only` path called `RegisterEndpointAndSyncAsync(...)` (returns `EndpointRegistrationResult`) and discarded it, so `NotConfigured` / contract-mismatch / `Failed` all exited 0.
+
+Full detection rules and real examples are in `.claude/agents/pr-code-reviewer.md` Step 9, Rules N through V (plus W–Y above).
 
 ### Context Awareness
 The skill differentiates between:

src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AllSubcommand.cs

@@ -198,9 +198,19 @@ public static Command CreateCommand(
             var tenantIdFlag = context.ParseResult.GetValueForOption(tenantIdOption);
             bool isM365 = context.ParseResult.GetValueForOption(m365Option);
             var authMode = context.ParseResult.GetValueForOption(authModeOption)?.ToLowerInvariant();
+            // Distinguish "option omitted" from "option explicitly passed empty" — the latter must be a
+            // hard error, not silently treated as omitted (which would prompt/defer instead).
+            var messagingEndpointSpecified = context.ParseResult.CommandResult.FindResultFor(messagingEndpointOption) != null;
             var messagingEndpointFlag = context.ParseResult.GetValueForOption(messagingEndpointOption)?.Trim();
             var ct = context.GetCancellationToken();
 
+            if (messagingEndpointSpecified && string.IsNullOrWhiteSpace(messagingEndpointFlag))
+            {
+                logger.LogError("--messaging-endpoint requires an HTTPS URL value (e.g. https://my-agent.example.com/api/messages).");
+                context.ExitCode = 1;
+                return;
+            }
+
             // --messaging-endpoint validation: must be a well-formed HTTPS URL when supplied.
             if (!string.IsNullOrWhiteSpace(messagingEndpointFlag) &&
                 (!Uri.TryCreate(messagingEndpointFlag, UriKind.Absolute, out var msgEndpointUri) ||
@@ -377,6 +387,15 @@ effectiveAuthModeForValidation is not ("obo" or "s2s" or "both"))
             if (nonDwConfig is null)
                 isM365 = true;
 
+            // --messaging-endpoint only takes effect for M365 agents (the messaging endpoint step is
+            // skipped otherwise). Fail fast rather than silently ignoring the supplied value.
+            if (messagingEndpointSpecified && !isM365)
+            {
+                logger.LogError("--messaging-endpoint applies only to M365 agents. Add --m365 (or use --aiteammate).");
+                context.ExitCode = 1;
+                return;
+            }
+
             if (nonDwConfig is not null)
             {
                 if (dryRun)

src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/BlueprintSubcommand.cs

@@ -204,6 +204,9 @@ public static Command CreateCommand(
             var skipEndpointRegistration = context.ParseResult.GetValueForOption(skipEndpointRegistrationOption);
             var endpointOnly = context.ParseResult.GetValueForOption(endpointOnlyOption);
             var updateEndpoint = context.ParseResult.GetValueForOption(updateEndpointOption);
+            // Distinguish "option omitted" from "explicitly passed empty" — the latter is a hard error,
+            // not silently treated as omitted (which would later no-op or fall back to config).
+            var messagingEndpointSpecified = context.ParseResult.CommandResult.FindResultFor(messagingEndpointOption) != null;
             var messagingEndpointFlag = context.ParseResult.GetValueForOption(messagingEndpointOption)?.Trim();
             var skipRequirements = context.ParseResult.GetValueForOption(skipRequirementsOption);
             var isM365 = context.ParseResult.GetValueForOption(m365Option);
@@ -287,6 +290,22 @@ public static Command CreateCommand(
                 return;
             }
 
+            if (messagingEndpointSpecified && string.IsNullOrWhiteSpace(messagingEndpointFlag))
+            {
+                logger.LogError("--messaging-endpoint requires an HTTPS URL value (e.g. https://my-agent.example.com/api/messages).");
+                context.ExitCode = 1;
+                return;
+            }
+
+            // --messaging-endpoint is consumed only by the --endpoint-only registration path. Fail fast
+            // rather than accepting a value that would be silently ignored on a normal blueprint run.
+            if (messagingEndpointSpecified && !endpointOnly)
+            {
+                logger.LogError("--messaging-endpoint applies only with --endpoint-only. To replace an existing endpoint, use --update-endpoint <url>.");
+                context.ExitCode = 1;
+                return;
+            }
+
             // --messaging-endpoint validation: must be a well-formed HTTPS URL when supplied.
             if (!string.IsNullOrWhiteSpace(messagingEndpointFlag) &&
                 (!Uri.TryCreate(messagingEndpointFlag, UriKind.Absolute, out var msgEndpointUri) ||
@@ -398,7 +417,7 @@ await RequirementsSubcommand.RunChecksOrExitAsync(
                     return;
                 }
 
-                await RegisterEndpointAndSyncAsync(
+                var endpointResult = await RegisterEndpointAndSyncAsync(
                     config.FullName,
                     logger,
                     configService,
@@ -407,6 +426,14 @@ await RegisterEndpointAndSyncAsync(
                     overrideEndpointUrl: messagingEndpointFlag,
                     correlationId: correlationId,
                     cancellationToken: ct);
+
+                // Non-zero exit when the endpoint did not actually register (not configured, contract
+                // mismatch, or other failure) so scripts and CI can detect it.
+                if (endpointResult != Models.EndpointRegistrationResult.Created &&
+                    endpointResult != Models.EndpointRegistrationResult.AlreadyExists)
+                {
+                    context.ExitCode = 1;
+                }
                 return;
             }
 

src/Tests/Microsoft.Agents.A365.DevTools.Cli.Tests/Commands/BlueprintSubcommandTests.cs

@@ -591,6 +591,58 @@ public void DryRunDisplay_ShouldShowMessagingUrl()
 
     #endregion
 
+    #region --messaging-endpoint option validation
+
+    [Fact]
+    public async Task SetupBlueprint_MessagingEndpointWithoutEndpointOnly_ExitsOne()
+    {
+        var command = BlueprintSubcommand.CreateCommand(
+            _mockLogger, _mockConfigService, _mockExecutor, _mockAuthValidator, _mockPlatformDetector,
+            _mockBackendConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService);
+        var parser = new CommandLineBuilder(command).Build();
+
+        var result = await parser.InvokeAsync(
+            new[] { "--messaging-endpoint", "https://agent.contoso.com/api/messages" }, new TestConsole());
+
+        result.Should().Be(1,
+            because: "--messaging-endpoint only applies with --endpoint-only; supplying it alone must fail fast, not be silently ignored");
+    }
+
+    [Fact]
+    public async Task SetupBlueprint_EndpointOnlyWithEmptyMessagingEndpoint_ExitsOne()
+    {
+        var command = BlueprintSubcommand.CreateCommand(
+            _mockLogger, _mockConfigService, _mockExecutor, _mockAuthValidator, _mockPlatformDetector,
+            _mockBackendConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService);
+        var parser = new CommandLineBuilder(command).Build();
+
+        var result = await parser.InvokeAsync(
+            new[] { "--endpoint-only", "--m365", "--messaging-endpoint", "" }, new TestConsole());
+
+        result.Should().Be(1,
+            because: "an explicitly-empty --messaging-endpoint must error, not be treated as omitted");
+    }
+
+    [Fact]
+    public async Task SetupBlueprint_EndpointOnlyWhenEndpointNotConfigured_ExitsOne()
+    {
+        // Blueprint exists but no messaging endpoint is configured → registration returns Failed.
+        var config = new Agent365Config { TenantId = "test-tenant", AgentBlueprintId = "blueprint-123" };
+        _mockConfigService.LoadAsync(Arg.Any<string>()).Returns(Task.FromResult(config));
+
+        var command = BlueprintSubcommand.CreateCommand(
+            _mockLogger, _mockConfigService, _mockExecutor, _mockAuthValidator, _mockPlatformDetector,
+            _mockBackendConfigurator, _mockGraphApiService, _mockBlueprintService, _mockClientAppValidator, _mockBlueprintLookupService, _mockFederatedCredentialService);
+        var parser = new CommandLineBuilder(command).Build();
+
+        var result = await parser.InvokeAsync(new[] { "--endpoint-only", "--m365", "--skip-requirements" }, new TestConsole());
+
+        result.Should().Be(1,
+            because: "endpoint registration that doesn't complete must surface a non-zero exit code for scripting");
+    }
+
+    #endregion
+
     #region RegisterEndpointAndSyncAsync Tests
 
     [Fact]