diff --git a/Sources/ContainerCommands/Application.swift b/Sources/ContainerCommands/Application.swift index c77f51a59..2588a4096 100644 --- a/Sources/ContainerCommands/Application.swift +++ b/Sources/ContainerCommands/Application.swift @@ -94,9 +94,14 @@ public struct Application: AsyncLoggableCommand { name: "Other", subcommands: Self.otherCommands() ), - ], - // Hidden command to handle plugins on unrecognized input. - defaultSubcommand: DefaultCommand.self + ] + // Note: intentionally no `defaultSubcommand`. Routing unrecognized + // top-level input to `DefaultCommand` via `defaultSubcommand` combined + // with its `.captureForPassthrough` argument breaks flag parsing for + // unrelated, legitimately-matched subcommands throughout the tree + // (e.g. `container system status --debug` would fail to recognize + // `--debug`). See https://github.com/apple/container/issues/1936. + // Plugin dispatch is instead handled manually in `main()` below. ) public static func main() async throws { @@ -117,6 +122,30 @@ public struct Application: AsyncLoggableCommand { let fullArgs = CommandLine.arguments let args = Array(fullArgs.dropFirst()) + // If the input doesn't correspond to one of our own subcommands, hand + // it off to `DefaultCommand` for plugin dispatch (or to show the + // plugin-aware help text) before ever invoking `Application.parseAsRoot`. + // This intentionally avoids ArgumentParser's `defaultSubcommand` + // mechanism; see the note on `configuration` above. + if let dispatch = Self.pluginDispatchArguments(args) { + do { + // Mirror the `validate()` side effects (debug log level, + // Rosetta check) that `Application.parseAsRoot` would + // otherwise have applied on its way to the default subcommand. + var app = Application() + app.logOptions = dispatch.logOptions + try app.validate() + + var defaultCommand = DefaultCommand() + defaultCommand.logOptions = dispatch.logOptions + defaultCommand.remaining = dispatch.remaining + try await defaultCommand.run() + } catch { + Application.exit(withError: error) + } + return + } + do { var command = try Application.parseAsRoot(args) if var asyncCommand = command as? AsyncParsableCommand { @@ -220,6 +249,56 @@ public struct Application: AsyncLoggableCommand { } } + /// Determines whether `args` should be routed directly to `DefaultCommand` + /// (i.e. treated as a plugin invocation, or as a bare/`--debug`-only + /// invocation that should show the plugin-aware help text), rather than + /// being parsed as one of `Application`'s own subcommands. + /// + /// Returns `nil` when `args` should instead be parsed normally by + /// `Application.parseAsRoot` — either because it starts with a + /// recognized subcommand name/alias, or because it's requesting `--help`, + /// `-h`, or `--version`, which ArgumentParser already handles. + static func pluginDispatchArguments(_ args: [String]) -> (logOptions: Flags.Logging, remaining: [String])? { + let knownNames = Self.knownSubcommandNames() + var debug = false + var index = args.startIndex + while index < args.endIndex { + switch args[index] { + case "--debug": + debug = true + index += 1 + case "--help", "-h", "--version": + return nil + default: + if knownNames.contains(args[index]) { + return nil + } + return (Flags.Logging(debug: debug), Array(args[index...])) + } + } + // Nothing left but recognized root-level flags (or no args at all): + // fall through to `DefaultCommand`, which shows the plugin-aware + // help text when `remaining` is empty. + return (Flags.Logging(debug: debug), []) + } + + /// All subcommand names and aliases directly reachable from the root, + /// whether declared via `subcommands` or `groupedSubcommands`. + private static func knownSubcommandNames() -> Set { + var names = Set() + func add(_ command: any ParsableCommand.Type) { + if let commandName = command.configuration.commandName { + names.insert(commandName) + } + names.formUnion(command.configuration.aliases) + } + configuration.subcommands.forEach(add) + configuration.groupedSubcommands.forEach { group in + group.subcommands.forEach(add) + } + return names + } + private static func otherCommands() -> [any ParsableCommand.Type] { guard #available(macOS 26, *) else { return [ diff --git a/Tests/ContainerCommandsTests/PluginDispatchArgumentsTests.swift b/Tests/ContainerCommandsTests/PluginDispatchArgumentsTests.swift new file mode 100644 index 000000000..e71c7b60f --- /dev/null +++ b/Tests/ContainerCommandsTests/PluginDispatchArgumentsTests.swift @@ -0,0 +1,96 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Testing + +@testable import ContainerCommands + +/// Regression coverage for https://github.com/apple/container/issues/1936. +/// +/// `Application` no longer relies on ArgumentParser's `defaultSubcommand` +/// mechanism to route unrecognized input to `DefaultCommand` (plugin +/// dispatch), because combining `defaultSubcommand` with a +/// `.captureForPassthrough` argument breaks flag parsing for legitimately +/// matched subcommands throughout the whole tree (e.g. `container system +/// status --debug` would fail with "Unknown option '--debug'"). Instead, +/// `Application.pluginDispatchArguments` decides up front whether input +/// should go to `DefaultCommand`. +struct PluginDispatchArgumentsTests { + @Test + func knownSubcommandIsNotTreatedAsPluginDispatch() { + #expect(Application.pluginDispatchArguments(["system", "status", "--debug"]) == nil) + #expect(Application.pluginDispatchArguments(["s", "status", "--debug"]) == nil) + #expect(Application.pluginDispatchArguments(["image", "list"]) == nil) + #expect(Application.pluginDispatchArguments(["list"]) == nil) + #expect(Application.pluginDispatchArguments(["help"]) == nil) + } + + @Test + func helpAndVersionAreNotTreatedAsPluginDispatch() { + #expect(Application.pluginDispatchArguments(["--help"]) == nil) + #expect(Application.pluginDispatchArguments(["-h"]) == nil) + #expect(Application.pluginDispatchArguments(["--version"]) == nil) + } + + @Test + func unrecognizedTopLevelWordIsPluginDispatch() { + guard let dispatch = Application.pluginDispatchArguments(["some-plugin", "arg1", "--flag"]) else { + Issue.record("expected plugin dispatch for an unrecognized subcommand") + return + } + #expect(dispatch.logOptions.debug == false) + #expect(dispatch.remaining == ["some-plugin", "arg1", "--flag"]) + } + + @Test + func leadingDebugFlagIsConsumedBeforePluginDispatch() { + guard let dispatch = Application.pluginDispatchArguments(["--debug", "some-plugin", "arg1"]) else { + Issue.record("expected plugin dispatch") + return + } + #expect(dispatch.logOptions.debug == true) + #expect(dispatch.remaining == ["some-plugin", "arg1"]) + } + + @Test + func pluginsOwnDebugFlagIsPassedThroughUnchanged() { + // A plugin that accepts its own `--debug` flag must receive it + // verbatim, rather than having it stolen by `container`'s own + // global `--debug` flag. + guard let dispatch = Application.pluginDispatchArguments(["some-plugin", "--debug"]) else { + Issue.record("expected plugin dispatch") + return + } + #expect(dispatch.logOptions.debug == false) + #expect(dispatch.remaining == ["some-plugin", "--debug"]) + } + + @Test + func bareAndDebugOnlyInvocationsDispatchWithEmptyRemaining() { + guard let bare = Application.pluginDispatchArguments([]) else { + Issue.record("expected dispatch for bare invocation") + return + } + #expect(bare.remaining.isEmpty) + + guard let debugOnly = Application.pluginDispatchArguments(["--debug"]) else { + Issue.record("expected dispatch for --debug-only invocation") + return + } + #expect(debugOnly.logOptions.debug == true) + #expect(debugOnly.remaining.isEmpty) + } +} diff --git a/Tests/IntegrationTests/System/TestCLIDebugFlag.swift b/Tests/IntegrationTests/System/TestCLIDebugFlag.swift new file mode 100644 index 000000000..b6d07dad1 --- /dev/null +++ b/Tests/IntegrationTests/System/TestCLIDebugFlag.swift @@ -0,0 +1,57 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Testing + +/// Regression coverage for https://github.com/apple/container/issues/1936: +/// `--debug` was rejected with "Unknown option '--debug'. Did you mean +/// '--debug'?" for many subcommands even though it's listed in their own +/// usage text, because ArgumentParser's `defaultSubcommand` mechanism +/// (used for plugin dispatch) broke flag parsing for unrelated subcommands +/// when combined with a `.captureForPassthrough` argument. +@Suite +struct TestCLIDebugFlag { + private static func expectDebugAccepted(_ f: ContainerFixture, _ args: [String]) throws { + let result = try f.run(args) + #expect( + !result.error.contains("Unknown option '--debug'"), + "expected '--debug' to be accepted for \(args.joined(separator: " ")), stderr: \(result.error)" + ) + } + + @Test func systemStatusAcceptsDebugFlag() async throws { + try await ContainerFixture.with { f in + try Self.expectDebugAccepted(f, ["system", "status", "--debug"]) + try Self.expectDebugAccepted(f, ["s", "status", "--debug"]) + } + } + + @Test func systemVersionAndDfAcceptDebugFlag() async throws { + try await ContainerFixture.with { f in + try Self.expectDebugAccepted(f, ["system", "version", "--debug"]) + try Self.expectDebugAccepted(f, ["system", "df", "--debug"]) + } + } + + @Test func nestedInspectAndDeleteCommandsAcceptDebugFlag() async throws { + try await ContainerFixture.with { f in + try Self.expectDebugAccepted(f, ["image", "inspect", "nonexistent", "--debug"]) + try Self.expectDebugAccepted(f, ["volume", "inspect", "nonexistent", "--debug"]) + try Self.expectDebugAccepted(f, ["network", "inspect", "nonexistent", "--debug"]) + try Self.expectDebugAccepted(f, ["builder", "status", "--debug"]) + } + } +}