Make main module optional for commands

[RiSKeD]

Main modules are now optional. Commands can have 0 or 1 main modules instead of requiring exactly 1. Arguments are only passed to commands that have a main module else they are ignored. This is not a breaking change as all old commands are still valid.

resolves #240

[Copilot]

Pull request overview

This PR refactors the command structure to make main modules optional, allowing commands to have 0 or 1 main modules instead of requiring exactly 1. Arguments are now only passed to commands that have a main module, and a new error type ErrNoMainForArgs is introduced to handle cases where arguments are provided without a main module. This change maintains backward compatibility while enabling more flexible command configurations.

Key changes:

• Modified validation logic to allow commands with 0 or 1 main modules (previously exactly 1)
• Updated FindCmd signature to accept arguments for validation
• Added ErrNoMainForArgs error for cases where arguments are provided without a main module

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
pkg/dut/dut.go	Updated FindCmd to accept args parameter, modified validation to allow 0-1 main modules, made CountMain method public, added ErrNoMainForArgs error, removed implicit main module assignment
pkg/dut/dut_test.go	Added comprehensive test cases for commands with/without main modules and argument validation scenarios
cmds/dutagent/runRPC.go	Updated findDUTCmd to pass command arguments to FindCmd and handle new ErrNoMainForArgs error
cmds/dutagent/rpc.go	Updated Details RPC to pass empty args to FindCmd, removed redundant main module validation since it's now optional
docs/dutagent-config.md	Updated documentation to reflect that main modules are now optional (0 or 1 instead of exactly 1)

Comments suppressed due to low confidence (1)

pkg/dut/dut_test.go:202

• Consider adding a test case for a command with no modules (empty Modules slice) to verify that FindCmd properly returns ErrInvalidCommand as expected by the validation logic in dut.go:76.

	tests := []struct {
		name    string
		device  string
		command string
		args    []string
		wantDev Device
		wantCmd Command
		err     error
	}{
		{
			name:    "device and command found with main module and args",
			device:  "device1",
			command: "cmd1",
			args:    []string{"arg1", "arg2"},
			wantDev: devs["device1"],
			wantCmd: devs["device1"].Cmds["cmd1"],
			err:     nil,
		},
		{
			name:    "device and command found with main module and no args",
			device:  "device1",
			command: "cmd1",
			args:    []string{},
			wantDev: devs["device1"],
			wantCmd: devs["device1"].Cmds["cmd1"],
			err:     nil,
		},
		{
			name:    "command without main module and no args",
			device:  "device1",
			command: "cmd4",
			args:    []string{},
			wantDev: devs["device1"],
			wantCmd: devs["device1"].Cmds["cmd4"],
			err:     nil,
		},
		{
			name:    "args provided but no main module",
			device:  "device1",
			command: "cmd4",
			args:    []string{"arg1"},
			wantDev: devs["device1"],
			wantCmd: devs["device1"].Cmds["cmd4"],
			err:     ErrNoMainForArgs,
		},
		{
			name:    "device found, command not found",
			device:  "device1",
			command: "cmd5",
			args:    []string{},
			wantDev: devs["device1"],
			wantCmd: Command{},
			err:     ErrCommandNotFound,
		},
		{
			name:    "device not found",
			device:  "device2",
			command: "cmd1",
			args:    []string{},
			wantDev: Device{},
			wantCmd: Command{},
			err:     ErrDeviceNotFound,
		},
		{
			name:    "command with module but no main module",
			device:  "device1",
			command: "cmd2",
			args:    []string{},
			wantDev: devs["device1"],
			wantCmd: devs["device1"].Cmds["cmd2"],
			err:     nil,
		},
		{
			name:    "invalid command with multiple main modules",
			device:  "device1",
			command: "cmd3",
			args:    []string{},
			wantDev: devs["device1"],
			wantCmd: devs["device1"].Cmds["cmd3"],
			err:     ErrInvalidCommand,
		},
	}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

[nitpick] When a command has no main module, the Details RPC will return an empty help string. Consider providing a more informative response, such as a message indicating that this command has no main module and therefore no detailed help is available. This would improve the user experience when querying details for commands without a main module.

Suggested change

	if helpStr == "" {
	helpStr = "No main module found for this command; detailed help is not available."
	}

[Copilot]

The documentation in the Mandatory column is confusing. It says "no, only applies if main is set" which contradicts itself - the args field should only be used when main is not set (as stated in the Description column). Consider changing this to "no, only for non-main modules" for clarity.

Suggested change

	| args | []string | nil | If a module is **not** an commands main module, it does not get any arguments passed at runtime, instead arguments can be passed here. | no, only applies if `main` is set |
	| args | []string | nil | If a module is **not** an commands main module, it does not get any arguments passed at runtime, instead arguments can be passed here. | no, only for non-main modules |

[sylv-io]

Does this only cover cases where multiple modules are set? So, if there is only one module, will it automatically be set as the main module?

[RiSKeD]

No, the idea is to let the user decide if args are passed on to the cmd or not. I encountered the following issue:
I wrote a cmd which holds one module. This module is the ssh module executing some ssh cmd. But it is impossible to always execute the same fixed arguments. If it is the only module it is automatically the main module. And the main modules arguments are always overwritten by the user input. Therefore i needed to add another dummy module and make this one the main module instead. This seems awkward to me. Hence i would suggest letting users decide if they want to have a main module or not. So basically give users the possiblity to specify a cmd with a singular module which is not the main module but has its own fixed args. This way we just ignore additional user input.

[jenstopp]

We need to take care of #191 before doing this change. Otherwise, the current implementation of help will break, as it expects a main module in every command to print this module's help message.
No big deal to change that, just need to find another solution for help (see #191).

[sylv-io]

I see. Perhaps we should give it a name other than main to clarify its function. No better option came to mind while I was working on multi-module support.

It should be something that intuitively indicates that the module handles the user command line. But I'm out of ideas for what to choose. 😅

What do you think, @jenstopp? We could create a gh discussion about this topic.

[RiSKeD]

Yes this came to my mind as well. If we change this behaviour, the name main doesnt really fit anymore. I guess smth. like interactive would be better? Shall i close this PR for now, if this needs to be discussed first?

[sylv-io]

interactive sounds great 👍

[jenstopp]

The naming can be part of discussion #191
Agree that we would need another name then "main" then.

[RiSKeD]

closed in favor of #248