From 917b436668fc8c201d4f7110b9869c1b1484123d Mon Sep 17 00:00:00 2001 From: autocarl Date: Sun, 26 Jul 2026 09:14:55 -0400 Subject: [PATCH 1/3] feat: add hidden option and alias visibility --- .../Autocomplete/AutocompleteEngine.cs | 10 +- src/Repl.Core/CommandBuilder.cs | 206 ++++- src/Repl.Core/CoreReplApp.Documentation.cs | 8 +- src/Repl.Core/CoreReplApp.Execution.cs | 3 +- src/Repl.Core/CoreReplApp.cs | 42 +- .../Documentation/DocumentationEngine.cs | 241 +++++- src/Repl.Core/Documentation/ReplDocOption.cs | 29 +- src/Repl.Core/GlobalOptionBuilder.cs | 53 ++ .../Help/HelpTextBuilder.Rendering.cs | 74 +- src/Repl.Core/Help/HelpTextBuilder.cs | 12 +- .../Interaction/InteractionProgressFactory.cs | 13 + .../Options/HiddenRequiredOptionException.cs | 19 + .../Internal/Options/OptionSchema.cs | 288 ++++++- .../Internal/Options/OptionSchemaBuilder.cs | 105 ++- .../Internal/Options/OptionSchemaEntry.cs | 3 +- .../Internal/Options/OptionSchemaParameter.cs | 11 +- .../Options/OptionTokenCompletionSource.cs | 50 +- src/Repl.Core/OptionBuilder.cs | 80 ++ .../Output/MarkdownOutputTransformer.cs | 25 +- .../Attributes/ReplOptionAttribute.cs | 39 + .../Parsing/GlobalInvocationOptions.cs | 3 + .../Parsing/GlobalOptionDefinition.cs | 4 +- src/Repl.Core/Parsing/GlobalOptionParser.cs | 102 ++- .../Parsing/InvocationOptionParser.cs | 16 +- src/Repl.Core/ParsingOptions.cs | 198 ++++- src/Repl.Core/Routing/RouteDefinition.cs | 8 +- src/Repl.Core/RoutingInvalidatedEventArgs.cs | 6 + src/Repl.Core/Session/InteractiveSession.cs | 11 +- .../ShellCompletion/ShellCompletionEngine.cs | 41 +- src/Repl.Defaults/GlobalOptionsExtensions.cs | 25 +- src/Repl.Defaults/ReplApp.cs | 29 +- .../Given_Completions.cs | 141 +++- .../Given_CustomGlobalOptions.cs | 30 + .../Given_DocumentationExport.cs | 121 +++ .../Given_GlobalOptionsAccessor.cs | 235 ++++++ .../Given_HelpDiscovery.cs | 718 ++++++++++++++++++ .../Given_OptionAttributeOverrides.cs | 16 + .../Given_OptionsGroupBinding.cs | 73 ++ src/Repl.Mcp/McpAutomationProjection.cs | 57 ++ src/Repl.Mcp/McpInteractionChannel.cs | 42 +- src/Repl.Mcp/McpSchemaGenerator.cs | 1 + src/Repl.Mcp/McpServerHandler.cs | 146 +++- src/Repl.Mcp/McpToolAdapter.cs | 224 +++++- src/Repl.McpTests/Given_McpDebounce.cs | 110 ++- .../Given_McpFallbackEndToEnd.cs | 21 + .../Given_McpInteractionChannel.cs | 54 ++ src/Repl.McpTests/Given_McpSchemaGenerator.cs | 14 + src/Repl.McpTests/Given_McpServerEndToEnd.cs | 532 +++++++++++++ .../Given_McpServerOptionsBuilder.cs | 18 + src/Repl.McpTests/Given_McpToolAdapter.cs | 269 +++++++ src/Repl.McpTests/McpTestFixture.cs | 49 +- .../Given_CommandBuilderEnrichment.cs | 116 +++ src/Repl.Tests/Given_GlobalOptionsAccessor.cs | 78 ++ ...nteractiveAutocomplete_OptionCandidates.cs | 270 ++++++- src/Repl.Tests/Given_OptionSchema.cs | 34 + 55 files changed, 4832 insertions(+), 291 deletions(-) create mode 100644 src/Repl.Core/GlobalOptionBuilder.cs create mode 100644 src/Repl.Core/Internal/Options/HiddenRequiredOptionException.cs create mode 100644 src/Repl.Core/OptionBuilder.cs create mode 100644 src/Repl.Core/RoutingInvalidatedEventArgs.cs create mode 100644 src/Repl.Mcp/McpAutomationProjection.cs create mode 100644 src/Repl.Tests/Given_OptionSchema.cs diff --git a/src/Repl.Core/Autocomplete/AutocompleteEngine.cs b/src/Repl.Core/Autocomplete/AutocompleteEngine.cs index fe6730b9..47f410b1 100644 --- a/src/Repl.Core/Autocomplete/AutocompleteEngine.cs +++ b/src/Repl.Core/Autocomplete/AutocompleteEngine.cs @@ -624,7 +624,7 @@ private ConsoleLineReader.AutocompleteSuggestion[] AppendInvalidAlternativeIfNee var comparer = StringComparer.FromComparison(comparison); var tokens = new List(); var dedupe = new HashSet(comparer); - OptionTokenCompletionSource.CollectGlobalOptionTokens( + var customGlobalOwnership = OptionTokenCompletionSource.CollectGlobalOptionTokens( app.OptionsSnapshot, currentTokenPrefix, comparison, dedupe, tokens); // Source route options from the single route this prefix resolves to (already @@ -636,7 +636,8 @@ private ConsoleLineReader.AutocompleteSuggestion[] AppendInvalidAlternativeIfNee && commandPrefix.Length == match.Route.Template.Segments.Count) { OptionTokenCompletionSource.CollectRouteOptionTokens( - match.Route, + match.Route.OptionSchema, + customGlobalOwnership, currentTokenPrefix, app.OptionsSnapshot.Parsing.OptionCaseSensitivity, dedupe, @@ -1592,6 +1593,11 @@ internal static bool IsControlFreeValue(string value) => pendingOptionToken, app.OptionsSnapshot.Parsing.OptionCaseSensitivity); foreach (var entry in entries) { + if (!match.Route.OptionSchema.IsEntryDiscoverable(entry)) + { + continue; + } + // Same keystroke rule as the positional path: providers only run for an explicit // completion request; live-hint refreshes fall through to the static enum fallback. if (providersAllowed diff --git a/src/Repl.Core/CommandBuilder.cs b/src/Repl.Core/CommandBuilder.cs index 724dc4fb..806290fa 100644 --- a/src/Repl.Core/CommandBuilder.cs +++ b/src/Repl.Core/CommandBuilder.cs @@ -1,3 +1,6 @@ +using System.Reflection; +using Repl.Internal.Options; + namespace Repl; /// @@ -11,15 +14,37 @@ public sealed class CommandBuilder private readonly Dictionary _completionScopes = new(StringComparer.OrdinalIgnoreCase); + // Option visibility lives on this immutable schema, not on the builder, so every discovery + // surface reads one source. Swapped whole via CompareExchange rather than mutated: no lock, + // because a blocking wait on another managed thread deadlocks on single-threaded WASM. + private OptionSchema _optionSchema = OptionSchema.Empty; + + // Derived discovery state (most visibly the MCP tool snapshot) is rebuilt only when routing + // is invalidated, so any visibility change made after Map has to say so. + private readonly Action? _invalidateRouting; + private readonly Func? _resolveOptionCaseSensitivity; + /// /// Initializes a new instance of the class. /// /// The command route template. /// The command handler delegate. - internal CommandBuilder(string route, Delegate handler) + /// + /// Invoked when metadata that discovery surfaces derive from changes after registration. + /// + /// + /// Resolves the current global option-token comparison mode for post-registration visibility changes. + /// + internal CommandBuilder( + string route, + Delegate handler, + Action? invalidateRouting = null, + Func? resolveOptionCaseSensitivity = null) { Route = route; Handler = handler; + _invalidateRouting = invalidateRouting; + _resolveOptionCaseSensitivity = resolveOptionCaseSensitivity; SupportsHostedProtocolPassthrough = ComputeSupportsHostedProtocolPassthrough(handler); } @@ -139,6 +164,182 @@ public CommandBuilder WithAlias(params string[] aliases) return this; } + /// + /// Configures one option's metadata. + /// + /// + /// The callback shape is deliberate: this type and both expose + /// Hidden and AutomationHidden, so an API returning the option builder would let + /// .Option("x").Hidden() sit in a chain reading exactly like the command-level call while + /// meaning something quite different, with nothing to signal that the subject changed. Passing a + /// callback keeps the subject explicit and the chain on the command. + /// + /// Handler parameter or options-group property name. + /// Receives the option metadata builder. + /// The same command builder instance. + /// No such option target is registered for this command. + /// + /// hid an options-group property that a required arity or + /// non-omittable CLR shape makes impossible to omit from an invocation. + /// + public CommandBuilder WithOption(string targetName, Action configure) + { + ArgumentNullException.ThrowIfNull(configure); + configure(SelectOption(targetName)); + + return this; + } + + private OptionBuilder SelectOption(string targetName) + { + targetName = string.IsNullOrWhiteSpace(targetName) + ? throw new ArgumentException("Option target name cannot be empty.", nameof(targetName)) + : targetName; + var schema = OptionSchema; + if (!schema.TryGetParameter(targetName, out var parameter) + || parameter.Mode == ReplParameterMode.ArgumentOnly) + { + // List the candidates: the target is the CLR parameter or property name, not the rendered + // token, and that mismatch is the mistake this exception almost always reports. + var candidates = schema.Parameters.Values + .Where(static candidate => candidate.Mode != ReplParameterMode.ArgumentOnly) + .Select(static candidate => candidate.Name) + .Order(StringComparer.Ordinal); + + throw new KeyNotFoundException( + $"No option target named '{targetName}' is registered for command '{Route}'. " + + $"Known option targets: {string.Join(", ", candidates)}."); + } + + return new OptionBuilder( + isHidden => UpdateOptionParameter( + targetName, + current => current with { IsHidden = isHidden }, + isVisibilityRetraction: isHidden), + isAutomationHidden => UpdateOptionParameter( + targetName, + current => current with { IsAutomationHidden = isAutomationHidden }, + isVisibilityRetraction: isAutomationHidden), + (alias, isHidden) => UpdateOptionAliasVisibility(targetName, alias, isHidden)); + } + + internal OptionSchema OptionSchema => Volatile.Read(ref _optionSchema); + + internal void AttachOptionSchema(OptionSchema schema) => Volatile.Write(ref _optionSchema, schema); + + /// + /// Publishes a new schema with the target parameter's metadata updated. + /// + /// + /// A retry loop rather than a plain write: the schema is also the parsing contract, so a + /// concurrent update must not drop the other one. Unknown targets are impossible here because + /// already rejected them. + /// + private void UpdateOptionParameter( + string targetName, + Func update, + bool isVisibilityRetraction) + { + while (true) + { + var current = Volatile.Read(ref _optionSchema); + if (!current.TryGetParameter(targetName, out var parameter)) + { + return; + } + + var updated = update(parameter); + if (updated == parameter) + { + return; + } + + var candidate = current.WithParameter(updated); + + // Validate before publishing so the throw carries the caller's own Hidden() frame and + // the schema is never left in a state no invocation could satisfy. + ValidateOptionVisibility(candidate); + if (ReferenceEquals(Interlocked.CompareExchange(ref _optionSchema, candidate, current), current)) + { + _invalidateRouting?.Invoke(isVisibilityRetraction); + return; + } + } + } + + private void UpdateOptionAliasVisibility(string targetName, string alias, bool isHidden) + { + while (true) + { + var current = Volatile.Read(ref _optionSchema); + var candidate = current.WithAliasVisibility( + targetName, + alias, + isHidden, + _resolveOptionCaseSensitivity?.Invoke()); + if (ReferenceEquals(candidate, current)) + { + // Already in the requested state: WithAliasVisibility returned the same instance. + // Mirrors UpdateOptionParameter's no-op early exit — no schema swap, no routing + // invalidation, over a call that changed nothing. + return; + } + + if (ReferenceEquals(Interlocked.CompareExchange(ref _optionSchema, candidate, current), current)) + { + _invalidateRouting?.Invoke(isHidden); + return; + } + } + } + + internal void ValidateOptionVisibility() => ValidateOptionVisibility(OptionSchema); + + /// + /// Rejects a hidden option that no invocation could omit. + /// + /// + /// Runs at configuration time only — from for the + /// declarative form and from the fluent setter for the other. It deliberately does not run on any + /// execution or completion path: Run reports failures as an exit code and has no general + /// catch, so throwing from there escapes the host unhandled. + /// + private void ValidateOptionVisibility(OptionSchema schema) + { + foreach (var parameter in schema.Parameters.Values) + { + if (parameter.Mode == ReplParameterMode.ArgumentOnly || !parameter.IsHidden) + { + continue; + } + + // Two independent ways an option can be mandatory, and each needs the right evidence. + // + // Only an EXPLICITLY declared arity counts: an inferred ExactlyOne describes how many + // values the token consumes when present, not whether the option may be absent, so a + // nullable reference parameter infers ExactlyOne yet binds null happily when omitted. + // + // The CLR shape is the other authority. Direct handler parameters cannot be rejected at + // mapping time, though: Run and MCP may supply an external provider later, and the binder + // consults it before enforcing either lower bound. Provider-aware documentation validates + // those paths at the discovery boundary. Options-group properties never reach that fallback, + // so their impossible hidden configuration remains an immediate error. + var requiresFallback = parameter.ExplicitArity is ReplArity.ExactlyOne or ReplArity.OneOrMore + || !parameter.CanBeOmitted; + if (!requiresFallback || parameter.SupportsServiceFallback) + { + continue; + } + + // Name the rendered token as well as the CLR target: an operator greps argv for + // '--internal-token' and would not find the parameter name anywhere. + throw new HiddenRequiredOptionException( + parameter.Name, + schema.ResolveDisplayToken(parameter.Name), + Route); + } + } + /// /// Adds a completion provider for a target parameter, invoked by in-process surfaces only /// (the interactive Tab menu and the complete ambient command). @@ -214,6 +415,7 @@ public CommandBuilder WithBanner(string text) public CommandBuilder Hidden(bool isHidden = true) { IsHidden = isHidden; + _invalidateRouting?.Invoke(isHidden); return this; } @@ -333,6 +535,7 @@ public CommandBuilder LongRunning(bool value = true) public CommandBuilder AutomationHidden(bool value = true) { Annotations = (Annotations ?? new CommandAnnotations()) with { AutomationHidden = value }; + _invalidateRouting?.Invoke(value); return this; } @@ -348,6 +551,7 @@ public CommandBuilder WithAnnotations(Action configur var builder = new CommandAnnotationsBuilder(); configure(builder); Annotations = builder.Build(); + _invalidateRouting?.Invoke(Annotations.AutomationHidden); return this; } diff --git a/src/Repl.Core/CoreReplApp.Documentation.cs b/src/Repl.Core/CoreReplApp.Documentation.cs index 7f93d609..3540b0c0 100644 --- a/src/Repl.Core/CoreReplApp.Documentation.cs +++ b/src/Repl.Core/CoreReplApp.Documentation.cs @@ -9,7 +9,13 @@ public sealed partial class CoreReplApp public ReplDocumentationModel CreateDocumentationModel(string? targetPath = null) => DocumentationEng.CreateDocumentationModel(targetPath); - internal ReplDocumentationModel CreateDocumentationModel( + /// + /// Builds a structured documentation model using the specified provider for service-backed parameters. + /// + /// Provider used to determine whether direct handler parameters can be omitted. + /// Optional target path to scope the model. + /// A structured documentation model. + public ReplDocumentationModel CreateDocumentationModel( IServiceProvider serviceProvider, string? targetPath = null) => DocumentationEng.CreateDocumentationModel(serviceProvider, targetPath); diff --git a/src/Repl.Core/CoreReplApp.Execution.cs b/src/Repl.Core/CoreReplApp.Execution.cs index c53ab934..37660fa7 100644 --- a/src/Repl.Core/CoreReplApp.Execution.cs +++ b/src/Repl.Core/CoreReplApp.Execution.cs @@ -479,7 +479,8 @@ private async ValueTask TryHandleContextDeeplinkAsync( var parsedOptions = InvocationOptionParser.Parse( match.RemainingTokens, match.Route.OptionSchema, - commandParsingOptions); + commandParsingOptions, + globalOptions.CustomGlobalTokenOwnership); if (parsedOptions.HasErrors) { var firstError = parsedOptions.Diagnostics diff --git a/src/Repl.Core/CoreReplApp.cs b/src/Repl.Core/CoreReplApp.cs index bfc75d18..d3e4f22a 100644 --- a/src/Repl.Core/CoreReplApp.cs +++ b/src/Repl.Core/CoreReplApp.cs @@ -40,6 +40,7 @@ public sealed partial class CoreReplApp : ICoreReplApp internal GlobalOptionsSnapshot GlobalOptionsSnapshotInstance => _globalOptionsSnapshot; internal ImplicitServiceParameterRegistry ImplicitServiceParameters => _implicitServiceParameters; internal ShellCompletionRuntime ShellCompletionRuntimeInstance => _shellCompletionRuntime; + internal IServiceProvider CurrentServiceProvider => _runtimeState.Value?.ServiceProvider ?? _services; internal IReplExecutionObserver? ExecutionObserver { get; set; } internal List Contexts => _contexts; internal AsyncLocal BannerRendered => _bannerRendered; @@ -136,7 +137,23 @@ public CoreReplApp Use(Func middlewar public CoreReplApp Options(Action configure) { ArgumentNullException.ThrowIfNull(configure); + var previousCaseSensitivity = _options.Parsing.OptionCaseSensitivity; + var previousGlobalOptions = _options.Parsing.GlobalOptions.ToDictionary( + static pair => pair.Key, + static pair => pair.Value, + StringComparer.OrdinalIgnoreCase); + configure(_options); + + var globalDiscoveryChanged = previousGlobalOptions.Count != _options.Parsing.GlobalOptions.Count + || previousGlobalOptions.Any(pair => + !_options.Parsing.GlobalOptions.TryGetValue(pair.Key, out var current) + || !ReferenceEquals(pair.Value, current)); + if (previousCaseSensitivity != _options.Parsing.OptionCaseSensitivity || globalDiscoveryChanged) + { + InvalidateRouting(isVisibilityRetraction: true); + } + return this; } @@ -144,15 +161,17 @@ public CoreReplApp Options(Action configure) /// Occurs after routing has been invalidated. /// Subscribers can use this to refresh derived state (e.g. MCP tool lists). /// - internal event EventHandler? RoutingInvalidated; + internal event EventHandler? RoutingInvalidated; /// /// Invalidates active routing cache so module presence predicates are re-evaluated on next resolution. /// - public void InvalidateRouting() + public void InvalidateRouting() => InvalidateRouting(isVisibilityRetraction: false); + + private void InvalidateRouting(bool isVisibilityRetraction) { Interlocked.Increment(ref _routingCacheVersion); - RoutingInvalidated?.Invoke(this, EventArgs.Empty); + RoutingInvalidated?.Invoke(this, new RoutingInvalidatedEventArgs(isVisibilityRetraction)); } /// @@ -168,7 +187,11 @@ public CommandBuilder Map(string route, Delegate handler) : route; ArgumentNullException.ThrowIfNull(handler); - var command = new CommandBuilder(route, handler); + var command = new CommandBuilder( + route, + handler, + InvalidateRouting, + () => _options.Parsing.OptionCaseSensitivity); ApplyMetadataFromAttributes(command, handler); var parsedTemplate = RouteTemplateParser.Parse(route, _options.Parsing); var template = InferRouteConstraintsFromHandler(parsedTemplate, handler); @@ -180,8 +203,15 @@ public CommandBuilder Map(string route, Delegate handler) .Select(existingRoute => existingRoute.Template)); _commands.Add(command); - var optionSchema = OptionSchemaBuilder.Build(template, command, _options.Parsing, _implicitServiceParameters); - var routeDefinition = new RouteDefinition(template, command, moduleId, optionSchema); + var optionSchema = OptionSchemaBuilder.Build( + template, + command, + _options.Parsing, + _implicitServiceParameters, + () => _options.Parsing.OptionCaseSensitivity); + command.AttachOptionSchema(optionSchema); + command.ValidateOptionVisibility(); + var routeDefinition = new RouteDefinition(template, command, moduleId); _routes.Add(routeDefinition); InvalidateRouting(); return command; diff --git a/src/Repl.Core/Documentation/DocumentationEngine.cs b/src/Repl.Core/Documentation/DocumentationEngine.cs index 8c77d2e2..6dbecc85 100644 --- a/src/Repl.Core/Documentation/DocumentationEngine.cs +++ b/src/Repl.Core/Documentation/DocumentationEngine.cs @@ -1,6 +1,7 @@ using System.ComponentModel; using System.Diagnostics.CodeAnalysis; using System.Reflection; +using Repl.Interaction; using Repl.Internal.Options; namespace Repl; @@ -65,7 +66,23 @@ public object CreateDocumentationModelInternal(string? targetPath) out var notFoundResult); var contexts = SelectDocumentationContexts(normalizedTargetPath, commands, discoverableContexts); - var commandDocs = commands.Select(BuildDocumentationCommand).ToArray(); + + // Mirrors the command axis: a targeted command exports its hidden options, flagged, the way + // a targeted hidden command exports itself. Aggregate models omit them — which is also what + // keeps them out of the MCP tool schema and its argument allow-list, since MCP always builds + // the aggregate model. + var isExactCommandTarget = commands.Length == 1 + && !string.IsNullOrWhiteSpace(normalizedTargetPath) + && string.Equals(commands[0].Template.Template, normalizedTargetPath, StringComparison.OrdinalIgnoreCase); + var serviceAvailability = new Dictionary(); + var customGlobalOwnership = GlobalOptionParser.BuildCustomTokenOwnership(app.OptionsSnapshot.Parsing); + var commandDocs = commands + .Select(route => BuildDocumentationCommand( + route, + includeHiddenOptions: isExactCommandTarget, + serviceAvailability, + customGlobalOwnership)) + .ToArray(); var contextDocs = contexts .Select(context => new ReplDocContext( Path: context.Template.Template, @@ -74,15 +91,7 @@ public object CreateDocumentationModelInternal(string? targetPath) IsHidden: context.IsHidden, Details: context.Details)) .ToArray(); - var resourceDocs = commandDocs - .Where(cmd => cmd.IsResource || cmd.Annotations?.ReadOnly == true) - .Select(cmd => new ReplDocResource( - Path: cmd.Path, - Description: cmd.Description, - Details: cmd.Details, - Arguments: cmd.Arguments, - Options: cmd.Options)) - .ToArray(); + var resourceDocs = BuildDocumentationResources(commandDocs); var model = new ReplDocumentationModel( App: BuildDocumentationApp(), Contexts: contextDocs, @@ -91,6 +100,17 @@ public object CreateDocumentationModelInternal(string? targetPath) return (model, notFoundResult); } + private static ReplDocResource[] BuildDocumentationResources(IReadOnlyList commands) => + commands + .Where(static command => command.IsResource || command.Annotations?.ReadOnly == true) + .Select(static command => new ReplDocResource( + Path: command.Path, + Description: command.Description, + Details: command.Details, + Arguments: command.Arguments, + Options: command.Options)) + .ToArray(); + private static RouteDefinition[] SelectDocumentationCommands( string? normalizedTargetPath, IReadOnlyList routes, @@ -171,8 +191,17 @@ private static ContextDefinition[] SelectDocumentationContexts( return selected; } - private ReplDocCommand BuildDocumentationCommand(RouteDefinition route) + private ReplDocCommand BuildDocumentationCommand( + RouteDefinition route, + bool includeHiddenOptions, + Dictionary serviceAvailability, + IReadOnlyDictionary customGlobalOwnership) { + if (!includeHiddenOptions) + { + ValidateHiddenOptionInvocability(route, serviceAvailability); + } + var dynamicSegments = route.Template.Segments .OfType() .ToArray(); @@ -181,7 +210,13 @@ private ReplDocCommand BuildDocumentationCommand(RouteDefinition route) .ToHashSet(StringComparer.OrdinalIgnoreCase); var handlerParams = route.Command.Handler.Method.GetParameters(); var arguments = BuildDocumentationArguments(dynamicSegments, handlerParams); - var options = BuildDocumentationOptions(route, routeParameterNames, handlerParams); + var options = BuildDocumentationOptions( + route, + routeParameterNames, + handlerParams, + includeHiddenOptions, + serviceAvailability, + customGlobalOwnership); var answers = BuildDocumentationAnswers(route.Command); var acceptsPagingInput = handlerParams.Any(static parameter => parameter.ParameterType == typeof(IReplPagingContext)); var emitsPagedResult = IsPagedReturnType(route.Command.Handler.Method.ReturnType); @@ -206,26 +241,39 @@ private ReplDocCommand BuildDocumentationCommand(RouteDefinition route) private ReplDocOption[] BuildDocumentationOptions( RouteDefinition route, HashSet routeParameterNames, - ParameterInfo[] handlerParams) + ParameterInfo[] handlerParams, + bool includeHiddenOptions, + Dictionary serviceAvailability, + IReadOnlyDictionary customGlobalOwnership) { + var schema = route.OptionSchema; var regularOptions = handlerParams .Where(parameter => - !string.IsNullOrWhiteSpace(parameter.Name) + parameter.Name is { } name && parameter.ParameterType != typeof(CancellationToken) - && !routeParameterNames.Contains(parameter.Name!) + && !routeParameterNames.Contains(name) && !app.ImplicitServiceParameters.IsImplicitServiceParameter(parameter.ParameterType) && parameter.GetCustomAttribute() is null && parameter.GetCustomAttribute() is null - && !Attribute.IsDefined(parameter.ParameterType, typeof(ReplOptionsGroupAttribute), inherit: true)) - .Select(parameter => BuildDocumentationOption(route.OptionSchema, parameter)); + && !Attribute.IsDefined(parameter.ParameterType, typeof(ReplOptionsGroupAttribute), inherit: true) + && (includeHiddenOptions || !schema.IsOptionHidden(name)) + && ShouldIncludeDocumentationOption( + route, name, includeHiddenOptions, serviceAvailability, customGlobalOwnership)) + .Select(parameter => BuildDocumentationOption( + schema, parameter, serviceAvailability, customGlobalOwnership)); var groupOptions = handlerParams .Where(parameter => Attribute.IsDefined(parameter.ParameterType, typeof(ReplOptionsGroupAttribute), inherit: true)) .SelectMany(parameter => { var defaultInstance = CreateOptionsGroupDefault(parameter.ParameterType); return GetOptionsGroupProperties(parameter.ParameterType) - .Where(prop => prop.CanWrite) - .Select(prop => BuildDocumentationOptionFromProperty(route.OptionSchema, prop, defaultInstance)); + .Where(prop => + prop.CanWrite + && (includeHiddenOptions || !schema.IsOptionHidden(prop.Name)) + && ShouldIncludeDocumentationOption( + route, prop.Name, includeHiddenOptions, serviceAvailability, customGlobalOwnership)) + .Select(prop => BuildDocumentationOptionFromProperty( + schema, prop, defaultInstance, customGlobalOwnership)); }); return regularOptions.Concat(groupOptions).ToArray(); } @@ -294,26 +342,120 @@ internal ReplDocApp BuildDocumentationApp() return new ReplDocApp(name, version, description); } - // An explicit OneOrMore/ExactlyOne arity now fails binding when the option is absent - // (HandlerArgumentBinder), so exported docs must report it as required regardless of the - // CLR parameter shape. - private static bool HasExplicitRequiredArity(OptionSchema schema, string parameterName) => - schema.TryGetParameter(parameterName, out var schemaParameter) - && schemaParameter.ExplicitArity is ReplArity.OneOrMore or ReplArity.ExactlyOne; - - private static bool IsRequiredParameter(ParameterInfo parameter) + // Explicit lower bounds and non-omittable CLR shapes are required only when the binding path + // cannot obtain the value from the active provider. This must use GetService itself — not + // registration metadata — because custom/external providers and null-returning factories are + // part of the same contract as HandlerArgumentBinder. + private bool IsRequiredOption( + OptionSchema schema, + string parameterName, + Dictionary serviceAvailability) { - if (parameter.HasDefaultValue) + if (!schema.TryGetParameter(parameterName, out var parameter)) { return false; } - if (!parameter.ParameterType.IsValueType) + var requiresFallback = parameter.ExplicitArity is ReplArity.OneOrMore or ReplArity.ExactlyOne + || !parameter.CanBeOmitted; + return requiresFallback + && (!parameter.SupportsServiceFallback + || !CanResolveFromActiveServices(parameter.ParameterType, serviceAvailability)); + } + + private bool CanResolveFromActiveServices(Type parameterType, Dictionary serviceAvailability) + { + // HandlerArgumentBinder synthesizes these progress types from the interaction channel before + // direct service lookup. Discovery must apply that same fallback and still allow an explicitly + // registered IProgress when no channel is available. + if (InteractionProgressFactory.IsSupportedProgressType(parameterType) + && IsServiceAvailable(typeof(IReplInteractionChannel), serviceAvailability)) + { + return true; + } + + return IsServiceAvailable(parameterType, serviceAvailability); + } + + private bool IsServiceAvailable(Type serviceType, Dictionary serviceAvailability) + { + if (serviceAvailability.TryGetValue(serviceType, out var available)) { - return false; + return available; } - return Nullable.GetUnderlyingType(parameter.ParameterType) is null; + // GetService can activate a transient factory or throw outright — a scoped registration + // resolved from a root provider under ValidateScopes is a common way this happens. A + // misbehaving registration must not crash discovery for every other route over one option's + // fallback check; treat a throw here the same as a null result, unavailable. + try + { + available = app.CurrentServiceProvider.GetService(serviceType) is not null; + } + catch (Exception) + { + available = false; + } + + serviceAvailability[serviceType] = available; + return available; + } + + private bool ShouldIncludeDocumentationOption( + RouteDefinition route, + string parameterName, + bool includeHiddenOptions, + Dictionary serviceAvailability, + IReadOnlyDictionary customGlobalOwnership) + { + var schema = route.OptionSchema; + if (includeHiddenOptions && schema.IsOptionHidden(parameterName)) + { + return true; + } + + var displayToken = schema.ResolveDisplayToken(parameterName); + var hasReachableToken = schema.ResolveDiscoverableAliases(parameterName) + .Any(entry => !customGlobalOwnership.ContainsKey(entry.Token)); + if (displayToken is null || hasReachableToken) + { + return true; + } + + if (IsRequiredOption(schema, parameterName, serviceAvailability)) + { + throw new HiddenRequiredOptionException( + parameterName, + displayToken, + route.Template.Template); + } + + return false; + } + + private static bool HasExplicitRequiredArity(OptionSchema schema, string parameterName) => + schema.TryGetParameter(parameterName, out var schemaParameter) + && schemaParameter.ExplicitArity is ReplArity.OneOrMore or ReplArity.ExactlyOne; + + private void ValidateHiddenOptionInvocability( + RouteDefinition route, + Dictionary serviceAvailability) + { + var schema = route.OptionSchema; + foreach (var parameter in schema.Parameters.Values) + { + if (parameter.Mode == ReplParameterMode.ArgumentOnly + || !parameter.IsHidden + || !IsRequiredOption(schema, parameter.Name, serviceAvailability)) + { + continue; + } + + throw new HiddenRequiredOptionException( + parameter.Name, + schema.ResolveDisplayToken(parameter.Name), + route.Template.Template); + } } internal static string GetConstraintTypeName(RouteConstraintKind kind) => @@ -381,10 +523,12 @@ private static string GetFriendlyTypeName(Type type) private static ReplDocOption BuildDocumentationOptionFromProperty( OptionSchema schema, PropertyInfo property, - object defaultInstance) + object defaultInstance, + IReadOnlyDictionary customGlobalOwnership) { - var entries = schema.Entries - .Where(entry => string.Equals(entry.ParameterName, property.Name, StringComparison.OrdinalIgnoreCase)) + var displayToken = schema.ResolveDisplayToken(property.Name); + var entries = schema.ResolveDiscoverableAliases(property.Name) + .Where(entry => !customGlobalOwnership.ContainsKey(entry.Token)) .ToArray(); var aliases = entries .Where(entry => entry.TokenKind is OptionSchemaTokenKind.NamedOption or OptionSchemaTokenKind.BoolFlag) @@ -411,7 +555,7 @@ private static ReplDocOption BuildDocumentationOptionFromProperty( ? propDefault.ToString() : null; return new ReplDocOption( - Name: aliases.Length > 0 ? aliases[0].TrimStart('-') : property.Name, + Name: displayToken?.TrimStart('-') ?? property.Name, Type: GetFriendlyTypeName(property.PropertyType), Required: HasExplicitRequiredArity(schema, property.Name), Description: property.GetCustomAttribute()?.Description, @@ -419,13 +563,22 @@ private static ReplDocOption BuildDocumentationOptionFromProperty( ReverseAliases: reverseAliases, ValueAliases: valueAliases, EnumValues: enumValues, - DefaultValue: defaultValue); + DefaultValue: defaultValue) + { + IsHidden = schema.IsOptionHidden(property.Name), + IsAutomationHidden = schema.IsOptionAutomationHidden(property.Name), + }; } - private static ReplDocOption BuildDocumentationOption(OptionSchema schema, ParameterInfo parameter) + private ReplDocOption BuildDocumentationOption( + OptionSchema schema, + ParameterInfo parameter, + Dictionary serviceAvailability, + IReadOnlyDictionary customGlobalOwnership) { - var entries = schema.Entries - .Where(entry => string.Equals(entry.ParameterName, parameter.Name, StringComparison.OrdinalIgnoreCase)) + var displayToken = schema.ResolveDisplayToken(parameter.Name!); + var entries = schema.ResolveDiscoverableAliases(parameter.Name!) + .Where(entry => !customGlobalOwnership.ContainsKey(entry.Token)) .ToArray(); var aliases = entries .Where(entry => entry.TokenKind is OptionSchemaTokenKind.NamedOption or OptionSchemaTokenKind.BoolFlag) @@ -451,15 +604,19 @@ private static ReplDocOption BuildDocumentationOption(OptionSchema schema, Param ? parameter.DefaultValue.ToString() : null; return new ReplDocOption( - Name: aliases.Length > 0 ? aliases[0].TrimStart('-') : parameter.Name!, + Name: displayToken?.TrimStart('-') ?? parameter.Name!, Type: GetFriendlyTypeName(parameter.ParameterType), - Required: IsRequiredParameter(parameter) || HasExplicitRequiredArity(schema, parameter.Name!), + Required: IsRequiredOption(schema, parameter.Name!, serviceAvailability), Description: parameter.GetCustomAttribute()?.Description, Aliases: aliases, ReverseAliases: reverseAliases, ValueAliases: valueAliases, EnumValues: enumValues, - DefaultValue: defaultValue); + DefaultValue: defaultValue) + { + IsHidden = schema.IsOptionHidden(parameter.Name!), + IsAutomationHidden = schema.IsOptionAutomationHidden(parameter.Name!), + }; } [UnconditionalSuppressMessage( diff --git a/src/Repl.Core/Documentation/ReplDocOption.cs b/src/Repl.Core/Documentation/ReplDocOption.cs index e1508117..4c70ccfe 100644 --- a/src/Repl.Core/Documentation/ReplDocOption.cs +++ b/src/Repl.Core/Documentation/ReplDocOption.cs @@ -12,4 +12,31 @@ public sealed record ReplDocOption( IReadOnlyList ReverseAliases, IReadOnlyList ValueAliases, IReadOnlyList EnumValues, - string? DefaultValue); + string? DefaultValue) +{ + // Declared in the record body rather than as a positional parameter: this record is public and + // its nine positional parameters have no defaults, so adding a tenth would change the + // constructor signature and the Deconstruct arity for every already-compiled consumer. + + /// + /// Gets a value indicating whether the option is hidden from discovery surfaces. + /// + /// + /// Mirrors : an aggregate model omits hidden options + /// entirely, while a model built for an explicitly targeted command includes them with this + /// flag set, so an app author can still inventory them. Hidden options remain parsable from the + /// command line and the REPL; consumers that generate agent-facing schemas must omit them. + /// + public bool IsHidden { get; init; } + + /// + /// Gets a value indicating whether the option is suppressed on programmatic surfaces only. + /// + /// + /// Unlike , an automation-hidden option is always present in this model — + /// human-facing exports keep it, which is why the flag exists rather than an omission. + /// Consumers that generate agent-facing schemas must omit it. Not an access-control boundary. + /// + public bool IsAutomationHidden { get; init; } + +} diff --git a/src/Repl.Core/GlobalOptionBuilder.cs b/src/Repl.Core/GlobalOptionBuilder.cs new file mode 100644 index 00000000..9806d947 --- /dev/null +++ b/src/Repl.Core/GlobalOptionBuilder.cs @@ -0,0 +1,53 @@ +namespace Repl; + +/// +/// Configures discovery metadata for a registered global option. +/// +/// +/// Deliberately separate from rather than sharing it. Global options are +/// consumed before routing and never enter the documentation model, so they cannot reach an MCP tool +/// schema — an automation-visibility knob here could never do anything. Splitting the type makes that +/// unrepresentable instead of offering a method that silently does nothing. +/// +public sealed class GlobalOptionBuilder +{ + private readonly ParsingOptions _owner; + private readonly string _canonicalName; + + internal GlobalOptionBuilder(ParsingOptions owner, string canonicalName) + { + _owner = owner; + _canonicalName = canonicalName; + } + + /// + /// Hides or shows one registered alias while leaving the global option's canonical token visible. + /// Parsing and binding continue to accept the alias. + /// + /// A registered alias token, with or without its long-option prefix. + /// Whether the alias is hidden from discovery. + /// The same builder instance. + public GlobalOptionBuilder HiddenAlias(string alias, bool isHidden = true) + { + ArgumentException.ThrowIfNullOrWhiteSpace(alias); + _owner.SetGlobalOptionAliasHidden(_canonicalName, alias, isHidden); + return this; + } + + /// + /// Hides or shows the global option on discovery surfaces without changing parsing or binding. + /// + /// + /// The option is omitted from help and from interactive and shell completion, but remains a valid + /// parser input and binds normally when supplied explicitly. This is a discovery filter, not an + /// access-control boundary: the token stays an invocable part of the command line. + /// + /// Whether the option is hidden. + /// The same builder instance. + public GlobalOptionBuilder Hidden(bool isHidden = true) + { + _owner.SetGlobalOptionHidden(_canonicalName, isHidden); + + return this; + } +} diff --git a/src/Repl.Core/Help/HelpTextBuilder.Rendering.cs b/src/Repl.Core/Help/HelpTextBuilder.Rendering.cs index caa47fdf..ceba7e3a 100644 --- a/src/Repl.Core/Help/HelpTextBuilder.Rendering.cs +++ b/src/Repl.Core/Help/HelpTextBuilder.Rendering.cs @@ -15,11 +15,15 @@ internal static partial class HelpTextBuilder new($"{ReplResultFlowOptionNames.Pager}=auto|off|more|inline|full", "Control the integrated pager for human output."), ]; - private static string BuildCommandHelp(RouteDefinition[] routes, bool useAnsi, AnsiPalette palette) + private static string BuildCommandHelp( + RouteDefinition[] routes, + IReadOnlyDictionary customGlobalOwnership, + bool useAnsi, + AnsiPalette palette) { if (routes.Length == 1) { - return BuildSingleCommandHelp(routes[0], useAnsi, palette); + return BuildSingleCommandHelp(routes[0], customGlobalOwnership, useAnsi, palette); } var rows = routes @@ -46,7 +50,11 @@ private static string BuildCommandHelp(RouteDefinition[] routes, bool useAnsi, A return $"{header}{Environment.NewLine}{table}"; } - private static string BuildSingleCommandHelp(RouteDefinition route, bool useAnsi, AnsiPalette palette) + private static string BuildSingleCommandHelp( + RouteDefinition route, + IReadOnlyDictionary customGlobalOwnership, + bool useAnsi, + AnsiPalette palette) { var displayTemplate = FormatRouteTemplate(route.Template); var description = route.Command.Description ?? "No description."; @@ -54,7 +62,7 @@ private static string BuildSingleCommandHelp(RouteDefinition route, bool useAnsi ? string.Empty : $"{Environment.NewLine}Aliases: {string.Join(", ", route.Command.Aliases)}"; var argumentSection = BuildArgumentSection(route, useAnsi, palette); - var optionSection = BuildOptionSection(route, useAnsi, palette); + var optionSection = BuildOptionSection(route, customGlobalOwnership, useAnsi, palette); var resultFlowSection = BuildResultFlowSection(route, useAnsi, palette); var answerSection = BuildAnswerSection(route, useAnsi, palette); if (!useAnsi) @@ -141,9 +149,13 @@ private static string BuildResultFlowSection(RouteDefinition route, bool useAnsi return builder.ToString(); } - private static string BuildOptionSection(RouteDefinition route, bool useAnsi, AnsiPalette palette) + private static string BuildOptionSection( + RouteDefinition route, + IReadOnlyDictionary customGlobalOwnership, + bool useAnsi, + AnsiPalette palette) { - var optionRows = BuildOptionRows(route); + var optionRows = BuildOptionRows(route, customGlobalOwnership); if (optionRows.Length == 0) { return string.Empty; @@ -169,11 +181,13 @@ private static string BuildOptionSection(RouteDefinition route, bool useAnsi, An OptionSchema schema, OptionSchemaParameter schemaParameter, Dictionary parameters, + IReadOnlyDictionary customGlobalOwnership, Dictionary? groupProperties = null) { - var entries = schema.Entries + var entries = schema.DiscoverableEntries .Where(entry => string.Equals(entry.ParameterName, schemaParameter.Name, StringComparison.OrdinalIgnoreCase) + && !customGlobalOwnership.ContainsKey(entry.Token) && entry.TokenKind is OptionSchemaTokenKind.NamedOption or OptionSchemaTokenKind.BoolFlag or OptionSchemaTokenKind.ReverseFlag @@ -260,7 +274,9 @@ private static HelpRenderEntry[] BuildAnswerRows(RouteDefinition route) ]; } - private static HelpRenderEntry[] BuildOptionRows(RouteDefinition route) + private static HelpRenderEntry[] BuildOptionRows( + RouteDefinition route, + IReadOnlyDictionary customGlobalOwnership) { var parameters = route.Command.Handler.Method.GetParameters() .Where(parameter => !string.IsNullOrWhiteSpace(parameter.Name)) @@ -281,9 +297,13 @@ private static HelpRenderEntry[] BuildOptionRows(RouteDefinition route) } } - return route.OptionSchema.Parameters.Values - .Where(parameter => parameter.Mode != ReplParameterMode.ArgumentOnly) - .Select(parameter => BuildOptionRow(route.OptionSchema, parameter, parameters, groupProperties)) + return route.OptionSchema.DiscoverableParameters + .Select(parameter => BuildOptionRow( + route.OptionSchema, + parameter, + parameters, + customGlobalOwnership, + groupProperties)) .Where(row => row is not null) .Select(row => new HelpRenderEntry(row![0], row[1])) .ToArray(); @@ -590,22 +610,30 @@ private static string[][] BuildGlobalCommandRows(AmbientCommandOptions ambientOp private static string[][] BuildGlobalOptionRows(ParsingOptions parsingOptions) { ArgumentNullException.ThrowIfNull(parsingOptions); + + // Visibility is decided per TOKEN, not per definition: GlobalOptionParser gives a colliding + // token to the LAST registration. Build that ownership once, then keep a token only on the + // exact definition that owns it and only when that owner is visible. The canonical token has + // no special weight: a definition whose canonical token was claimed elsewhere may still be + // reachable through an alias nobody took. + var ownership = GlobalOptionParser.BuildCustomTokenOwnership(parsingOptions); var customRows = parsingOptions.GlobalOptions.Values .OrderBy(option => option.Name, StringComparer.OrdinalIgnoreCase) - .Select(option => + .Select(option => new + { + Option = option, + OwnedTokens = option.Aliases + .Prepend(option.CanonicalToken) + .Where(token => GlobalOptionParser.IsGlobalTokenDiscoverable(token, option, ownership, parsingOptions)) + .ToArray(), + }) + .Where(static row => row.OwnedTokens.Length > 0) + .Select(static row => new[] { - var aliases = option.Aliases.Count == 0 - ? string.Empty - : $", {string.Join(", ", option.Aliases)}"; - var description = string.IsNullOrWhiteSpace(option.Description) + string.Join(", ", row.OwnedTokens), + string.IsNullOrWhiteSpace(row.Option.Description) ? "Custom global option." - : option.Description; - - return new[] - { - $"{option.CanonicalToken}{aliases}", - description, - }; + : row.Option.Description, }); return [.. BuiltInGlobalOptionRows.Concat(customRows)]; } diff --git a/src/Repl.Core/Help/HelpTextBuilder.cs b/src/Repl.Core/Help/HelpTextBuilder.cs index a215c5d1..a66a5663 100644 --- a/src/Repl.Core/Help/HelpTextBuilder.cs +++ b/src/Repl.Core/Help/HelpTextBuilder.cs @@ -70,10 +70,11 @@ public static HelpRenderDocument BuildRenderModel( var scope = scopeTokens.Count == 0 ? "root" : string.Join(' ', scopeTokens); if (TryGetCommandHelpRoutes(visibleRoutes, scopeTokens, parsingOptions, out var commandHelpRoutes)) { + var customGlobalOwnership = GlobalOptionParser.BuildCustomTokenOwnership(parsingOptions); return new HelpRenderDocument( scope, IsCommandHelp: true, - Commands: commandHelpRoutes.Select(CreateRenderCommand).ToArray(), + Commands: commandHelpRoutes.Select(route => CreateRenderCommand(route, customGlobalOwnership)).ToArray(), Scopes: [], GlobalOptions: [], GlobalCommands: []); @@ -155,7 +156,8 @@ public static string Build( var effectiveAmbientOptions = ambientOptions ?? new AmbientCommandOptions(); if (TryGetCommandHelpRoutes(visibleRoutes, scopeTokens, parsingOptions, out var commandHelpRoutes)) { - return BuildCommandHelp(commandHelpRoutes, useAnsi, effectivePalette); + var customGlobalOwnership = GlobalOptionParser.BuildCustomTokenOwnership(parsingOptions); + return BuildCommandHelp(commandHelpRoutes, customGlobalOwnership, useAnsi, effectivePalette); } var matchingRoutes = visibleRoutes @@ -258,7 +260,9 @@ private static HelpCommandModel CreateCommandModel(RouteDefinition route) Aliases: route.Command.Aliases.ToArray()); } - private static HelpRenderCommand CreateRenderCommand(RouteDefinition route) + private static HelpRenderCommand CreateRenderCommand( + RouteDefinition route, + IReadOnlyDictionary customGlobalOwnership) { var displayTemplate = FormatRouteTemplate(route.Template); return new HelpRenderCommand( @@ -267,7 +271,7 @@ private static HelpRenderCommand CreateRenderCommand(RouteDefinition route) Usage: displayTemplate, Aliases: route.Command.Aliases.ToArray(), Arguments: BuildArgumentRows(route), - Options: BuildOptionRows(route), + Options: BuildOptionRows(route, customGlobalOwnership), ResultFlow: UsesResultFlow(route) ? ResultFlowRows : [], Answers: BuildAnswerRows(route)); } diff --git a/src/Repl.Core/Interaction/InteractionProgressFactory.cs b/src/Repl.Core/Interaction/InteractionProgressFactory.cs index 35cf94c8..70c17757 100644 --- a/src/Repl.Core/Interaction/InteractionProgressFactory.cs +++ b/src/Repl.Core/Interaction/InteractionProgressFactory.cs @@ -12,6 +12,12 @@ public static bool TryCreate( ArgumentNullException.ThrowIfNull(parameterType); ArgumentNullException.ThrowIfNull(context); + if (!IsSupportedProgressType(parameterType)) + { + progress = null; + return false; + } + var channel = context.ServiceProvider.GetService(typeof(IReplInteractionChannel)) as IReplInteractionChannel; if (channel is null) { @@ -35,6 +41,13 @@ public static bool TryCreate( return false; } + internal static bool IsSupportedProgressType(Type parameterType) + { + ArgumentNullException.ThrowIfNull(parameterType); + return parameterType == typeof(IProgress) + || parameterType == typeof(IProgress); + } + private sealed class PercentageProgress( IReplInteractionChannel channel, InteractionOptions options, diff --git a/src/Repl.Core/Internal/Options/HiddenRequiredOptionException.cs b/src/Repl.Core/Internal/Options/HiddenRequiredOptionException.cs new file mode 100644 index 00000000..5efb3327 --- /dev/null +++ b/src/Repl.Core/Internal/Options/HiddenRequiredOptionException.cs @@ -0,0 +1,19 @@ +namespace Repl.Internal.Options; + +/// +/// Raised when discovery would hide an option that the active invocation contract cannot omit. +/// +internal sealed class HiddenRequiredOptionException : InvalidOperationException +{ + internal HiddenRequiredOptionException(string parameterName, string? renderedToken, string route) + : base(BuildMessage(parameterName, renderedToken, route)) + { + } + + private static string BuildMessage(string parameterName, string? renderedToken, string route) + { + var rendered = renderedToken is null ? string.Empty : $" (rendered as '{renderedToken}')"; + return $"Option target '{parameterName}'{rendered} for command '{route}' cannot be hidden because it is required. " + + "Either drop the Required/Arity constraint on the option, or hide a different one."; + } +} diff --git a/src/Repl.Core/Internal/Options/OptionSchema.cs b/src/Repl.Core/Internal/Options/OptionSchema.cs index 5f88fdce..d5362530 100644 --- a/src/Repl.Core/Internal/Options/OptionSchema.cs +++ b/src/Repl.Core/Internal/Options/OptionSchema.cs @@ -3,16 +3,31 @@ namespace Repl.Internal.Options; internal sealed class OptionSchema { public static OptionSchema Empty { get; } = - new([], new Dictionary(StringComparer.OrdinalIgnoreCase)); + new( + [], + new Dictionary(StringComparer.OrdinalIgnoreCase), + ReplCaseSensitivity.CaseSensitive); public OptionSchema( IReadOnlyList entries, - IReadOnlyDictionary parameters) + IReadOnlyDictionary parameters, + ReplCaseSensitivity globalCaseSensitivity) + : this(entries, parameters, () => globalCaseSensitivity) + { + } + + public OptionSchema( + IReadOnlyList entries, + IReadOnlyDictionary parameters, + Func resolveGlobalCaseSensitivity) { Entries = entries; Parameters = parameters; + _resolveGlobalCaseSensitivity = resolveGlobalCaseSensitivity; } + private readonly Func _resolveGlobalCaseSensitivity; + public IReadOnlyList Entries { get; } public IReadOnlyDictionary Parameters { get; } @@ -28,6 +43,255 @@ public OptionSchema( public IReadOnlyCollection KnownTokens => _knownTokens ??= [.. Entries.Select(entry => entry.Token).Distinct(StringComparer.Ordinal)]; + // Same lazy-materialization contract as KnownTokens: the schema is immutable, every + // discovery surface reads these per keystroke, and a concurrent first read recomputes + // the same result. A visibility change does not mutate a schema — it publishes a new + // one (see WithParameter) — so these caches can never go stale. + private OptionSchemaParameter[]? _discoverableParameters; + private DiscoveryProjection? _sensitiveDiscovery; + private DiscoveryProjection? _insensitiveDiscovery; + + /// + /// Parameters that discovery surfaces may advertise: option-bearing and not hidden. + /// This is the single projection help, documentation and completion consume, so a new + /// surface cannot forget to filter. + /// + public IReadOnlyList DiscoverableParameters => + _discoverableParameters ??= + [ + .. Parameters.Values.Where(parameter => + parameter.Mode != ReplParameterMode.ArgumentOnly && !parameter.IsHidden), + ]; + + /// + /// Entries whose owning parameter is discoverable. Entries outnumber parameters (aliases, + /// value aliases, negated flags), so filtering here keeps every token of a hidden option + /// out of completion, not just its canonical form. + /// + public IReadOnlyList DiscoverableEntries => + ResolveDiscoveryProjection().DiscoverableEntries; + + /// + /// minus the tokens of hidden options. + /// + /// + /// Suggestions must read this, never : proposing "did you mean + /// '--secret'?" turns a validation error into a way to enumerate hidden options by probing at + /// small edit distance. Parsing keeps the full set, so a hidden option still binds when supplied. + /// + public IReadOnlyCollection DiscoverableTokens => + ResolveDiscoveryProjection().DiscoverableTokens; + + /// + /// Whether the named parameter is hidden from discovery. Unknown names are not hidden: + /// callers resolve tokens that may belong to route segments rather than options. + /// + public bool IsOptionHidden(string parameterName) => + Parameters.TryGetValue(parameterName, out var parameter) && parameter.IsHidden; + + /// + /// Whether the named parameter is hidden from programmatic surfaces only. Independent of + /// : an option can be withheld from agents while staying in help. + /// + public bool IsOptionAutomationHidden(string parameterName) => + Parameters.TryGetValue(parameterName, out var parameter) && parameter.IsAutomationHidden; + + /// + /// Returns a schema with replacing its same-named entry. + /// + /// + /// The list is reused verbatim, which is what makes a post-registration + /// visibility change safe: tokens, arities and case sensitivity all live on the entries, so + /// parsing and binding observe an identical schema. Only the derived discovery projections + /// differ, and they are recomputed lazily on the new instance. + /// + public OptionSchema WithParameter(OptionSchemaParameter parameter) + { + var parameters = new Dictionary(Parameters, StringComparer.OrdinalIgnoreCase) + { + [parameter.Name] = parameter, + }; + + return new OptionSchema(Entries, parameters, _resolveGlobalCaseSensitivity); + } + + public OptionSchema WithAliasVisibility( + string parameterName, + string alias, + bool isHidden, + ReplCaseSensitivity? currentGlobalCaseSensitivity = null) + { + var canonicalEntry = FindNamedEntry(parameterName); + if (canonicalEntry is not null + && TokensAreEquivalent(canonicalEntry, alias, currentGlobalCaseSensitivity)) + { + throw new ArgumentException( + $"Token '{alias}' is the canonical token for option target '{parameterName}', not an alias.", + nameof(alias)); + } + + // Explicit loop, not a Select with a captured `found` flag: a projection that relies on a + // side effect only works because ToArray happens to enumerate eagerly, and breaks silently + // if that materialization is ever removed. + var found = false; + var changed = false; + var entries = new OptionSchemaEntry[Entries.Count]; + for (var i = 0; i < Entries.Count; i++) + { + var entry = Entries[i]; + if (string.Equals(entry.ParameterName, parameterName, StringComparison.OrdinalIgnoreCase) + && TokensAreEquivalent(entry, alias, currentGlobalCaseSensitivity)) + { + found = true; + // Accumulate with OR, not a plain assignment: multiple parser-equivalent aliases + // (e.g. --ACCOUNT and --account under case-insensitive mode) can match the same + // request. An earlier entry's real change must not be discarded just because a + // later equivalent entry happens to already be in the requested state. + var entryChanged = entry.IsHidden != isHidden; + changed |= entryChanged; + entries[i] = entryChanged ? entry with { IsHidden = isHidden } : entry; + } + else + { + entries[i] = entry; + } + } + + if (!found) + { + throw new KeyNotFoundException( + $"No alias token '{alias}' is registered for option target '{parameterName}'."); + } + + // A no-op call (the alias is already in the requested state) returns this same instance + // rather than an equivalent copy, so the caller's CAS loop can recognize nothing changed + // and skip publishing a schema and invalidating routing over it. + return changed ? new OptionSchema(entries, Parameters, _resolveGlobalCaseSensitivity) : this; + } + + private bool TokensAreEquivalent( + OptionSchemaEntry entry, + string token, + ReplCaseSensitivity? currentGlobalCaseSensitivity) + { + var effectiveCaseSensitivity = entry.CaseSensitivity + ?? currentGlobalCaseSensitivity + ?? _resolveGlobalCaseSensitivity(); + var comparison = effectiveCaseSensitivity == ReplCaseSensitivity.CaseInsensitive + ? StringComparison.OrdinalIgnoreCase + : StringComparison.Ordinal; + return string.Equals(entry.Token, token, comparison); + } + + private DiscoveryProjection ResolveDiscoveryProjection() + { + var globalCaseSensitivity = _resolveGlobalCaseSensitivity(); + return globalCaseSensitivity == ReplCaseSensitivity.CaseInsensitive + ? _insensitiveDiscovery ??= BuildDiscoveryProjection(globalCaseSensitivity) + : _sensitiveDiscovery ??= BuildDiscoveryProjection(globalCaseSensitivity); + } + + private DiscoveryProjection BuildDiscoveryProjection(ReplCaseSensitivity globalCaseSensitivity) + { + var exactHidden = new HashSet(AliasVisibilityKeyComparer.Ordinal); + var insensitiveHidden = new HashSet(AliasVisibilityKeyComparer.OrdinalIgnoreCase); + foreach (var hidden in Entries.Where(static entry => entry.IsHidden)) + { + var key = AliasVisibilityKey.From(hidden); + var effectiveCaseSensitivity = hidden.CaseSensitivity ?? globalCaseSensitivity; + _ = effectiveCaseSensitivity == ReplCaseSensitivity.CaseInsensitive + ? insensitiveHidden.Add(key) + : exactHidden.Add(key); + } + + var canonicalEntries = ResolveNamedEntries().Values.ToHashSet(); + var visibleAliases = new HashSet(); + var visibleAliasesByParameter = new Dictionary>(StringComparer.OrdinalIgnoreCase); + var discoverableEntries = new List(Entries.Count); + foreach (var entry in Entries) + { + var key = AliasVisibilityKey.From(entry); + var aliasIsVisible = !entry.IsHidden + && (canonicalEntries.Contains(entry) + || (!exactHidden.Contains(key) && !insensitiveHidden.Contains(key))); + if (!aliasIsVisible) + { + continue; + } + + visibleAliases.Add(entry); + if (!visibleAliasesByParameter.TryGetValue(entry.ParameterName, out var parameterAliases)) + { + parameterAliases = []; + visibleAliasesByParameter.Add(entry.ParameterName, parameterAliases); + } + parameterAliases.Add(entry); + if (!IsOptionHidden(entry.ParameterName)) + { + discoverableEntries.Add(entry); + } + } + + var entries = discoverableEntries.ToArray(); + return new DiscoveryProjection( + entries, + [.. entries.Select(static entry => entry.Token).Distinct(StringComparer.Ordinal)], + visibleAliases, + entries.ToHashSet(), + visibleAliasesByParameter.ToDictionary( + static pair => pair.Key, + static pair => (IReadOnlyList)pair.Value.ToArray(), + StringComparer.OrdinalIgnoreCase)); + } + + internal IReadOnlyList ResolveDiscoverableAliases(string parameterName) => + ResolveDiscoveryProjection().VisibleAliasesByParameter.GetValueOrDefault(parameterName) ?? []; + + internal bool IsEntryDiscoverable(OptionSchemaEntry entry) => + ResolveDiscoveryProjection().DiscoverableEntrySet.Contains(entry); + + internal bool IsAliasDiscoverable(OptionSchemaEntry entry) => + ResolveDiscoveryProjection().VisibleAliasSet.Contains(entry); + + private sealed record DiscoveryProjection( + OptionSchemaEntry[] DiscoverableEntries, + string[] DiscoverableTokens, + HashSet VisibleAliasSet, + HashSet DiscoverableEntrySet, + IReadOnlyDictionary> VisibleAliasesByParameter); + + private readonly record struct AliasVisibilityKey( + string ParameterName, + OptionSchemaTokenKind TokenKind, + string? InjectedValue, + string Token) + { + public static AliasVisibilityKey From(OptionSchemaEntry entry) => + new(entry.ParameterName, entry.TokenKind, entry.InjectedValue, entry.Token); + } + + private sealed class AliasVisibilityKeyComparer(StringComparer tokenComparer) : IEqualityComparer + { + public static AliasVisibilityKeyComparer Ordinal { get; } = new(StringComparer.Ordinal); + public static AliasVisibilityKeyComparer OrdinalIgnoreCase { get; } = new(StringComparer.OrdinalIgnoreCase); + + public bool Equals(AliasVisibilityKey left, AliasVisibilityKey right) => + left.TokenKind == right.TokenKind + && string.Equals(left.ParameterName, right.ParameterName, StringComparison.OrdinalIgnoreCase) + && string.Equals(left.InjectedValue, right.InjectedValue, StringComparison.Ordinal) + && tokenComparer.Equals(left.Token, right.Token); + + public int GetHashCode(AliasVisibilityKey value) + { + var hash = new HashCode(); + hash.Add(value.ParameterName, StringComparer.OrdinalIgnoreCase); + hash.Add(value.TokenKind); + hash.Add(value.InjectedValue, StringComparer.Ordinal); + hash.Add(value.Token, tokenComparer); + return hash.ToHashCode(); + } + } + public IReadOnlyList ResolveToken(string token, ReplCaseSensitivity globalCaseSensitivity) { if (string.IsNullOrWhiteSpace(token)) @@ -68,17 +332,27 @@ public ReplArity ResolveParameterArity(string parameterName) => public string? ResolveDisplayToken(string parameterName) => FindNamedEntry(parameterName)?.Token; - private OptionSchemaEntry? FindNamedEntry(string parameterName) + private Dictionary? _namedEntries; + + private OptionSchemaEntry? FindNamedEntry(string parameterName) => + ResolveNamedEntries().GetValueOrDefault(parameterName); + + private Dictionary ResolveNamedEntries() { + if (_namedEntries is { } cached) + { + return cached; + } + + var entries = new Dictionary(StringComparer.OrdinalIgnoreCase); foreach (var entry in Entries) { - if (string.Equals(entry.ParameterName, parameterName, StringComparison.OrdinalIgnoreCase) - && entry.TokenKind is OptionSchemaTokenKind.NamedOption or OptionSchemaTokenKind.BoolFlag) + if (entry.TokenKind is OptionSchemaTokenKind.NamedOption or OptionSchemaTokenKind.BoolFlag) { - return entry; + entries.TryAdd(entry.ParameterName, entry); } } - return null; + return _namedEntries = entries; } } diff --git a/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs b/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs index d1e69bde..6d0985a3 100644 --- a/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs +++ b/src/Repl.Core/Internal/Options/OptionSchemaBuilder.cs @@ -10,7 +10,8 @@ public static OptionSchema Build( RouteTemplate template, CommandBuilder command, ParsingOptions parsingOptions, - ImplicitServiceParameterRegistry implicitServiceParameters) + ImplicitServiceParameterRegistry implicitServiceParameters, + Func? resolveGlobalCaseSensitivity = null) { ArgumentNullException.ThrowIfNull(template); ArgumentNullException.ThrowIfNull(command); @@ -39,12 +40,13 @@ public static OptionSchema Build( parameter.ParameterType, entries, parameters, - groupPositionalPropertyNames); + groupPositionalPropertyNames, + parsingOptions); } #pragma warning restore IL2072 else { - AppendParameterSchemaEntries(parameter, entries, parameters); + AppendParameterSchemaEntries(parameter, entries, parameters, parsingOptions); if (IsExplicitPositionalParameter(parameter)) { regularPositionalParameterNames.Add(parameter.Name!); @@ -54,7 +56,10 @@ public static OptionSchema Build( ValidatePositionalBindingCompatibility(regularPositionalParameterNames, groupPositionalPropertyNames); ValidateTokenCollisions(entries, parsingOptions, template); - return new OptionSchema(entries, parameters); + return new OptionSchema( + entries, + parameters, + resolveGlobalCaseSensitivity ?? (() => parsingOptions.OptionCaseSensitivity)); } private static bool ShouldSkipSchemaParameter( @@ -105,7 +110,8 @@ private static bool ShouldSkipSchemaParameter( private static void AppendParameterSchemaEntries( ParameterInfo parameter, List entries, - Dictionary parameters) + Dictionary parameters, + ParsingOptions parsingOptions) { var optionAttribute = parameter.GetCustomAttribute(inherit: true); var mode = ResolveParameterMode(parameter); @@ -120,7 +126,11 @@ private static void AppendParameterSchemaEntries( parameter.ParameterType, mode, CaseSensitivity: optionAttribute?.CaseSensitivityOverride, - ExplicitArity: optionAttribute?.ArityOverride); + ExplicitArity: optionAttribute?.ArityOverride, + IsHidden: optionAttribute?.Hidden ?? false, + IsAutomationHidden: optionAttribute?.AutomationHidden ?? false, + CanBeOmitted: CanParameterBeOmitted(parameter), + SupportsServiceFallback: true); if (mode == ReplParameterMode.ArgumentOnly) { return; @@ -135,7 +145,8 @@ private static void AppendParameterSchemaEntries( tokenKind, arity, CaseSensitivity: optionAttribute?.CaseSensitivityOverride)); - AppendOptionAliases(parameter, tokenKind, arity, optionAttribute, entries); + AppendOptionAliases(parameter, tokenKind, arity, optionAttribute, parsingOptions, entries); + AppendHiddenOptionAliases(parameter, tokenKind, arity, optionAttribute, entries); AppendReverseAliases(parameter, optionAttribute, entries); AppendValueAliases(parameter, optionAttribute, entries); AppendEnumAliases(parameter, optionAttribute, entries); @@ -159,10 +170,18 @@ private static void AppendOptionAliases( OptionSchemaTokenKind tokenKind, ReplArity arity, ReplOptionAttribute? optionAttribute, + ParsingOptions parsingOptions, List entries) { foreach (var alias in optionAttribute?.Aliases ?? []) { + // Preserve case-distinct aliases in the parse schema so inherited visibility can be + // reevaluated if the global case mode changes after mapping. + if ((optionAttribute?.HiddenAliases ?? []).Contains(alias, StringComparer.Ordinal)) + { + continue; + } + ValidateOptionToken(alias, parameter.Name!); entries.Add(new OptionSchemaEntry( alias, @@ -173,6 +192,26 @@ private static void AppendOptionAliases( } } + private static void AppendHiddenOptionAliases( + ParameterInfo parameter, + OptionSchemaTokenKind tokenKind, + ReplArity arity, + ReplOptionAttribute? optionAttribute, + List entries) + { + foreach (var alias in optionAttribute?.HiddenAliases ?? []) + { + ValidateOptionToken(alias, parameter.Name!); + entries.Add(new OptionSchemaEntry( + alias, + parameter.Name!, + tokenKind, + arity, + CaseSensitivity: optionAttribute?.CaseSensitivityOverride, + IsHidden: true)); + } + } + private static void AppendReverseAliases( ParameterInfo parameter, ReplOptionAttribute? optionAttribute, @@ -350,7 +389,8 @@ private static void AppendOptionsGroupSchemaEntries( Type groupType, List entries, Dictionary parameters, - List positionalPropertyNames) + List positionalPropertyNames, + ParsingOptions parsingOptions) { ValidateOptionsGroupType(groupType); @@ -367,7 +407,7 @@ private static void AppendOptionsGroupSchemaEntries( $"Nested options groups are not supported. Property '{property.Name}' on '{groupType.Name}' is itself an options group."); } - AppendPropertySchemaEntries(property, entries, parameters, positionalPropertyNames); + AppendPropertySchemaEntries(property, entries, parameters, positionalPropertyNames, parsingOptions); } } @@ -392,11 +432,21 @@ private static void ValidateOptionsGroupType( } } + // Mirrors the requiredness rule the documentation model reports: a value type with no default and + // no nullable wrapper has nothing to bind from when the option is absent, so the binder rejects + // it. Options-group properties always carry the default instance's value and so are never + // mandatory by shape — only an explicit arity can make them so. + private static bool CanParameterBeOmitted(ParameterInfo parameter) => + parameter.HasDefaultValue + || !parameter.ParameterType.IsValueType + || Nullable.GetUnderlyingType(parameter.ParameterType) is not null; + private static void AppendPropertySchemaEntries( PropertyInfo property, List entries, Dictionary parameters, - List positionalPropertyNames) + List positionalPropertyNames, + ParsingOptions parsingOptions) { var optionAttribute = property.GetCustomAttribute(inherit: true); var argumentAttribute = property.GetCustomAttribute(inherit: true); @@ -414,7 +464,9 @@ private static void AppendPropertySchemaEntries( property.PropertyType, mode, CaseSensitivity: optionAttribute?.CaseSensitivityOverride, - ExplicitArity: optionAttribute?.ArityOverride); + ExplicitArity: optionAttribute?.ArityOverride, + IsHidden: optionAttribute?.Hidden ?? false, + IsAutomationHidden: optionAttribute?.AutomationHidden ?? false); if (mode != ReplParameterMode.OptionOnly) { positionalPropertyNames.Add(property.Name); @@ -433,7 +485,8 @@ private static void AppendPropertySchemaEntries( tokenKind, arity, CaseSensitivity: optionAttribute?.CaseSensitivityOverride)); - AppendPropertyOptionAliases(property.Name, tokenKind, arity, optionAttribute, entries); + AppendPropertyOptionAliases(property.Name, tokenKind, arity, optionAttribute, parsingOptions, entries); + AppendPropertyHiddenOptionAliases(property.Name, tokenKind, arity, optionAttribute, entries); AppendPropertyReverseAliases(property.Name, optionAttribute, entries); AppendPropertyValueAliases(property, optionAttribute, entries); AppendPropertyEnumAliases(property, optionAttribute, entries); @@ -464,10 +517,18 @@ private static void AppendPropertyOptionAliases( OptionSchemaTokenKind tokenKind, ReplArity arity, ReplOptionAttribute? optionAttribute, + ParsingOptions parsingOptions, List entries) { foreach (var alias in optionAttribute?.Aliases ?? []) { + // Preserve case-distinct aliases in the parse schema so inherited visibility can be + // reevaluated if the global case mode changes after mapping. + if ((optionAttribute?.HiddenAliases ?? []).Contains(alias, StringComparer.Ordinal)) + { + continue; + } + ValidateOptionToken(alias, propertyName); entries.Add(new OptionSchemaEntry( alias, @@ -478,6 +539,26 @@ private static void AppendPropertyOptionAliases( } } + private static void AppendPropertyHiddenOptionAliases( + string propertyName, + OptionSchemaTokenKind tokenKind, + ReplArity arity, + ReplOptionAttribute? optionAttribute, + List entries) + { + foreach (var alias in optionAttribute?.HiddenAliases ?? []) + { + ValidateOptionToken(alias, propertyName); + entries.Add(new OptionSchemaEntry( + alias, + propertyName, + tokenKind, + arity, + CaseSensitivity: optionAttribute?.CaseSensitivityOverride, + IsHidden: true)); + } + } + private static void AppendPropertyReverseAliases( string propertyName, ReplOptionAttribute? optionAttribute, diff --git a/src/Repl.Core/Internal/Options/OptionSchemaEntry.cs b/src/Repl.Core/Internal/Options/OptionSchemaEntry.cs index 62e69b70..2029e7b6 100644 --- a/src/Repl.Core/Internal/Options/OptionSchemaEntry.cs +++ b/src/Repl.Core/Internal/Options/OptionSchemaEntry.cs @@ -8,4 +8,5 @@ internal sealed record OptionSchemaEntry( OptionSchemaTokenKind TokenKind, ReplArity Arity, ReplCaseSensitivity? CaseSensitivity = null, - string? InjectedValue = null); + string? InjectedValue = null, + bool IsHidden = false); diff --git a/src/Repl.Core/Internal/Options/OptionSchemaParameter.cs b/src/Repl.Core/Internal/Options/OptionSchemaParameter.cs index e506c84d..fd3caf68 100644 --- a/src/Repl.Core/Internal/Options/OptionSchemaParameter.cs +++ b/src/Repl.Core/Internal/Options/OptionSchemaParameter.cs @@ -7,4 +7,13 @@ internal sealed record OptionSchemaParameter( Type ParameterType, ReplParameterMode Mode, ReplCaseSensitivity? CaseSensitivity = null, - ReplArity? ExplicitArity = null); + ReplArity? ExplicitArity = null, + bool IsHidden = false, + bool IsAutomationHidden = false, + // Whether a caller may leave the option out entirely. Distinct from Arity, which counts the + // values a token consumes: a bool flag is ZeroOrOne yet a non-nullable bool with no default + // still fails to bind when absent, so arity alone cannot answer "is this option mandatory". + bool CanBeOmitted = true, + // Direct handler parameters may fall back to DI after named binding; options-group properties + // never do, because the binder constructs the group before reaching its service fallback. + bool SupportsServiceFallback = false); diff --git a/src/Repl.Core/Internal/Options/OptionTokenCompletionSource.cs b/src/Repl.Core/Internal/Options/OptionTokenCompletionSource.cs index e703c159..ce97c387 100644 --- a/src/Repl.Core/Internal/Options/OptionTokenCompletionSource.cs +++ b/src/Repl.Core/Internal/Options/OptionTokenCompletionSource.cs @@ -40,7 +40,8 @@ internal static class OptionTokenCompletionSource /// static framework options, output-format aliases, --output:<format> /// selectors, and custom global options with their aliases. /// - internal static void CollectGlobalOptionTokens( + /// The custom global token ownership used to filter lower-precedence route candidates. + internal static IReadOnlyDictionary CollectGlobalOptionTokens( ReplOptions options, string currentTokenPrefix, StringComparison comparison, @@ -72,39 +73,66 @@ internal static void CollectGlobalOptionTokens( TryAddComposed("--output:", format, currentTokenPrefix, StringComparison.OrdinalIgnoreCase, dedupe, results); } + // Visibility is decided per TOKEN, not per definition: GlobalOptionParser gives a colliding + // token to the LAST registration, so a token owned by a visible definition here may in fact + // bind a hidden one. Build that authoritative ownership map once for this completion pass. + var ownership = GlobalOptionParser.BuildCustomTokenOwnership(options.Parsing); foreach (var custom in options.Parsing.GlobalOptions.Values) { - TryAdd(custom.CanonicalToken, currentTokenPrefix, comparison, dedupe, results); + TryAddGlobalToken(custom.CanonicalToken, custom, ownership, options.Parsing, currentTokenPrefix, comparison, dedupe, results); foreach (var alias in custom.Aliases) { - TryAdd(alias, currentTokenPrefix, comparison, dedupe, results); + TryAddGlobalToken(alias, custom, ownership, options.Parsing, currentTokenPrefix, comparison, dedupe, results); } } + + return ownership; } - /// Collects the route's declared option tokens matching the prefix. - internal static void CollectRouteOptionTokens( - RouteDefinition route, + private static void TryAddGlobalToken( + string token, + GlobalOptionDefinition expectedOwner, + IReadOnlyDictionary ownership, + ParsingOptions parsingOptions, string currentTokenPrefix, - ReplCaseSensitivity globalCaseSensitivity, + StringComparison comparison, HashSet dedupe, - List results) => - CollectRouteOptionTokens(route.OptionSchema, currentTokenPrefix, globalCaseSensitivity, dedupe, results); + List results) + { + if (!GlobalOptionParser.IsGlobalTokenDiscoverable(token, expectedOwner, ownership, parsingOptions)) + { + return; + } + + TryAdd(token, currentTokenPrefix, comparison, dedupe, results); + } + /// Collects the route's discoverable option tokens matching the prefix. // Filters against the schema entries — not the flattened KnownTokens — so each option's // own case sensitivity is honored: an entry declared case-insensitive is offered for a // differently-cased prefix even under a case-sensitive global default (and vice versa), - // matching exactly what the invocation parser accepts. + // matching exactly what the invocation parser accepts. Hidden options are excluded by + // DiscoverableEntries rather than by a caller-supplied predicate: a predicate parameter + // was the very drift vector this single source exists to prevent. internal static void CollectRouteOptionTokens( OptionSchema schema, + IReadOnlyDictionary customGlobalOwnership, string currentTokenPrefix, ReplCaseSensitivity globalCaseSensitivity, HashSet dedupe, List results) { - foreach (var entry in schema.Entries) + foreach (var entry in schema.DiscoverableEntries) { + // GlobalOptionParser consumes custom globals before route parsing. Suppress every + // route token owned by that projection — including hidden globals that contributed no + // candidate to dedupe — so completion never advertises a route binding execution cannot reach. + if (customGlobalOwnership.ContainsKey(entry.Token)) + { + continue; + } + var comparison = (entry.CaseSensitivity ?? globalCaseSensitivity) == ReplCaseSensitivity.CaseInsensitive ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal; diff --git a/src/Repl.Core/OptionBuilder.cs b/src/Repl.Core/OptionBuilder.cs new file mode 100644 index 00000000..3cb8953a --- /dev/null +++ b/src/Repl.Core/OptionBuilder.cs @@ -0,0 +1,80 @@ +namespace Repl; + +/// +/// Configures discovery metadata for an option. +/// +public sealed class OptionBuilder +{ + private readonly Action _setHidden; + private readonly Action _setAutomationHidden; + private readonly Action _setAliasHidden; + + internal OptionBuilder( + Action setHidden, + Action setAutomationHidden, + Action setAliasHidden) + { + _setHidden = setHidden; + _setAutomationHidden = setAutomationHidden; + _setAliasHidden = setAliasHidden; + } + + /// + /// Hides or shows the option on every discovery surface: help, interactive and shell completion + /// including value providers, exported documentation, and generated MCP tool and prompt schemas. + /// + /// + /// Parsing and binding are unaffected, so the option still binds when a command-line or REPL + /// caller supplies it explicitly. An MCP tools/call that supplies it is rejected, because + /// the option is absent from the advertised tool schema. A hidden option must be optional. For an + /// options-group property this fails immediately, here; a direct handler parameter may still be + /// satisfiable from DI or a synthesized progress channel, which is only knowable once a real + /// service provider exists, so that case instead fails the first time discovery runs against one + /// (an explicit documentation request or MCP's aggregate tools/list). This is a discovery + /// filter, not an access-control boundary: the token remains an invocable part of the + /// command line for anyone who knows it. + /// + /// Whether the option is hidden. + /// The same builder instance. + public OptionBuilder Hidden(bool isHidden = true) + { + _setHidden(isHidden); + + return this; + } + + /// + /// Hides or shows one registered alias without changing parsing or the visibility of the + /// canonical option token. + /// + /// A full alias token already declared on the option. + /// Whether the alias is hidden from discovery. + /// The same builder instance. + public OptionBuilder HiddenAlias(string alias, bool isHidden = true) + { + ArgumentException.ThrowIfNullOrWhiteSpace(alias); + _setAliasHidden(alias, isHidden); + return this; + } + + /// + /// Hides or shows the option on programmatic surfaces only. + /// + /// + /// Unlike , which hides from all surfaces, this suppresses only MCP tool + /// input schemas and prompt arguments — and, because the advertised schema and the accepted + /// argument list share one option list, an MCP tools/call that supplies the option is + /// rejected too. The option stays visible in help, in interactive and shell completion, and in + /// exported documentation, and binds normally from the command line and the REPL. Mirrors + /// at the option level. This is a discovery + /// filter, not an access-control boundary. + /// + /// Whether the option is hidden from programmatic surfaces. + /// The same builder instance. + public OptionBuilder AutomationHidden(bool isAutomationHidden = true) + { + _setAutomationHidden(isAutomationHidden); + + return this; + } +} diff --git a/src/Repl.Core/Output/MarkdownOutputTransformer.cs b/src/Repl.Core/Output/MarkdownOutputTransformer.cs index bdba66e0..35037bca 100644 --- a/src/Repl.Core/Output/MarkdownOutputTransformer.cs +++ b/src/Repl.Core/Output/MarkdownOutputTransformer.cs @@ -488,8 +488,31 @@ private static void AppendOptions(StringBuilder builder, IReadOnlyList alias.Token)) + .Distinct(StringComparer.Ordinal) + .ToArray(); + var displayTokens = tokens.Length == 0 + ? $"`--{option.Name}`" + : string.Join(", ", tokens.Select(static token => $"`{token}`")); + builder.Append(" - ").Append(displayTokens).Append(" (") .Append(option.Type).Append(')'); + + // Commands print their own Hidden line, and the structured formats get these flags for + // free by serializing the record. Markdown formats each field by hand, so without this an + // exact-target export would render a hidden option indistinguishably from a public one — + // breaking the promise that targeted exports include hidden options *flagged*. + if (option.IsHidden) + { + builder.Append(" [hidden]"); + } + + if (option.IsAutomationHidden) + { + builder.Append(" [automation-hidden]"); + } + if (!string.IsNullOrWhiteSpace(option.Description)) { builder.Append(" - ").Append(option.Description); diff --git a/src/Repl.Core/Parameters/Attributes/ReplOptionAttribute.cs b/src/Repl.Core/Parameters/Attributes/ReplOptionAttribute.cs index 96858fab..cbf8a060 100644 --- a/src/Repl.Core/Parameters/Attributes/ReplOptionAttribute.cs +++ b/src/Repl.Core/Parameters/Attributes/ReplOptionAttribute.cs @@ -16,6 +16,16 @@ public sealed class ReplOptionAttribute : Attribute /// public string[] Aliases { get; set; } = []; + /// + /// Legacy aliases that remain accepted by parsing and binding but are omitted from every + /// discovery surface. Values are full tokens (for example: --old-mode or -o). + /// + /// + /// Use this for backwards-compatible migrations where stays visible while an + /// old spelling remains callable. This is not an access-control boundary. + /// + public string[] HiddenAliases { get; set; } = []; + /// /// Reverse aliases as full tokens (for example: --no-verbose). /// @@ -26,6 +36,35 @@ public sealed class ReplOptionAttribute : Attribute /// public ReplParameterMode Mode { get; set; } = ReplParameterMode.OptionAndPositional; + /// + /// Gets or sets a value indicating whether the option is hidden from every discovery surface: + /// help, interactive and shell completion including value providers, exported documentation, + /// and generated MCP tool and prompt schemas. + /// + /// + /// Parsing and binding are unaffected, so the option still binds when a command-line or REPL + /// caller supplies it explicitly. An MCP tools/call that supplies it is rejected, because + /// the option is absent from the advertised tool schema. A hidden option must be optional. + /// This is a discovery filter, not an access-control boundary: the token remains an + /// invocable part of the command line for anyone who knows it. + /// + public bool Hidden { get; set; } + + /// + /// Gets or sets a value indicating whether the option is hidden from programmatic surfaces only. + /// + /// + /// Unlike , which hides from all surfaces, this suppresses only MCP tool + /// input schemas and prompt arguments — and, because the advertised schema and the accepted + /// argument list share one option list, an MCP tools/call that supplies the option is + /// rejected too. The option stays visible in help, in interactive and shell completion, and in + /// exported documentation, and binds normally from the command line and the REPL. Mirrors + /// at the option level. Not supported on typed + /// global-options properties, which never reach a programmatic surface. This is a discovery + /// filter, not an access-control boundary. + /// + public bool AutomationHidden { get; set; } + // Nullable enums are not legal attribute named arguments (CS0655), so the optional // overrides expose a non-nullable property and track the unset state in a nullable // backing field surfaced through the read-only *Override properties. diff --git a/src/Repl.Core/Parsing/GlobalInvocationOptions.cs b/src/Repl.Core/Parsing/GlobalInvocationOptions.cs index 4baaaba0..86efd551 100644 --- a/src/Repl.Core/Parsing/GlobalInvocationOptions.cs +++ b/src/Repl.Core/Parsing/GlobalInvocationOptions.cs @@ -21,6 +21,9 @@ internal sealed record GlobalInvocationOptions( public IReadOnlyDictionary> CustomGlobalNamedOptions { get; init; } = new Dictionary>(StringComparer.OrdinalIgnoreCase); + internal IReadOnlyDictionary CustomGlobalTokenOwnership { get; init; } = + new Dictionary(StringComparer.Ordinal); + public IReadOnlyList Diagnostics { get; init; } = []; // The original input index of each surviving token in . diff --git a/src/Repl.Core/Parsing/GlobalOptionDefinition.cs b/src/Repl.Core/Parsing/GlobalOptionDefinition.cs index 6fd93bc0..3b7ed947 100644 --- a/src/Repl.Core/Parsing/GlobalOptionDefinition.cs +++ b/src/Repl.Core/Parsing/GlobalOptionDefinition.cs @@ -7,4 +7,6 @@ internal sealed record GlobalOptionDefinition( string? DefaultValue, string? Description, Type ValueType, - Type? OwnerType); + Type? OwnerType, + bool IsHidden, + IReadOnlyList HiddenAliases); diff --git a/src/Repl.Core/Parsing/GlobalOptionParser.cs b/src/Repl.Core/Parsing/GlobalOptionParser.cs index 65743ca3..78416241 100644 --- a/src/Repl.Core/Parsing/GlobalOptionParser.cs +++ b/src/Repl.Core/Parsing/GlobalOptionParser.cs @@ -4,6 +4,12 @@ namespace Repl; internal static class GlobalOptionParser { + // The overwhelming majority of invocations supply no custom global option — reuse one empty, + // immutable instance instead of allocating a fresh dictionary on every Parse call to project + // nothing into it. + private static readonly IReadOnlyDictionary> EmptyCustomGlobalValues = + new Dictionary>(StringComparer.OrdinalIgnoreCase); + [SuppressMessage( "Maintainability", "MA0051:Method is too long", @@ -25,7 +31,7 @@ public static GlobalInvocationOptions Parse( var promptAnswers = new Dictionary(StringComparer.OrdinalIgnoreCase); var customGlobalValues = new Dictionary>(tokenComparer); var diagnostics = new List(); - var customTokenMap = BuildCustomTokenMap(parsingOptions.GlobalOptions, tokenComparer); + var customTokenMap = BuildCustomTokenOwnership(parsingOptions); var options = new GlobalInvocationOptions(remaining); var optionComparison = parsingOptions.OptionCaseSensitivity == ReplCaseSensitivity.CaseInsensitive ? StringComparison.OrdinalIgnoreCase @@ -94,7 +100,6 @@ public static GlobalInvocationOptions Parse( ref index, argument, customTokenMap, - parsingOptions.GlobalOptions, customGlobalValues)) { continue; @@ -104,14 +109,17 @@ public static GlobalInvocationOptions Parse( remainingIndices.Add(index); } - var readonlyCustomGlobalValues = customGlobalValues.ToDictionary( - pair => pair.Key, - pair => (IReadOnlyList)pair.Value, - StringComparer.OrdinalIgnoreCase); + var readonlyCustomGlobalValues = customGlobalValues.Count == 0 + ? EmptyCustomGlobalValues + : customGlobalValues.ToDictionary( + pair => pair.Key, + pair => (IReadOnlyList)pair.Value, + StringComparer.OrdinalIgnoreCase); return options with { PromptAnswers = promptAnswers, CustomGlobalNamedOptions = readonlyCustomGlobalValues, + CustomGlobalTokenOwnership = customTokenMap, Diagnostics = diagnostics, RemainingTokenIndices = remainingIndices, }; @@ -274,58 +282,49 @@ private static int ClampPageSize(int pageSize, int maxPageSize) => private static void AddResultFlowDiagnostic(List diagnostics, string message) => diagnostics.Add(new ParseDiagnostic(ParseDiagnosticSeverity.Error, message)); - // Resolves a custom-global token to the definition the parser would actually use — the - // LAST registered definition wins a token/alias collision (BuildCustomTokenMap overwrites), - // so callers must not scan definitions independently and pick a different one. - internal static bool TryResolveCustomGlobalDefinition( - string token, - ParsingOptions parsingOptions, - out GlobalOptionDefinition definition) + // The projection itself is cached on ParsingOptions (invalidated by its own mutators), since + // parsing, help, and completion all resolve it — completion on every keystroke. + internal static IReadOnlyDictionary BuildCustomTokenOwnership( + ParsingOptions parsingOptions) { - definition = null!; - var comparer = parsingOptions.OptionCaseSensitivity == ReplCaseSensitivity.CaseInsensitive - ? StringComparer.OrdinalIgnoreCase - : StringComparer.Ordinal; - var tokenMap = BuildCustomTokenMap(parsingOptions.GlobalOptions, comparer); - if (!TryResolveCustomGlobalName(token, tokenMap, out var optionName, out _)) - { - return false; - } - - return parsingOptions.GlobalOptions.TryGetValue(optionName, out definition!); + ArgumentNullException.ThrowIfNull(parsingOptions); + return parsingOptions.ResolveCustomTokenOwnership(); } - private static Dictionary BuildCustomTokenMap( - IReadOnlyDictionary definitions, - StringComparer comparer) - { - var tokenMap = new Dictionary(comparer); - foreach (var definition in definitions.Values) - { - tokenMap[definition.CanonicalToken] = definition.Name; - foreach (var alias in definition.Aliases) - { - tokenMap[alias] = definition.Name; - } - } - - return tokenMap; - } + internal static bool TryResolveCustomGlobalDefinition( + string token, + ParsingOptions parsingOptions, + out GlobalOptionDefinition definition) => + BuildCustomTokenOwnership(parsingOptions).TryGetValue(token, out definition!); + + // Shared by help and completion: a token only belongs to a discoverable surface when the + // ownership projection still resolves it back to the SAME definition (ReferenceEquals — the + // last-registration-wins rule may have reassigned it to a different one) and neither the + // definition nor this specific alias spelling is hidden. + internal static bool IsGlobalTokenDiscoverable( + string token, + GlobalOptionDefinition expectedOwner, + IReadOnlyDictionary ownership, + ParsingOptions parsingOptions) => + ownership.TryGetValue(token, out var actualOwner) + && ReferenceEquals(actualOwner, expectedOwner) + && !actualOwner.IsHidden + && !parsingOptions.IsGlobalOptionAliasHidden(actualOwner, token); private static bool TryParseCustomGlobalOption( IReadOnlyList args, ref int index, string argument, - IReadOnlyDictionary tokenMap, - IReadOnlyDictionary definitions, + IReadOnlyDictionary tokenMap, Dictionary> customGlobalValues) { - if (!TryResolveCustomGlobalName(argument, tokenMap, out var optionName, out var inlineValue)) + if (!TryResolveCustomGlobalDefinition(argument, tokenMap, out var definition, out var inlineValue)) { return false; } - var isBool = definitions.TryGetValue(optionName, out var def) && def.ValueType == typeof(bool); + var optionName = definition.Name; + var isBool = definition.ValueType == typeof(bool); var value = inlineValue; if (value is null && !isBool @@ -346,13 +345,13 @@ private static bool TryParseCustomGlobalOption( return true; } - private static bool TryResolveCustomGlobalName( + private static bool TryResolveCustomGlobalDefinition( string argument, - IReadOnlyDictionary tokenMap, - out string optionName, + IReadOnlyDictionary tokenMap, + out GlobalOptionDefinition definition, out string? inlineValue) { - optionName = string.Empty; + definition = null!; inlineValue = null; if (!argument.StartsWith('-')) { @@ -367,12 +366,7 @@ private static bool TryResolveCustomGlobalName( inlineValue = valuePart; } - if (!tokenMap.TryGetValue(optionToken, out optionName!)) - { - return false; - } - - return true; + return tokenMap.TryGetValue(optionToken, out definition!); } private static bool IsSignedNumericLiteral(string token) => diff --git a/src/Repl.Core/Parsing/InvocationOptionParser.cs b/src/Repl.Core/Parsing/InvocationOptionParser.cs index e8eed28a..1642cdc0 100644 --- a/src/Repl.Core/Parsing/InvocationOptionParser.cs +++ b/src/Repl.Core/Parsing/InvocationOptionParser.cs @@ -127,7 +127,8 @@ public static OptionParsingResult Parse( public static OptionParsingResult Parse( IReadOnlyList tokens, OptionSchema schema, - ParsingOptions options) + ParsingOptions options, + IReadOnlyDictionary? customGlobalOwnership = null) { ArgumentNullException.ThrowIfNull(tokens); ArgumentNullException.ThrowIfNull(schema); @@ -142,6 +143,7 @@ public static OptionParsingResult Parse( : tokens; var namedOptions = new Dictionary>(tokenComparer); var positionalArguments = new List(tokens.Count); + customGlobalOwnership ??= new Dictionary(StringComparer.Ordinal); var parseAsPositional = false; for (var index = 0; index < effectiveTokens.Count; index++) @@ -187,6 +189,7 @@ public static OptionParsingResult Parse( inlineValue, schema, options, + customGlobalOwnership, namedOptions, diagnostics); continue; @@ -210,7 +213,11 @@ public static OptionParsingResult Parse( return new OptionParsingResult(readonlyNamedOptions, positionalArguments, diagnostics); } - private static bool LooksLikeOptionToken(string token) => + // Internal (not private): the MCP adapter reuses this to reject route-segment argument + // values that would be re-lexed as an option token once substituted into the CLI stream, + // since a positional segment has no separator that can escape it the way an inline + // "--token=value" pair does for a named option. + internal static bool LooksLikeOptionToken(string token) => token.Length >= 2 && token[0] == '-'; // Shared with the completion engines: a signed numeric literal (-42) is a positional @@ -270,14 +277,17 @@ private static void HandleUnknownOption( string? inlineValue, OptionSchema schema, ParsingOptions options, + IReadOnlyDictionary customGlobalOwnership, Dictionary> namedOptions, List diagnostics) { if (!options.AllowUnknownOptions) { + // DiscoverableTokens, not KnownTokens: parsing accepts every token, but suggesting one + // would let a caller enumerate hidden options by probing at small edit distance. var suggestion = TryResolveSuggestion( optionToken, - schema.KnownTokens, + [.. schema.DiscoverableTokens.Where(token => !customGlobalOwnership.ContainsKey(token))], options.OptionCaseSensitivity == ReplCaseSensitivity.CaseInsensitive); var message = suggestion is null ? $"Unknown option '{optionToken}'." diff --git a/src/Repl.Core/ParsingOptions.cs b/src/Repl.Core/ParsingOptions.cs index 1b4084b0..c2f610b5 100644 --- a/src/Repl.Core/ParsingOptions.cs +++ b/src/Repl.Core/ParsingOptions.cs @@ -38,6 +38,14 @@ public sealed class ParsingOptions private readonly Dictionary _globalOptions = new(StringComparer.OrdinalIgnoreCase); + // Built once per registration/visibility generation, not per lookup: help renders it twice, + // completion rebuilds it on every keystroke, and it was previously reconstructed — one + // dictionary plus an insert per alias — on every single call. Any mutator that can change + // which token maps to which definition (registration, visibility, or the comparer itself) + // must null this out; a stale cache would silently keep serving a token to its old owner. + private IReadOnlyDictionary? _customTokenOwnershipCache; + private ReplCaseSensitivity _optionCaseSensitivity = ReplCaseSensitivity.CaseSensitive; + /// /// Gets or sets a value indicating whether unknown options are allowed. /// @@ -46,7 +54,15 @@ public sealed class ParsingOptions /// /// Gets or sets option-name case-sensitivity mode. /// - public ReplCaseSensitivity OptionCaseSensitivity { get; set; } = ReplCaseSensitivity.CaseSensitive; + public ReplCaseSensitivity OptionCaseSensitivity + { + get => _optionCaseSensitivity; + set + { + _optionCaseSensitivity = value; + _customTokenOwnershipCache = null; + } + } /// /// Gets or sets a value indicating whether response files (for example: @args.rsp) are expanded. @@ -156,7 +172,135 @@ public void AddGlobalOption(string name, string constraintOrTypeName, string[]? public void AddGlobalOption(string name, string constraintOrTypeName, string[]? aliases, string? defaultValue, string description) => AddGlobalOptionCore(name, ResolveConstraintOrTypeName(constraintOrTypeName, _customRouteConstraints), aliases, defaultValue, description); - internal void AddGlobalOptionCore(string name, Type valueType, string[]? aliases, string? defaultValue, string? description = null, Type? ownerType = null) + /// + /// Selects a registered global option for discovery metadata configuration. + /// + /// Canonical option name, with or without the -- prefix. + /// A global-option metadata builder. + /// No global option with the supplied canonical name is registered. + public GlobalOptionBuilder GlobalOption(string name) + { + var requested = string.IsNullOrWhiteSpace(name) + ? throw new ArgumentException("Global option name cannot be empty.", nameof(name)) + : name.Trim(); + + return new GlobalOptionBuilder(this, ResolveGlobalOptionKey(requested)); + } + + private string ResolveGlobalOptionKey(string requested) + { + // Keyed probe first; the scan below only covers a caller passing the rendered token + // ("--tenant") for an option registered under its bare name, or the reverse. + if (_globalOptions.ContainsKey(requested)) + { + return requested; + } + + var canonicalToken = NormalizeLongToken(requested); + + return _globalOptions.Values.FirstOrDefault(option => + string.Equals(option.CanonicalToken, canonicalToken, StringComparison.OrdinalIgnoreCase)) + ?.Name + ?? throw new KeyNotFoundException($"No global option named '{requested}' is registered."); + } + + // Re-reads the entry instead of closing over the definition the builder was created from: a + // retained builder must not write back a stale record. Not reachable today, since duplicate + // registration throws, but the closure form was one added mutator away from silently reverting. + internal void SetGlobalOptionHidden(string canonicalName, bool isHidden) + { + if (_globalOptions.TryGetValue(canonicalName, out var definition)) + { + _globalOptions[canonicalName] = definition with { IsHidden = isHidden }; + _customTokenOwnershipCache = null; + } + } + + internal void SetGlobalOptionAliasHidden(string canonicalName, string alias, bool isHidden) + { + if (!_globalOptions.TryGetValue(canonicalName, out var definition)) + { + return; + } + + var normalizedAlias = NormalizeAliasToken(alias.Trim()); + var comparer = ResolveOptionTokenComparer(); + var registeredAlias = definition.Aliases.FirstOrDefault(candidate => + string.Equals(candidate, normalizedAlias, StringComparison.Ordinal)); + if (registeredAlias is null) + { + var matches = definition.Aliases.Where(candidate => comparer.Equals(candidate, normalizedAlias)).ToArray(); + registeredAlias = matches.Length switch + { + 0 => throw new KeyNotFoundException( + $"No alias token '{alias}' is registered for global option '{canonicalName}'."), + 1 => matches[0], + _ => throw new InvalidOperationException( + $"Alias token '{alias}' is ambiguous for global option '{canonicalName}' because registered aliases differ only by casing."), + }; + } + + // Store the selected registered spelling exactly. Runtime membership still uses the active + // comparer, but exact storage prevents a temporary case-insensitive mode from collapsing + // distinct tokens if the application later restores case-sensitive parsing. + var hiddenAliases = definition.HiddenAliases.ToHashSet(StringComparer.Ordinal); + if (isHidden) + { + hiddenAliases.Add(registeredAlias); + } + else + { + hiddenAliases.RemoveWhere(candidate => comparer.Equals(candidate, registeredAlias)); + } + + _globalOptions[canonicalName] = definition with { HiddenAliases = [.. hiddenAliases] }; + _customTokenOwnershipCache = null; + } + + // The canonical token's visibility is governed solely by the definition's own IsHidden, never + // by HiddenAliases: a case-distinct hidden alias (registered while case-sensitive) can become + // equivalent to the canonical token once parsing switches to case-insensitive, and without this + // guard that would incorrectly hide the canonical spelling too. + // + // The canonical check is deliberately Ordinal, not the effective comparer: the effective + // comparer is exactly what makes "--TENANT" equivalent to canonical "--tenant" once parsing + // turns case-insensitive, and using it here would ALSO exempt that distinct alias string from + // its own HiddenAliases membership — the opposite of what HiddenAlias("--TENANT") asked for. + // Ordinal identifies only the canonical spelling itself; every other alias string still falls + // through to the normal effective-comparer membership check below. + internal bool IsGlobalOptionAliasHidden(GlobalOptionDefinition definition, string token) => + !string.Equals(token, definition.CanonicalToken, StringComparison.Ordinal) + && definition.HiddenAliases.Contains(token, ResolveOptionTokenComparer()); + + private StringComparer ResolveOptionTokenComparer() => + OptionCaseSensitivity == ReplCaseSensitivity.CaseInsensitive + ? StringComparer.OrdinalIgnoreCase + : StringComparer.Ordinal; + + // Kept as a distinct six-parameter signature, not folded into the overload below as a trailing + // optional parameter. Optional parameters are a compile-time convenience: the call site emits + // the full signature, so folding would rename the method as far as the CLR is concerned. + // Repl.Defaults ships as its own package and declares `Repl.Core >= ` — a + // minimum, not an exact pin — so a consumer can legitimately run an older compiled + // Repl.Defaults against a newer Repl.Core. That binary calls this arity; removing it would + // raise MissingMethodException at runtime rather than fail anyone's build. + internal void AddGlobalOptionCore( + string name, + Type valueType, + string[]? aliases, + string? defaultValue, + string? description = null, + Type? ownerType = null) => + AddGlobalOptionCore(name, valueType, aliases, defaultValue, description, ownerType, isHidden: false); + + internal void AddGlobalOptionCore( + string name, + Type valueType, + string[]? aliases, + string? defaultValue, + string? description, + Type? ownerType, + bool isHidden) { name = string.IsNullOrWhiteSpace(name) ? throw new ArgumentException("Global option name cannot be empty.", nameof(name)) @@ -168,11 +312,25 @@ internal void AddGlobalOptionCore(string name, Type valueType, string[]? aliases throw new InvalidOperationException(BuildDuplicateGlobalOptionMessage(name, existing.OwnerType, ownerType)); } + // Two different Names ("tenant" and "--tenant") can normalize to the identical canonical + // token. Left unrejected, GlobalOption(name) resolves that token to whichever definition + // happens to enumerate first — an ambiguity, not a deterministic choice — so ANY caller + // mutating "the" option by that token could silently be mutating the wrong one. + var canonicalCollision = _globalOptions.Values.FirstOrDefault(candidate => + string.Equals(candidate.CanonicalToken, normalizedCanonical, StringComparison.OrdinalIgnoreCase)); + if (canonicalCollision is not null) + { + throw new InvalidOperationException( + $"A global option named '{canonicalCollision.Name}' already renders as the same token " + + $"'{normalizedCanonical}' that registering '{name}' would produce."); + } + + var tokenComparer = ResolveOptionTokenComparer(); var normalizedAliases = (aliases ?? []) .Where(alias => !string.IsNullOrWhiteSpace(alias)) .Select(alias => NormalizeAliasToken(alias.Trim())) - .Distinct(StringComparer.OrdinalIgnoreCase) - .Where(alias => !string.Equals(alias, normalizedCanonical, StringComparison.OrdinalIgnoreCase)) + .Distinct(tokenComparer) + .Where(alias => !tokenComparer.Equals(alias, normalizedCanonical)) .ToArray(); _globalOptions[name] = new GlobalOptionDefinition( @@ -182,7 +340,37 @@ internal void AddGlobalOptionCore(string name, Type valueType, string[]? aliases DefaultValue: defaultValue, Description: description, ValueType: valueType, - OwnerType: ownerType); + OwnerType: ownerType, + IsHidden: isHidden, + HiddenAliases: []); + _customTokenOwnershipCache = null; + } + + // The last-registered definition wins a token/alias collision: assigning an existing + // dictionary key replaces its owner while preserving the token's first-seen output order. + // Cached because parsing, help, and completion all need this projection, completion rebuilds + // it on every keystroke, and Values/Aliases are immutable between the mutators above that + // invalidate it. + internal IReadOnlyDictionary ResolveCustomTokenOwnership() + { + if (_customTokenOwnershipCache is { } cached) + { + return cached; + } + + var comparer = ResolveOptionTokenComparer(); + var ownership = new Dictionary(comparer); + foreach (var definition in _globalOptions.Values) + { + ownership[definition.CanonicalToken] = definition; + foreach (var alias in definition.Aliases) + { + ownership[alias] = definition; + } + } + + _customTokenOwnershipCache = ownership; + return ownership; } private static string BuildDuplicateGlobalOptionMessage(string name, Type? existingOwner, Type? newOwner) diff --git a/src/Repl.Core/Routing/RouteDefinition.cs b/src/Repl.Core/Routing/RouteDefinition.cs index 5cdcac54..308ddc7b 100644 --- a/src/Repl.Core/Routing/RouteDefinition.cs +++ b/src/Repl.Core/Routing/RouteDefinition.cs @@ -5,8 +5,7 @@ namespace Repl; internal sealed class RouteDefinition( RouteTemplate template, CommandBuilder command, - int moduleId, - OptionSchema optionSchema) + int moduleId) { public RouteTemplate Template { get; } = template; @@ -14,5 +13,8 @@ internal sealed class RouteDefinition( public int ModuleId { get; } = moduleId; - public OptionSchema OptionSchema { get; } = optionSchema; + // Read through to the builder rather than snapshotting: a fluent visibility change after + // Map publishes a new schema, and every already-cached routing graph holds these same + // RouteDefinition instances — so the change is observed with no cache plumbing. + public OptionSchema OptionSchema => Command.OptionSchema; } diff --git a/src/Repl.Core/RoutingInvalidatedEventArgs.cs b/src/Repl.Core/RoutingInvalidatedEventArgs.cs new file mode 100644 index 00000000..a18c23ce --- /dev/null +++ b/src/Repl.Core/RoutingInvalidatedEventArgs.cs @@ -0,0 +1,6 @@ +namespace Repl; + +internal sealed class RoutingInvalidatedEventArgs(bool isVisibilityRetraction) : EventArgs +{ + internal bool IsVisibilityRetraction { get; } = isVisibilityRetraction; +} diff --git a/src/Repl.Core/Session/InteractiveSession.cs b/src/Repl.Core/Session/InteractiveSession.cs index 88de4bfa..2add4b6b 100644 --- a/src/Repl.Core/Session/InteractiveSession.cs +++ b/src/Repl.Core/Session/InteractiveSession.cs @@ -733,9 +733,14 @@ internal async ValueTask HandleCompletionAmbientCommandAsync( return false; } - if (!match.Route.Command.Completions.TryGetValue(target, out var completion)) - { - await ReplSessionIO.Output.WriteLineAsync($"Error: no completion provider registered for '{target}'.").ConfigureAwait(false); + // A hidden target and an unknown one are deliberately conflated, and the wording avoids + // "no provider registered" because for a hidden option one usually is. Distinguishing the + // two, or admitting a provider exists, would let a caller probe for hidden options through + // this command — the exact discovery this flag exists to prevent. + if (match.Route.OptionSchema.IsOptionHidden(target) + || !match.Route.Command.Completions.TryGetValue(target, out var completion)) + { + await ReplSessionIO.Output.WriteLineAsync($"Error: no completion is available for '{target}'.").ConfigureAwait(false); return false; } diff --git a/src/Repl.Core/ShellCompletion/ShellCompletionEngine.cs b/src/Repl.Core/ShellCompletion/ShellCompletionEngine.cs index 41c9e547..975e17da 100644 --- a/src/Repl.Core/ShellCompletion/ShellCompletionEngine.cs +++ b/src/Repl.Core/ShellCompletion/ShellCompletionEngine.cs @@ -410,6 +410,25 @@ private static void AbandonProvider(CancellationTokenSource deadline, Task provi deadline.Dispose(); }); + // The pending route option has a provider this bridge may run: it resolves, it is discoverable, + // and it opted into shell scope. A quoted value context is unsafe for provider output (see the + // positional path), and bailing here still lets the static enum fallback run. + private bool TryResolveShellScopedProvider( + RouteMatch match, + string currentTokenPrefix, + out OptionSchemaEntry entry, + out CompletionDelegate completion) + { + entry = null!; + completion = null!; + + return !PrefixHasQuoteContext(currentTokenPrefix) + && TryResolvePendingRouteOption(match, out entry) + && match.Route.OptionSchema.IsEntryDiscoverable(entry) + && match.Route.Command.Completions.TryGetValue(entry.ParameterName, out completion!) + && match.Route.Command.IsCompletionShellScoped(entry.ParameterName); + } + // Runs the pending route option's value provider when it opted into the shell bridge. // Returns true when the provider ran (its answer is final, even when empty), so an enum // fallback never overrides an explicit provider — the interactive menu's precedence. @@ -421,12 +440,7 @@ private async ValueTask TryAddPendingOptionProviderCandidatesAsync( List candidates, CancellationToken cancellationToken) { - // A quoted value context is unsafe for provider output (see the positional path); - // skipping here lets the static enum fallback still run. - if (PrefixHasQuoteContext(currentTokenPrefix) - || !TryResolvePendingRouteOption(match, out var entry) - || !match.Route.Command.Completions.TryGetValue(entry.ParameterName, out var completion) - || !match.Route.Command.IsCompletionShellScoped(entry.ParameterName)) + if (!TryResolveShellScopedProvider(match, currentTokenPrefix, out var entry, out var completion)) { return false; } @@ -589,7 +603,8 @@ private bool TryAddRouteEnumValueCandidates( HashSet dedupe, List candidates) { - if (!TryResolvePendingRouteOption(match, out var entry)) + if (!TryResolvePendingRouteOption(match, out var entry) + || !match.Route.OptionSchema.IsEntryDiscoverable(entry)) { return false; } @@ -722,23 +737,23 @@ private void AddShellOptionCandidates( app.OptionsSnapshot.Parsing.OptionCaseSensitivity == ReplCaseSensitivity.CaseInsensitive ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); - AddGlobalShellOptionCandidates(currentTokenPrefix, optionDedupe, candidates); + var customGlobalOwnership = AddGlobalShellOptionCandidates(currentTokenPrefix, optionDedupe, candidates); if (route is null) { return; } - AddRouteShellOptionCandidates(route, currentTokenPrefix, optionDedupe, candidates); + AddRouteShellOptionCandidates(route, customGlobalOwnership, currentTokenPrefix, optionDedupe, candidates); } - private void AddGlobalShellOptionCandidates( + private IReadOnlyDictionary AddGlobalShellOptionCandidates( string currentTokenPrefix, HashSet dedupe, List candidates) { var options = app.OptionsSnapshot; - OptionTokenCompletionSource.CollectGlobalOptionTokens( + return OptionTokenCompletionSource.CollectGlobalOptionTokens( options, currentTokenPrefix, options.Parsing.OptionCaseSensitivity.ToStringComparison(), @@ -748,12 +763,14 @@ private void AddGlobalShellOptionCandidates( private void AddRouteShellOptionCandidates( RouteDefinition route, + IReadOnlyDictionary customGlobalOwnership, string currentTokenPrefix, HashSet dedupe, List candidates) { OptionTokenCompletionSource.CollectRouteOptionTokens( - route, + route.OptionSchema, + customGlobalOwnership, currentTokenPrefix, app.OptionsSnapshot.Parsing.OptionCaseSensitivity, dedupe, diff --git a/src/Repl.Defaults/GlobalOptionsExtensions.cs b/src/Repl.Defaults/GlobalOptionsExtensions.cs index 34765b7d..a895d455 100644 --- a/src/Repl.Defaults/GlobalOptionsExtensions.cs +++ b/src/Repl.Defaults/GlobalOptionsExtensions.cs @@ -36,7 +36,8 @@ public static class GlobalOptionsExtensions { var optionAttr = property.GetCustomAttribute(); var name = optionAttr?.Name ?? ToKebabCase(property.Name); - var aliases = optionAttr?.Aliases; + var hiddenAliases = optionAttr?.HiddenAliases ?? []; + var aliases = (optionAttr?.Aliases ?? []).Concat(hiddenAliases).ToArray(); // The effective default of a typed global option is always the prototype value: // PopulateInstance starts from new T() and only overwrites parsed values. Keep // the metadata aligned (even when the value equals the CLR default) so @@ -44,7 +45,18 @@ public static class GlobalOptionsExtensions var defaultValue = property.GetValue(prototype)?.ToString(); var description = property.GetCustomAttribute()?.Description; - options.Parsing.AddGlobalOptionCore(name, property.PropertyType, aliases, defaultValue, description, typeof(T)); + options.Parsing.AddGlobalOptionCore( + name, + property.PropertyType, + aliases, + defaultValue, + description, + typeof(T), + isHidden: optionAttr?.Hidden ?? false); + foreach (var hiddenAlias in hiddenAliases) + { + options.Parsing.GlobalOption(name).HiddenAlias(hiddenAlias); + } } }); @@ -80,6 +92,15 @@ private static void ThrowIfUnsupportedOverrides(Type optionsType, IReadOnlyList< $"Global option property '{optionsType.Name}.{property.Name}' declares an Arity override, which is not supported for typed global options. Remove the override or expose the option through a per-command options type."); } + // Hidden is supported here and flows through to GlobalOptionDefinition, but + // AutomationHidden has nothing to act on: global options never enter the documentation + // model, so they never reach an MCP tool schema. Rejecting beats a silent no-op. + if (optionAttr?.AutomationHidden == true) + { + throw new NotSupportedException( + $"Global option property '{optionsType.Name}.{property.Name}' declares AutomationHidden, which is not supported for typed global options: global options are never exposed to programmatic surfaces, so the flag would have no effect. Remove it, or expose the option through a per-command options type."); + } + foreach (var valueAlias in property.GetCustomAttributes()) { if (valueAlias.CaseSensitivityOverride is not null) diff --git a/src/Repl.Defaults/ReplApp.cs b/src/Repl.Defaults/ReplApp.cs index 530e3d4c..0865b82e 100644 --- a/src/Repl.Defaults/ReplApp.cs +++ b/src/Repl.Defaults/ReplApp.cs @@ -114,8 +114,31 @@ public ReplApp Options(Action configure) public void InvalidateRouting() => _core.InvalidateRouting(); /// - public ReplDocumentationModel CreateDocumentationModel(string? targetPath = null) => - _core.CreateDocumentationModel(targetPath); + public ReplDocumentationModel CreateDocumentationModel(string? targetPath = null) + { + if (_sharedProvider is { } sharedProvider) + { + return _core.CreateDocumentationModel(sharedProvider, targetPath); + } + + // Documentation needs the configured services for provider-aware requiredness, but it + // must not finalize the app's mutable registration phase. A short-lived provider gives + // discovery the current descriptors while leaving Run free to build the shared provider + // after later Use* extensions have registered their services. + using var discoveryProvider = _services.BuildServiceProvider(); + return _core.CreateDocumentationModel(discoveryProvider, targetPath); + } + + /// + /// Builds a structured documentation model using an externally managed provider for service-backed parameters. + /// + /// Provider used to determine whether direct handler parameters can be omitted. + /// Optional target path to scope the model. + /// A structured documentation model. + public ReplDocumentationModel CreateDocumentationModel( + IServiceProvider serviceProvider, + string? targetPath = null) => + _core.CreateDocumentationModel(serviceProvider, targetPath); /// /// Maps a route and command handler. @@ -758,7 +781,7 @@ public IReplApp MapModule(IReplModule module, Delegate isPresent) public void InvalidateRouting() => _map.InvalidateRouting(); public ReplDocumentationModel CreateDocumentationModel(string? targetPath = null) => - _map.CreateDocumentationModel(targetPath); + root.CreateDocumentationModel(targetPath); IContextBuilder ICoreReplApp.Context(string segment, Action configure, Delegate? validation) => Context(segment, scoped => configure(scoped), validation); diff --git a/src/Repl.IntegrationTests/Given_Completions.cs b/src/Repl.IntegrationTests/Given_Completions.cs index e3f20228..b0961f34 100644 --- a/src/Repl.IntegrationTests/Given_Completions.cs +++ b/src/Repl.IntegrationTests/Given_Completions.cs @@ -25,6 +25,32 @@ public void When_InteractiveCompleteCommandIsUsed_Then_CompletionProviderCandida output.Text.Should().Contain("ab002"); } + [TestMethod] + [Description("A hidden option's completion provider must not be reachable through the complete ambient command, and the refusal must not reveal that the option exists. Asserting the wording is identical to the unknown-target refusal — bar the name — makes the non-disclosure a contract instead of a coincidence, and stops the message from claiming no provider is registered when one is.")] + public void When_InteractiveCompleteTargetsHiddenOption_Then_ItIsIndistinguishableFromAnUnknownTarget() + { + var sut = ReplApp.Create().UseDefaultInteractive(); + var command = sut.Map( + "deploy", + static string ([ReplOption(Name = "secret-mode")] string? secretMode = null) => secretMode ?? "none"); + command.WithCompletion( + "secretMode", + static (_, _, _) => ValueTask.FromResult>(["internal-debug"])); + command.WithOption("secretMode", option => option.Hidden()); + + var hidden = ConsoleCaptureHelper.CaptureWithInput( + "complete deploy --target secretMode\nexit\n", + () => sut.Run([])); + var unknown = ConsoleCaptureHelper.CaptureWithInput( + "complete deploy --target ga-bu-zo-meu\nexit\n", + () => sut.Run([])); + + hidden.ExitCode.Should().Be(0); + hidden.Text.Should().Contain("Error: no completion is available for 'secretMode'."); + hidden.Text.Should().NotContain("internal-debug"); + unknown.Text.Should().Contain("Error: no completion is available for 'ga-bu-zo-meu'."); + } + [TestMethod] [Description("Regression guard: verifies interactive complete command uses unknown target so that error is rendered.")] public void When_InteractiveCompleteCommandUsesUnknownTarget_Then_ErrorIsRendered() @@ -37,7 +63,7 @@ public void When_InteractiveCompleteCommandUsesUnknownTarget_Then_ErrorIsRendere () => sut.Run([])); output.ExitCode.Should().Be(0); - output.Text.Should().Contain("Error: no completion provider registered for 'missing'."); + output.Text.Should().Contain("Error: no completion is available for 'missing'."); } [TestMethod] @@ -88,6 +114,119 @@ public void When_AutocompleteModeCommandIsUsed_Then_SessionOverrideIsApplied() output.Text.Should().Contain("override=Off"); } + [TestMethod] + [Description("Hidden custom global options and their aliases are omitted from shell completion while visible framework options remain discoverable.")] + public void When_CompletingGlobalPrefixWithHiddenCustomOption_Then_HiddenTokensAreNotSuggested() + { + var sut = ReplApp.Create() + .Options(options => + { + options.Parsing.AddGlobalOption("region", aliases: ["-r"]); + options.Parsing.AddGlobalOption("tenant", aliases: ["-t"]); + options.Parsing.GlobalOption("tenant").Hidden(); + }); + sut.Map("ping", () => "pong"); + + var longCandidates = Complete("repl --"); + var shortCandidates = Complete("repl -"); + + longCandidates.Should().Contain("--help"); + longCandidates.Should().NotContain("--tenant"); + shortCandidates.Should().Contain("-r"); + shortCandidates.Should().NotContain("-t"); + + string[] Complete(string line) + { + var output = ConsoleCaptureHelper.Capture(() => sut.Run( + [ + "completion", + "__complete", + "--shell", + "bash", + "--line", + line, + "--cursor", + line.Length.ToString(CultureInfo.InvariantCulture), + "--no-logo", + ])); + + output.ExitCode.Should().Be(0); + + // The completion emitter separates candidates with '\n', but the final line is flushed + // with Environment.NewLine — so on Windows the last candidate carries a trailing '\r'. + // TrimEntries strips it; without it the last candidate never compares equal, which made + // this test pass on the Linux/macOS CI legs and fail only on Windows. + return output.Text.Split('\n', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + } + } + + [TestMethod] + [Description("A token registered as a visible option's alias and later as a hidden option's canonical form belongs, per GlobalOptionParser, to the LAST registration — the hidden one. Enumerating definitions independently would advertise the token from the visible definition while accepting it would bind the hidden one, which is the mismatch that file's own comment warns callers against.")] + public void When_AVisibleGlobalAliasCollidesWithALaterHiddenOption_Then_TheTokenIsNotSuggested() + { + var sut = ReplApp.Create() + .Options(options => + { + options.Parsing.AddGlobalOption("region", aliases: ["--tenant"]); + options.Parsing.AddGlobalOption("tenant"); + options.Parsing.GlobalOption("tenant").Hidden(); + }); + sut.Map("ping", () => "pong"); + const string line = "repl --t"; + + var output = ConsoleCaptureHelper.Capture(() => sut.Run( + [ + "completion", + "__complete", + "--shell", + "bash", + "--line", + line, + "--cursor", + line.Length.ToString(CultureInfo.InvariantCulture), + "--no-logo", + ])); + + output.ExitCode.Should().Be(0); + output.Text.Split('\n', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries) + .Should().NotContain("--tenant"); + } + + [TestMethod] + [Description("In case-insensitive mode, a losing hidden token that differs only by case must not enter deduplication before the later visible owner. Completion must emit the winning definition's spelling, matching parser/help ownership and registration order.")] + public void When_AVisibleGlobalClaimsACaseVariantOfAHiddenAlias_Then_CompletionUsesTheVisibleSpelling() + { + var sut = ReplApp.Create() + .Options(options => + { + options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive; + options.Parsing.AddGlobalOption("legacy", aliases: ["--TENANT"]); + options.Parsing.GlobalOption("legacy").Hidden(); + options.Parsing.AddGlobalOption("tenant"); + }); + sut.Map("ping", () => "pong"); + const string line = "repl --t"; + + var output = ConsoleCaptureHelper.Capture(() => sut.Run( + [ + "completion", + "__complete", + "--shell", + "bash", + "--line", + line, + "--cursor", + line.Length.ToString(CultureInfo.InvariantCulture), + "--no-logo", + ])); + var candidates = output.Text.Split( + '\n', + StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + + output.ExitCode.Should().Be(0); + candidates.Should().ContainSingle().Which.Should().Be("--tenant"); + } + [TestMethod] [Description("Regression guard: verifies shell completion suggests custom global options so app-registered globals remain discoverable from tab completion.")] public void When_CompletingGlobalPrefix_Then_CustomGlobalOptionsAreSuggested() diff --git a/src/Repl.IntegrationTests/Given_CustomGlobalOptions.cs b/src/Repl.IntegrationTests/Given_CustomGlobalOptions.cs index 77e39ae4..d58e3fa6 100644 --- a/src/Repl.IntegrationTests/Given_CustomGlobalOptions.cs +++ b/src/Repl.IntegrationTests/Given_CustomGlobalOptions.cs @@ -50,6 +50,36 @@ public void When_RequestingRootHelp_Then_CustomGlobalOptionIsListedInGlobalOptio output.Text.Should().Contain("Custom global option."); } + [TestMethod] + [Description("Hidden(false) re-exposes a manually registered global option after it was hidden.")] + public void When_HiddenGlobalOptionIsShownAgain_Then_RootHelpListsIt() + { + var sut = ReplApp.Create() + .Options(options => + { + options.Parsing.AddGlobalOption("tenant"); + options.Parsing.GlobalOption("tenant").Hidden().Hidden(isHidden: false); + }); + sut.Map("ping", () => "ok"); + + var output = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + + output.ExitCode.Should().Be(0); + output.Text.Should().Contain("--tenant"); + } + + [TestMethod] + [Description("Selecting an unregistered global option fails clearly instead of silently creating ineffective metadata.")] + public void When_SelectingUnknownGlobalOption_Then_ConfigurationThrows() + { + var parsing = new ParsingOptions(); + + var act = () => parsing.GlobalOption("missing"); + + act.Should().Throw() + .WithMessage("*missing*registered*"); + } + [TestMethod] [Description("Regression guard: verifies typed global option descriptions are rendered in root help without adding default-value display.")] public void When_RequestingRootHelpForTypedGlobalOptions_Then_DescriptionsAreListed() diff --git a/src/Repl.IntegrationTests/Given_DocumentationExport.cs b/src/Repl.IntegrationTests/Given_DocumentationExport.cs index fff64ac2..7bc91474 100644 --- a/src/Repl.IntegrationTests/Given_DocumentationExport.cs +++ b/src/Repl.IntegrationTests/Given_DocumentationExport.cs @@ -59,6 +59,127 @@ public void When_ExportingExactHiddenCommand_Then_HiddenCommandIsIncluded() output.Text.Should().Contain("\"isHidden\": true"); } + [TestMethod] + [Description("Mirrors the command axis one level down: an explicitly targeted command exports its hidden options, flagged, exactly as a targeted hidden command exports itself. Aggregate export still omits them, which is what keeps them out of the MCP tool schema and its argument allow-list. This targeted export is the only surface an app author has for asking which options are hidden.")] + public void When_ExportingExactCommandWithHiddenOption_Then_TheOptionIsIncludedAndFlagged() + { + var sut = ReplApp.Create() + .UseDocumentationExport(); + sut.Map( + "deploy", + ([ReplOption(Name = "environment")] string environment, [ReplOption(Name = "internalMode")] bool internalMode = false) => + $"{environment}:{internalMode}") + .WithOption("internalMode", static option => option.Hidden()); + + var aggregate = ConsoleCaptureHelper.Capture(() => sut.Run(["doc", "export", "--json", "--no-logo"])); + var targeted = ConsoleCaptureHelper.Capture(() => sut.Run(["doc", "export", "deploy", "--json", "--no-logo"])); + + aggregate.ExitCode.Should().Be(0, aggregate.Text); + aggregate.Text.Should().NotContain("internalMode"); + targeted.ExitCode.Should().Be(0, targeted.Text); + targeted.Text.Should().Contain("internalMode"); + targeted.Text.Should().Contain("\"isHidden\": true"); + } + + [TestMethod] + [Description("The exact-target export contract promises hidden options are included AND flagged. The structured formats get that for free by serializing the record, but markdown formats each field by hand, so without this it rendered a hidden option indistinguishably from a public one. Commands already print their own Hidden line; options now carry the same information.")] + public void When_ExportingExactCommandAsMarkdown_Then_HiddenOptionsAreFlagged() + { + var sut = ReplApp.Create() + .UseDocumentationExport(); + sut.Map( + "deploy", + ([ReplOption(Name = "environment")] string environment, + [ReplOption(Name = "internalMode")] bool internalMode = false, + [ReplOption(Name = "traceId")] string? traceId = null) => + $"{environment}:{internalMode}:{traceId}") + .WithOption("internalMode", static option => option.Hidden()) + .WithOption("traceId", static option => option.AutomationHidden()); + + var markdown = ConsoleCaptureHelper.Capture(() => sut.Run(["doc", "export", "deploy", "--markdown", "--no-logo"])); + + markdown.ExitCode.Should().Be(0, markdown.Text); + markdown.Text.Should().Contain("`--internalMode`"); + markdown.Text.Should().Contain("hidden"); + markdown.Text.Should().Contain("`--traceId`"); + markdown.Text.Should().Contain("automation-hidden"); + markdown.Text.Should().Contain("`--environment`"); + } + + [TestMethod] + [Description("Markdown renders the surviving invocable reverse alias when global precedence removes the ordinary route token, rather than inventing the unreachable canonical spelling.")] + public void When_OnlyReverseAliasRemainsReachable_Then_MarkdownRendersThatAlias() + { + var sut = ReplApp.Create().UseDocumentationExport(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("force"); + options.Parsing.GlobalOption("force").Hidden(); + }); + sut.Map( + "deploy", + static string ([ReplOption(ReverseAliases = ["--no-force"])] bool force = true) => force.ToString()); + + var markdown = ConsoleCaptureHelper.Capture(() => + sut.Run(["doc", "export", "deploy", "--markdown", "--no-logo"])); + + markdown.ExitCode.Should().Be(0, markdown.Text); + markdown.Text.Should().Contain("`--no-force`"); + markdown.Text.Should().NotContain("`--force`"); + } + + [TestMethod] + [Description("Exact-target inventory retains a wholly hidden route option even when a hidden global owns its only token; every export format identifies it and preserves the hidden flag.")] + public void When_HiddenGlobalOwnsWhollyHiddenOptionToken_Then_ExactExportsRetainTheOption() + { + var sut = ReplApp.Create().UseDocumentationExport(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("internal-mode"); + options.Parsing.GlobalOption("internal-mode").Hidden(); + }); + sut.Map( + "deploy", + static string ([ReplOption(Name = "internal-mode")] bool internalMode = false) => internalMode.ToString()) + .WithOption("internalMode", static option => option.Hidden()); + + var modelOption = sut.CreateDocumentationModel("deploy").Commands.Single().Options.Single(); + var json = ConsoleCaptureHelper.Capture(() => sut.Run(["doc", "export", "deploy", "--json", "--no-logo"])); + var yaml = ConsoleCaptureHelper.Capture(() => sut.Run(["doc", "export", "deploy", "--yaml", "--no-logo"])); + var xml = ConsoleCaptureHelper.Capture(() => sut.Run(["doc", "export", "deploy", "--xml", "--no-logo"])); + var markdown = ConsoleCaptureHelper.Capture(() => sut.Run(["doc", "export", "deploy", "--markdown", "--no-logo"])); + + modelOption.Name.Should().Be("internal-mode"); + modelOption.IsHidden.Should().BeTrue(); + foreach (var export in new[] { json, yaml, xml, markdown }) + { + export.ExitCode.Should().Be(0, export.Text); + export.Text.Should().Contain("internal-mode"); + export.Text.Should().ContainEquivalentOf("hidden"); + } + } + + [TestMethod] + [Description("The visibility flags are new members on a serialized public record, and yaml and xml emit that record wholesale rather than field by field. Only json and markdown had coverage, so this exercises the two formats that would have broken silently.")] + public void When_ExportingExactCommandAsYamlOrXml_Then_VisibilityFlagsSerialize() + { + var sut = ReplApp.Create() + .UseDocumentationExport(); + sut.Map( + "deploy", + ([ReplOption(Name = "environment")] string environment, [ReplOption(Name = "internalMode")] bool internalMode = false) => + $"{environment}:{internalMode}") + .WithOption("internalMode", static option => option.Hidden()); + + var yaml = ConsoleCaptureHelper.Capture(() => sut.Run(["doc", "export", "deploy", "--yaml", "--no-logo"])); + var xml = ConsoleCaptureHelper.Capture(() => sut.Run(["doc", "export", "deploy", "--xml", "--no-logo"])); + + yaml.ExitCode.Should().Be(0, yaml.Text); + yaml.Text.Should().Contain("internalMode"); + xml.ExitCode.Should().Be(0, xml.Text); + xml.Text.Should().Contain("internalMode"); + } + [TestMethod] [Description("Regression guard: verifies hidden context is explicitly targeted so exact-path export includes the hidden context metadata.")] public void When_ExportingExactHiddenContext_Then_HiddenContextIsIncluded() diff --git a/src/Repl.IntegrationTests/Given_GlobalOptionsAccessor.cs b/src/Repl.IntegrationTests/Given_GlobalOptionsAccessor.cs index 05cca86d..10064208 100644 --- a/src/Repl.IntegrationTests/Given_GlobalOptionsAccessor.cs +++ b/src/Repl.IntegrationTests/Given_GlobalOptionsAccessor.cs @@ -22,6 +22,116 @@ public void When_GlobalOptionProvided_Then_HandlerCanReadItViaAccessor() output.Text.Should().Contain("acme"); } + [TestMethod] + [Description("Help renders one row per global option and lists that option's aliases, but GlobalOptionParser gives a colliding token to the LAST registration. A token listed as a visible option's alias can therefore bind a hidden option, so help must decide visibility per token rather than per definition — the same mismatch the completion source had.")] + public void When_AVisibleGlobalAliasCollidesWithALaterHiddenOption_Then_HelpDoesNotAdvertiseTheToken() + { + var sut = ReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("region", aliases: ["--tenant", "-r"]); + options.Parsing.AddGlobalOption("tenant"); + options.Parsing.GlobalOption("tenant").Hidden(); + }); + sut.Map("show", () => "ok"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + + help.ExitCode.Should().Be(0); + help.Text.Should().Contain("--region", "the visible option keeps its own row"); + help.Text.Should().Contain("-r", "an alias with no collision stays listed"); + help.Text.Should().NotContain("--tenant", "accepting this token would bind the hidden option"); + } + + [TestMethod] + [Description("The reverse collision: a later hidden definition claims a visible option's CANONICAL token while leaving its aliases alone. The alias is still accepted by the parser and offered by completion, so dropping the whole row would hide a reachable option. Rows are therefore built from whichever tokens the definition still owns, with the canonical one carrying no special weight.")] + public void When_AHiddenGlobalClaimsAVisibleCanonicalToken_Then_HelpStillListsTheSurvivingAlias() + { + var sut = ReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("region", aliases: ["-r"]); + options.Parsing.AddGlobalOption("zone", aliases: ["--region"]); + options.Parsing.GlobalOption("zone").Hidden(); + }); + sut.Map("show", () => "ok"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + + help.ExitCode.Should().Be(0); + help.Text.Should().Contain("-r", "the alias nobody claimed keeps the option discoverable"); + help.Text.Should().NotContain("--region", "that token now binds the hidden option"); + help.Text.Should().NotContain("--zone"); + } + + [TestMethod] + [Description("A hidden definition registered first must not contribute a token that a later visible definition owns. Otherwise root help renders the token twice and leaks the hidden definition's description under the duplicate row.")] + public void When_AVisibleGlobalClaimsAHiddenAlias_Then_HelpRendersOnlyTheVisibleOwner() + { + var sut = ReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption( + "legacy", + aliases: ["--tenant"], + defaultValue: null, + description: "Hidden legacy tenant."); + options.Parsing.AddGlobalOption( + "tenant", + aliases: null, + defaultValue: null, + description: "Visible tenant."); + options.Parsing.GlobalOption("legacy").Hidden(); + }); + sut.Map("show", () => "ok"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + var tenantRows = help.Text + .Split(Environment.NewLine, StringSplitOptions.RemoveEmptyEntries) + .Where(static line => line.Contains("--tenant", StringComparison.Ordinal)) + .ToArray(); + + help.ExitCode.Should().Be(0); + tenantRows.Should().ContainSingle(); + tenantRows[0].Should().Contain("Visible tenant."); + help.Text.Should().NotContain("Hidden legacy tenant."); + } + + [TestMethod] + [Description("A manually registered hidden global option is omitted from help — token, alias, description and default alike — while staying bindable through the accessor. A visible sibling whose own token contains the hidden one's alias is registered on purpose: asserting the bare substring \"-t\" would pass only for as long as no other rendered token happened to contain it, so the assertions target the rendered option row instead.")] + public void When_ManualGlobalOptionIsHidden_Then_HelpOmitsItsRowAndExplicitInvocationStillBinds() + { + var sut = ReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption( + "tenant", + aliases: ["-t"], + defaultValue: "internal-default", + description: "Internal tenant selector."); + options.Parsing.AddGlobalOption( + "denim-tint", + aliases: ["-d"], + defaultValue: null, + description: "Visible control."); + options.Parsing.GlobalOption("tenant").Hidden(); + }); + sut.Map("show", (IGlobalOptionsAccessor globals) => globals.GetValue("tenant") ?? "none"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + var invocation = ConsoleCaptureHelper.Capture(() => + sut.Run(["show", "--tenant", "acme", "--no-logo"])); + + help.ExitCode.Should().Be(0); + help.Text.Should().NotContain("--tenant"); + help.Text.Should().NotContain("--tenant, -t"); + help.Text.Should().NotContain("Internal tenant selector."); + help.Text.Should().NotContain("internal-default"); + help.Text.Should().Contain("--denim-tint, -d", "the visible sibling proves the whole option table was not simply empty"); + invocation.ExitCode.Should().Be(0, invocation.Text); + invocation.Text.Should().Contain("acme"); + } + [TestMethod] [Description("Global option is accessible in middleware via DI.")] public void When_GlobalOptionProvided_Then_MiddlewareCanReadIt() @@ -128,6 +238,22 @@ public void When_NullableGlobalOptionDeclaresUnderlyingClrDefault_Then_Registere output.Text.Should().Contain("port:0"); } + [TestMethod] + [Description("Creating documentation must inspect configured services without freezing the shared provider. A typed global-options extension registered afterward must still be visible to the provider that Run reuses.")] + public void When_DocumentationPrecedesTypedGlobalRegistration_Then_RunUsesTheLateServiceRegistration() + { + var sut = ReplApp.Create(); + _ = sut.CreateDocumentationModel(); + sut.UseGlobalOptions(); + sut.Map("show", (TestGlobalOptions options) => options.Tenant ?? "none"); + + var output = ConsoleCaptureHelper.Capture(() => + sut.Run(["show", "--tenant", "acme", "--no-logo"])); + + output.ExitCode.Should().Be(0, output.Text); + output.Text.Should().Contain("acme"); + } + [TestMethod] [Description("UseGlobalOptions registers typed class accessible via DI.")] public void When_UsingTypedGlobalOptions_Then_ClassIsPopulatedFromParsedValues() @@ -143,6 +269,101 @@ public void When_UsingTypedGlobalOptions_Then_ClassIsPopulatedFromParsedValues() output.Text.Should().Contain("acme:9090"); } + [TestMethod] + [Description("ReplOption.Hidden on a typed global-options property hides discovery while preserving typed injection and binding.")] + public void When_TypedGlobalOptionPropertyIsHidden_Then_HelpOmitsItAndExplicitInvocationStillBinds() + { + var sut = ReplApp.Create(); + sut.UseGlobalOptions(); + sut.Map("show", (HiddenGlobalOptions options) => $"{options.Region}:{options.InternalToken}"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + var invocation = ConsoleCaptureHelper.Capture(() => sut.Run( + ["show", "--region", "east", "--internal-token", "secret", "--no-logo"])); + + help.ExitCode.Should().Be(0); + help.Text.Should().Contain("--region"); + help.Text.Should().NotContain("--internal-token"); + invocation.ExitCode.Should().Be(0, invocation.Text); + invocation.Text.Should().Contain("east:secret"); + } + + [TestMethod] + [Description("ReplOption.HiddenAliases on a typed global property preserves legacy parsing while root help keeps the canonical token and omits the old spelling.")] + public void When_TypedGlobalOptionHasHiddenAlias_Then_HelpOmitsOnlyTheAliasAndBindingRetainsIt() + { + var sut = ReplApp.Create(); + sut.UseGlobalOptions(); + sut.Map("show", static string (LegacyGlobalOptions options) => options.Tenant ?? "none"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + var invocation = ConsoleCaptureHelper.Capture(() => sut.Run(["--account", "acme", "show", "--no-logo"])); + + help.Text.Should().Contain("--tenant"); + help.Text.Should().NotContain("--account"); + invocation.ExitCode.Should().Be(0, invocation.Text); + invocation.Text.Should().Contain("acme"); + } + + [TestMethod] + [Description("In case-sensitive mode, hiding one global alias spelling leaves a differently-cased alias visible and both spellings remain parsable.")] + public void When_GlobalAliasesDifferOnlyByCase_Then_HidingOnePreservesTheOther() + { + var sut = ReplApp.Create(); + sut.Options(options => + { + options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseSensitive; + options.Parsing.AddGlobalOption("tenant", aliases: ["--account", "--ACCOUNT"]); + options.Parsing.GlobalOption("tenant").HiddenAlias("--account"); + }); + sut.Map("show", static string () => "ok"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + var hiddenInvocation = ConsoleCaptureHelper.Capture(() => sut.Run(["--account", "acme", "show", "--no-logo"])); + var visibleInvocation = ConsoleCaptureHelper.Capture(() => sut.Run(["--ACCOUNT", "acme", "show", "--no-logo"])); + + help.Text.Should().Contain("--ACCOUNT"); + help.Text.Should().NotContain("--account"); + hiddenInvocation.ExitCode.Should().Be(0, hiddenInvocation.Text); + visibleInvocation.ExitCode.Should().Be(0, visibleInvocation.Text); + } + + [TestMethod] + [Description("HiddenAlias prefers an exact registered token before the active case-insensitive fallback, so changing comparison modes cannot hide the wrong case-distinct alias when case sensitivity is restored.")] + public void When_CaseModeChangesBeforeHidingAnExactGlobalAlias_Then_TheExactAliasIsRetained() + { + var sut = ReplApp.Create(); + sut.Options(options => + { + options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseSensitive; + options.Parsing.AddGlobalOption("tenant", aliases: ["--account", "--ACCOUNT"]); + options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive; + options.Parsing.GlobalOption("tenant").HiddenAlias("--ACCOUNT"); + options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseSensitive; + }); + sut.Map("show", static string () => "ok"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + + help.Text.Should().Contain("--account"); + help.Text.Should().NotContain("--ACCOUNT"); + } + + [TestMethod] + [Description("A fluent Hidden(false) override re-exposes a typed global option hidden by attribute.")] + public void When_TypedGlobalOptionHiddenAttributeIsOverriddenWithFalse_Then_RootHelpListsIt() + { + var sut = ReplApp.Create() + .UseGlobalOptions() + .Options(options => options.Parsing.GlobalOption("internal-token").Hidden(isHidden: false)); + sut.Map("show", (HiddenGlobalOptions globals) => globals.InternalToken ?? "none"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + + help.ExitCode.Should().Be(0); + help.Text.Should().Contain("--internal-token"); + } + [TestMethod] [Description("UseGlobalOptions handler parameters are injected from DI even when the parameter name matches a global option.")] public void When_TypedGlobalOptionsParameterNameMatchesGlobalOption_Then_HandlerReceivesDiInstance() @@ -485,6 +706,20 @@ private sealed class ClrDefaultGlobals public int Retries { get; set; } } + private sealed class LegacyGlobalOptions + { + [ReplOption(HiddenAliases = ["--account"])] + public string? Tenant { get; set; } + } + + private sealed class HiddenGlobalOptions + { + public string? Region { get; set; } + + [ReplOption(Hidden = true)] + public string? InternalToken { get; set; } + } + private interface IInterfaceGlobalOptions { string? Tenant { get; } diff --git a/src/Repl.IntegrationTests/Given_HelpDiscovery.cs b/src/Repl.IntegrationTests/Given_HelpDiscovery.cs index 5733fe40..441764b2 100644 --- a/src/Repl.IntegrationTests/Given_HelpDiscovery.cs +++ b/src/Repl.IntegrationTests/Given_HelpDiscovery.cs @@ -1,4 +1,5 @@ using ComponentDescriptionAttribute = System.ComponentModel.DescriptionAttribute; +using Microsoft.Extensions.DependencyInjection; using Repl.Spectre; namespace Repl.IntegrationTests; @@ -83,6 +84,716 @@ public void When_RequestingCommandHelp_Then_UsageAndDescriptionAreRendered() output.Text.Should().Contain("Description: List contacts"); } + [TestMethod] + [Description("A hidden legacy alias remains a parser fallback but never appears in command help, aggregate documentation, or typo suggestions; attribute and fluent forms keep the canonical token visible.")] + public void When_RouteAliasIsHidden_Then_OnlyParsingRetainsIt() + { + var sut = ReplApp.Create(); + sut.Map( + "deploy", + static string ([ReplOption(Name = "tenant", Aliases = ["--ACCOUNT"], HiddenAliases = ["--account"])] string? tenant = null) => tenant ?? "none"); + sut.Map( + "publish", + static string ([ReplOption(Name = "tenant", Aliases = ["--account", "--ACCOUNT"])] string? tenant = null) => tenant ?? "none") + .WithOption("tenant", static option => option.HiddenAlias("--account")); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--help", "--no-logo"])); + var fluentHelp = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--help", "--no-logo"])); + var legacy = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--account", "acme", "--no-logo"])); + var fluentLegacy = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--account", "acme", "--no-logo"])); + var uppercaseAlias = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--ACCOUNT", "north", "--no-logo"])); + var typo = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--accoun", "acme", "--no-logo"])); + var option = sut.CreateDocumentationModel().Commands + .Single(command => string.Equals(command.Path, "deploy", StringComparison.Ordinal)) + .Options.Single(); + + help.Text.Should().Contain("--tenant"); + help.Text.Should().NotContain("--account"); + help.Text.Should().Contain("--ACCOUNT"); + fluentHelp.Text.Should().Contain("--tenant"); + fluentHelp.Text.Should().Contain("--ACCOUNT"); + fluentHelp.Text.Should().NotContain("--account"); + legacy.ExitCode.Should().Be(0, legacy.Text); + legacy.Text.Should().Contain("acme"); + fluentLegacy.ExitCode.Should().Be(0, fluentLegacy.Text); + fluentLegacy.Text.Should().Contain("acme"); + uppercaseAlias.ExitCode.Should().Be(0, uppercaseAlias.Text); + uppercaseAlias.Text.Should().Contain("north"); + typo.Text.Should().NotContain("Did you mean '--account'"); + option.Name.Should().Be("tenant"); + option.Aliases.Should().Contain(["--tenant", "--ACCOUNT"]); + option.Aliases.Should().NotContain("--account"); + } + + [TestMethod] + [Description("ReplOptionAttribute.HiddenAliases can be explicitly set to null (valid attribute metadata despite the nullable warning). Mapping a command with at least one visible alias must not depend on the implicit null-array-to-Span conversion happening to make '.Contains' return false — that reads as a dereference of a null array and should stay explicit (?? []), matching every other consumer of this field.")] + public void When_HiddenAliasesIsExplicitlyNull_Then_MappingDoesNotThrowAndAliasStaysVisible() + { + var sut = ReplApp.Create(); + sut.Map( + "deploy", + static string ([ReplOption(Aliases = ["--tenant-name"], HiddenAliases = null!)] string? tenant = null) => tenant ?? "none"); + + var option = sut.CreateDocumentationModel().Commands.Single().Options.Single(); + + option.Aliases.Should().Contain("--tenant-name"); + } + + [TestMethod] + [Description("Same null-HiddenAliases contract for an options-group property, which builds its schema entries through a separate code path from direct handler parameters.")] + public void When_GroupPropertyHiddenAliasesIsExplicitlyNull_Then_MappingDoesNotThrowAndAliasStaysVisible() + { + var sut = ReplApp.Create(); + sut.Map("deploy", (NullHiddenAliasesOptions options) => options.Tenant ?? "none"); + + var option = sut.CreateDocumentationModel().Commands.Single().Options.Single(); + + option.Aliases.Should().Contain("--tenant-name"); + } + + [ReplOptionsGroup] + private sealed class NullHiddenAliasesOptions + { + [ReplOption(Aliases = ["--tenant-name"], HiddenAliases = null!)] + public string? Tenant { get; set; } + } + + [TestMethod] + [Description("Fluent alias visibility follows a route option's case-insensitive override: parser-equivalent aliases are all hidden from help and documentation but remain accepted by parsing under a case-sensitive global default.")] + public void When_FluentAliasesUseCaseInsensitiveOverride_Then_AllEquivalentSpellingsAreHiddenFromDiscovery() + { + var sut = ReplApp.Create(); + sut.Map( + "publish", + static string ([ReplOption(Name = "tenant", Aliases = ["--account", "--ACCOUNT"], CaseSensitivity = ReplCaseSensitivity.CaseInsensitive)] string? tenant = null) => tenant ?? "none") + .WithOption("tenant", static option => option.HiddenAlias("--account")); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--help", "--no-logo"])); + var lower = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--account", "south", "--no-logo"])); + var upper = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--ACCOUNT", "north", "--no-logo"])); + var option = sut.CreateDocumentationModel().Commands.Single().Options.Single(); + + help.Text.Should().Contain("--tenant").And.NotContain("--account").And.NotContain("--ACCOUNT"); + option.Aliases.Should().Contain("--tenant").And.NotContain(["--account", "--ACCOUNT"]); + lower.ExitCode.Should().Be(0, lower.Text); + lower.Text.Should().Contain("south"); + upper.ExitCode.Should().Be(0, upper.Text); + upper.Text.Should().Contain("north"); + } + + [TestMethod] + [Description("After mapping under the sensitive default, switching to case-insensitive mode and unhiding through opposite casing restores one deduplicated discovery representative while both spellings remain parsable.")] + public void When_GlobalCaseModeChangesAfterMappingAndAliasIsUnhidden_Then_OneEquivalentSpellingReturnsToDiscovery() + { + var sut = ReplApp.Create(); + var command = sut.Map( + "publish", + static string ([ReplOption(Name = "tenant", Aliases = ["--account", "--ACCOUNT"])] string? tenant = null) => tenant ?? "none"); + sut.Options(static options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + command.WithOption("tenant", static option => + { + option.HiddenAlias("--account"); + option.HiddenAlias("--ACCOUNT", isHidden: false); + }); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--help", "--no-logo"])); + var lower = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--account", "south", "--no-logo"])); + var upper = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--ACCOUNT", "north", "--no-logo"])); + var option = sut.CreateDocumentationModel().Commands.Single().Options.Single(); + + help.Text.Should().Contain("--account"); + option.Aliases.Should().Contain("--account"); + lower.ExitCode.Should().Be(0, lower.Text); + upper.ExitCode.Should().Be(0, upper.Text); + } + + [TestMethod] + [Description("A per-option case-sensitive override wins over a case-insensitive global mode, so hiding one exact fluent alias leaves its case-distinct sibling discoverable and both exact spellings parsable.")] + public void When_FluentAliasUsesSensitiveOverrideUnderInsensitiveGlobal_Then_OnlyExactSpellingIsHiddenFromDiscovery() + { + var sut = ReplApp.Create(); + sut.Options(static options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + sut.Map( + "publish", + static string ([ReplOption(Name = "tenant", Aliases = ["--account", "--ACCOUNT"], CaseSensitivity = ReplCaseSensitivity.CaseSensitive)] string? tenant = null) => tenant ?? "none") + .WithOption("tenant", static option => option.HiddenAlias("--account")); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--help", "--no-logo"])); + var lower = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--account", "south", "--no-logo"])); + var upper = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--ACCOUNT", "north", "--no-logo"])); + var option = sut.CreateDocumentationModel().Commands.Single().Options.Single(); + + help.Text.Should().NotContain("--account").And.Contain("--ACCOUNT"); + option.Aliases.Should().NotContain("--account").And.Contain("--ACCOUNT"); + lower.ExitCode.Should().Be(0, lower.Text); + upper.ExitCode.Should().Be(0, upper.Text); + } + + [TestMethod] + [Description("A manually registered global alias can be hidden independently: root help omits only that alias while the pre-routing parser still accepts it and exposes its value through the global accessor.")] + public void When_GlobalAliasIsHidden_Then_HelpOmitsItButParsingRetainsIt() + { + var sut = ReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("tenant", aliases: ["--account"]); + options.Parsing.GlobalOption("tenant").HiddenAlias("--account"); + }); + sut.Map("show", static string (IGlobalOptionsAccessor globals) => globals.GetValue("tenant") ?? "none"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + var legacy = ConsoleCaptureHelper.Capture(() => sut.Run(["--account", "acme", "show", "--no-logo"])); + + help.Text.Should().Contain("--tenant"); + help.Text.Should().NotContain("--account"); + legacy.ExitCode.Should().Be(0, legacy.Text); + legacy.Text.Should().Contain("acme"); + } + + [TestMethod] + [Description("Global hide followed by opposite-case unhide uses the current case-insensitive comparer, restoring one deduplicated help representative while both spellings remain parsable.")] + public void When_GlobalAliasIsUnhiddenThroughEquivalentCasing_Then_OneRepresentativeReturnsToHelp() + { + var sut = ReplApp.Create(); + sut.Options(options => + options.Parsing.AddGlobalOption("organization", aliases: ["--legacy-org", "--LEGACY-ORG"])); + sut.Options(options => + { + options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive; + options.Parsing.GlobalOption("organization").HiddenAlias("--legacy-org"); + options.Parsing.GlobalOption("organization").HiddenAlias("--LEGACY-ORG", isHidden: false); + }); + sut.Map("show", static string (IGlobalOptionsAccessor globals) => + globals.GetValue("organization") ?? "none"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + var lower = ConsoleCaptureHelper.Capture(() => + sut.Run(["--legacy-org", "south", "show", "--no-logo"])); + var upper = ConsoleCaptureHelper.Capture(() => + sut.Run(["--LEGACY-ORG", "north", "show", "--no-logo"])); + + help.Text.Should().Contain("--legacy-org"); + lower.ExitCode.Should().Be(0, lower.Text); + lower.Text.Should().Contain("south"); + upper.ExitCode.Should().Be(0, upper.Text); + upper.Text.Should().Contain("north"); + } + + [TestMethod] + [Description("A global registered under case-sensitive parsing with canonical --tenant and a case-distinct hidden alias --TENANT must keep its canonical token visible after switching to case-insensitive parsing: the canonical token's visibility is governed by the definition's own IsHidden, never by HiddenAliases becoming case-equivalent to it.")] + public void When_CaseDistinctHiddenAliasBecomesEquivalentToCanonicalAfterModeChange_Then_CanonicalStaysVisible() + { + var sut = ReplApp.Create(); + sut.Options(options => + options.Parsing.AddGlobalOption("tenant", aliases: ["--TENANT"])); + sut.Options(options => options.Parsing.GlobalOption("tenant").HiddenAlias("--TENANT")); + sut.Options(options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + sut.Map("show", static string (IGlobalOptionsAccessor globals) => globals.GetValue("tenant") ?? "none"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["--help", "--no-logo"])); + var canonical = ConsoleCaptureHelper.Capture(() => sut.Run(["--tenant", "acme", "show", "--no-logo"])); + + help.Text.Should().Contain("--tenant"); + help.Text.Should().NotContain("--TENANT"); + canonical.ExitCode.Should().Be(0, canonical.Text); + canonical.Text.Should().Contain("acme"); + } + + [TestMethod] + [Description("A hidden command option stays bindable when explicitly provided but is omitted from command help.")] + public void When_CommandOptionIsHidden_Then_HelpOmitsItAndExplicitInvocationStillBinds() + { + var sut = ReplApp.Create(); + sut.Map( + "deploy", + ([ReplOption(Name = "environment")] string environment, [ReplOption(Name = "internal-mode")] bool internalMode = false) => + $"{environment}:{internalMode}") + .WithOption("internalMode", static option => option.Hidden()); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--help", "--no-logo"])); + var invocation = ConsoleCaptureHelper.Capture(() => + sut.Run(["deploy", "--environment", "prod", "--internal-mode", "--no-logo"])); + + help.ExitCode.Should().Be(0); + help.Text.Should().Contain("--environment"); + help.Text.Should().NotContain("--internal-mode"); + invocation.ExitCode.Should().Be(0, invocation.Text); + invocation.Text.Should().Contain("prod:True"); + } + + [TestMethod] + [Description("Command help must follow global-parser precedence token by token: omit a route canonical token owned by a hidden global, retain a route-owned alias, and leave that alias directly invocable.")] + public void When_HiddenGlobalOwnsRouteCanonicalToken_Then_CommandHelpKeepsOnlyTheRouteAlias() + { + var sut = ReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("tenant"); + options.Parsing.GlobalOption("tenant").Hidden(); + }); + sut.Map( + "deploy", + static string ([ReplOption(Aliases = ["-t"])] string? tenant = null) => tenant ?? "none"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--help", "--no-logo"])); + var invocation = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "-t", "acme", "--no-logo"])); + + help.ExitCode.Should().Be(0, help.Text); + help.Text.Should().Contain("-t"); + help.Text.Should().NotContain("--tenant"); + invocation.ExitCode.Should().Be(0, invocation.Text); + invocation.Text.Should().Contain("acme"); + } + + [TestMethod] + [Description("Global case-insensitive parsing makes case-equivalent visible and hidden aliases one logical token; hidden precedence keeps both spellings out of help/docs while parsing remains backwards-compatible.")] + public void When_GlobalCaseInsensitiveModeMakesVisibleAliasEquivalentToHiddenAlias_Then_HiddenPrecedenceWins() + { + var sut = CoreReplApp.Create(); + sut.Options(options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + sut.Map( + "deploy", + static string ([ReplOption( + Name = "tenant", + Aliases = ["--ACCOUNT"], + HiddenAliases = ["--account"])] string? tenant = null) => tenant ?? "none"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--help", "--no-logo"])); + var invocation = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--ACCOUNT", "acme", "--no-logo"])); + var option = sut.CreateDocumentationModel().Commands.Single().Options.Single(); + + help.Text.Should().Contain("--tenant"); + help.Text.Should().NotContain("--account"); + help.Text.Should().NotContain("--ACCOUNT"); + invocation.ExitCode.Should().Be(0, invocation.Text); + invocation.Text.Should().Contain("acme"); + option.Aliases.Should().ContainSingle().Which.Should().Be("--tenant"); + } + + [TestMethod] + [Description("The programmatic-only axis must not leak into human surfaces: an AutomationHidden option stays listed in command help and binds normally from the command line. Only the MCP tool schema drops it.")] + public void When_CommandOptionIsAutomationHidden_Then_HelpStillListsItAndItBinds() + { + var sut = ReplApp.Create(); + sut.Map( + "deploy", + ([ReplOption(Name = "environment")] string environment, [ReplOption(Name = "internal-mode")] bool internalMode = false) => + $"{environment}:{internalMode}") + .WithOption("internalMode", static option => option.AutomationHidden()); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--help", "--no-logo"])); + var invocation = ConsoleCaptureHelper.Capture(() => + sut.Run(["deploy", "--environment", "prod", "--internal-mode", "--no-logo"])); + + help.ExitCode.Should().Be(0); + help.Text.Should().Contain("--internal-mode"); + invocation.ExitCode.Should().Be(0, invocation.Text); + invocation.Text.Should().Contain("prod:True"); + } + + [TestMethod] + [Description("A direct handler option may receive an external provider only after fluent metadata is complete, so Hidden() must defer the provider-dependent decision. Aggregate discovery without a fallback then rejects the impossible hidden contract before omitting the option.")] + public void When_RequiredCommandOptionIsHiddenFluentlyWithoutAService_Then_AggregateDocumentationThrows() + { + var sut = ReplApp.Create(); + var command = sut.Map( + "deploy", + ([ReplOption(Name = "internal-token", Arity = ReplArity.ExactlyOne)] string internalToken) => internalToken); + + var hide = () => command.WithOption("internalToken", option => option.Hidden()); + hide.Should().NotThrow(); + var document = () => sut.CreateDocumentationModel(); + + document.Should().Throw() + .WithMessage("Option target 'internalToken' (rendered as '--internal-token') for command 'deploy' cannot be hidden because it is required. Either drop the Required/Arity constraint on the option, or hide a different one."); + } + + [TestMethod] + [Description("A service registration that throws on resolution — a scoped registration resolved from the root provider, or any constructor that fails — must be treated as unavailable rather than letting that registration's own exception crash discovery. The caller must see the standard hidden-required diagnosis, not whatever the broken registration happened to throw.")] + public void When_ServiceFallbackRegistrationThrowsOnResolution_Then_TreatedAsUnavailable() + { + var sut = ReplApp.Create(services => + services.AddSingleton(_ => throw new InvalidOperationException("Simulated resolution failure"))); + var command = sut.Map( + "deploy", + ([ReplOption(Name = "internal-token", Arity = ReplArity.ExactlyOne)] string internalToken) => internalToken); + command.WithOption("internalToken", option => option.Hidden()); + + var document = () => sut.CreateDocumentationModel(); + + document.Should().Throw() + .WithMessage("Option target 'internalToken' (rendered as '--internal-token') for command 'deploy' cannot be hidden because it is required.*"); + } + + [TestMethod] + [Description("A registered service is an established omission fallback in HandlerArgumentBinder and runs before the explicit-arity failure. Both fluent and declarative hiding must therefore remain legal: callers can omit the options and DI still supplies the handler values.")] + public void When_RequiredOptionTypeHasAServiceFallback_Then_HidingAndOmissionAreAllowed() + { + var sut = ReplApp.Create(services => services.AddSingleton("service-token")); + var command = sut.Map( + "deploy", + ([ReplOption(Name = "token", Arity = ReplArity.ExactlyOne)] string token) => token); + + var fluentAct = () => command.WithOption("token", static option => option.Hidden()); + var declarativeAct = () => sut.Map( + "publish", + ([ReplOption(Name = "token", Arity = ReplArity.ExactlyOne, Hidden = true)] string token) => token); + + fluentAct.Should().NotThrow(); + declarativeAct.Should().NotThrow(); + var deploy = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--no-logo"])); + var publish = ConsoleCaptureHelper.Capture(() => sut.Run(["publish", "--no-logo"])); + deploy.ExitCode.Should().Be(0, deploy.Text); + deploy.Text.Should().Contain("service-token"); + publish.ExitCode.Should().Be(0, publish.Text); + publish.Text.Should().Contain("service-token"); + } + + [TestMethod] + [Description("IProgress is synthesized from IReplInteractionChannel before direct service lookup. Provider-aware hidden-option validation must recognize that established fallback so documentation and argument-free binding agree.")] + public void When_RequiredHiddenProgressOptionUsesInteractionChannel_Then_DiscoveryAndOmissionAreAllowed() + { + var sut = ReplApp.Create(); + sut.Map( + "sync", + ([ReplOption(Name = "progress", Arity = ReplArity.ExactlyOne, Hidden = true)] IProgress progress) => + progress is not null ? "progress-ready" : "missing"); + + var document = () => sut.CreateDocumentationModel(); + var invocation = ConsoleCaptureHelper.Capture(() => sut.Run(["sync", "--no-logo"])); + + document.Should().NotThrow(); + invocation.ExitCode.Should().Be(0); + invocation.Text.Should().Contain("progress-ready"); + } + + [TestMethod] + [Description("The DI-enabled facade must build programmatic documentation with the same shared provider used by Run. Otherwise a configured service-backed hidden option runs successfully but aggregate documentation rejects it as impossible.")] + public void When_ProgrammaticDocumentationUsesConfiguredServices_Then_HiddenRequiredOptionIsAllowed() + { + var sut = ReplApp.Create(services => services.AddSingleton("service-token")); + sut.Map( + "deploy", + ([ReplOption(Name = "token", Arity = ReplArity.ExactlyOne, Hidden = true)] string token) => token); + + var document = () => sut.CreateDocumentationModel(); + + document.Should().NotThrow(); + } + + [TestMethod] + [Description("Visible option requiredness in programmatic documentation must use the configured service provider too, matching the argument-free invocation that HandlerArgumentBinder accepts.")] + public void When_ProgrammaticDocumentationUsesConfiguredServices_Then_VisibleRequiredOptionIsOptional() + { + var sut = ReplApp.Create(services => services.AddSingleton("service-token")); + sut.Map( + "deploy", + ([ReplOption(Name = "token", Arity = ReplArity.ExactlyOne)] string token) => token); + + var model = sut.CreateDocumentationModel(); + var option = model.Commands + .Single(command => string.Equals(command.Path, "deploy", StringComparison.Ordinal)) + .Options.Single(); + + option.Required.Should().BeFalse(); + } + + [TestMethod] + [Description("Programmatic documentation needs the same explicit-provider seam as Run so externally managed/custom providers supplied after mapping can determine hidden-option invocability.")] + public void When_ProgrammaticDocumentationUsesAnExternalProvider_Then_RequiredHiddenOptionIsAllowed() + { + using var services = new ServiceCollection() + .AddSingleton("external-token") + .BuildServiceProvider(); + var sut = ReplApp.Create(); + sut.Map( + "deploy", + ([ReplOption(Name = "token", Arity = ReplArity.ExactlyOne, Hidden = true)] string token) => token); + var document = () => sut.CreateDocumentationModel(services); + + document.Should().NotThrow(); + } + + [TestMethod] + [Description("An external provider is supplied only at Run time, after mapping and fluent metadata are complete. Required hidden validation must therefore defer its provider-dependent decision and let the binder use that provider when the option is omitted.")] + public void When_RequiredHiddenOptionUsesAnExternalServiceProvider_Then_OmissionIsAllowed() + { + using var services = new ServiceCollection() + .AddSingleton("external-token") + .BuildServiceProvider(); + var sut = ReplApp.Create(); + + var command = sut.Map( + "deploy", + ([ReplOption(Name = "token", Arity = ReplArity.ExactlyOne, Hidden = true)] string token) => token); + var run = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--no-logo"], services)); + + command.Should().NotBeNull(); + run.ExitCode.Should().Be(0, run.Text); + run.Text.Should().Contain("external-token"); + } + + [TestMethod] + [Description("Custom globals parse before route options. Aggregate documentation must omit globally owned route tokens so MCP cannot advertise an argument that execution will bind to the global instead; unrelated route options remain visible.")] + public void When_HiddenGlobalOwnsRouteOptionToken_Then_AggregateDocumentationOmitsThatRouteOption() + { + var sut = CoreReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("tenant"); + options.Parsing.GlobalOption("tenant").Hidden(); + }); + sut.Map( + "deploy", + static string ( + [ReplOption] string? tenant = null, + [ReplOption] string? region = null) => $"{tenant}:{region}"); + + var command = sut.CreateDocumentationModel().Commands.Single(candidate => + string.Equals(candidate.Path, "deploy", StringComparison.Ordinal)); + + command.Options.Should().NotContain(option => string.Equals(option.Name, "tenant", StringComparison.Ordinal)); + command.Options.Should().Contain(option => string.Equals(option.Name, "region", StringComparison.Ordinal)); + } + + [TestMethod] + [Description("A globally owned secondary route alias must be removed from documentation without changing the route option's canonical MCP argument name, which the adapter reconstructs as a long option.")] + public void When_HiddenGlobalOwnsSecondaryRouteAlias_Then_DocumentationKeepsOnlyTheCanonicalRouteToken() + { + var sut = CoreReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("t"); + options.Parsing.GlobalOption("t").Hidden(); + }); + sut.Map( + "deploy", + static string ([ReplOption(Aliases = ["--t"])] string? tenant = null) => tenant ?? "none"); + + var option = sut.CreateDocumentationModel().Commands.Single().Options.Single(); + + option.Name.Should().Be("tenant"); + option.Aliases.Should().Contain("--tenant"); + option.Aliases.Should().NotContain("--t"); + } + + [TestMethod] + [Description("A route option remains documentable when its canonical token belongs to a global but an alias remains route-owned. The semantic programmatic name stays stable while the surviving alias remains the exact CLI invocation token.")] + public void When_HiddenGlobalOwnsRouteCanonicalToken_Then_DocumentationKeepsStableNameAndRouteAlias() + { + var sut = CoreReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("tenant"); + options.Parsing.GlobalOption("tenant").Hidden(); + }); + sut.Map( + "deploy", + static string ([ReplOption(Aliases = ["-t"])] string? tenant = null) => tenant ?? "none"); + + var option = sut.CreateDocumentationModel().Commands.Single().Options.Single(); + + option.Name.Should().Be("tenant"); + option.Aliases.Should().ContainSingle().Which.Should().Be("-t"); + } + + [TestMethod] + [Description("If a global shadows the only canonical token of a required route option, omitting the unreachable field would advertise a dead programmatic contract. Aggregate discovery must fail closed unless a runtime fallback can satisfy it.")] + public void When_HiddenGlobalOwnsRequiredRouteOptionToken_Then_AggregateDocumentationFailsClosed() + { + var sut = CoreReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("tenant"); + options.Parsing.GlobalOption("tenant").Hidden(); + }); + sut.Map( + "deploy", + static string ([ReplOption(Arity = ReplArity.ExactlyOne)] string tenant) => tenant); + + var act = () => sut.CreateDocumentationModel(); + + act.Should().Throw() + .WithMessage("*'tenant'*'--tenant'*cannot be hidden because it is required*"); + } + + [TestMethod] + [Description("DI fallback applies to direct handler parameters only. The binder constructs an options group before its service fallback, so registering the same property type must not make a hidden required group property look invocable.")] + public void When_HiddenRequiredGroupPropertyTypeIsRegisteredAsAService_Then_MappingStillThrows() + { + var sut = ReplApp.Create(services => services.AddSingleton("service-token")); + + var act = () => sut.Map("deploy", (HiddenRequiredOptions options) => options.Token); + + act.Should().Throw() + .WithMessage("*'Token'*cannot be hidden because it is required*"); + } + + [TestMethod] + [Description("The typo suggester is a discovery surface like any other. Searching the full token set turns a validation error into a way to enumerate hidden options by probing at small edit distance, which defeats hiding on the one surface that has no listing of its own.")] + public void When_MistypingAHiddenOption_Then_TheSuggestionDoesNotRevealIt() + { + var sut = ReplApp.Create(); + sut.Map( + "deploy", + ([ReplOption(Name = "environment")] string environment, [ReplOption(Name = "secret")] string? secret = null) => + $"{environment}:{secret}") + .WithOption("secret", static option => option.Hidden()); + + var hidden = ConsoleCaptureHelper.Capture(() => + sut.Run(["deploy", "--environment", "prod", "--secre", "x", "--no-logo"])); + var visible = ConsoleCaptureHelper.Capture(() => + sut.Run(["deploy", "--environmnet", "prod", "--no-logo"])); + + hidden.Text.Should().NotContain("--secret"); + visible.Text.Should().Contain("--environment", "a visible option is still suggested, so the filter is not simply disabling suggestions"); + } + + [TestMethod] + [Description("A hidden global that owns a visible route token must suppress that unreachable token from typo suggestions just as it is suppressed from help, completion, and documentation; an unrelated route option remains a positive-control suggestion.")] + public void When_HiddenGlobalOwnsVisibleRouteToken_Then_TypoSuggestionDoesNotRevealIt() + { + var sut = CoreReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("secret"); + options.Parsing.GlobalOption("secret").Hidden(); + }); + sut.Map( + "deploy", + static string ([ReplOption] string? secret = null, [ReplOption] string? region = null) => $"{secret}:{region}"); + + var hidden = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--secre", "x", "--no-logo"])); + var visible = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--regoin", "x", "--no-logo"])); + + hidden.Text.Should().NotContain("Did you mean '--secret'"); + visible.Text.Should().Contain("Did you mean '--region'", "route suggestions must remain enabled"); + } + + [TestMethod] + [Description("An inherited declarative hidden alias is reevaluated when the global case mode changes after mapping: once the parser considers the visible and hidden spellings equivalent, discovery hides both aliases while the canonical token and parsing remain available.")] + public void When_GlobalCaseModeChangesAfterMapping_Then_DeclarativeHiddenAliasPrecedenceIsReevaluated() + { + var sut = CoreReplApp.Create(); + sut.Map( + "deploy", + static string ([ReplOption(Name = "tenant", Aliases = ["--ACCOUNT"], HiddenAliases = ["--account"])] string? tenant = null) => tenant ?? "none"); + sut.Options(options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--help", "--no-logo"])); + var invocation = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--ACCOUNT", "acme", "--no-logo"])); + var option = sut.CreateDocumentationModel().Commands.Single().Options.Single(); + + help.Text.Should().Contain("--tenant").And.NotContain("--account").And.NotContain("--ACCOUNT"); + option.Aliases.Should().ContainSingle().Which.Should().Be("--tenant"); + invocation.ExitCode.Should().Be(0, invocation.Text); + invocation.Text.Should().Contain("acme"); + } + + [TestMethod] + [Description("Human documentation retains an option when a global shadows its ordinary flag but an unowned reverse alias remains invocable; help and execution expose the same reachable reverse token.")] + public void When_GlobalOwnsOrdinaryFlagButReverseAliasRemains_Then_HumanDocumentationRetainsReverseAlias() + { + var sut = CoreReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("force"); + options.Parsing.GlobalOption("force").Hidden(); + }); + sut.Map( + "deploy", + static string ([ReplOption(ReverseAliases = ["--no-force"])] bool force = true) => force.ToString()); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--help", "--no-logo"])); + var invocation = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--no-force", "--no-logo"])); + var option = sut.CreateDocumentationModel().Commands.Single().Options.Single(); + + help.Text.Should().Contain("--no-force").And.NotContain("--force"); + option.Aliases.Should().BeEmpty(); + option.ReverseAliases.Should().ContainSingle().Which.Should().Be("--no-force"); + var publicRoundTrip = new Repl.Documentation.ReplDocOption( + option.Name, + option.Type, + option.Required, + option.Description, + option.Aliases, + option.ReverseAliases, + option.ValueAliases, + option.EnumValues, + option.DefaultValue) + { + IsHidden = option.IsHidden, + IsAutomationHidden = option.IsAutomationHidden, + }; + option.Equals(publicRoundTrip).Should().BeTrue("MCP-only derivations must not alter public record equality"); + option.GetHashCode().Should().Be(publicRoundTrip.GetHashCode()); + invocation.ExitCode.Should().Be(0, invocation.Text); + invocation.Text.Should().Contain("False"); + } + + [TestMethod] + [Description("A nullable reference option with no default has its arity inferred as ExactlyOne, but arity governs how many values the token consumes when present — omitting it binds null perfectly well. Hiding it must therefore be allowed; only an explicitly declared required arity should override what the CLR shape says.")] + public void When_HidingAnOmittableOptionWithInferredArity_Then_ItIsAllowed() + { + var sut = ReplApp.Create(); + var command = sut.Map( + "deploy", + ([ReplOption(Name = "token", Mode = ReplParameterMode.OptionOnly)] string? token) => token ?? "none"); + + var act = () => command.WithOption("token", static option => option.Hidden()); + + act.Should().NotThrow(); + var run = ConsoleCaptureHelper.Capture(() => sut.Run(["deploy", "--no-logo"])); + run.ExitCode.Should().Be(0, run.Text); + run.Text.Should().Contain("none"); + } + + [TestMethod] + [Description("A non-defaulted bool in OptionAndPositional mode may still come from an external provider, so hiding is deferred. Without such a provider, aggregate discovery rejects it because MCP cannot express the positional fallback for an omitted hidden option.")] + public void When_HidingANonOmittableFlagInPositionalModeWithoutAService_Then_AggregateDocumentationThrows() + { + var sut = ReplApp.Create(); + var command = sut.Map("deploy", ([ReplOption(Name = "force")] bool force) => force.ToString()); + + command.WithOption("force", static option => option.Hidden()); + var act = () => sut.CreateDocumentationModel(); + + act.Should().Throw() + .WithMessage("*'force'*cannot be hidden because it is required*"); + } + + [TestMethod] + [Description("A non-nullable value-type option with no default requires either a token or a service. Fluent hiding remains compatible with a provider supplied later, but aggregate discovery without that provider must reject the otherwise impossible command.")] + public void When_HidingAnOptionThatCannotBeOmittedWithoutAService_Then_AggregateDocumentationThrows() + { + var sut = ReplApp.Create(); + var command = sut.Map( + "deploy", + ([ReplOption(Name = "force", Mode = ReplParameterMode.OptionOnly)] bool force) => force.ToString()); + + command.WithOption("force", static option => option.Hidden()); + var act = () => sut.CreateDocumentationModel(); + + act.Should().Throw() + .WithMessage("*'force'*cannot be hidden because it is required*"); + } + + [TestMethod] + [Description("Declarative hiding has the same provider timing as fluent hiding: Map cannot reject a direct parameter before an external provider is known. Aggregate discovery later evaluates the active provider and rejects the no-service contract.")] + public void When_RequiredCommandOptionIsHiddenByAttributeWithoutAService_Then_AggregateDocumentationThrows() + { + var sut = ReplApp.Create(); + + var map = () => sut.Map( + "deploy", + ([ReplOption(Name = "internal-token", Arity = ReplArity.ExactlyOne, Hidden = true)] string internalToken) => internalToken); + map.Should().NotThrow(); + var document = () => sut.CreateDocumentationModel(); + + document.Should().Throw() + .WithMessage("Option target 'internalToken' (rendered as '--internal-token') for command 'deploy' cannot be hidden because it is required. Either drop the Required/Arity constraint on the option, or hide a different one."); + } + [TestMethod] [Description("Regression guard: verifies requesting command help for aliased command so that aliases are shown.")] public void When_RequestingCommandHelpForAliasedCommand_Then_AliasesAreShown() @@ -514,6 +1225,13 @@ public void When_RequestingCommandHelpWithGlobalOptionsAccessor_Then_AccessorIsN private static string SendHandler([ComponentDescriptionAttribute("Message to send to all watching sessions")] string message) => message; + [ReplOptionsGroup] + private sealed class HiddenRequiredOptions + { + [ReplOption(Name = "token", Arity = ReplArity.ExactlyOne, Hidden = true)] + public string Token { get; set; } = null!; + } + private enum HelpMode { Fast, diff --git a/src/Repl.IntegrationTests/Given_OptionAttributeOverrides.cs b/src/Repl.IntegrationTests/Given_OptionAttributeOverrides.cs index 24187f78..aa54f510 100644 --- a/src/Repl.IntegrationTests/Given_OptionAttributeOverrides.cs +++ b/src/Repl.IntegrationTests/Given_OptionAttributeOverrides.cs @@ -37,6 +37,12 @@ public sealed class OverrideArityGlobals public int Retries { get; set; } = 42; } + public sealed class AutomationHiddenGlobals + { + [ReplOption(Name = "internal-tenant", AutomationHidden = true)] + public string? InternalTenant { get; set; } + } + public sealed class ValueAliasOverrideGlobals { [ReplValueAlias("--prod", "production", CaseSensitivity = ReplCaseSensitivity.CaseInsensitive)] @@ -356,6 +362,16 @@ public void When_GlobalOptionsPropertyDeclaresArityOverride_Then_RegistrationFai act.Should().Throw().WithMessage("*Arity*"); } + [TestMethod] + [Description("Global options are consumed before routing and never enter the documentation model, so AutomationHidden on one could never do anything. The fluent path prevents this by type — GlobalOptionBuilder has no such method — but an attribute cannot be type-split, so this branch must fail fast rather than silently discard the flag. Hidden, by contrast, is supported here and must keep working.")] + public void When_GlobalOptionsPropertyDeclaresAutomationHidden_Then_RegistrationFailsFast() + { + var act = () => ReplApp.Create().UseGlobalOptions(); + + act.Should().Throw() + .WithMessage("*AutomationHiddenGlobals.InternalTenant*AutomationHidden*never exposed to programmatic surfaces*"); + } + [TestMethod] [Description("Guards the value-alias branch of the typed-global-options fail-fast: a [ReplValueAlias(..., CaseSensitivity = ...)] override on a global property is equally newly settable and must be rejected instead of silently discarded.")] public void When_GlobalOptionsValueAliasDeclaresOverride_Then_RegistrationFailsFast() diff --git a/src/Repl.IntegrationTests/Given_OptionsGroupBinding.cs b/src/Repl.IntegrationTests/Given_OptionsGroupBinding.cs index d0f1ff38..530b9b11 100644 --- a/src/Repl.IntegrationTests/Given_OptionsGroupBinding.cs +++ b/src/Repl.IntegrationTests/Given_OptionsGroupBinding.cs @@ -15,6 +15,27 @@ public class TestOutputOptions public bool Verbose { get; set; } } + [ReplOptionsGroup] + public class HiddenOutputOptions + { + [ReplOption] + public string Format { get; set; } = "text"; + + [ReplOption(Name = "internal-token", Hidden = true)] + public string? InternalToken { get; set; } + } + + [ReplOptionsGroup] + public class LegacyOutputOptions + { + [ReplOption( + Name = "format", + Aliases = ["--OUTPUT-FORMAT"], + HiddenAliases = ["--output-format"], + CaseSensitivity = ReplCaseSensitivity.CaseInsensitive)] + public string Format { get; set; } = "text"; + } + [ReplOptionsGroup] public class TestPagingOptions { @@ -138,6 +159,58 @@ public void When_UsingTwoGroups_Then_BothBindIndependently() output.Text.Should().Contain("json:20:5"); } + [TestMethod] + [Description("A hidden options-group property is omitted from help while remaining bindable when explicitly provided.")] + public void When_OptionsGroupPropertyIsHidden_Then_HelpOmitsItAndExplicitInvocationStillBinds() + { + var sut = ReplApp.Create(); + sut.Map("list", (HiddenOutputOptions options) => $"{options.Format}:{options.InternalToken}"); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["list", "--help", "--no-logo"])); + var invocation = ConsoleCaptureHelper.Capture(() => sut.Run( + ["list", "--format", "json", "--internal-token", "secret", "--no-logo"])); + + help.ExitCode.Should().Be(0); + help.Text.Should().Contain("--format"); + help.Text.Should().NotContain("--internal-token"); + invocation.ExitCode.Should().Be(0, invocation.Text); + invocation.Text.Should().Contain("json:secret"); + } + + [TestMethod] + [Description("A hidden alias declared on an options-group property remains bindable while help and documentation expose only the canonical token.")] + public void When_OptionsGroupPropertyAliasIsHidden_Then_OnlyParsingRetainsIt() + { + var sut = ReplApp.Create(); + sut.Map("list", static string (LegacyOutputOptions options) => options.Format); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["list", "--help", "--no-logo"])); + var invocation = ConsoleCaptureHelper.Capture(() => sut.Run(["list", "--output-format", "json", "--no-logo"])); + var option = sut.CreateDocumentationModel().Commands.Single().Options.Single(); + + help.Text.Should().Contain("--format"); + help.Text.Should().NotContain("--output-format"); + help.Text.Should().NotContain("--OUTPUT-FORMAT"); + invocation.ExitCode.Should().Be(0, invocation.Text); + invocation.Text.Should().Contain("json"); + option.Aliases.Should().Contain("--format"); + option.Aliases.Should().NotContain("--output-format"); + } + + [TestMethod] + [Description("A fluent Hidden(false) override re-exposes an options-group property hidden by attribute.")] + public void When_HiddenAttributeIsOverriddenFluentlyWithFalse_Then_OptionIsVisibleAgain() + { + var sut = ReplApp.Create(); + sut.Map("list", (HiddenOutputOptions options) => "ok") + .WithOption(nameof(HiddenOutputOptions.InternalToken), static option => option.Hidden(isHidden: false)); + + var help = ConsoleCaptureHelper.Capture(() => sut.Run(["list", "--help", "--no-logo"])); + + help.ExitCode.Should().Be(0); + help.Text.Should().Contain("--internal-token"); + } + [TestMethod] [Description("Regression guard: a nullable group property initialized to the CLR default of its underlying type (int? = 0, bool? = false) is a deliberate default the binder preserves, so command help must advertise it — while implicit defaults of non-nullable properties stay hidden.")] public void When_NullableGroupPropertyInitializedToUnderlyingClrDefault_Then_HelpShowsDefault() diff --git a/src/Repl.Mcp/McpAutomationProjection.cs b/src/Repl.Mcp/McpAutomationProjection.cs new file mode 100644 index 00000000..cf72059a --- /dev/null +++ b/src/Repl.Mcp/McpAutomationProjection.cs @@ -0,0 +1,57 @@ +using Repl.Documentation; + +namespace Repl.Mcp; + +/// +/// Removes automation-hidden options from a documentation model before anything MCP-facing reads it, +/// and withdraws any command that this would leave impossible to invoke. +/// +/// +/// Applied once, at the model boundary, rather than at each MCP emitter. The generated tool schema +/// does not declare additionalProperties: false, so 's argument +/// allow-list is the only thing that turns an unadvertised option into a hard error. Should schema +/// generation and allow-listing ever read different option lists, an option would silently vanish +/// from the schema while remaining callable — projecting once makes that divergence unrepresentable. +/// +/// Fully hidden options never arrive here: MCP always builds the aggregate documentation model, which +/// omits them already. Only the programmatic-only axis needs filtering. +/// +/// +internal static class McpAutomationProjection +{ + public static ReplDocumentationModel Apply(ReplDocumentationModel model) + { + if (!model.Commands.Any(HasUnavailableOption)) + { + return model; + } + + var projected = model.Commands + .Where(static command => !command.Options.Any(static option => IsUnavailable(option) && option.Required)) + .Select(static command => HasUnavailableOption(command) + ? command with { Options = [.. command.Options.Where(static option => !IsUnavailable(option))] } + : command) + .ToArray(); + + // Resources are projected from the command list, so a withdrawn command must not linger there. + var retained = projected + .Select(static command => command.Path) + .ToHashSet(StringComparer.OrdinalIgnoreCase); + + return model with + { + Commands = projected, + Resources = [.. model.Resources.Where(resource => retained.Contains(resource.Path))], + }; + } + + // Withholding an option a caller cannot omit leaves no valid invocation at all: the client cannot + // supply it and omitting it fails to bind, so every call would fail. Withdrawing the tool is + // preferable to advertising an impossible one — and unlike the all-surfaces Hidden axis this is + // not rejected at configuration time, because the human command line remains perfectly usable. + private static bool HasUnavailableOption(ReplDocCommand command) => + command.Options.Any(static option => IsUnavailable(option)); + + private static bool IsUnavailable(ReplDocOption option) => + option.IsAutomationHidden || option.Aliases.Count == 0; +} diff --git a/src/Repl.Mcp/McpInteractionChannel.cs b/src/Repl.Mcp/McpInteractionChannel.cs index 1d8ea662..ab6531ad 100644 --- a/src/Repl.Mcp/McpInteractionChannel.cs +++ b/src/Repl.Mcp/McpInteractionChannel.cs @@ -13,7 +13,7 @@ namespace Repl.Mcp; /// internal sealed class McpInteractionChannel : IReplInteractionChannel { - private readonly IReadOnlyDictionary _prefillAnswers; + private readonly Dictionary _prefillAnswers; private readonly InteractivityMode _mode; private readonly McpServer? _server; private readonly ProgressToken? _progressToken; @@ -26,7 +26,7 @@ public McpInteractionChannel( ProgressToken? progressToken = null, IMcpFeedback? feedback = null) { - _prefillAnswers = prefillAnswers; + _prefillAnswers = new Dictionary(prefillAnswers, StringComparer.Ordinal); _mode = mode; _server = server; _progressToken = progressToken; @@ -40,7 +40,7 @@ public async ValueTask AskChoiceAsync( int? defaultIndex = null, AskOptions? options = null) { - if (_prefillAnswers.TryGetValue(name, out var prefill)) + if (TryGetPrefill(name, out var prefill)) { return ResolveChoiceIndex(prefill, choices); } @@ -77,7 +77,7 @@ public async ValueTask AskConfirmationAsync( bool? defaultValue = null, AskOptions? options = null) { - if (_prefillAnswers.TryGetValue(name, out var prefill)) + if (TryGetPrefill(name, out var prefill)) { return ParseBool(prefill); } @@ -113,7 +113,7 @@ public async ValueTask AskTextAsync( string? defaultValue = null, AskOptions? options = null) { - if (_prefillAnswers.TryGetValue(name, out var prefill)) + if (TryGetPrefill(name, out var prefill)) { return prefill; } @@ -149,7 +149,7 @@ public ValueTask AskSecretAsync( AskSecretOptions? options = null) { // Secrets: prefill ONLY, never elicitation or sampling (security). - if (_prefillAnswers.TryGetValue(name, out var prefill)) + if (TryGetPrefill(name, out var prefill)) { return ValueTask.FromResult(prefill); } @@ -166,7 +166,7 @@ public async ValueTask> AskMultiChoiceAsync( IReadOnlyList? defaultIndices = null, AskMultiChoiceOptions? options = null) { - if (_prefillAnswers.TryGetValue(name, out var prefill)) + if (TryGetPrefill(name, out var prefill)) { return ParseMultiChoice(prefill, choices, options); } @@ -538,6 +538,34 @@ private static int ResolveChoiceIndex(string value, IReadOnlyList choice $"Cannot resolve choice '{value}'. Available: {string.Join(", ", choices)}"); } + private bool TryGetPrefill(string name, out string prefill) + { + if (_prefillAnswers.TryGetValue(name, out var exactPrefill)) + { + prefill = exactPrefill; + return true; + } + + var matches = _prefillAnswers + .Where(pair => string.Equals(pair.Key, name, StringComparison.OrdinalIgnoreCase)) + .ToArray(); + if (matches.Length > 1) + { + throw new McpInteractionException( + $"Interactive prompt name '{name}' is ambiguous between prefills " + + $"{string.Join(", ", matches.Select(static pair => $"'{pair.Key}'"))}."); + } + + if (matches.Length == 1) + { + prefill = matches[0].Value; + return true; + } + + prefill = string.Empty; + return false; + } + private static bool ParseBool(string value) { if (string.Equals(value, "true", StringComparison.OrdinalIgnoreCase) diff --git a/src/Repl.Mcp/McpSchemaGenerator.cs b/src/Repl.Mcp/McpSchemaGenerator.cs index a277b270..8fd8fd78 100644 --- a/src/Repl.Mcp/McpSchemaGenerator.cs +++ b/src/Repl.Mcp/McpSchemaGenerator.cs @@ -16,6 +16,7 @@ internal static class McpSchemaGenerator /// public static JsonElement BuildInputSchema(ReplDocCommand command) { + McpToolAdapter.ValidateArgumentNames(command); var properties = new JsonObject(); var required = new JsonArray(); diff --git a/src/Repl.Mcp/McpServerHandler.cs b/src/Repl.Mcp/McpServerHandler.cs index 2ff84ebe..1658a87e 100644 --- a/src/Repl.Mcp/McpServerHandler.cs +++ b/src/Repl.Mcp/McpServerHandler.cs @@ -5,6 +5,8 @@ using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; using Repl.Documentation; +using Repl.Interaction; +using Repl.Internal.Options; namespace Repl.Mcp; @@ -35,15 +37,20 @@ internal sealed class McpServerHandler private readonly Lock _attachLock = new(); private McpGeneratedSnapshot? _snapshot; - private long _snapshotVersion = 1; + private SnapshotVersionState _snapshotState = new(Version: 1, LastVisibilityRetractionVersion: 0); private long _builtSnapshotVersion; private McpServer? _server; - private EventHandler? _routingChangedHandler; + private EventHandler? _routingChangedHandler; private ITimer? _debounceTimer; private int _rootsNotificationRegistered; private int _compatibilityIntroServed; private static readonly TimeSpan DebounceDelay = TimeSpan.FromMilliseconds(100); + // Notifications are fire-and-forget best-effort — a stuck stdio peer must not hang this + // indefinitely, since nothing awaits it and it would otherwise pile up one task per + // invalidation forever. + private static readonly TimeSpan NotificationSendTimeout = TimeSpan.FromSeconds(5); + public McpServerHandler( ICoreReplApp app, ReplMcpServerOptions options, @@ -303,7 +310,7 @@ private async ValueTask GetSnapshotAsync( { AttachServer(server); - var snapshotVersion = Volatile.Read(ref _snapshotVersion); + var snapshotVersion = Volatile.Read(ref _snapshotState).Version; if (Volatile.Read(ref _builtSnapshotVersion) == snapshotVersion && _snapshot is { } cached) { @@ -313,7 +320,7 @@ private async ValueTask GetSnapshotAsync( await _snapshotGate.WaitAsync(cancellationToken).ConfigureAwait(false); try { - snapshotVersion = Volatile.Read(ref _snapshotVersion); + snapshotVersion = Volatile.Read(ref _snapshotState).Version; if (Volatile.Read(ref _builtSnapshotVersion) == snapshotVersion && _snapshot is { } refreshed) { @@ -323,26 +330,25 @@ private async ValueTask GetSnapshotAsync( var previousSnapshot = _snapshot; try { - await _roots.GetAsync(cancellationToken).ConfigureAwait(false); - var built = BuildSnapshotCore(); - _snapshot = built; - if (Volatile.Read(ref _snapshotVersion) == snapshotVersion) - { - Volatile.Write(ref _builtSnapshotVersion, snapshotVersion); - } - return built; + return await BuildCurrentSnapshotAsync(snapshotVersion, cancellationToken).ConfigureAwait(false); } catch (OperationCanceledException) { throw; } - catch (Exception) when (previousSnapshot is not null) + catch (HiddenRequiredOptionException) { + ThrowSanitizedIfAClientAlreadyHasASchema(previousSnapshot); + throw; + } + catch (Exception) when ( + previousSnapshot is not null + && Volatile.Read(ref _snapshotState).LastVisibilityRetractionVersion + <= Volatile.Read(ref _builtSnapshotVersion)) + { + // Preserve availability for transient projection failures, but leave the version dirty + // so the next request retries without requiring another routing mutation. _snapshot = previousSnapshot; - if (Volatile.Read(ref _snapshotVersion) == snapshotVersion) - { - Volatile.Write(ref _builtSnapshotVersion, snapshotVersion); - } return previousSnapshot; } } @@ -352,9 +358,57 @@ private async ValueTask GetSnapshotAsync( } } + // No snapshot has ever been served: a cold-start configuration error, not a runtime retraction + // reaching an already-connected client. Let the caller's rethrow carry the detailed exception so + // the operator sees exactly which option and route are misconfigured. + // + // Once a client HAS a working schema, the same failure must fail closed (returning the previous + // snapshot would keep advertising an option the app explicitly hid), but the exception's own + // message names that option's target, rendered token and route — precisely the identity hiding it + // was meant to withhold — so it must not reach the client verbatim. A generic McpException (the + // pattern this handler already uses for other client-facing failures) reports the failure without + // disclosing what triggered it. + private static void ThrowSanitizedIfAClientAlreadyHasASchema(McpGeneratedSnapshot? previousSnapshot) + { + if (previousSnapshot is not null) + { + throw new McpException("Tool discovery is temporarily unavailable due to a server configuration error."); + } + } + + private async ValueTask BuildCurrentSnapshotAsync( + long snapshotVersion, + CancellationToken cancellationToken) + { + while (true) + { + cancellationToken.ThrowIfCancellationRequested(); + await _roots.GetAsync(cancellationToken).ConfigureAwait(false); + var built = BuildSnapshotCore(); + var observedState = Volatile.Read(ref _snapshotState); + + // Version and retraction watermark are one atomically published state. A reader can + // therefore never observe the new version without the visibility retraction that caused it. + // If that state appeared after projection started, discard the result and rebuild. + if (observedState.LastVisibilityRetractionVersion > snapshotVersion) + { + snapshotVersion = observedState.Version; + continue; + } + + _snapshot = built; + if (observedState.Version == snapshotVersion) + { + Volatile.Write(ref _builtSnapshotVersion, snapshotVersion); + } + return built; + } + } + private McpGeneratedSnapshot BuildSnapshotCore() { - var model = CreateDocumentationModel(); + // Project once here so tools/list, tools/call and prompts/list all read the same option list. + var model = McpAutomationProjection.Apply(CreateDocumentationModel()); var adapter = new McpToolAdapter(_app, _options, _sessionServices); var commandsByPath = model.Commands.ToDictionary( command => command.Path, @@ -376,7 +430,18 @@ private ReplDocumentationModel CreateDocumentationModel() ReplSessionIO.IsProgrammatic = true; try { - return coreApp.CreateDocumentationModel(_sessionServices); + // Every MCP tool invocation overlays a concrete interaction channel before entering + // the binder. Discovery must expose that guaranteed fallback even when the caller did + // not supply a base provider (or supplied one without the channel). + var discoveryServices = new McpServiceProviderOverlay( + _sessionServices, + new Dictionary + { + [typeof(IReplInteractionChannel)] = new McpInteractionChannel( + new Dictionary(StringComparer.Ordinal), + _options.InteractivityMode), + }); + return coreApp.CreateDocumentationModel(discoveryServices); } finally { @@ -427,6 +492,10 @@ private void AttachServer(McpServer? server) } } + internal sealed record SnapshotVersionState( + long Version, + long LastVisibilityRetractionVersion); + private void EnsureRoutingSubscription() { if (_routingChangedHandler is not null || _app is not CoreReplApp coreApp) @@ -435,8 +504,8 @@ private void EnsureRoutingSubscription() } var weakSelf = new WeakReference(this); - EventHandler? handler = null; - handler = (_, _) => + EventHandler? handler = null; + handler = (_, args) => { if (!weakSelf.TryGetTarget(out var target)) { @@ -444,7 +513,7 @@ private void EnsureRoutingSubscription() return; } - target.OnRoutingInvalidated(); + target.OnRoutingInvalidated(args.IsVisibilityRetraction); }; _routingChangedHandler = handler; @@ -472,9 +541,35 @@ private void EnsureRootsNotificationHandler(McpServer server) }); } - private void OnRoutingInvalidated() + internal static SnapshotVersionState PublishSnapshotInvalidation( + ref SnapshotVersionState snapshotState, + bool isVisibilityRetraction, + Action? beforePublish = null) + { + SnapshotVersionState currentState; + SnapshotVersionState invalidatedState; + do + { + currentState = Volatile.Read(ref snapshotState); + var invalidatedVersion = currentState.Version + 1; + invalidatedState = new SnapshotVersionState( + invalidatedVersion, + isVisibilityRetraction + ? invalidatedVersion + : currentState.LastVisibilityRetractionVersion); + beforePublish?.Invoke(invalidatedState); + } + while (!ReferenceEquals( + Interlocked.CompareExchange(ref snapshotState, invalidatedState, currentState), + currentState)); + + return invalidatedState; + } + + private void OnRoutingInvalidated(bool isVisibilityRetraction) { - Interlocked.Increment(ref _snapshotVersion); + PublishSnapshotInvalidation(ref _snapshotState, isVisibilityRetraction); + if (_options.DynamicToolCompatibility == DynamicToolCompatibilityMode.DiscoverAndCallShim) { Interlocked.Exchange(ref _compatibilityIntroServed, 0); @@ -508,7 +603,8 @@ private async Task SendNotificationSafeAsync(string method) return; } - await server.SendNotificationAsync(method, CancellationToken.None).ConfigureAwait(false); + using var timeoutCts = new CancellationTokenSource(NotificationSendTimeout); + await server.SendNotificationAsync(method, timeoutCts.Token).ConfigureAwait(false); } catch (OperationCanceledException) { diff --git a/src/Repl.Mcp/McpToolAdapter.cs b/src/Repl.Mcp/McpToolAdapter.cs index f12d9b2e..cfe07e61 100644 --- a/src/Repl.Mcp/McpToolAdapter.cs +++ b/src/Repl.Mcp/McpToolAdapter.cs @@ -2,6 +2,7 @@ using System.Text.RegularExpressions; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; +using Repl; using Repl.Documentation; using Repl.Interaction; @@ -322,54 +323,73 @@ internal static (List Tokens, Dictionary Prefills) Prepa IDictionary arguments) { var allowedArgumentNames = BuildAllowedArgumentNames(command); - return PrepareExecution(command.Path, arguments, allowedArgumentNames); + var optionTokens = command.Options.ToDictionary( + static option => option.Name, + static option => option.Aliases.Count > 0 ? option.Aliases[0] : $"--{option.Name}", + StringComparer.Ordinal); + // A bool-flag option's value token is only ever consumed on a best-effort basis: when it + // looks like a fresh option token, ApplyBoolFlagValue declines it WITHOUT a diagnostic + // (that decline is required so legitimate flag-chaining like "--verbose --other" keeps + // working), leaving it to be re-lexed as its own token on the parser's next iteration — + // which can bind a Hidden() option's alias. Every other option kind either has no value to + // smuggle or fails the whole call with a diagnostic when its value looks option-like, so + // only bool options need the inline "--name=value" form that makes re-lexing impossible. + var boolOptionNames = command.Options + .Where(static option => string.Equals(option.Type, "bool", StringComparison.Ordinal)) + .Select(static option => option.Name) + .ToHashSet(StringComparer.Ordinal); + return PrepareExecution(command.Path, arguments, allowedArgumentNames, optionTokens, boolOptionNames); } private static (List Tokens, Dictionary Prefills) PrepareExecution( string routePath, IDictionary arguments, - HashSet? allowedArgumentNames) + AllowedArgumentNames? allowedArgumentNames, + IReadOnlyDictionary optionTokens, + IReadOnlySet? boolOptionNames = null) { - var stringArgs = new Dictionary(StringComparer.OrdinalIgnoreCase); - var prefills = new Dictionary(StringComparer.OrdinalIgnoreCase); + var stringArgs = new Dictionary(StringComparer.Ordinal); + var prefills = new Dictionary(StringComparer.Ordinal); var resultFlowTokens = new List(); + var suppliedSchemaFields = new HashSet(StringComparer.Ordinal); - foreach (var (key, value) in arguments) + foreach (var (requestedKey, value) in arguments) { - var strValue = value.ValueKind == JsonValueKind.String - ? value.GetString() ?? "" - : value.GetRawText(); - - if (allowedArgumentNames is not null && !allowedArgumentNames.Contains(key)) + var field = allowedArgumentNames?.Resolve(requestedKey) + ?? new AllowedArgumentField(requestedKey, McpArgumentFieldKind.Command); + var key = field.Name; + if (!suppliedSchemaFields.Add(key)) { throw new InvalidOperationException( - $"The MCP argument '{key}' is not defined by the tool schema."); + $"The MCP argument '{requestedKey}' duplicates the schema field '{key}'."); } + var strValue = value.ValueKind == JsonValueKind.String + ? value.GetString() ?? "" + : value.GetRawText(); - if (key.StartsWith("answer.", StringComparison.OrdinalIgnoreCase)) + switch (field.Kind) { - prefills[key["answer.".Length..]] = strValue; - } - else if (string.Equals(key, McpResultFlowArgumentNames.Cursor, StringComparison.OrdinalIgnoreCase)) - { - ValidateResultCursor(strValue); - resultFlowTokens.Add(ReplResultFlowOptionNames.Cursor); - resultFlowTokens.Add(strValue); - } - else if (string.Equals(key, McpResultFlowArgumentNames.PageSize, StringComparison.OrdinalIgnoreCase)) - { - ValidateResultPageSize(strValue); - resultFlowTokens.Add(ReplResultFlowOptionNames.PageSize); - resultFlowTokens.Add(strValue); - } - else - { - ValidateCommandArgumentValue(strValue); - stringArgs[key] = strValue; + case McpArgumentFieldKind.Answer: + prefills.Add(key["answer.".Length..], strValue); + break; + case McpArgumentFieldKind.Cursor: + ValidateResultCursor(strValue); + resultFlowTokens.Add(ReplResultFlowOptionNames.Cursor); + resultFlowTokens.Add(strValue); + break; + case McpArgumentFieldKind.PageSize: + ValidateResultPageSize(strValue); + resultFlowTokens.Add(ReplResultFlowOptionNames.PageSize); + resultFlowTokens.Add(strValue); + break; + default: + ValidateCommandArgumentValue(strValue); + stringArgs.Add(key, strValue); + break; } } - var tokens = ReconstructTokens(routePath, stringArgs); + var tokens = ReconstructTokens(routePath, stringArgs, optionTokens, boolOptionNames); tokens.InsertRange(0, resultFlowTokens); return (tokens, prefills); } @@ -404,45 +424,105 @@ private static void ValidateCommandArgumentValue(string value) } } - private static HashSet BuildAllowedArgumentNames(ReplDocCommand command) + internal static void ValidateArgumentNames(ReplDocCommand command) => + _ = BuildAllowedArgumentNames(command); + + private static AllowedArgumentNames BuildAllowedArgumentNames(ReplDocCommand command) { - var names = new HashSet(StringComparer.OrdinalIgnoreCase); + var names = new AllowedArgumentNames(); foreach (var argument in command.Arguments) { - names.Add(argument.Name); + names.Add(argument.Name, McpArgumentFieldKind.Command); } foreach (var option in command.Options) { - names.Add(option.Name); + names.Add(option.Name, McpArgumentFieldKind.Command); } if (command.Answers is { Count: > 0 }) { foreach (var answer in command.Answers) { - names.Add($"answer.{answer.Name}"); + names.Add($"answer.{answer.Name}", McpArgumentFieldKind.Answer); } } if (command.AcceptsPagingInput || command.EmitsPagedResult) { - names.Add(McpResultFlowArgumentNames.Cursor); - names.Add(McpResultFlowArgumentNames.PageSize); + names.Add(McpResultFlowArgumentNames.Cursor, McpArgumentFieldKind.Cursor); + names.Add(McpResultFlowArgumentNames.PageSize, McpArgumentFieldKind.PageSize); } return names; } + private enum McpArgumentFieldKind + { + Command, + Answer, + Cursor, + PageSize, + } + + private readonly record struct AllowedArgumentField(string Name, McpArgumentFieldKind Kind); + + private sealed class AllowedArgumentNames + { + private readonly Dictionary _exactFields = new(StringComparer.Ordinal); + private readonly List _orderedFields = []; + + internal void Add(string name, McpArgumentFieldKind kind) + { + var field = new AllowedArgumentField(name, kind); + if (!_exactFields.TryAdd(name, field)) + { + throw new InvalidOperationException( + $"MCP argument name collision: '{name}' is declared more than once by the tool schema."); + } + + _orderedFields.Add(field); + } + + internal AllowedArgumentField Resolve(string requestedName) + { + if (_exactFields.TryGetValue(requestedName, out var exact)) + { + return exact; + } + + var matches = _orderedFields + .Where(field => string.Equals(field.Name, requestedName, StringComparison.OrdinalIgnoreCase)) + .ToArray(); + return matches.Length switch + { + 0 => throw new InvalidOperationException( + $"The MCP argument '{requestedName}' is not defined by the tool schema."), + 1 => matches[0], + _ => throw new InvalidOperationException( + $"The MCP argument '{requestedName}' is ambiguous between schema fields " + + $"{string.Join(", ", matches.Select(static field => $"'{field.Name}'"))}."), + }; + } + } + /// /// Reconstructs CLI tokens from a route template and MCP arguments. /// internal static List ReconstructTokens( string routePath, - IDictionary arguments) + IDictionary arguments) => + ReconstructTokens(routePath, arguments, optionTokens: null, boolOptionNames: null); + + private static List ReconstructTokens( + string routePath, + IDictionary arguments, + IReadOnlyDictionary? optionTokens, + IReadOnlySet? boolOptionNames) { var tokens = new List(); - var consumedArgs = new HashSet(StringComparer.OrdinalIgnoreCase); + var consumedArgs = new HashSet( + optionTokens is null ? StringComparer.OrdinalIgnoreCase : StringComparer.Ordinal); foreach (var part in routePath.Split(' ', StringSplitOptions.RemoveEmptyEntries)) { @@ -456,6 +536,7 @@ internal static List ReconstructTokens( var strValue = value.ToString() ?? ""; if (strValue.Length > 0) { + ValidatePositionalArgumentValue(strValue); tokens.Add(strValue); } @@ -475,14 +556,73 @@ internal static List ReconstructTokens( { if (!consumedArgs.Contains(key)) { - tokens.Add($"--{key}"); - tokens.Add(value?.ToString() ?? ""); + var token = ResolveOptionToken(key, optionTokens); + var strValue = value?.ToString() ?? ""; + if (boolOptionNames?.Contains(key) == true) + { + // Single inline token: TrySplitOptionToken (InvocationOptionParser) splits on + // the first '=' only, so the value survives intact even when it starts with + // '-' or contains '=' itself, and it can never be left dangling by + // ApplyBoolFlagValue to be re-lexed as a fresh option on the next iteration. + tokens.Add($"{token}={strValue}"); + } + else + { + tokens.Add(token); + tokens.Add(strValue); + } } } return tokens; } + // Guards the ONE place an MCP-supplied value becomes a bare CLI token: a positional route + // segment. It has no separator that can escape the value the way an inline "--token=value" + // pair does for a bool option, so a value that would itself be lexed as an option token (and + // could then resolve to a route/global option — including a Hidden() one, since parsing still + // accepts those — via the general-purpose parser) must be rejected here instead. + private static void ValidatePositionalArgumentValue(string value) + { + if (InvocationOptionParser.LooksLikeOptionToken(value) && !InvocationOptionParser.IsSignedNumericLiteral(value)) + { + throw new InvalidOperationException( + "The MCP argument value cannot start like a CLI option because it fills a positional route segment, which has no way to escape it."); + } + } + + private static string ResolveOptionToken(string key, IReadOnlyDictionary? optionTokens) + { + if (optionTokens is null) + { + return $"--{key}"; + } + + if (optionTokens.TryGetValue(key, out var exactToken)) + { + return exactToken; + } + + string? matchedToken = null; + foreach (var (optionName, token) in optionTokens) + { + if (!string.Equals(optionName, key, StringComparison.OrdinalIgnoreCase)) + { + continue; + } + + if (matchedToken is not null) + { + throw new InvalidOperationException( + $"The MCP argument '{key}' is ambiguous because option names differ only by casing."); + } + + matchedToken = token; + } + + return matchedToken ?? $"--{key}"; + } + private static CallToolResult ErrorResult(string message) => new() { Content = [new TextContentBlock { Text = message }], diff --git a/src/Repl.McpTests/Given_McpDebounce.cs b/src/Repl.McpTests/Given_McpDebounce.cs index 45a14b0d..72d79567 100644 --- a/src/Repl.McpTests/Given_McpDebounce.cs +++ b/src/Repl.McpTests/Given_McpDebounce.cs @@ -4,6 +4,7 @@ using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; using Repl.Mcp; +using Repl.Parameters; namespace Repl.McpTests; @@ -53,16 +54,103 @@ public void When_RebuildThrows_Then_ServerContinuesWithStaleRoutes() var tools = SyncWait(fixture.Client.ListToolsAsync().AsTask()); tools.Should().ContainSingle(t => string.Equals(t.Name, "initial", StringComparison.Ordinal)); - // Add a command filter that will throw during the next rebuild. + // Add a route that only a successful refresh can reveal, then make the first rebuild fail. + fixture.App.Map("added-after", () => "new"); fixture.Options.CommandFilter = _ => throw new InvalidOperationException("Simulated rebuild failure"); fixture.App.Core.InvalidateRouting(); // Advance time to trigger the rebuild — should not crash. fakeTime.Advance(TimeSpan.FromMilliseconds(150)); - // Server should still respond — existing tools remain. - var toolsAfter = SyncWait(fixture.Client.ListToolsAsync().AsTask()); - toolsAfter.Should().NotBeEmpty("server should continue with stale routes after rebuild failure"); + // Server should still respond with the previous snapshot while the failure is active. + var staleTools = SyncWait(fixture.Client.ListToolsAsync().AsTask()); + staleTools.Should().ContainSingle(tool => string.Equals(tool.Name, "initial", StringComparison.Ordinal)); + + // Clearing a transient failure must let the same invalidated version retry without another mutation. + fixture.Options.CommandFilter = null; + var recoveredTools = SyncWait(fixture.Client.ListToolsAsync().AsTask()); + recoveredTools.Should().Contain(tool => string.Equals(tool.Name, "added-after", StringComparison.Ordinal)); + } + + [TestMethod] + [Description("Pausing immediately before invalidation publication leaves readers on the complete old version/watermark pair; releasing publication exposes the complete new pair atomically.")] + public void When_VisibilityRetractionPublicationIsPaused_Then_ReaderObservesOnlyCompleteStates() + { + var initial = new McpServerHandler.SnapshotVersionState( + Version: 7, + LastVisibilityRetractionVersion: 3); + var holder = new SnapshotStateHolder(initial); + using var publicationReady = new ManualResetEventSlim(initialState: false); + using var releasePublication = new ManualResetEventSlim(initialState: false); + + var publication = Task.Run(() => McpServerHandler.PublishSnapshotInvalidation( + ref holder.State, + isVisibilityRetraction: true, + beforePublish: candidate => + { + candidate.Version.Should().Be(8); + candidate.LastVisibilityRetractionVersion.Should().Be(8); + publicationReady.Set(); + releasePublication.Wait(TimeSpan.FromSeconds(10)).Should().BeTrue(); + })); + + publicationReady.Wait(TimeSpan.FromSeconds(10)).Should().BeTrue(); + Volatile.Read(ref holder.State).Should().BeSameAs(initial); + + releasePublication.Set(); + SyncWait(publication); + var published = Volatile.Read(ref holder.State); + published.Version.Should().Be(8); + published.LastVisibilityRetractionVersion.Should().Be(8); + } + + [TestMethod] + [Description("If an option is hidden while a successful MCP snapshot projection is in flight, that request retries against the newer visibility version instead of returning the stale schema it already constructed.")] + public void When_VisibilityRetractionOccursDuringSuccessfulBuild_Then_InFlightRequestRetries() + { + var fakeTime = new FakeTimeProvider(); + using var fixture = CreateServerFixture(fakeTime); + var initial = SyncWait(fixture.Client.ListToolsAsync().AsTask()).Single(); + initial.JsonSchema.GetProperty("properties").TryGetProperty("tenant", out _).Should().BeTrue(); + + using var projectionEntered = new ManualResetEventSlim(initialState: false); + using var releaseProjection = new ManualResetEventSlim(initialState: false); + fixture.Options.CommandFilter = _ => + { + projectionEntered.Set(); + return releaseProjection.Wait(TimeSpan.FromSeconds(10)); + }; + fixture.App.Core.InvalidateRouting(); + + var refresh = fixture.Client.ListToolsAsync().AsTask(); + projectionEntered.Wait(TimeSpan.FromSeconds(10)).Should().BeTrue("the test must interleave the visibility change with snapshot projection"); + fixture.InitialCommand.WithOption("tenant", static option => option.Hidden()); + releaseProjection.Set(); + + var rebuilt = SyncWait(refresh).Single(); + rebuilt.JsonSchema.GetProperty("properties").TryGetProperty("tenant", out _).Should().BeFalse(); + } + + [TestMethod] + [Description("A visibility retraction fails closed while snapshot projection is broken, rather than serving the previous MCP schema that still exposes the hidden option; a later successful rebuild clears the fail-closed state.")] + public void When_VisibilityRetractionRefreshFails_Then_StaleSnapshotIsNotServed() + { + var fakeTime = new FakeTimeProvider(); + using var fixture = CreateServerFixture(fakeTime); + + var initial = SyncWait(fixture.Client.ListToolsAsync().AsTask()).Single(); + initial.JsonSchema.GetProperty("properties").TryGetProperty("tenant", out _).Should().BeTrue(); + + fixture.Options.CommandFilter = _ => throw new InvalidOperationException("Simulated rebuild failure"); + fixture.InitialCommand.WithOption("tenant", static option => option.Hidden()); + fakeTime.Advance(TimeSpan.FromMilliseconds(150)); + + Action staleRead = () => _ = SyncWait(fixture.Client.ListToolsAsync().AsTask()); + staleRead.Should().Throw("a stale schema would still advertise and accept the explicitly hidden option"); + + fixture.Options.CommandFilter = null; + var recovered = SyncWait(fixture.Client.ListToolsAsync().AsTask()).Single(); + recovered.JsonSchema.GetProperty("properties").TryGetProperty("tenant", out _).Should().BeFalse(); } // ── Sync-over-async helper ────────────────────────────────────────── @@ -88,7 +176,9 @@ private static ServerFixture CreateServerFixture(TimeProvider timeProvider) { var app = ReplApp.Create(); app.UseMcpServer(); - app.Map("initial", () => "ok"); + var initialCommand = app.Map( + "initial", + static string ([ReplOption] string? tenant = null) => tenant ?? "ok"); var clientToServer = new Pipe(); var serverToClient = new Pipe(); @@ -109,15 +199,16 @@ private static ServerFixture CreateServerFixture(TimeProvider timeProvider) clientToServer.Writer.AsStream(), serverToClient.Reader.AsStream()))); - return new ServerFixture(app, options, client, cts, clientToServer, serverToClient, serverTask); + return new ServerFixture(app, options, initialCommand, client, cts, clientToServer, serverToClient, serverTask); } private sealed class ServerFixture( - ReplApp app, ReplMcpServerOptions options, McpClient client, + ReplApp app, ReplMcpServerOptions options, CommandBuilder initialCommand, McpClient client, CancellationTokenSource cts, Pipe c2s, Pipe s2c, Task serverTask) : IDisposable { public ReplApp App => app; public ReplMcpServerOptions Options => options; + public CommandBuilder InitialCommand => initialCommand; public McpClient Client => client; #pragma warning disable VSTHRD002 @@ -148,4 +239,9 @@ private sealed class NullIoContext : IReplIoContext public bool IsHostedSession => false; public string? SessionId => null; } + private sealed class SnapshotStateHolder(McpServerHandler.SnapshotVersionState state) + { + public McpServerHandler.SnapshotVersionState State = state; + } + } diff --git a/src/Repl.McpTests/Given_McpFallbackEndToEnd.cs b/src/Repl.McpTests/Given_McpFallbackEndToEnd.cs index 344ee59a..e78ed76e 100644 --- a/src/Repl.McpTests/Given_McpFallbackEndToEnd.cs +++ b/src/Repl.McpTests/Given_McpFallbackEndToEnd.cs @@ -1,4 +1,5 @@ using ModelContextProtocol.Protocol; +using Repl.Parameters; namespace Repl.McpTests; @@ -91,6 +92,26 @@ public async Task When_PromptFallbackEnabled_Then_StillInPromptsList() prompts.Should().ContainSingle(p => string.Equals(p.Name, "explain", StringComparison.Ordinal)); } + [TestMethod] + [Description("Proves the automation filter is applied once at the documentation-model boundary rather than per emitter: prompts/list builds its arguments from its own loop over command options, untouched by this change, yet it still omits an automation-hidden option.")] + public async Task When_PromptCommandHasAnAutomationHiddenOption_Then_PromptArgumentsOmitIt() + { + await using var fixture = await McpTestFixture.CreateAsync( + app => app.Map( + "explain {topic}", + (string topic, [ReplOption(Name = "verbosity")] string? verbosity = null, [ReplOption(Name = "traceId")] string? traceId = null) => + $"Explain {topic} {verbosity} {traceId}") + .AsPrompt() + .WithOption("traceId", static option => option.AutomationHidden())); + + var prompts = await fixture.Client.ListPromptsAsync().ConfigureAwait(false); + + var prompt = prompts.Should().ContainSingle(p => string.Equals(p.Name, "explain", StringComparison.Ordinal)).Which; + var argumentNames = prompt.ProtocolPrompt.Arguments?.Select(static argument => argument.Name).ToArray() ?? []; + argumentNames.Should().Contain("verbosity"); + argumentNames.Should().NotContain("traceId"); + } + // ── Mixed scenarios ──────────────────────────────────────────────── [TestMethod] diff --git a/src/Repl.McpTests/Given_McpInteractionChannel.cs b/src/Repl.McpTests/Given_McpInteractionChannel.cs index c8b86767..f1a5a355 100644 --- a/src/Repl.McpTests/Given_McpInteractionChannel.cs +++ b/src/Repl.McpTests/Given_McpInteractionChannel.cs @@ -76,6 +76,60 @@ public async Task When_PrefillIsFalse_Then_ReturnsFalse() result.Should().BeFalse(); } + [TestMethod] + [Description("Runtime answer lookup preserves the historical case-insensitive contract when exactly one stored prefill matches.")] + public async Task When_RuntimeAnswerNameDiffersOnlyByCase_Then_UniquePrefillStillResolves() + { + var channel = CreateChannel(new Dictionary(StringComparer.Ordinal) { ["confirm"] = "false" }); + + var result = await channel.AskConfirmationAsync("CONFIRM", "Proceed?"); + + result.Should().BeFalse(); + } + + [TestMethod] + [Description("Case-distinct prefill names resolve exactly, while a third non-exact casing is rejected as ambiguous instead of selecting an arbitrary answer.")] + public async Task When_CaseDistinctPrefillsExist_Then_ExactLookupWinsAndNonExactLookupIsAmbiguous() + { + var channel = CreateChannel(new Dictionary(StringComparer.Ordinal) + { + ["confirm"] = "false", + ["CONFIRM"] = "true", + }); + + var lower = await channel.AskConfirmationAsync("confirm", "Proceed?"); + var upper = await channel.AskConfirmationAsync("CONFIRM", "Proceed?"); + var ambiguous = () => channel.AskConfirmationAsync("Confirm", "Proceed?").AsTask(); + + lower.Should().BeFalse(); + upper.Should().BeTrue(); + await ambiguous.Should().ThrowAsync() + .WithMessage("*ambiguous*confirm*CONFIRM*"); + } + + [TestMethod] + [Description("Every interaction kind uses the shared exact-first, unique case-insensitive prefill fallback when an ordinal dictionary contains one differently-cased answer name.")] + public async Task When_AllRuntimeAnswerKindsDifferOnlyByCase_Then_UniquePrefillsStillResolve() + { + var channel = CreateChannel(new Dictionary(StringComparer.Ordinal) + { + ["color"] = "green", + ["name"] = "Alice", + ["password"] = "s3cret", + ["tags"] = "red,blue", + }); + + var choice = await channel.AskChoiceAsync("COLOR", "Pick a color", ["red", "green", "blue"]); + var text = await channel.AskTextAsync("NAME", "Enter name"); + var secret = await channel.AskSecretAsync("PASSWORD", "Enter password"); + var multiChoice = await channel.AskMultiChoiceAsync("TAGS", "Select tags", ["red", "green", "blue"]); + + choice.Should().Be(1); + text.Should().Be("Alice"); + secret.Should().Be("s3cret"); + multiChoice.Should().BeEquivalentTo([0, 2]); + } + [TestMethod] [Description("Missing prefill with explicit default returns default even in PrefillThenFail mode.")] public async Task When_NoBoolPrefillInFailModeWithDefaultTrue_Then_ReturnsDefault() diff --git a/src/Repl.McpTests/Given_McpSchemaGenerator.cs b/src/Repl.McpTests/Given_McpSchemaGenerator.cs index 71d6bfd0..640427de 100644 --- a/src/Repl.McpTests/Given_McpSchemaGenerator.cs +++ b/src/Repl.McpTests/Given_McpSchemaGenerator.cs @@ -252,6 +252,20 @@ public void When_DetailsPresent_Then_CombinesDescriptionAndDetails() McpSchemaGenerator.BuildDescription(cmd).Should().Contain("Deploy").And.Contain("Deploys to env."); } + [TestMethod] + [Description("MCP input schema generation rejects an exact field-name collision across route arguments and options instead of overwriting the first JSON property.")] + public void When_ArgumentAndOptionShareAnMcpFieldName_Then_SchemaGenerationFailsClosed() + { + var command = CreateCommand( + arguments: [new ReplDocArgument("scope", "string", Required: true, Description: null)], + options: [CreateOption("scope", "string")]); + + var action = () => McpSchemaGenerator.BuildInputSchema(command); + + action.Should().Throw() + .WithMessage("*MCP argument name collision*scope*"); + } + // ── Helpers ───────────────────────────────────────────────────────── private static ReplDocCommand CreateCommand( diff --git a/src/Repl.McpTests/Given_McpServerEndToEnd.cs b/src/Repl.McpTests/Given_McpServerEndToEnd.cs index 8df6fb42..2bf425af 100644 --- a/src/Repl.McpTests/Given_McpServerEndToEnd.cs +++ b/src/Repl.McpTests/Given_McpServerEndToEnd.cs @@ -1,6 +1,8 @@ using System.Text.Json; using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol; using ModelContextProtocol.Protocol; +using Repl.Interaction; using Repl.Mcp; using Repl.Parameters; @@ -508,6 +510,536 @@ public async Task When_PromptReturnsString_Then_TextIsPlainString() text.Should().Be("Investigate the checkout service for this symptom: 'queue depth rising'. Start with ops_status, inspect failed checks, then propose the smallest safe next step."); } + [TestMethod] + [Description("Hidden command options are omitted from MCP tool schemas while visible sibling options remain discoverable.")] + public async Task When_CommandOptionIsHidden_Then_McpToolSchemaOmitsIt() + { + await using var fixture = await McpTestFixture.CreateAsync(app => + { + app.Map( + "deploy", + ([ReplOption(Name = "environment")] string environment, [ReplOption(Name = "internalMode")] bool internalMode = false) => + $"{environment}:{internalMode}") + .WithOption("internalMode", static option => option.Hidden()); + }); + + var tools = await fixture.Client.ListToolsAsync(); + var tool = tools.Single(candidate => string.Equals(candidate.Name, "deploy", StringComparison.Ordinal)); + var properties = tool.JsonSchema.GetProperty("properties"); + + properties.TryGetProperty("environment", out _).Should().BeTrue(); + properties.TryGetProperty("internalMode", out _).Should().BeFalse(); + } + + [TestMethod] + [Description("A hidden option is absent from the advertised schema, so supplying it to tools/call is rejected — the same hard block a hidden command gets. This pins the guarantee that the advertised schema and the accepted argument list can never diverge, because the generated schema omits additionalProperties:false and the allow-list is the only thing enforcing it.")] + public async Task When_HiddenCommandOptionIsSuppliedToToolCall_Then_CallIsRejected() + { + await using var fixture = await McpTestFixture.CreateAsync(app => + { + app.Map( + "deploy", + ([ReplOption(Name = "environment")] string environment, [ReplOption(Name = "internalMode")] bool internalMode = false) => + $"{environment}:{internalMode}") + .WithOption("internalMode", static option => option.Hidden()); + }); + + var result = await fixture.Client.CallToolAsync( + toolName: "deploy", + arguments: new Dictionary(StringComparer.Ordinal) + { + ["environment"] = "denim", + ["internalMode"] = true, + }).ConfigureAwait(false); + + var text = string.Join('\n', result.Content.OfType().Select(static block => block.Text)); + + result.IsError.Should().BeTrue(); + text.Should().NotContain("denim", "the handler must not run when an undeclared argument is supplied"); + + // The adapter's own diagnostic ("The MCP argument 'internalMode' is not defined by the + // tool schema.") is deliberately not asserted: the SDK replaces it with a generic + // "An error occurred invoking ''." before it reaches the client. That is the right + // outcome for a hidden option — a caller probing for one learns nothing from the failure. + text.Should().Contain("error occurred invoking"); + } + + [TestMethod] + [Description("Human documentation may retain reverse/value-only reachability, but MCP cannot reconstruct an arbitrary semantic value without an ordinary named token; the optional field is therefore omitted from both schema and allow-list while the tool remains callable.")] + public async Task When_OnlyReverseAliasRemainsReachable_Then_McpOmitsTheSemanticOption() + { + await using var fixture = await McpTestFixture.CreateAsync(app => + { + app.Options(options => + { + options.Parsing.AddGlobalOption("force"); + options.Parsing.GlobalOption("force").Hidden(); + }); + app.Map( + "deploy", + static string ([ReplOption(ReverseAliases = ["--no-force"])] bool force = true) => force.ToString()); + }); + + var advertised = await ReadDeployPropertiesAsync(fixture).ConfigureAwait(false); + var result = await fixture.Client.CallToolAsync( + toolName: "deploy", + arguments: new Dictionary(StringComparer.Ordinal)).ConfigureAwait(false); + + advertised.Should().NotContain("force"); + result.IsError.Should().NotBeTrue(); + string.Join('\n', result.Content.OfType().Select(static block => block.Text)) + .Should().Contain("True"); + } + + [TestMethod] + [Description("Changing inherited option casing after an MCP snapshot retracts aliases that become equivalent to a hidden alias; the next list and call must use the rebuilt schema rather than the stale adapter.")] + public async Task When_OptionCaseModeChangesAfterInitialList_Then_McpRetractsNewlyHiddenField() + { + await using var fixture = await McpTestFixture.CreateAsync(app => + { + app.Options(options => + { + options.Parsing.AddGlobalOption("tenant"); + options.Parsing.GlobalOption("tenant").Hidden(); + }); + app.Map( + "deploy", + static string ([ReplOption(Name = "tenant", Aliases = ["--ACCOUNT"], HiddenAliases = ["--account"])] string? tenant = null) => tenant ?? "none"); + }); + + var before = await ReadDeployPropertiesAsync(fixture).ConfigureAwait(false); + before.Should().Contain("tenant"); + + fixture.App.Options(options => + options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + + var after = await ReadDeployPropertiesAsync(fixture).ConfigureAwait(false); + var call = await fixture.Client.CallToolAsync( + toolName: "deploy", + arguments: new Dictionary(StringComparer.Ordinal) + { + ["tenant"] = "north", + }).ConfigureAwait(false); + + after.Should().NotContain("tenant"); + call.IsError.Should().BeTrue(); + } + + [TestMethod] + [Description("The MCP half of the AutomationHidden pair: the advertised tool schema omits the option, and because the schema and the accepted argument list are built from the same option list, tools/call rejects it too. The help half is asserted separately, where the same option stays listed and binds.")] + public async Task When_CommandOptionIsAutomationHidden_Then_McpOmitsItAndRejectsIt() + { + await using var fixture = await McpTestFixture.CreateAsync(app => + { + app.Map( + "deploy", + ([ReplOption(Name = "environment")] string environment, [ReplOption(Name = "internalMode")] bool internalMode = false) => + $"{environment}:{internalMode}") + .WithOption("internalMode", static option => option.AutomationHidden()); + }); + + var advertised = await ReadDeployPropertiesAsync(fixture).ConfigureAwait(false); + var result = await fixture.Client.CallToolAsync( + toolName: "deploy", + arguments: new Dictionary(StringComparer.Ordinal) + { + ["environment"] = "denim", + ["internalMode"] = true, + }).ConfigureAwait(false); + + advertised.Should().Contain("environment"); + advertised.Should().NotContain("internalMode"); + result.IsError.Should().BeTrue(); + string.Join('\n', result.Content.OfType().Select(static block => block.Text)) + .Should().NotContain("denim", "the handler must not run for an argument the schema never advertised"); + } + + [TestMethod] + [Description("An automation-hidden option that cannot be omitted leaves no valid MCP invocation: the client cannot supply it, and omitting it fails to bind. Advertising such a tool guarantees every call fails, so the command is withdrawn from MCP instead — the human command line keeps working, which is why this is not rejected at configuration time the way an all-surfaces hidden required option is.")] + public async Task When_AutomationHiddenOptionCannotBeOmitted_Then_TheToolIsWithdrawn() + { + await using var fixture = await McpTestFixture.CreateAsync(app => + { + app.Map("ping", () => "pong"); + app.Map( + "deploy", + ([ReplOption(Name = "internal-token", Arity = ReplArity.ExactlyOne)] string internalToken) => internalToken) + .WithOption("internalToken", static option => option.AutomationHidden()); + }); + + var tools = await fixture.Client.ListToolsAsync().ConfigureAwait(false); + + tools.Should().Contain(tool => string.Equals(tool.Name, "ping", StringComparison.Ordinal)); + tools.Should().NotContain(tool => string.Equals(tool.Name, "deploy", StringComparison.Ordinal)); + } + + [TestMethod] + [Description("A DI service fallback makes an explicitly required option omittable at the same precedence point used by HandlerArgumentBinder. AutomationHidden must omit that option without withdrawing the tool, and tools/call without the argument must receive the service value.")] + public async Task When_AutomationHiddenRequiredOptionHasAServiceFallback_Then_TheToolRemainsInvocable() + { + await using var fixture = await McpTestFixture.CreateAsync( + app => app.Map( + "deploy", + ([ReplOption(Name = "token", Arity = ReplArity.ExactlyOne)] string token) => token) + .WithOption("token", static option => option.AutomationHidden()), + configureServices: static services => services.AddSingleton("service-token")); + + var tools = await fixture.Client.ListToolsAsync().ConfigureAwait(false); + var tool = tools.Single(candidate => string.Equals(candidate.Name, "deploy", StringComparison.Ordinal)); + var result = await fixture.Client.CallToolAsync( + toolName: "deploy", + arguments: new Dictionary(StringComparer.Ordinal)).ConfigureAwait(false); + var text = string.Join('\n', result.Content.OfType().Select(static block => block.Text)); + + tool.JsonSchema.GetProperty("properties").TryGetProperty("token", out _).Should().BeFalse(); + result.IsError.Should().BeFalse(); + text.Should().Contain("service-token"); + } + + [TestMethod] + [Description("MCP metadata and its call allow-list must omit a route option whose canonical token belongs to a higher-precedence hidden global. Otherwise the adapter accepts a field that GlobalOptionParser consumes before route binding.")] + public async Task When_HiddenGlobalOwnsRouteOptionToken_Then_McpOmitsAndRejectsThatArgument() + { + await using var fixture = await McpTestFixture.CreateAsync(app => + { + app.Options(options => + { + options.Parsing.AddGlobalOption("tenant"); + options.Parsing.GlobalOption("tenant").Hidden(); + }); + app.Map( + "deploy", + static string ( + [ReplOption] string? tenant = null, + [ReplOption] string? region = null) => $"{tenant}:{region}"); + }); + + var tools = await fixture.Client.ListToolsAsync().ConfigureAwait(false); + var tool = tools.Single(candidate => string.Equals(candidate.Name, "deploy", StringComparison.Ordinal)); + var result = await fixture.Client.CallToolAsync( + toolName: "deploy", + arguments: new Dictionary(StringComparer.Ordinal) + { + ["tenant"] = "acme", + }).ConfigureAwait(false); + tool.JsonSchema.GetProperty("properties").TryGetProperty("tenant", out _).Should().BeFalse(); + tool.JsonSchema.GetProperty("properties").TryGetProperty("region", out _).Should().BeTrue(); + result.IsError.Should().BeTrue(); + } + + [TestMethod] + [Description("A hidden legacy alias remains a CLI fallback but is omitted from the MCP schema and allow-list; the visible canonical argument remains callable.")] + public async Task When_RouteAliasIsHidden_Then_McpExposesOnlyTheCanonicalArgument() + { + await using var fixture = await McpTestFixture.CreateAsync(app => app.Map( + "deploy", + static string ([ReplOption(Name = "tenant", HiddenAliases = ["--account"])] string? tenant = null) => tenant ?? "none")); + + var tool = (await fixture.Client.ListToolsAsync().ConfigureAwait(false)).Single(); + var canonical = await fixture.Client.CallToolAsync( + toolName: "deploy", + arguments: new Dictionary(StringComparer.Ordinal) { ["tenant"] = "acme" }) + .ConfigureAwait(false); + var legacy = await fixture.Client.CallToolAsync( + toolName: "deploy", + arguments: new Dictionary(StringComparer.Ordinal) { ["account"] = "acme" }) + .ConfigureAwait(false); + + tool.JsonSchema.GetProperty("properties").TryGetProperty("tenant", out _).Should().BeTrue(); + tool.JsonSchema.GetProperty("properties").TryGetProperty("account", out _).Should().BeFalse(); + canonical.IsError.Should().NotBeTrue(); + legacy.IsError.Should().BeTrue(); + } + + [TestMethod] + [Description("When a global shadows only the canonical route token, MCP keeps the stable semantic argument name, maps it to the exact surviving short alias, and does not collide with another option whose canonical name matches that alias.")] + public async Task When_HiddenGlobalOwnsRouteCanonicalToken_Then_McpUsesTheRouteAliasWithoutRenamingTheArgument() + { + await using var fixture = await McpTestFixture.CreateAsync(app => + { + app.Options(options => + { + options.Parsing.AddGlobalOption("tenant"); + options.Parsing.GlobalOption("tenant").Hidden(); + }); + app.Map( + "deploy", + static string ( + [ReplOption(Aliases = ["-t"])] string? tenant = null, + [ReplOption(Name = "t")] string? shortName = null) => $"{tenant ?? "none"}:{shortName ?? "none"}"); + }); + + var tool = (await fixture.Client.ListToolsAsync().ConfigureAwait(false)) + .Single(candidate => string.Equals(candidate.Name, "deploy", StringComparison.Ordinal)); + var result = await fixture.Client.CallToolAsync( + toolName: "deploy", + arguments: new Dictionary(StringComparer.Ordinal) + { + ["tenant"] = "acme", + ["t"] = "north", + }).ConfigureAwait(false); + var text = string.Join('\n', result.Content.OfType().Select(static block => block.Text)); + var properties = tool.JsonSchema.GetProperty("properties"); + + properties.TryGetProperty("tenant", out _).Should().BeTrue(); + properties.TryGetProperty("t", out _).Should().BeTrue(); + result.IsError.Should().NotBeTrue(); + text.Should().Contain("acme:north"); + } + + [TestMethod] + [Description("A route argument and an option whose stable MCP name becomes identical after global canonical-token shadowing must fail closed instead of overwriting the JSON schema property and binding the submitted value positionally.")] + public async Task When_ShadowedOptionStableNameCollidesWithRouteArgument_Then_McpStartupFailsClosed() + { + var start = async () => + { + var fixture = await McpTestFixture.CreateAsync(app => + { + app.Options(options => + { + options.Parsing.AddGlobalOption("scope"); + options.Parsing.GlobalOption("scope").Hidden(); + }); + app.Map( + "deploy {scope}", + static string (string scope, [ReplOption(Name = "scope", Aliases = ["-s"])] string? selected = null) => + $"{scope}:{selected ?? "none"}"); + }).ConfigureAwait(false); + try + { + _ = await fixture.Client.ListToolsAsync().ConfigureAwait(false); + } + finally + { + await fixture.DisposeAsync().ConfigureAwait(false); + } + }; + + var faulted = await start.Should() + .ThrowAsync() + .WaitAsync(TimeSpan.FromSeconds(15)) + .ConfigureAwait(false); + + faulted.WithMessage("*error occurred*"); + } + + [TestMethod] + [Description("A case-distinct ordinary option and the synthetic MCP cursor remain separate schema fields and bind to the option and paging context respectively in one real tool call.")] + public async Task When_OptionDiffersFromSyntheticCursorOnlyByCase_Then_EndToEndBindingKeepsBothDestinations() + { + await using var fixture = await McpTestFixture.CreateAsync(app => app.Map( + "inspect", + static string ( + [ReplOption(Name = "_replcursor")] string? ordinary, + IReplPagingContext paging) => $"{ordinary ?? "none"}:{paging.Cursor ?? "none"}")); + + var tool = (await fixture.Client.ListToolsAsync().ConfigureAwait(false)) + .Single(candidate => string.Equals(candidate.Name, "inspect", StringComparison.Ordinal)); + var result = await fixture.Client.CallToolAsync( + toolName: "inspect", + arguments: new Dictionary(StringComparer.Ordinal) + { + ["_replcursor"] = "ordinary", + [McpResultFlowArgumentNames.Cursor] = "opaque", + }).ConfigureAwait(false); + var text = string.Join('\n', result.Content.OfType().Select(static block => block.Text)); + var properties = tool.JsonSchema.GetProperty("properties"); + + properties.TryGetProperty("_replcursor", out _).Should().BeTrue(); + properties.TryGetProperty(McpResultFlowArgumentNames.Cursor, out _).Should().BeTrue(); + result.IsError.Should().NotBeTrue(); + text.Should().Contain("ordinary:opaque"); + } + + [TestMethod] + [Description("A declared answer and a case-distinct ordinary option under the answer prefix remain separate destinations through schema generation, tool adaptation, and runtime interaction lookup.")] + public async Task When_OptionDiffersFromDeclaredAnswerOnlyByCase_Then_EndToEndBindingKeepsBothDestinations() + { + await using var fixture = await McpTestFixture.CreateAsync(app => app.Map( + "wizard", + static async Task ( + [ReplOption(Name = "answer.CONFIRM")] string? ordinary, + IReplInteractionChannel interaction) => + { + var answer = await interaction.AskConfirmationAsync("confirm", "Proceed?").ConfigureAwait(false); + return $"{ordinary ?? "none"}:{answer}"; + }) + .WithAnswer("confirm", "bool")); + + var tool = (await fixture.Client.ListToolsAsync().ConfigureAwait(false)) + .Single(candidate => string.Equals(candidate.Name, "wizard", StringComparison.Ordinal)); + var result = await fixture.Client.CallToolAsync( + toolName: "wizard", + arguments: new Dictionary(StringComparer.Ordinal) + { + ["answer.CONFIRM"] = "ordinary", + ["answer.confirm"] = false, + }).ConfigureAwait(false); + var text = string.Join('\n', result.Content.OfType().Select(static block => block.Text)); + var properties = tool.JsonSchema.GetProperty("properties"); + + properties.TryGetProperty("answer.CONFIRM", out _).Should().BeTrue(); + properties.TryGetProperty("answer.confirm", out _).Should().BeTrue(); + result.IsError.Should().NotBeTrue(); + text.Should().Contain("ordinary:False"); + } + + [TestMethod] + [Description("MCP supplies IReplInteractionChannel, so the binder synthesizes structured progress before direct service lookup. AutomationHidden requiredness must use that same fallback, retain the tool, omit the field, and allow an argument-free call.")] + public async Task When_AutomationHiddenRequiredProgressUsesInteractionChannel_Then_TheToolRemainsInvocable() + { + await using var fixture = await McpTestFixture.CreateAsync(app => app.Map( + "sync", + ([ReplOption(Name = "progress", Arity = ReplArity.ExactlyOne, AutomationHidden = true)] IProgress progress) => + progress is not null ? "progress-ready" : "missing")); + + var tools = await fixture.Client.ListToolsAsync().ConfigureAwait(false); + var tool = tools.Single(candidate => string.Equals(candidate.Name, "sync", StringComparison.Ordinal)); + var result = await fixture.Client.CallToolAsync( + toolName: "sync", + arguments: new Dictionary(StringComparer.Ordinal)).ConfigureAwait(false); + var text = string.Join('\n', result.Content.OfType().Select(static block => block.Text)); + + tool.JsonSchema.GetProperty("properties").TryGetProperty("progress", out _).Should().BeFalse(); + result.IsError.Should().BeFalse(); + text.Should().Contain("progress-ready"); + } + + [TestMethod] + [Description("An all-surfaces hidden required option with no service fallback would disappear from both schema and call allow-list while remaining mandatory. MCP startup must reject that provider-specific impossible contract promptly rather than advertise a dead tool.")] + public async Task When_HiddenRequiredOptionHasNoServiceFallback_Then_McpStartupFailsFast() + { + var start = async () => await McpTestFixture.CreateAsync(app => + app.Map( + "deploy", + ([ReplOption(Name = "token", Arity = ReplArity.ExactlyOne, Hidden = true)] string token) => token)).ConfigureAwait(false); + + var faulted = await start.Should() + .ThrowAsync() + .WaitAsync(TimeSpan.FromSeconds(15)) + .ConfigureAwait(false); + + faulted.WithMessage("Option target 'token' (rendered as '--token') for command 'deploy' cannot be hidden because it is required.*"); + } + + [TestMethod] + [Description("Provider-aware requiredness also applies to visible options: if omission reaches a registered service before explicit lower-bound enforcement, MCP must not mark the field required and an argument-free call must receive that service value.")] + public async Task When_VisibleRequiredOptionHasAServiceFallback_Then_McpSchemaMakesItOptional() + { + await using var fixture = await McpTestFixture.CreateAsync( + app => app.Map( + "deploy", + ([ReplOption(Name = "token", Arity = ReplArity.ExactlyOne)] string token) => token), + configureServices: static services => services.AddSingleton("service-token")); + + var tools = await fixture.Client.ListToolsAsync().ConfigureAwait(false); + var tool = tools.Single(candidate => string.Equals(candidate.Name, "deploy", StringComparison.Ordinal)); + var result = await fixture.Client.CallToolAsync( + toolName: "deploy", + arguments: new Dictionary(StringComparer.Ordinal)).ConfigureAwait(false); + var text = string.Join('\n', result.Content.OfType().Select(static block => block.Text)); + + tool.JsonSchema.GetProperty("properties").TryGetProperty("token", out _).Should().BeTrue(); + if (tool.JsonSchema.TryGetProperty("required", out var required)) + { + required.EnumerateArray().Select(static item => item.GetString()).Should().NotContain("token"); + } + result.IsError.Should().BeFalse(); + text.Should().Contain("service-token"); + } + + [TestMethod] + [Description("Hiding an option after the first tools/list must withdraw it from the next one. The MCP snapshot is rebuilt only when routing is invalidated, so a visibility change that forgets to invalidate leaves an agent seeing an option the app has retracted.")] + public async Task When_OptionIsHiddenAfterFirstToolsList_Then_SecondToolsListOmitsIt() + { + CommandBuilder? deploy = null; + await using var fixture = await McpTestFixture.CreateAsync(app => + deploy = app.Map( + "deploy", + ([ReplOption(Name = "environment")] string environment, [ReplOption(Name = "internalMode")] bool internalMode = false) => + $"{environment}:{internalMode}")); + + var advertisedBefore = await ReadDeployPropertiesAsync(fixture).ConfigureAwait(false); + deploy!.WithOption("internalMode", option => option.Hidden()); + var advertisedAfter = await ReadDeployPropertiesAsync(fixture).ConfigureAwait(false); + + advertisedBefore.Should().Contain("internalMode"); + advertisedAfter.Should().NotContain("internalMode"); + advertisedAfter.Should().Contain("environment", "hiding one option must not withdraw its siblings"); + } + + [TestMethod] + [Description("If a post-start visibility change makes the new MCP contract impossible, refresh must fail closed rather than restore and permanently cache the old snapshot that still advertises the retracted option. Re-exposing it must then recover normally. Because a client has already received a working schema by this point, the error must not name the option, its rendered token, or the route — that identity is exactly what Hidden() was meant to withhold.")] + public async Task When_RequiredOptionIsHiddenAfterFirstToolsList_Then_RefreshFailsClosedUntilConfigurationRecovers() + { + CommandBuilder? deploy = null; + await using var fixture = await McpTestFixture.CreateAsync(app => + deploy = app.Map( + "deploy", + ([ReplOption(Name = "token", Arity = ReplArity.ExactlyOne)] string token) => token)); + var advertisedBefore = await ReadDeployPropertiesAsync(fixture).ConfigureAwait(false); + + deploy!.WithOption("token", static option => option.Hidden()); + var firstRefresh = async () => await fixture.Client.ListToolsAsync().ConfigureAwait(false); + var secondRefresh = async () => await fixture.Client.ListToolsAsync().ConfigureAwait(false); + + advertisedBefore.Should().Contain("token"); + var firstFault = await firstRefresh.Should().ThrowAsync().ConfigureAwait(false); + var secondFault = await secondRefresh.Should().ThrowAsync().ConfigureAwait(false); + + firstFault.Which.Message.Should().NotContainAny("token", "--token", "deploy"); + secondFault.Which.Message.Should().NotContainAny("token", "--token", "deploy"); + + deploy.WithOption("token", static option => option.Hidden(isHidden: false)); + var advertisedAfterRecovery = await ReadDeployPropertiesAsync(fixture).ConfigureAwait(false); + advertisedAfterRecovery.Should().Contain("token"); + } + + [TestMethod] + [Description("Same contract one level up: hiding a whole command after the first tools/list withdraws its tool. This axis predates option-level visibility and shares the missing-invalidation cause, so it is pinned alongside it.")] + public async Task When_CommandIsHiddenAfterFirstToolsList_Then_SecondToolsListOmitsIt() + { + CommandBuilder? wizard = null; + await using var fixture = await McpTestFixture.CreateAsync(app => + { + app.Map("ping", () => "pong"); + wizard = app.Map("wizard", () => "interactive"); + }); + + var before = await fixture.Client.ListToolsAsync().ConfigureAwait(false); + wizard!.Hidden(); + var after = await fixture.Client.ListToolsAsync().ConfigureAwait(false); + + before.Should().Contain(tool => string.Equals(tool.Name, "wizard", StringComparison.Ordinal)); + after.Should().NotContain(tool => string.Equals(tool.Name, "wizard", StringComparison.Ordinal)); + after.Should().Contain(tool => string.Equals(tool.Name, "ping", StringComparison.Ordinal)); + } + + [TestMethod] + [Description("A server that fails while starting must surface its own exception from CreateAsync. The fixture used to launch RunAsync fire-and-forget and then await the client handshake, so a start failure was observable only as an initialize timeout carrying the wrong exception — which is what pushed an earlier iteration to make production code throw synchronously just to be testable.")] + public async Task When_ServerStartFails_Then_FixtureSurfacesTheServerFault() + { + var start = async () => await McpTestFixture.CreateAsync( + app => app.Map("ping", () => "pong"), + configureOptions: static options => options.TransportFactory = + static (_, _) => throw new InvalidOperationException("ga-bu-zo-meu: transport refused to start")).ConfigureAwait(false); + + var faulted = await start.Should() + .ThrowAsync() + .WaitAsync(TimeSpan.FromSeconds(15)) + .ConfigureAwait(false); + + faulted.WithMessage("*ga-bu-zo-meu*"); + } + + private static async Task> ReadDeployPropertiesAsync(McpTestFixture fixture) + { + var tools = await fixture.Client.ListToolsAsync().ConfigureAwait(false); + var tool = tools.Single(candidate => string.Equals(candidate.Name, "deploy", StringComparison.Ordinal)); + + return [.. tool.JsonSchema.GetProperty("properties").EnumerateObject().Select(static property => property.Name)]; + } + // ── Options group camelCase naming ───────────────────────────────── [TestMethod] diff --git a/src/Repl.McpTests/Given_McpServerOptionsBuilder.cs b/src/Repl.McpTests/Given_McpServerOptionsBuilder.cs index d9e45e2b..3500d955 100644 --- a/src/Repl.McpTests/Given_McpServerOptionsBuilder.cs +++ b/src/Repl.McpTests/Given_McpServerOptionsBuilder.cs @@ -2,7 +2,9 @@ using ModelContextProtocol.Client; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; +using Repl.Interaction; using Repl.Mcp; +using Repl.Parameters; namespace Repl.McpTests; @@ -24,6 +26,22 @@ public void When_AppHasCommands_Then_OptionsContainToolCollection() options.ToolCollection.Should().Contain(tool => string.Equals(tool.ProtocolTool.Name, "ping", StringComparison.Ordinal)); } + [TestMethod] + [Description("Core MCP dispatch guarantees a per-call interaction channel even when no base provider is supplied. Static discovery must retain a tool whose AutomationHidden required progress parameter is therefore synthesizable.")] + public void When_CoreMcpOptionsHaveNoBaseProvider_Then_RequiredHiddenProgressToolIsRetained() + { + var app = CoreReplApp.Create(); + app.Map( + "sync", + ([ReplOption(Name = "progress", Arity = ReplArity.ExactlyOne, AutomationHidden = true)] IProgress progress) => + progress is not null ? "progress-ready" : "missing"); + + var options = app.BuildMcpServerOptions(); + + options.ToolCollection.Should().Contain(tool => + string.Equals(tool.ProtocolTool.Name, "sync", StringComparison.Ordinal)); + } + [TestMethod] [Description("BuildMcpServerOptions captures the current resources as a pre-populated collection.")] public void When_AppHasReadOnlyCommands_Then_OptionsContainResourceCollection() diff --git a/src/Repl.McpTests/Given_McpToolAdapter.cs b/src/Repl.McpTests/Given_McpToolAdapter.cs index f2b682b4..0774af78 100644 --- a/src/Repl.McpTests/Given_McpToolAdapter.cs +++ b/src/Repl.McpTests/Given_McpToolAdapter.cs @@ -1,5 +1,8 @@ +using Repl; using Repl.Mcp; using Repl.Documentation; +using Repl.Internal.Options; +using Repl.Parameters; using System.Text.Json; namespace Repl.McpTests; @@ -264,6 +267,142 @@ public void When_ArgumentIsNotInToolSchema_Then_Rejected() .WithMessage("*not defined*schema*"); } + [TestMethod] + [Description("A differently-cased MCP key remains backwards-compatible when it identifies one schema option unambiguously, including custom short-token reconstruction.")] + public void When_McpOptionNameUsesDifferentCaseAndHasOneMatch_Then_ConfiguredTokenIsRetained() + { + var command = new ReplDocCommand( + Path: "deploy", + Description: null, + Aliases: [], + IsHidden: false, + Arguments: [], + Options: [new ReplDocOption("tenant", "string", Required: false, Description: null, Aliases: ["-t"], ReverseAliases: [], ValueAliases: [], EnumValues: [], DefaultValue: null)]); + + var (tokens, _) = McpToolAdapter.PrepareExecution( + command, + new Dictionary(StringComparer.Ordinal) + { + ["TENANT"] = JsonSerializer.SerializeToElement("acme"), + }); + + tokens.Should().Equal("deploy", "-t", "acme"); + } + + [TestMethod] + [Description("For a single advertised field, submitting both its exact spelling and a unique case-insensitive fallback is rejected as a duplicate canonical field instead of silently overwriting one value.")] + public void When_ExactAndCaseFallbackNameTheSameMcpField_Then_RejectedAsDuplicate() + { + var command = new ReplDocCommand( + Path: "deploy", + Description: null, + Aliases: [], + IsHidden: false, + Arguments: [], + Options: [new ReplDocOption("tenant", "string", Required: false, Description: null, Aliases: ["-t"], ReverseAliases: [], ValueAliases: [], EnumValues: [], DefaultValue: null)]); + var action = () => McpToolAdapter.PrepareExecution( + command, + new Dictionary(StringComparer.Ordinal) + { + ["tenant"] = JsonSerializer.SerializeToElement("south"), + ["TENANT"] = JsonSerializer.SerializeToElement("north"), + }); + + action.Should().Throw() + .WithMessage("*TENANT*duplicates*tenant*"); + } + + [TestMethod] + [Description("Case-sensitive option names that differ only by casing preserve both exact MCP fields and reconstruct each distinct CLI token in one invocation.")] + public void When_OptionNamesDifferOnlyByCase_Then_BothExactMcpFieldsRemainDistinct() + { + var command = CreateCaseDistinctOptionsCommand(); + + var (tokens, _) = McpToolAdapter.PrepareExecution( + command, + new Dictionary(StringComparer.Ordinal) + { + ["tenant"] = JsonSerializer.SerializeToElement("south"), + ["TENANT"] = JsonSerializer.SerializeToElement("north"), + }); + + tokens.Should().Equal("deploy", "--tenant", "south", "--TENANT", "north"); + } + + [TestMethod] + [Description("A non-exact MCP field casing is rejected when multiple advertised fields match it case-insensitively.")] + public void When_NonExactMcpFieldMatchesMultipleCaseDistinctOptions_Then_RejectedAsAmbiguous() + { + var action = () => McpToolAdapter.PrepareExecution( + CreateCaseDistinctOptionsCommand(), + new Dictionary(StringComparer.Ordinal) + { + ["Tenant"] = JsonSerializer.SerializeToElement("acme"), + }); + + action.Should().Throw() + .WithMessage("*ambiguous*tenant*TENANT*"); + } + + [TestMethod] + [Description("Case-distinct ordinary fields retain command provenance beside exact synthetic cursor and page-size fields instead of being reclassified as result-flow controls.")] + public void When_CaseDistinctOptionsMatchResultFlowNames_Then_EachFieldKeepsItsOwnSink() + { + var command = new ReplDocCommand( + Path: "contacts", + Description: null, + Aliases: [], + IsHidden: false, + Arguments: [], + Options: + [ + new ReplDocOption("_replcursor", "string", Required: false, Description: null, Aliases: ["--_replcursor"], ReverseAliases: [], ValueAliases: [], EnumValues: [], DefaultValue: null), + new ReplDocOption("_replpagesize", "string", Required: false, Description: null, Aliases: ["--_replpagesize"], ReverseAliases: [], ValueAliases: [], EnumValues: [], DefaultValue: null), + ], + AcceptsPagingInput: true); + + var (tokens, prefills) = McpToolAdapter.PrepareExecution( + command, + new Dictionary(StringComparer.Ordinal) + { + [McpResultFlowArgumentNames.Cursor] = JsonSerializer.SerializeToElement("opaque"), + ["_replcursor"] = JsonSerializer.SerializeToElement("ordinary-cursor"), + [McpResultFlowArgumentNames.PageSize] = JsonSerializer.SerializeToElement(25), + ["_replpagesize"] = JsonSerializer.SerializeToElement("ordinary-size"), + }); + + tokens.Should().Equal( + ReplResultFlowOptionNames.Cursor, "opaque", + ReplResultFlowOptionNames.PageSize, "25", + "contacts", "--_replcursor", "ordinary-cursor", "--_replpagesize", "ordinary-size"); + prefills.Should().BeEmpty(); + } + + [TestMethod] + [Description("A declared answer and a case-distinct ordinary option under the answer prefix keep separate prefill and CLI destinations.")] + public void When_CaseDistinctOptionMatchesAnswerField_Then_DeclaredProvenanceControlsDispatch() + { + var command = new ReplDocCommand( + Path: "wizard", + Description: null, + Aliases: [], + IsHidden: false, + Arguments: [], + Options: [new ReplDocOption("answer.CONFIRM", "string", Required: false, Description: null, Aliases: ["--answer.CONFIRM"], ReverseAliases: [], ValueAliases: [], EnumValues: [], DefaultValue: null)], + Answers: [new ReplDocAnswer("confirm", "bool", Description: null)]); + + var (tokens, prefills) = McpToolAdapter.PrepareExecution( + command, + new Dictionary(StringComparer.Ordinal) + { + ["answer.confirm"] = JsonSerializer.SerializeToElement(value: false), + ["answer.CONFIRM"] = JsonSerializer.SerializeToElement("ordinary"), + }); + + tokens.Should().Equal("wizard", "--answer.CONFIRM", "ordinary"); + prefills.Should().ContainSingle().Which.Should().Be(new KeyValuePair("confirm", "false")); + } + [TestMethod] [Description("PrepareExecution rejects token-like MCP argument values before reconstructing CLI tokens.")] public void When_ArgumentValueStartsWithDashDash_Then_Rejected() @@ -333,6 +472,136 @@ public void When_ResultFlowInputIsNotInToolSchema_Then_Rejected() .WithMessage("*not defined*schema*"); } + [TestMethod] + [Description("A bool option's value is embedded as a single inline '--name=value' token instead of a '--name' / 'value' pair, so it can never be split apart and re-lexed as a fresh option by the downstream parser.")] + public void When_BoolOptionValueIsReconstructed_Then_EmbeddedAsSingleInlineToken() + { + var command = new ReplDocCommand( + Path: "deploy", + Description: null, + Aliases: [], + IsHidden: false, + Arguments: [], + Options: [new ReplDocOption("verbose", "bool", Required: false, Description: null, Aliases: [], ReverseAliases: [], ValueAliases: [], EnumValues: [], DefaultValue: null)]); + + var (tokens, _) = McpToolAdapter.PrepareExecution( + command, + new Dictionary(StringComparer.Ordinal) + { + ["verbose"] = JsonSerializer.SerializeToElement("true"), + }); + + tokens.Should().Equal("deploy", "--verbose=true"); + } + + [TestMethod] + [Description("PrepareExecution rejects positional route-segment values that look like a CLI option token: unlike an option, a positional segment has no separator that can escape the value, so it would be re-lexed as a fresh option once substituted into the token stream.")] + public void When_PositionalArgumentValueLooksLikeOptionToken_Then_Rejected() + { + var command = new ReplDocCommand( + Path: "contacts {id}", + Description: null, + Aliases: [], + IsHidden: false, + Arguments: [new ReplDocArgument("id", "string", Required: true, Description: null)], + Options: []); + + var action = () => McpToolAdapter.PrepareExecution( + command, + new Dictionary(StringComparer.Ordinal) + { + ["id"] = JsonSerializer.SerializeToElement("-t=denim"), + }); + + action.Should().Throw() + .WithMessage("*argument value*CLI option*positional*"); + } + + [TestMethod] + [Description("A signed numeric literal remains a valid positional route-segment value: it starts with a dash but IsSignedNumericLiteral carves it out, matching the CLI parser's own positional-vs-option rule.")] + public void When_PositionalArgumentValueIsSignedNumericLiteral_Then_Accepted() + { + var command = new ReplDocCommand( + Path: "contacts {id}", + Description: null, + Aliases: [], + IsHidden: false, + Arguments: [new ReplDocArgument("id", "string", Required: true, Description: null)], + Options: []); + + var (tokens, _) = McpToolAdapter.PrepareExecution( + command, + new Dictionary(StringComparer.Ordinal) + { + ["id"] = JsonSerializer.SerializeToElement("-42"), + }); + + tokens.Should().Equal("contacts", "-42"); + } + + [TestMethod] + [Description( + "End-to-end regression for the option-smuggling vulnerability: a tool call supplies a value " + + "for a visible bool option that itself looks like '-t=denim', an alias of a Hidden() option on " + + "the same route. Before the fix, ReconstructTokens emitted the value as its own token, which the " + + "bool flag declined to consume without a diagnostic (that decline is required so legitimate " + + "flag-chaining like '--verbose --other' keeps working) and which the parser then re-lexed as a " + + "fresh '-t' option, binding the hidden 'tenant' target. The inline '--verbose=-t=denim' token this " + + "test asserts on cannot be split apart that way, so 'tenant' must never appear as bound.")] + public void When_ToolCallValueLooksLikeHiddenOptionToken_Then_ItIsNotBoundAsAnOption() + { + var command = new ReplDocCommand( + Path: "deploy", + Description: null, + Aliases: [], + IsHidden: false, + Arguments: [], + Options: [new ReplDocOption("verbose", "bool", Required: false, Description: null, Aliases: [], ReverseAliases: [], ValueAliases: [], EnumValues: [], DefaultValue: null)]); + + var (tokens, _) = McpToolAdapter.PrepareExecution( + command, + new Dictionary(StringComparer.Ordinal) + { + ["verbose"] = JsonSerializer.SerializeToElement("-t=denim"), + }); + + tokens.Should().Equal("deploy", "--verbose=-t=denim"); + + var schema = new OptionSchema( + [ + new OptionSchemaEntry("--verbose", "verbose", OptionSchemaTokenKind.BoolFlag, ReplArity.ZeroOrOne), + new OptionSchemaEntry("-t", "tenant", OptionSchemaTokenKind.NamedOption, ReplArity.ZeroOrOne, IsHidden: true), + ], + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["verbose"] = new OptionSchemaParameter("verbose", typeof(bool), ReplParameterMode.OptionOnly), + ["tenant"] = new OptionSchemaParameter("tenant", typeof(string), ReplParameterMode.OptionOnly, IsHidden: true), + }, + ReplCaseSensitivity.CaseSensitive); + + var parseResult = InvocationOptionParser.Parse( + [.. tokens.Skip(1)], + schema, + new ParsingOptions()); + + parseResult.NamedOptions.Should().NotContainKey("tenant"); + parseResult.NamedOptions.Should().ContainKey("verbose"); + parseResult.NamedOptions["verbose"].Should().ContainSingle().Which.Should().Be("-t=denim"); + } + + private static ReplDocCommand CreateCaseDistinctOptionsCommand() => + new( + Path: "deploy", + Description: null, + Aliases: [], + IsHidden: false, + Arguments: [], + Options: + [ + new ReplDocOption("tenant", "string", Required: false, Description: null, Aliases: ["--tenant"], ReverseAliases: [], ValueAliases: [], EnumValues: [], DefaultValue: null), + new ReplDocOption("TENANT", "string", Required: false, Description: null, Aliases: ["--TENANT"], ReverseAliases: [], ValueAliases: [], EnumValues: [], DefaultValue: null), + ]); + private static ReplDocCommand CreatePagedCommand(string path) => new( Path: path, diff --git a/src/Repl.McpTests/McpTestFixture.cs b/src/Repl.McpTests/McpTestFixture.cs index 23c90048..04279eee 100644 --- a/src/Repl.McpTests/McpTestFixture.cs +++ b/src/Repl.McpTests/McpTestFixture.cs @@ -62,7 +62,7 @@ public static async Task CreateAsync( var options = new ReplMcpServerOptions(); configureOptions?.Invoke(options); - var serviceProvider = configureServices is not null ? app.Services : EmptyServiceProvider.Instance; + var serviceProvider = app.Services; var handler = new McpServerHandler(app.Core, options, serviceProvider); var clientToServer = new Pipe(); @@ -84,9 +84,47 @@ public static async Task CreateAsync( var clientTransport = new StreamClientTransport( clientToServer.Writer.AsStream(), serverToClient.Reader.AsStream()); - var client = await McpClient.CreateAsync(clientTransport, clientOptions).ConfigureAwait(false); - return new McpTestFixture(app, client, serverTask, cts, clientToServer, serverToClient); + try + { + // Race the handshake against the server. Awaiting only the client means a server that + // fails while starting is observable solely as an initialize timeout carrying the wrong + // exception — which is what once pushed production code into throwing synchronously + // just to stay testable. + var clientTask = McpClient.CreateAsync(clientTransport, clientOptions); + if (ReferenceEquals(await Task.WhenAny(serverTask, clientTask).ConfigureAwait(false), serverTask)) + { + // Rethrows a start failure; a clean early exit means the handshake never completes. + await serverTask.ConfigureAwait(false); + + throw new InvalidOperationException( + "The MCP server stopped before the client completed its handshake."); + } + + var client = await clientTask.ConfigureAwait(false); + + return new McpTestFixture(app, client, serverTask, cts, clientToServer, serverToClient); + } + catch + { + await AbandonAsync(cts, clientToServer, serverToClient).ConfigureAwait(false); + throw; + } + } + + /// + /// Releases what + /// allocated when construction fails before the fixture takes ownership. + /// + private static async Task AbandonAsync( + CancellationTokenSource cts, + Pipe clientToServer, + Pipe serverToClient) + { + await cts.CancelAsync().ConfigureAwait(false); + await clientToServer.Writer.CompleteAsync().ConfigureAwait(false); + await serverToClient.Writer.CompleteAsync().ConfigureAwait(false); + cts.Dispose(); } public async ValueTask DisposeAsync() @@ -113,11 +151,6 @@ public async ValueTask DisposeAsync() _cts.Dispose(); } - private sealed class EmptyServiceProvider : IServiceProvider - { - public static readonly EmptyServiceProvider Instance = new(); - public object? GetService(Type serviceType) => null; - } internal sealed class PipeIoContext(Stream inputStream, Stream outputStream) : IReplIoContext { diff --git a/src/Repl.Tests/Given_CommandBuilderEnrichment.cs b/src/Repl.Tests/Given_CommandBuilderEnrichment.cs index 5d03633a..12985ab3 100644 --- a/src/Repl.Tests/Given_CommandBuilderEnrichment.cs +++ b/src/Repl.Tests/Given_CommandBuilderEnrichment.cs @@ -223,6 +223,122 @@ public void When_HandlerHasDescriptionAttribute_Then_ArgumentDescriptionIsPopula arg.Description.Should().Be("Contact numeric id"); } + [TestMethod] + [Description("An option hidden through the fluent builder disappears from the aggregate documentation model — the model MCP builds — but survives, flagged, when its command is targeted explicitly. That mirrors how a hidden command behaves and gives an app author the only way to inventory hidden options.")] + public void When_CommandOptionIsHiddenFluently_Then_OnlyTheAggregateModelOmitsIt() + { + var sut = CoreReplApp.Create(); + sut.Map( + "deploy", + ([ReplOption] string environment, [ReplOption] bool internalMode = false) => $"{environment}:{internalMode}") + .WithOption("internalMode", static option => option.Hidden()); + + var aggregate = sut.CreateDocumentationModel().Commands.Should().ContainSingle().Which; + var targeted = sut.CreateDocumentationModel("deploy").Commands.Should().ContainSingle().Which; + + aggregate.Options.Should().ContainSingle(option => option.Name == "environment"); + aggregate.Options.Should().NotContain(option => option.Name == "internalMode"); + targeted.Options.Should().Contain(option => option.Name == "environment" && !option.IsHidden); + targeted.Options.Should().Contain(option => option.Name == "internalMode" && option.IsHidden); + } + + [TestMethod] + [Description("Hiding an option after Map publishes a new option schema rather than mutating one, so this asserts the parsing contract survives the swap: the accepted tokens and the resolved arity must be byte-identical before and after, while only the discovery projection changes. Without this, a future change to the swap could silently narrow what the parser accepts.")] + public void When_OptionIsHiddenFluently_Then_ParsingContractIsUnchanged() + { + var sut = CoreReplApp.Create(); + var command = sut.Map( + "deploy", + ([ReplOption] string environment, [ReplOption] bool internalMode = false) => $"{environment}:{internalMode}"); + var before = command.OptionSchema; + string[] tokensBefore = [.. before.KnownTokens]; + var arityBefore = before.ResolveParameterArity("internalMode"); + + command.WithOption("internalMode", option => option.Hidden()); + + var after = command.OptionSchema; + after.Should().NotBeSameAs(before, "visibility is published as a new schema, never mutated in place"); + after.KnownTokens.Should().Equal(tokensBefore, "a hidden option stays fully parsable"); + after.ResolveParameterArity("internalMode").Should().Be(arityBefore); + after.Entries.Should().BeSameAs(before.Entries, "entries carry the parsing contract and are reused verbatim"); + after.IsOptionHidden("internalMode").Should().BeTrue(); + after.DiscoverableParameters.Should().NotContain(parameter => parameter.Name == "internalMode"); + after.DiscoverableParameters.Should().Contain(parameter => parameter.Name == "environment"); + } + + [TestMethod] + [Description("AutomationHidden is the programmatic-only axis: unlike Hidden it keeps the option in the documentation model, so human-facing exports and help still show it and only the MCP projection drops it. Mirrors CommandAnnotations.AutomationHidden one level down.")] + public void When_CommandOptionIsAutomationHidden_Then_TheDocumentationModelKeepsItFlagged() + { + var sut = CoreReplApp.Create(); + sut.Map( + "deploy", + ([ReplOption] string environment, [ReplOption] bool internalMode = false) => $"{environment}:{internalMode}") + .WithOption("internalMode", static option => option.AutomationHidden()); + + var aggregate = sut.CreateDocumentationModel().Commands.Should().ContainSingle().Which; + + aggregate.Options.Should().Contain(option => + option.Name == "internalMode" && option.IsAutomationHidden && !option.IsHidden); + aggregate.Options.Should().Contain(option => + option.Name == "environment" && !option.IsAutomationHidden); + } + + [TestMethod] + [Description("Fluent visibility wins over the attribute on both axes and in both directions, because the attribute seeds the schema at Map while a fluent call publishes a new one afterwards. Asserting a single direction would miss an inverted precedence.")] + public void When_AttributeAndFluentVisibilityDisagree_Then_FluentWins() + { + var sut = CoreReplApp.Create(); + var command = sut.Map( + "deploy", + ([ReplOption(Hidden = true)] bool attributeHidden = false, + [ReplOption(AutomationHidden = true)] bool attributeAutomationHidden = false, + [ReplOption] bool fluentHidden = false, + [ReplOption] bool fluentAutomationHidden = false) => + $"{attributeHidden}{attributeAutomationHidden}{fluentHidden}{fluentAutomationHidden}"); + + command.WithOption("attributeHidden", option => option.Hidden(isHidden: false)); + command.WithOption("attributeAutomationHidden", option => option.AutomationHidden(isAutomationHidden: false)); + command.WithOption("fluentHidden", option => option.Hidden()); + command.WithOption("fluentAutomationHidden", option => option.AutomationHidden()); + + var schema = command.OptionSchema; + schema.IsOptionHidden("attributeHidden").Should().BeFalse(); + schema.IsOptionAutomationHidden("attributeAutomationHidden").Should().BeFalse(); + schema.IsOptionHidden("fluentHidden").Should().BeTrue(); + schema.IsOptionAutomationHidden("fluentAutomationHidden").Should().BeTrue(); + } + + [TestMethod] + [Description("Selecting an unknown command option target fails clearly instead of leaving the intended option visible.")] + public void When_SelectingUnknownCommandOption_Then_ConfigurationThrows() + { + var sut = CoreReplApp.Create(); + var command = sut.Map("deploy", ([ReplOption] bool force) => force); + + var act = () => command.WithOption("missing", static option => option.Hidden()); + + act.Should().Throw() + .WithMessage("*option target*missing*deploy*"); + } + + [TestMethod] + [Description("The declarative form reaches the same documentation contract as the fluent one: omitted from the aggregate model, present and flagged when the command is targeted.")] + public void When_CommandOptionHasHiddenAttribute_Then_OnlyTheAggregateModelOmitsIt() + { + var sut = CoreReplApp.Create(); + sut.Map( + "deploy", + ([ReplOption] string environment, [ReplOption(Hidden = true)] bool internalMode = false) => $"{environment}:{internalMode}"); + + var aggregate = sut.CreateDocumentationModel().Commands.Should().ContainSingle().Which; + var targeted = sut.CreateDocumentationModel("deploy").Commands.Should().ContainSingle().Which; + + aggregate.Options.Should().ContainSingle(option => option.Name == "environment"); + aggregate.Options.Should().NotContain(option => option.Name == "internalMode"); + targeted.Options.Should().Contain(option => option.Name == "internalMode" && option.IsHidden); + } + [TestMethod] [Description("Verifies injected IGlobalOptionsAccessor parameters are omitted from documentation options.")] public void When_HandlerUsesGlobalOptionsAccessor_Then_DocumentationOmitsAccessorOption() diff --git a/src/Repl.Tests/Given_GlobalOptionsAccessor.cs b/src/Repl.Tests/Given_GlobalOptionsAccessor.cs index 34e066bd..dfa0962c 100644 --- a/src/Repl.Tests/Given_GlobalOptionsAccessor.cs +++ b/src/Repl.Tests/Given_GlobalOptionsAccessor.cs @@ -1,3 +1,4 @@ +using System.Reflection; using AwesomeAssertions; namespace Repl.Tests; @@ -5,6 +6,70 @@ namespace Repl.Tests; [TestClass] public sealed class Given_GlobalOptionsAccessor { + [TestMethod] + [Description("Repl.Defaults ships separately and depends on Repl.Core by minimum version, so an older compiled Repl.Defaults can run against a newer Repl.Core and will call the six-parameter arity it was built against. Asserting the method merely exists would pass even if it stopped registering anything, so this invokes it and checks the option really lands — visible, since that arity predates the visibility flag.")] + public void When_LegacyBinaryDescriptorIsInvoked_Then_TheOptionIsRegisteredAndVisible() + { + var sut = new ParsingOptions(); + var legacy = typeof(ParsingOptions).GetMethod( + "AddGlobalOptionCore", + BindingFlags.Instance | BindingFlags.NonPublic, + binder: null, + types: [typeof(string), typeof(Type), typeof(string[]), typeof(string), typeof(string), typeof(Type)], + modifiers: null); + + string[] aliases = ["-d"]; + + legacy.Should().NotBeNull("an already-compiled Repl.Defaults binds to this arity"); + legacy!.Invoke(sut, ["denim-tint", typeof(string), aliases, null, "Visible by construction.", null]); + + var registered = sut.GlobalOptions.Values.Should().ContainSingle().Which; + registered.Name.Should().Be("denim-tint"); + registered.IsHidden.Should().BeFalse(); + } + + [TestMethod] + [Description("Global options are consumed before routing and never enter the documentation model, so they cannot reach an MCP tool schema at all. Selecting one therefore returns a builder that deliberately cannot express automation visibility — enforced by the type rather than a runtime throw, so a permanent silent no-op is unrepresentable instead of merely undocumented. There is no behaviour to assert here because the point is that none exists.")] + public void When_SelectingAGlobalOption_Then_TheBuilderCannotExpressAutomationVisibility() + { + var sut = new ParsingOptions(); + sut.AddGlobalOption("tenant"); + + var builder = sut.GlobalOption("tenant"); + + builder.Should().BeOfType(); + typeof(GlobalOptionBuilder).GetMethod("AutomationHidden").Should().BeNull(); + } + + [TestMethod] + [Description("Selection accepts the name either bare or as the rendered token, so an option registered as \"tenant\" is still configurable as \"--tenant\". A keyed probe alone would miss this, which is why the prefixed form is pinned separately from the common case.")] + public void When_SelectingAGlobalOptionByItsRenderedToken_Then_ItResolves() + { + var sut = new ParsingOptions(); + sut.AddGlobalOption("tenant"); + + sut.GlobalOption("--tenant").Hidden(); + + sut.GlobalOptions.Values.Should().ContainSingle().Which.IsHidden.Should().BeTrue(); + } + + [TestMethod] + [Description("Hidden global aliases use the configured parser comparer: case-sensitive mode must not hide a differently-cased registered token, while the exact spelling remains configurable.")] + public void When_HidingGlobalAliasInCaseSensitiveMode_Then_OnlyTheExactRegisteredSpellingMatches() + { + var sut = new ParsingOptions + { + OptionCaseSensitivity = ReplCaseSensitivity.CaseSensitive, + }; + sut.AddGlobalOption("tenant", aliases: ["--ACCOUNT"]); + + var mismatchedCase = () => sut.GlobalOption("tenant").HiddenAlias("--account"); + + mismatchedCase.Should().Throw(); + sut.GlobalOption("tenant").HiddenAlias("--ACCOUNT"); + sut.GlobalOptions.Values.Single().HiddenAliases.Should().ContainSingle().Which.Should().Be("--ACCOUNT"); + } + [TestMethod] [Description("GetValue returns parsed string value after Update.")] public void When_StringOptionParsed_Then_GetValueReturnsIt() @@ -521,6 +586,19 @@ public void When_RegisteredWithStringTypeName_Email_Then_ResolvesAsString() parsing.GlobalOptions["contact"].ValueType.Should().Be(typeof(string)); } + [TestMethod] + [Description("Two different Names ('tenant' and '--tenant') can normalize to the identical canonical token. Left unrejected, GlobalOption(name) would resolve that token to whichever definition happens to enumerate first — an ambiguity, not a deterministic choice — so registering the second one must be rejected the same way an exact-name duplicate already is.")] + public void When_TwoGlobalOptionsNormalizeToTheSameCanonicalToken_Then_TheSecondRegistrationThrows() + { + var sut = new ParsingOptions(); + sut.AddGlobalOption("tenant"); + + var register = () => sut.AddGlobalOption("--tenant"); + + register.Should().Throw() + .WithMessage("*tenant*--tenant*"); + } + private sealed class TestTypedOptions { public string? Tenant { get; set; } diff --git a/src/Repl.Tests/Given_InteractiveAutocomplete_OptionCandidates.cs b/src/Repl.Tests/Given_InteractiveAutocomplete_OptionCandidates.cs index 56604727..dd8d4971 100644 --- a/src/Repl.Tests/Given_InteractiveAutocomplete_OptionCandidates.cs +++ b/src/Repl.Tests/Given_InteractiveAutocomplete_OptionCandidates.cs @@ -24,6 +24,246 @@ public async Task When_CurrentTokenIsOptionPrefix_Then_SuggestsRouteAndGlobalOpt result.HintLine.Should().Contain("--force"); } + [TestMethod] + [Description("Hidden route options and all their aliases are omitted from the completion source shared by interactive and shell completion.")] + public async Task When_RouteOptionIsHidden_Then_CompletionOmitsCanonicalAndAliasTokens() + { + var sut = CoreReplApp.Create(); + sut.Map( + "deploy", + static string ( + [ReplOption(Aliases = ["-e"])] string environment, + [ReplOption(Name = "internal-mode", Aliases = ["-i"], ReverseAliases = ["--no-internal-mode"])] + [ReplValueAlias("--internal", "true")] bool internalMode = false) => + $"{environment}:{internalMode}") + .WithOption("internalMode", static option => option.Hidden()); + + var interactiveLong = await ResolveAutocompleteAsync(sut, "deploy --").ConfigureAwait(false); + var interactiveShort = await ResolveAutocompleteAsync(sut, "deploy -").ConfigureAwait(false); + var shellEngine = new ShellCompletionEngine(sut); + var shellLong = await ResolveShellCandidatesAsync(shellEngine, "app deploy --").ConfigureAwait(false); + var shellShort = await ResolveShellCandidatesAsync(shellEngine, "app deploy -").ConfigureAwait(false); + + var interactiveLongValues = interactiveLong.Suggestions.Select(static suggestion => suggestion.Value).ToArray(); + var interactiveShortValues = interactiveShort.Suggestions.Select(static suggestion => suggestion.Value).ToArray(); + interactiveLongValues.Should().Contain("--environment"); + interactiveLongValues.Should().NotContain("--internal-mode"); + interactiveLongValues.Should().NotContain("--no-internal-mode"); + interactiveLongValues.Should().NotContain("--internal"); + interactiveShortValues.Should().Contain("-e"); + interactiveShortValues.Should().NotContain("-i"); + shellLong.Should().Contain("--environment"); + shellLong.Should().NotContain("--internal-mode"); + shellLong.Should().NotContain("--no-internal-mode"); + shellLong.Should().NotContain("--internal"); + shellShort.Should().Contain("-e"); + shellShort.Should().NotContain("-i"); + } + + [TestMethod] + [Description("A hidden legacy alias is absent from both completion surfaces and cannot activate value completion when typed manually, while the same option's canonical token remains fully discoverable.")] + public async Task When_RouteAliasIsHidden_Then_CompletionKeepsOnlyTheCanonicalTokenAndValues() + { + var sut = CoreReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("organization", aliases: ["--legacy-org", "--LEGACY-ORG"]); + options.Parsing.GlobalOption("organization").HiddenAlias("--legacy-org"); + }); + sut.Map( + "deploy", + static string ([ReplOption(Name = "mode", HiddenAliases = ["--legacy-mode"])] ProbeMode mode = ProbeMode.Debug) => mode.ToString()); + + var interactiveNames = await ResolveAutocompleteAsync(sut, "deploy --").ConfigureAwait(false); + var shellNames = await ResolveShellCandidatesAsync(new ShellCompletionEngine(sut), "app deploy --").ConfigureAwait(false); + var hiddenInteractiveValues = await ResolveAutocompleteAsync(sut, "deploy --legacy-mode ").ConfigureAwait(false); + var hiddenShellValues = await ResolveShellCandidatesAsync(new ShellCompletionEngine(sut), "app deploy --legacy-mode ").ConfigureAwait(false); + var visibleInteractiveValues = await ResolveAutocompleteAsync(sut, "deploy --mode ").ConfigureAwait(false); + var visibleShellValues = await ResolveShellCandidatesAsync(new ShellCompletionEngine(sut), "app deploy --mode ").ConfigureAwait(false); + + interactiveNames.Suggestions.Select(static suggestion => suggestion.Value) + .Should().Contain(["--mode", "--organization", "--LEGACY-ORG"]) + .And.NotContain(["--legacy-mode", "--legacy-org"]); + shellNames.Should().Contain(["--mode", "--organization", "--LEGACY-ORG"]) + .And.NotContain(["--legacy-mode", "--legacy-org"]); + hiddenInteractiveValues.Suggestions.Should().BeEmpty(); + hiddenShellValues.Should().BeEmpty(); + visibleInteractiveValues.Suggestions.Select(static suggestion => suggestion.Value).Should().Contain(nameof(ProbeMode.Debug)); + visibleShellValues.Should().Contain(nameof(ProbeMode.Debug)); + } + + [TestMethod] + [Description("A hidden alias shadows a visible alias with equivalent casing under the option's effective case-insensitive comparer, so neither spelling leaks through option-name or value completion.")] + public async Task When_CaseInsensitiveVisibleAliasMatchesHiddenAlias_Then_HiddenPrecedenceWins() + { + var sut = CoreReplApp.Create(); + sut.Map( + "deploy", + static string ([ReplOption( + Name = "mode", + Aliases = ["--LEGACY-MODE"], + HiddenAliases = ["--legacy-mode"], + CaseSensitivity = ReplCaseSensitivity.CaseInsensitive)] ProbeMode mode = ProbeMode.Debug) => mode.ToString()); + + var interactiveNames = await ResolveAutocompleteAsync(sut, "deploy --").ConfigureAwait(false); + var shellNames = await ResolveShellCandidatesAsync(new ShellCompletionEngine(sut), "app deploy --").ConfigureAwait(false); + var interactiveValues = await ResolveAutocompleteAsync(sut, "deploy --LEGACY-MODE ").ConfigureAwait(false); + var shellValues = await ResolveShellCandidatesAsync(new ShellCompletionEngine(sut), "app deploy --LEGACY-MODE ").ConfigureAwait(false); + + interactiveNames.Suggestions.Select(static suggestion => suggestion.Value) + .Should().Contain("--mode") + .And.NotContain(["--legacy-mode", "--LEGACY-MODE"]); + shellNames.Should().Contain("--mode").And.NotContain(["--legacy-mode", "--LEGACY-MODE"]); + interactiveValues.Suggestions.Should().BeEmpty(); + shellValues.Should().BeEmpty(); + } + + [TestMethod] + [Description("Fluent hiding uses the option's effective global case-insensitive comparer, so all parser-equivalent alias spellings disappear from option-name and value completion while the canonical token stays visible.")] + public async Task When_FluentAliasHasCaseEquivalentInInsensitiveMode_Then_AllEquivalentSpellingsAreHiddenFromCompletion() + { + var sut = CoreReplApp.Create(); + var command = sut.Map( + "deploy", + static string ([ReplOption(Name = "mode", Aliases = ["--legacy-mode", "--LEGACY-MODE"])] ProbeMode mode = ProbeMode.Debug) => mode.ToString()); + sut.Options(static options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + command.WithOption("mode", static option => option.HiddenAlias("--legacy-mode")); + + var interactiveNames = await ResolveAutocompleteAsync(sut, "deploy --").ConfigureAwait(false); + var shellNames = await ResolveShellCandidatesAsync(new ShellCompletionEngine(sut), "app deploy --").ConfigureAwait(false); + var hiddenInteractiveValues = await ResolveAutocompleteAsync(sut, "deploy --LEGACY-MODE ").ConfigureAwait(false); + var hiddenShellValues = await ResolveShellCandidatesAsync(new ShellCompletionEngine(sut), "app deploy --LEGACY-MODE ").ConfigureAwait(false); + var canonicalInteractiveValues = await ResolveAutocompleteAsync(sut, "deploy --mode ").ConfigureAwait(false); + + interactiveNames.Suggestions.Select(static suggestion => suggestion.Value) + .Should().Contain("--mode").And.NotContain(["--legacy-mode", "--LEGACY-MODE"]); + shellNames.Should().Contain("--mode").And.NotContain(["--legacy-mode", "--LEGACY-MODE"]); + hiddenInteractiveValues.Suggestions.Should().BeEmpty(); + hiddenShellValues.Should().BeEmpty(); + canonicalInteractiveValues.Suggestions.Select(static suggestion => suggestion.Value).Should().Contain(nameof(ProbeMode.Debug)); + } + + [TestMethod] + [Description("After mapping under the sensitive default, switching to case-insensitive mode and unhiding through opposite casing restores one deduplicated completion representative and values for both spellings.")] + public async Task When_GlobalCaseModeChangesAfterMappingAndAliasIsUnhidden_Then_OneEquivalentSpellingReturnsToCompletion() + { + var sut = CoreReplApp.Create(); + var command = sut.Map( + "deploy", + static string ([ReplOption(Name = "mode", Aliases = ["--legacy-mode", "--LEGACY-MODE"])] ProbeMode mode = ProbeMode.Debug) => mode.ToString()); + sut.Options(static options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + command.WithOption("mode", static option => + { + option.HiddenAlias("--legacy-mode"); + option.HiddenAlias("--LEGACY-MODE", isHidden: false); + }); + + var interactiveNames = await ResolveAutocompleteAsync(sut, "deploy --").ConfigureAwait(false); + var shell = new ShellCompletionEngine(sut); + var shellNames = await ResolveShellCandidatesAsync(shell, "app deploy --").ConfigureAwait(false); + var lowerValues = await ResolveAutocompleteAsync(sut, "deploy --legacy-mode ").ConfigureAwait(false); + var upperValues = await ResolveShellCandidatesAsync(shell, "app deploy --LEGACY-MODE ").ConfigureAwait(false); + + interactiveNames.Suggestions.Select(static suggestion => suggestion.Value) + .Should().Contain("--legacy-mode"); + shellNames.Should().Contain("--legacy-mode"); + lowerValues.Suggestions.Select(static suggestion => suggestion.Value).Should().Contain(nameof(ProbeMode.Debug)); + upperValues.Should().Contain(nameof(ProbeMode.Debug)); + } + + [TestMethod] + [Description("Global hide followed by opposite-case unhide uses the current case-insensitive comparer, restoring one deduplicated representative to interactive and shell completion.")] + public async Task When_GlobalAliasIsUnhiddenThroughEquivalentCasing_Then_OneRepresentativeReturnsToCompletion() + { + var sut = CoreReplApp.Create(); + sut.Options(options => + options.Parsing.AddGlobalOption("organization", aliases: ["--legacy-org", "--LEGACY-ORG"])); + sut.Options(options => + { + options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive; + options.Parsing.GlobalOption("organization").HiddenAlias("--legacy-org"); + options.Parsing.GlobalOption("organization").HiddenAlias("--LEGACY-ORG", isHidden: false); + }); + sut.Map("deploy", static () => "ok"); + + var interactive = await ResolveAutocompleteAsync(sut, "--").ConfigureAwait(false); + var shell = await ResolveShellCandidatesAsync(new ShellCompletionEngine(sut), "app --").ConfigureAwait(false); + + interactive.Suggestions.Select(static suggestion => suggestion.Value) + .Should().Contain("--legacy-org"); + shell.Should().Contain("--legacy-org"); + } + + [TestMethod] + [Description("A case-sensitive option override wins over the case-insensitive global mode in fluent visibility and completion: only the exact hidden alias and its values disappear.")] + public async Task When_FluentAliasUsesSensitiveOverrideUnderInsensitiveGlobal_Then_OnlyExactSpellingIsHiddenFromCompletion() + { + var sut = CoreReplApp.Create(); + sut.Options(static options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + sut.Map( + "deploy", + static string ([ReplOption(Name = "mode", Aliases = ["--legacy-mode", "--LEGACY-MODE"], CaseSensitivity = ReplCaseSensitivity.CaseSensitive)] ProbeMode mode = ProbeMode.Debug) => mode.ToString()) + .WithOption("mode", static option => option.HiddenAlias("--legacy-mode")); + + var interactiveNames = await ResolveAutocompleteAsync(sut, "deploy --").ConfigureAwait(false); + var shell = new ShellCompletionEngine(sut); + var shellNames = await ResolveShellCandidatesAsync(shell, "app deploy --").ConfigureAwait(false); + var hiddenValues = await ResolveAutocompleteAsync(sut, "deploy --legacy-mode ").ConfigureAwait(false); + var visibleValues = await ResolveShellCandidatesAsync(shell, "app deploy --LEGACY-MODE ").ConfigureAwait(false); + + interactiveNames.Suggestions.Select(static suggestion => suggestion.Value) + .Should().Contain("--LEGACY-MODE").And.NotContain("--legacy-mode"); + shellNames.Should().Contain("--LEGACY-MODE").And.NotContain("--legacy-mode"); + hiddenValues.Suggestions.Should().BeEmpty(); + visibleValues.Should().Contain(nameof(ProbeMode.Debug)); + } + + [TestMethod] + [Description("Changing the inherited global case mode after mapping reevaluates declarative hidden-alias precedence for interactive and shell name/value completion without hiding the canonical token.")] + public async Task When_GlobalCaseModeChangesAfterMapping_Then_DeclarativeHiddenAliasCompletionIsReevaluated() + { + var sut = CoreReplApp.Create(); + sut.Map( + "deploy", + static string ([ReplOption(Name = "mode", Aliases = ["--LEGACY-MODE"], HiddenAliases = ["--legacy-mode"])] ProbeMode mode = ProbeMode.Debug) => mode.ToString()); + sut.Options(options => options.Parsing.OptionCaseSensitivity = ReplCaseSensitivity.CaseInsensitive); + + var interactiveNames = await ResolveAutocompleteAsync(sut, "deploy --").ConfigureAwait(false); + var shell = new ShellCompletionEngine(sut); + var shellNames = await ResolveShellCandidatesAsync(shell, "app deploy --").ConfigureAwait(false); + var interactiveValues = await ResolveAutocompleteAsync(sut, "deploy --LEGACY-MODE ").ConfigureAwait(false); + var shellValues = await ResolveShellCandidatesAsync(shell, "app deploy --legacy-mode ").ConfigureAwait(false); + + interactiveNames.Suggestions.Select(static suggestion => suggestion.Value) + .Should().Contain("--mode").And.NotContain("--LEGACY-MODE").And.NotContain("--legacy-mode"); + shellNames.Should().Contain("--mode").And.NotContain("--LEGACY-MODE").And.NotContain("--legacy-mode"); + interactiveValues.Suggestions.Should().BeEmpty(); + shellValues.Should().BeEmpty(); + } + + [TestMethod] + [Description("A hidden option does not expose its enum values even after the caller types its token by hand — probing must not confirm the option exists. The visible sibling is asserted in the same pass as a positive control: two bare BeEmpty assertions would also pass if this shape offered no values at all, and would then stay green with the visibility filter deleted.")] + public async Task When_HiddenEnumOptionAwaitsValue_Then_OnlyTheVisibleSiblingOffersValues() + { + var sut = CoreReplApp.Create(); + sut.Map( + "deploy", + static string ( + [ReplOption(Name = "secret-mode")] ProbeMode secretMode = ProbeMode.Debug, + [ReplOption(Name = "denim-mode")] ProbeMode denimMode = ProbeMode.Debug) => $"{secretMode}{denimMode}") + .WithOption("secretMode", static option => option.Hidden()); + + var hiddenInteractive = await ResolveAutocompleteAsync(sut, "deploy --secret-mode ").ConfigureAwait(false); + var hiddenShell = await ResolveShellCandidatesAsync(new ShellCompletionEngine(sut), "app deploy --secret-mode ").ConfigureAwait(false); + var visibleInteractive = await ResolveAutocompleteAsync(sut, "deploy --denim-mode ").ConfigureAwait(false); + var visibleShell = await ResolveShellCandidatesAsync(new ShellCompletionEngine(sut), "app deploy --denim-mode ").ConfigureAwait(false); + + hiddenInteractive.Suggestions.Select(static suggestion => suggestion.Value).Should().BeEmpty(); + hiddenShell.Should().BeEmpty(); + visibleInteractive.Suggestions.Select(static suggestion => suggestion.Value).Should().Contain(nameof(ProbeMode.Debug)); + visibleShell.Should().Contain(nameof(ProbeMode.Debug)); + } + [TestMethod] [Description("Interactive autocomplete filters option suggestions by a partial option prefix.")] public async Task When_CurrentTokenIsPartialOptionPrefix_Then_FiltersOptionSuggestions() @@ -163,6 +403,32 @@ public async Task When_CurrentTokenIsOptionPrefix_Then_DynamicCompletionProvider values.Should().Contain("--force"); } + [TestMethod] + [Description("Global parsing runs before route parsing. When a hidden custom global owns a route option's token, neither interactive nor shell completion may re-advertise that unreachable route option; an unrelated route option remains as a positive control.")] + public async Task When_HiddenGlobalOwnsRouteOptionToken_Then_AllCompletionSurfacesOmitIt() + { + var sut = CoreReplApp.Create(); + sut.Options(options => + { + options.Parsing.AddGlobalOption("force"); + options.Parsing.GlobalOption("force").Hidden(); + }); + sut.Map( + "install {skillName}", + static string (string skillName, [ReplOption] bool force, [ReplOption] bool verbose) => skillName); + + var interactive = await ResolveAutocompleteAsync(sut, "install bib-overalls --").ConfigureAwait(false); + var shell = await ResolveShellCandidatesAsync( + new ShellCompletionEngine(sut), + "app install bib-overalls --").ConfigureAwait(false); + + var interactiveValues = interactive.Suggestions.Select(static suggestion => suggestion.Value).ToArray(); + interactiveValues.Should().NotContain("--force"); + interactiveValues.Should().Contain("--verbose"); + shell.Should().NotContain("--force"); + shell.Should().Contain("--verbose"); + } + [TestMethod] [Description("An option name shared by a global option and a route option appears exactly once in the menu — the dedup set spans both sources.")] public async Task When_GlobalAndRouteOptionsCollide_Then_SuggestionAppearsOnce() @@ -411,11 +677,13 @@ public void When_SchemaEntryIsCaseInsensitive_Then_DifferentlyCasedPrefixStillMa }; var schema = new Repl.Internal.Options.OptionSchema( entries, - new Dictionary(StringComparer.OrdinalIgnoreCase)); + new Dictionary(StringComparer.OrdinalIgnoreCase), + ReplCaseSensitivity.CaseSensitive); var results = new List(); Repl.Internal.Options.OptionTokenCompletionSource.CollectRouteOptionTokens( schema, + new Dictionary(StringComparer.Ordinal), "--FO", ReplCaseSensitivity.CaseSensitive, new HashSet(StringComparer.Ordinal), diff --git a/src/Repl.Tests/Given_OptionSchema.cs b/src/Repl.Tests/Given_OptionSchema.cs new file mode 100644 index 00000000..b7497235 --- /dev/null +++ b/src/Repl.Tests/Given_OptionSchema.cs @@ -0,0 +1,34 @@ +using Repl.Internal.Options; + +namespace Repl.Tests; + +[TestClass] +public sealed class Given_OptionSchema +{ + [TestMethod] + [Description("Given two parser-equivalent aliases under case-insensitive mode where the first needs to change and the second is already in the requested state, WithAliasVisibility must not let the second entry's no-op reset the accumulated 'changed' flag: the returned schema must reflect the first entry's real change instead of being discarded as a no-op.")] + public void When_AnEarlierEquivalentAliasChangesButALaterOneAlreadyMatches_Then_TheChangeIsNotLost() + { + var caseSensitivity = ReplCaseSensitivity.CaseSensitive; + var schema = new OptionSchema( + [ + new OptionSchemaEntry("--tenant", "tenant", OptionSchemaTokenKind.NamedOption, ReplArity.ZeroOrOne), + new OptionSchemaEntry("--ACCOUNT", "tenant", OptionSchemaTokenKind.NamedOption, ReplArity.ZeroOrOne, IsHidden: false), + new OptionSchemaEntry("--account", "tenant", OptionSchemaTokenKind.NamedOption, ReplArity.ZeroOrOne, IsHidden: true), + ], + new Dictionary(StringComparer.OrdinalIgnoreCase) + { + ["tenant"] = new OptionSchemaParameter("tenant", typeof(string), ReplParameterMode.OptionOnly), + }, + () => caseSensitivity); + + // Both --ACCOUNT and --account become equivalent to the requested alias "--ACCOUNT" once + // case sensitivity turns insensitive, even though they were registered as distinct tokens. + caseSensitivity = ReplCaseSensitivity.CaseInsensitive; + var updated = schema.WithAliasVisibility("tenant", "--ACCOUNT", isHidden: true); + + caseSensitivity = ReplCaseSensitivity.CaseSensitive; + updated.Entries.Should().ContainSingle(entry => entry.Token == "--ACCOUNT") + .Which.IsHidden.Should().BeTrue("the first matching entry genuinely changed and must not be lost because a later equivalent entry was already hidden"); + } +} From fde24c40ba93d9a6ac468c24b9328423f1ef89f4 Mon Sep 17 00:00:00 2001 From: autocarl Date: Sun, 26 Jul 2026 09:14:55 -0400 Subject: [PATCH 2/3] docs: document hidden option visibility contracts --- CHANGELOG.md | 46 +++++++++++++++ docs/commands.md | 115 ++++++++++++++++++++++++++++++++++++++ docs/for-coding-agents.md | 1 + docs/mcp-overview.md | 1 + docs/mcp-reference.md | 2 + docs/parameter-system.md | 6 ++ 6 files changed, 171 insertions(+) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 00000000..57bcb2fb --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,46 @@ +# Changelog + +Notable consumer-facing changes to the Repl packages. Versions are assigned automatically by +Nerdbank.GitVersioning at pack time; this file groups changes by theme instead of by release. + +## Unreleased + +### Added — option visibility + +- `.Hidden(bool isHidden = true)` on the option builder (`WithOption(name, option => option.Hidden())`) + hides an option's canonical token, aliases, description, default, and value candidates from help, + generated documentation, interactive/shell completion, and MCP tool schemas. The option remains a + fully parsable, invocable part of the command line — hiding is a discovery filter, not access + control. Available for direct command-handler parameters, options-group properties, manually + registered global options (`ParsingOptions.GlobalOption(name).Hidden()`), and typed global options. +- `.HiddenAlias(alias, isHidden = true)` and `[ReplOption(HiddenAliases = [...])]` mark specific + legacy/deprecated token spellings as parser-only: the canonical token and any current aliases stay + discoverable, while the hidden alias keeps binding from the CLI/REPL for backward compatibility. +- `doc export` (and `docs `) reports `isHidden` / `isAutomationHidden` per option so an + app author can inventory what a given command hides. Aggregate documentation (no target path) and + MCP's `tools/list` always omit hidden options entirely — see `docs/commands.md` for the full + visibility matrix. +- A hidden option must remain omittable for every provider that can build a discovery surface. + Hiding a required options-group property fails immediately at `Map` time. Hiding a required direct + handler parameter defers that check to the first time discovery runs against a real service + provider (aggregate documentation build or MCP startup), since a DI/synthesized-progress fallback + is only knowable once one exists — see the "Provider-aware requiredness" section of + `docs/commands.md`. + +### Changed — breaking + +- `WithOption(name, configure)` is now the only fluent entry point for configuring an existing + option's metadata (visibility included). This lands within the same change that introduces it — + no previously published `Option(...)` API is removed by this release. + +### Compatibility notes + +- `doc export --json` (and other structured documentation exports) now unconditionally include the + `isHidden` and `isAutomationHidden` fields on every option. A consumer validating that output + against a closed schema (`additionalProperties: false`) will need to allow these two additive + fields. +- The historical six-parameter `ParsingOptions.AddGlobalOptionCore` descriptor is preserved as a + distinct overload (not folded into a defaulted parameter) so an already-compiled `Repl.Defaults` + binary continues to work against a newer `Repl.Core`. The reverse is not guaranteed: this release's + `Repl.Defaults` calls APIs that only exist in this release's `Repl.Core`, so upgrading only one of + the two packages independently is not supported — upgrade them together. diff --git a/docs/commands.md b/docs/commands.md index caf23729..ae67582f 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -58,6 +58,121 @@ app.Map( Root help now includes a dedicated `Global Options:` section with built-ins plus custom options registered through `options.Parsing.AddGlobalOption(...)`. +### Hiding individual options + +Use `.WithOption(, option => option.Hidden())` to hide one command option without hiding its command. The target is the CLR handler parameter or options-group property name, not the rendered `--option-name` token: + +```csharp +app.Map( + "deploy", + ([ReplOption(Name = "internal-mode")] bool internalMode = false) => internalMode) + .WithOption("internalMode", option => option.Hidden()); +``` + +The callback shape is deliberate. `CommandBuilder` and `OptionBuilder` both expose `Hidden` and `AutomationHidden`, so an API returning the option builder would let `.Option("x").Hidden()` sit in a chain reading exactly like the command-level `.Hidden()` while meaning something quite different, with nothing to signal that the subject had changed. `WithOption` keeps the subject explicit and the chain on the command. + +For declarative registration, set `Hidden = true` on `ReplOptionAttribute`. This works on direct parameters, options-group properties, and typed global-options properties: + +```csharp +public sealed class DeploymentOptions +{ + [ReplOption(Hidden = true)] + public string? InternalToken { get; set; } +} +``` + +Manually registered global options can be selected after registration: + +```csharp +app.Options(options => +{ + options.Parsing.AddGlobalOption("internal-tenant"); + options.Parsing.GlobalOption("internal-tenant").Hidden(); +}); +``` + +#### Keeping a legacy alias callable but hidden + +Do not hide the whole option when only an old spelling is deprecated. Declare the legacy token in `HiddenAliases`; it remains accepted by CLI and REPL parsing while the canonical token stays visible: + +```csharp +app.Map( + "deploy", + ([ReplOption(Name = "tenant", HiddenAliases = ["--account", "-a"])] string? tenant = null) => tenant); +``` + +`HiddenAliases` registers those tokens itself; they do not also need to appear in `Aliases`. If the same exact token appears in both arrays, hidden visibility wins. Tokens remain case-sensitive when the option is case-sensitive, so `--account` can be hidden without hiding a distinct `--ACCOUNT` alias. + +The fluent form operates on an alias already declared in `Aliases`: + +```csharp +app.Map( + "deploy", + ([ReplOption(Name = "tenant", Aliases = ["--account"])] string? tenant = null) => tenant) + .WithOption("tenant", option => option.HiddenAlias("--account")); +``` + +The same contract applies to global options: + +```csharp +app.Options(options => +{ + options.Parsing.AddGlobalOption("tenant", aliases: ["--account"]); + options.Parsing.GlobalOption("tenant").HiddenAlias("--account"); +}); + +public sealed class GlobalOptions +{ + [ReplOption(HiddenAliases = ["--account"])] + public string? Tenant { get; set; } +} +``` + +A hidden alias is omitted from command/root help, typo suggestions, interactive and shell completion (including value completion after manually typing it), aggregate and exact-path documentation, and MCP schemas. MCP callers use the visible canonical argument name; the hidden alias is only a backwards-compatible CLI/REPL fallback. Like every hidden surface, this is discoverability metadata rather than authorization. + +Hidden options are omitted from help, interactive and shell completion (including value providers), documentation export, and generated MCP schemas. They remain valid parser inputs on the command line and in the REPL, and bind normally when supplied explicitly. + +Over MCP the omission is stricter. The advertised tool schema and the list of accepted arguments are built from the same option list, so a `tools/call` that supplies a hidden option is **rejected** — exactly like a call to a hidden command. A hidden option is therefore reachable from a human-driven CLI or REPL session, but not from an agent. + +A hidden option must be omittable for the provider that builds a discovery surface, because that client otherwise has no valid invocation path. Repl checks the same fallbacks and precedence as the handler binder: a CLR default or nullable shape, synthetic `IProgress`/`IProgress` from an available `IReplInteractionChannel`, then direct `IServiceProvider.GetService` before explicit `ExactlyOne`/`OneOrMore` lower bounds are enforced. Direct handler parameters are validated when aggregate documentation or MCP discovery is built, not by `Map` or `.Hidden()`, because `Run` and MCP may receive an external provider only after mapping. If the active provider cannot supply the value, discovery fails with the required-hidden diagnostic rather than advertising an impossible command. Required options-group properties still fail immediately during mapping because that binding path never consults DI. + +A fluent `.Hidden(isHidden: false)` overrides `ReplOptionAttribute.Hidden`. For direct handler parameters this can restore visibility before provider-aware discovery. A required hidden options-group property is rejected while the command is mapped, before a fluent override can run, so fix that attribute instead. + +> **Hiding is not access control.** A hidden option stays a fully invocable part of the command line for anyone who knows its name — nothing about it is authenticated, authorized, or secret. Use it for deprecated switches, diagnostic escape hatches and migration aliases. Gate privileged behavior with real authorization, never with obscurity. + +### Hiding an option from agents only + +`.AutomationHidden()` is the option-level counterpart of `CommandBuilder.AutomationHidden()`: it withholds the option from programmatic surfaces while leaving it fully visible to people. + +```csharp +app.Map("deploy", Deploy) + .WithOption("traceId", option => option.AutomationHidden()); +``` + +The declarative form is `[ReplOption(AutomationHidden = true)]`. It is **not** supported on typed global-options properties and fails fast there, because global options never reach a programmatic surface at all — the flag would have nothing to act on. `GlobalOptionBuilder` has no `AutomationHidden` for the same reason, enforced by its type rather than by a runtime check. + +The two axes are independent: + +| Surface | `.Hidden()` | `.AutomationHidden()` | +|---|---|---| +| Command help | omitted | **listed** | +| Interactive and shell completion | omitted | **offered** | +| Aggregate `doc export` | omitted | **exported, flagged** | +| `doc export ` (exact path) | **exported, flagged** | **exported, flagged** | +| MCP tool schema and prompt arguments | omitted | omitted | +| MCP `tools/call` | rejected | rejected | +| CLI and REPL parsing and binding | unaffected | unaffected | + +Both are discovery filters, and neither is an access-control boundary. + +### Troubleshooting an option that will not bind + +Almost always a target-name mistake. `WithOption` takes the **CLR** handler parameter or options-group property name, not the rendered `--option-name` token — `WithOption("internalMode", …)`, never `WithOption("internal-mode", …)`. An unknown target throws at configuration time and the message lists the targets that do exist, so read it rather than guessing. + +If the option is genuinely hidden and you want to confirm what the app thinks, export the command explicitly: `doc export --json` includes hidden options with `"isHidden": true`. The aggregate export omits them, so target the command. + +Note there is no built-in signal for *use* of a hidden option. If the point is retiring a deprecated switch, record that in the handler yourself — otherwise nothing will tell you when it has become safe to remove. + ### Accessing global options outside handlers Parsed global option values are available via `IGlobalOptionsAccessor`, registered in DI automatically. This enables access from middleware, DI service factories, and handlers: diff --git a/docs/for-coding-agents.md b/docs/for-coding-agents.md index 57964c1e..0e61fba8 100644 --- a/docs/for-coding-agents.md +++ b/docs/for-coding-agents.md @@ -144,6 +144,7 @@ Use these annotations to help agents make safer decisions: | `.OpenWorld()` | Talks to external systems; expect latency and failures. | | `.LongRunning()` | May take time; use call-now / poll-later patterns. | | `.AutomationHidden()` | Do not expose this command to MCP automation. | +| `.WithOption(name, o => o.AutomationHidden())` | Keep this one option out of the tool schema; the command stays visible. | Unannotated tools force agents to assume the worst. Annotate every command that will be visible through MCP. diff --git a/docs/mcp-overview.md b/docs/mcp-overview.md index 3cdee8f6..9f3f42a3 100644 --- a/docs/mcp-overview.md +++ b/docs/mcp-overview.md @@ -45,6 +45,7 @@ Commands map to MCP primitives automatically: | `.AsPrompt()` | Prompt | Reusable instruction template | | `.AsMcpAppResource()` | Tool + `ui://` HTML resource | Interactive UI for capable hosts | | `.AutomationHidden()` | _(nothing)_ | Excluded from MCP entirely | +| `.WithOption(name, o => o.AutomationHidden())` | Tool without that option | Option people may use but agents should not | ## Annotations diff --git a/docs/mcp-reference.md b/docs/mcp-reference.md index 5190316b..962da647 100644 --- a/docs/mcp-reference.md +++ b/docs/mcp-reference.md @@ -301,6 +301,8 @@ Notes: |---|---|---| | `.AutomationHidden()` | Per-command | Interactive-only commands | | `.Hidden()` | Per-command | Hidden from all surfaces | +| `.WithOption(name, o => o.AutomationHidden())` | Per-option | Option people may use but agents should not | +| `.WithOption(name, o => o.Hidden())` | Per-option | Deprecated or diagnostic switches | | `CommandFilter` | App-level | `o.CommandFilter = c => !c.Path.StartsWith("admin")` | | Module presence + `Programmatic` | Per-module | Entire feature areas | diff --git a/docs/parameter-system.md b/docs/parameter-system.md index 472d90b5..e6fa5068 100644 --- a/docs/parameter-system.md +++ b/docs/parameter-system.md @@ -26,6 +26,7 @@ Application-facing parameter DSL: - explicit `Aliases` (full tokens, for example `-m`, `--mode`) - explicit `ReverseAliases` (for example `--no-verbose`) - `Mode` (`OptionOnly`, `ArgumentOnly`, `OptionAndPositional`) + - `Hidden` to suppress discovery without changing parsing or binding - optional per-parameter `CaseSensitivity` - optional `Arity` - `ReplArgumentAttribute` @@ -41,6 +42,8 @@ Supporting enums: - `ReplParameterMode` - `ReplArity` +Option visibility can also be configured fluently with `CommandBuilder.WithOption(targetName, option => option.Hidden())` and `ParsingOptions.GlobalOption(name).Hidden()`. Fluent visibility overrides the attribute value, including `.Hidden(isHidden: false)`. Unknown targets fail during configuration instead of being ignored. Hidden options must be omittable, but inferred `ExactlyOne` alone does not make an omittable nullable/defaulted CLR parameter invalid. Required options-group properties fail immediately during mapping because their binder never consults DI. Required direct handler parameters are validated later, when aggregate documentation or MCP discovery knows the active provider; discovery accepts them when the binder can synthesize progress from `IReplInteractionChannel` or resolve the parameter from DI, and otherwise emits the required-hidden diagnostic. + ### Options groups - `ReplOptionsGroupAttribute` (on a class) marks it as a reusable parameter group @@ -107,6 +110,9 @@ This same schema drives: - command help option sections - shell option completion candidates - exported documentation option metadata +- generated MCP tool and prompt schemas + +Hidden options are filtered from those discovery surfaces, while the same schema continues to accept and bind their tokens during direct execution. ## System.CommandLine comparison From e084e20785f0c4b75cd215ae03ca83dbac012c25 Mon Sep 17 00:00:00 2001 From: autocarl Date: Sun, 26 Jul 2026 09:14:55 -0400 Subject: [PATCH 3/3] ci: strengthen pull-request test coverage --- .github/workflows/ci.yml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7a207a15..620f8a13 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,7 +33,9 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-latest] + # Windows is a first-class target for a CLI/REPL framework, and its absence let a + # CRLF-only test failure ship green (see the TrimEntries fix in Given_Completions). + os: [ubuntu-latest, macos-latest, windows-latest] steps: - name: Checkout uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 @@ -194,7 +196,15 @@ jobs: --coverage-output-format cobertura - name: Test report - if: always() + # Creating a check run needs `checks: write`, but GitHub caps GITHUB_TOKEN + # to read-only for `pull_request` events raised from a fork — the + # workflow-level `permissions:` block cannot lift that ceiling. The action + # then fails with "Resource not accessible by integration", which cascades + # into skipping Pack and the package validation steps below. Fork PRs still + # gate on the Test step and publish the .trx via the test-results artifact. + # The `github.event_name` clause keeps the report alive on push builds, + # where `github.event.pull_request` is null. + if: always() && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) uses: dorny/test-reporter@31a54ee7ebcacc03a09ea97a7e5465a47b84aea5 # v1.9.1 with: name: Test Results