refactor(internal/cli,librarian): move Run to cli package (#2122)

Move command execution logic from librarian.Run to a generic cli.Run
method.

Change the Command.Run field from:

```
Run func(ctx context.Context, cfg *config.Config) error
```

to

```
Action func(context.Context, *Command) error
```

This give the command handler access to the full Command context, rather
than just the config.

In a follow up PR, cli.Command will be refactored further to remove
business logic from Config out of the internal/cli framework.

Author: julieqiu

internal/cli/cli.go

@@ -38,8 +38,8 @@ type Command struct {
 	// Long is the full description of the command.
 	Long string
 
-	// Run executes the command.
-	Run func(ctx context.Context, cfg *config.Config) error
+	// Action executes the command.
+	Action func(context.Context, *Command) error
 
 	// Commands are the sub commands.
 	Commands []*Command
@@ -52,10 +52,24 @@ type Command struct {
 	Config *config.Config
 }
 
-// Parse parses the provided command-line arguments using the command's flag
-// set.
-func (c *Command) Parse(args []string) error {
-	return c.Flags.Parse(args)
+// Run executes the command with the provided arguments.
+func (c *Command) Run(ctx context.Context, args []string) error {
+	cmd, remaining, err := lookupCommand(c, args)
+	if err != nil {
+		return err
+	}
+	if err := cmd.Flags.Parse(remaining); err != nil {
+		return err
+	}
+
+	if cmd.Action == nil {
+		cmd.Flags.Usage()
+		if len(cmd.Commands) > 0 {
+			return nil
+		}
+		return fmt.Errorf("no action defined for command %q", cmd.Name())
+	}
+	return cmd.Action(ctx, cmd)
 }
 
 // Name is the command name. Command.Short is always expected to begin with
@@ -111,10 +125,10 @@ func hasFlags(fs *flag.FlagSet) bool {
 	return visited
 }
 
-// LookupCommand looks up the command specified by the given arguments.
+// lookupCommand looks up the command specified by the given arguments.
 // It returns the command, the remaining arguments, and an error if the command
 // is not found.
-func LookupCommand(cmd *Command, args []string) (*Command, []string, error) {
+func lookupCommand(cmd *Command, args []string) (*Command, []string, error) {
 	// If the first character of the argument is '-' assume it is a flag.
 	for len(args) > 0 && args[0][0] != '-' {
 		name := args[0]

internal/cli/cli_test.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
-	"github.com/googleapis/librarian/internal/config"
 )
 
 func TestParseAndSetFlags(t *testing.T) {
@@ -42,7 +41,7 @@ func TestParseAndSetFlags(t *testing.T) {
 	cmd.Flags.IntVar(&intFlag, "count", 0, "count flag")
 
 	args := []string{"-name=foo", "-count=5"}
-	if err := cmd.Parse(args); err != nil {
+	if err := cmd.Flags.Parse(args); err != nil {
 		t.Fatalf("Parse() failed: %v", err)
 	}
 
@@ -54,22 +53,21 @@ func TestParseAndSetFlags(t *testing.T) {
 	}
 }
 
-func TestRun(t *testing.T) {
+func TestAction(t *testing.T) {
 	executed := false
 	cmd := &Command{
 		Short: "run runs the command",
-		Run: func(ctx context.Context, cfg *config.Config) error {
+		Action: func(ctx context.Context, cmd *Command) error {
 			executed = true
 			return nil
 		},
 	}
 
-	cfg := &config.Config{}
-	if err := cmd.Run(t.Context(), cfg); err != nil {
+	if err := cmd.Action(t.Context(), cmd); err != nil {
 		t.Fatal(err)
 	}
 	if !executed {
-		t.Errorf("cmd.Run was not executed")
+		t.Errorf("cmd.Action was not executed")
 	}
 }
 
@@ -278,7 +276,7 @@ func TestLookupCommand(t *testing.T) {
 		},
 	} {
 		t.Run(test.name, func(t *testing.T) {
-			gotCmd, gotArgs, err := LookupCommand(test.cmd, test.args)
+			gotCmd, gotArgs, err := lookupCommand(test.cmd, test.args)
 			if (err != nil) != test.wantErr {
 				t.Fatalf("error = %v, wantErr %v", err, test.wantErr)
 			}
@@ -298,3 +296,77 @@ func TestLookupCommand(t *testing.T) {
 		})
 	}
 }
+
+func TestRun(t *testing.T) {
+	actionExecuted := false
+	subcmd := &Command{
+		Short:     "bar is a subcommand",
+		Long:      "bar is a subcommand.",
+		UsageLine: "bar",
+		Action: func(ctx context.Context, cmd *Command) error {
+			actionExecuted = true
+			return nil
+		},
+	}
+	subcmd.Init()
+
+	root := &Command{
+		Short:     "foo is the root command",
+		Long:      "foo is the root command.",
+		UsageLine: "foo",
+		Commands:  []*Command{subcmd},
+	}
+	root.Init()
+
+	noaction := &Command{
+		Short:     "noaction has no action",
+		Long:      "noaction has no action.",
+		UsageLine: "noaction",
+	}
+	noaction.Init()
+
+	for _, test := range []struct {
+		name           string
+		cmd            *Command
+		args           []string
+		wantErr        bool
+		actionExecuted bool
+	}{
+		{
+			name:           "execute foo with subcommand bar",
+			cmd:            root,
+			args:           []string{"bar"},
+			actionExecuted: true,
+		},
+		{
+			name:    "unknown subcommand",
+			cmd:     root,
+			args:    []string{"unknown"},
+			wantErr: true,
+		},
+		{
+			name:    "flag parse error",
+			cmd:     subcmd,
+			args:    []string{"-unknown"},
+			wantErr: true,
+		},
+		{
+			name:    "no action defined on command with no subcommands",
+			cmd:     noaction,
+			wantErr: true,
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			actionExecuted = false
+			err := test.cmd.Run(t.Context(), test.args)
+			if err != nil {
+				if !test.wantErr {
+					t.Errorf("error = %v, wantErr %v", err, test.wantErr)
+				}
+			}
+			if actionExecuted != test.actionExecuted {
+				t.Errorf("actionExecuted = %v, want %v", actionExecuted, test.actionExecuted)
+			}
+		})
+	}
+}

internal/config/config.go

@@ -55,7 +55,6 @@ const (
 	// after release.
 	ReleaseInitResponse = "release-init-response.json"
 	pipelineStateFile   = "state.yaml"
-	versionCmdName      = "version"
 )
 
 // are variables so it can be replaced during testing.
@@ -258,9 +257,6 @@ func (c *Config) setupUser() error {
 }
 
 func (c *Config) createWorkRoot() error {
-	if c.CommandName == versionCmdName {
-		return nil
-	}
 	if c.WorkRoot != "" {
 		slog.Info("Using specified working directory", "dir", c.WorkRoot)
 		return nil
@@ -286,9 +282,6 @@ func (c *Config) createWorkRoot() error {
 }
 
 func (c *Config) deriveRepo() error {
-	if c.CommandName == versionCmdName {
-		return nil
-	}
 	if c.Repo != "" {
 		slog.Debug("repo value provided by user", "repo", c.Repo)
 		return nil
@@ -327,7 +320,7 @@ func (c *Config) IsValid() (bool, error) {
 		return false, err
 	}
 
-	if c.CommandName != versionCmdName && c.Repo == "" {
+	if c.Repo == "" {
 		return false, errors.New("language repository not specified or detected")
 	}
 

internal/librarian/action_test.go

@@ -0,0 +1,89 @@
+// Copyright 2025 Google LLC
+//
+// 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.
+
+package librarian
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/googleapis/librarian/internal/cli"
+	"github.com/googleapis/librarian/internal/config"
+)
+
+func TestGenerateAction(t *testing.T) {
+	t.Parallel()
+	testActionConfig(t, cmdGenerate)
+}
+
+func TestInitAction(t *testing.T) {
+	t.Parallel()
+	testActionConfig(t, cmdInit)
+}
+
+func TestTagAndReleaseAction(t *testing.T) {
+	t.Parallel()
+	testActionConfig(t, cmdTagAndRelease)
+}
+
+// testActionConfig tests the execution flow for each Command.Action. The
+// functionality of the Config methods called inside these Actions are tested
+// separately in internal/config.
+func testActionConfig(t *testing.T, cmd *cli.Command) {
+	t.Helper()
+	for _, test := range []struct {
+		cfg     *config.Config
+		wantErr string
+	}{
+		{
+			cfg: &config.Config{
+				WorkRoot: t.TempDir(),
+			},
+			wantErr: "repo flag not specified",
+		},
+		{
+			cfg: &config.Config{
+				WorkRoot: t.TempDir(),
+				Repo:     "myrepo",
+			},
+			wantErr: "repository does not exist",
+		},
+		{
+			cfg: &config.Config{
+				WorkRoot:       t.TempDir(),
+				Repo:           "myrepo",
+				LibraryVersion: "1.0.0",
+			},
+			wantErr: "specified library version without library id",
+		},
+		{
+			cfg: &config.Config{
+				WorkRoot: t.TempDir(),
+				Repo:     "https://github.com/googleapis/language-repo",
+			},
+			wantErr: "remote branch is required when cloning",
+		},
+	} {
+		t.Run(test.wantErr, func(t *testing.T) {
+			cmd.Config = test.cfg
+			err := cmd.Action(t.Context(), cmd)
+			if err == nil {
+				t.Fatal("expected an error")
+			}
+			if !strings.Contains(err.Error(), test.wantErr) {
+				t.Errorf("error mismatch, want: %q, got: %q", test.wantErr, err.Error())
+			}
+		})
+	}
+}

internal/librarian/generate.go

@@ -86,8 +86,14 @@ in '.librarian/state.yaml'.
 
 Example with build and push:
   SDK_LIBRARIAN_GITHUB_TOKEN=xxx librarian generate --push --build`,
-	Run: func(ctx context.Context, cfg *config.Config) error {
-		runner, err := newGenerateRunner(cfg)
+	Action: func(ctx context.Context, cmd *cli.Command) error {
+		if err := cmd.Config.SetDefaults(); err != nil {
+			return fmt.Errorf("failed to initialize config: %w", err)
+		}
+		if _, err := cmd.Config.IsValid(); err != nil {
+			return fmt.Errorf("failed to validate config: %s", err)
+		}
+		runner, err := newGenerateRunner(cmd.Config)
 		if err != nil {
 			return err
 		}

internal/librarian/librarian.go

@@ -16,7 +16,6 @@ package librarian
 
 import (
 	"context"
-	"fmt"
 	"log/slog"
 
 	"github.com/googleapis/librarian/internal/cli"
@@ -27,51 +26,16 @@ var CmdLibrarian = &cli.Command{
 	Short:     "librarian manages client libraries for Google APIs",
 	UsageLine: "librarian <command> [arguments]",
 	Long:      "Librarian manages client libraries for Google APIs.",
-}
-
-func init() {
-	CmdLibrarian.Init()
-	CmdLibrarian.Commands = append(CmdLibrarian.Commands,
+	Commands: []*cli.Command{
 		cmdGenerate,
 		cmdRelease,
 		cmdVersion,
-	)
+	},
 }
 
-// Run executes the Librarian CLI with the given command line
-// arguments.
+// Run executes the Librarian CLI with the given command line arguments.
 func Run(ctx context.Context, arg ...string) error {
-	if err := CmdLibrarian.Parse(arg); err != nil {
-		return err
-	}
-	if len(arg) == 0 {
-		CmdLibrarian.Flags.Usage()
-		return nil
-	}
-	cmd, arg, err := cli.LookupCommand(CmdLibrarian, arg)
-	if err != nil {
-		return err
-	}
-
-	// If a command is just a container for subcommands, it won't have a
-	// Run function. In that case, display its usage instructions.
-	if cmd.Run == nil {
-		cmd.Flags.Usage()
-		return fmt.Errorf("command %q requires a subcommand", cmd.Name())
-	}
-
-	if err := cmd.Parse(arg); err != nil {
-		// We expect that if cmd.Parse fails, it will already
-		// have printed out a command-specific usage error,
-		// so we don't need to display the general usage.
-		return err
-	}
+	CmdLibrarian.Init()
 	slog.Info("librarian", "arguments", arg)
-	if err := cmd.Config.SetDefaults(); err != nil {
-		return fmt.Errorf("failed to initialize config: %w", err)
-	}
-	if _, err := cmd.Config.IsValid(); err != nil {
-		return fmt.Errorf("failed to validate config: %s", err)
-	}
-	return cmd.Run(ctx, cmd.Config)
+	return CmdLibrarian.Run(ctx, arg)
 }