Fix: run plugin hooks on command failure, not just success

Signed-off-by: Derek Misler <derek.misler@docker.com>

Author: derekmisler

cli-plugins/manager/hooks.go

@@ -42,11 +42,11 @@ func RunCLICommandHooks(ctx context.Context, dockerCLI config.Provider, rootCmd,
 
 // RunPluginHooks is the entrypoint for the hooks execution flow
 // after a plugin command was just executed by the CLI.
-func RunPluginHooks(ctx context.Context, dockerCLI config.Provider, rootCmd, subCommand *cobra.Command, args []string) {
+func RunPluginHooks(ctx context.Context, dockerCLI config.Provider, rootCmd, subCommand *cobra.Command, args []string, cmdErrorMessage string) {
 	commandName := strings.Join(args, " ")
 	flags := getNaiveFlags(args)
 
-	runHooks(ctx, dockerCLI.ConfigFile(), rootCmd, subCommand, commandName, flags, "")
+	runHooks(ctx, dockerCLI.ConfigFile(), rootCmd, subCommand, commandName, flags, cmdErrorMessage)
 }
 
 func runHooks(ctx context.Context, cfg *configfile.ConfigFile, rootCmd, subCommand *cobra.Command, invokedCommand string, flags map[string]string, cmdErrorMessage string) {

cli-plugins/manager/hooks_test.go

@@ -1,12 +1,23 @@
 package manager
 
 import (
+	"context"
 	"testing"
 
+	"github.com/docker/cli/cli/config/configfile"
+	"github.com/spf13/cobra"
 	"gotest.tools/v3/assert"
 	is "gotest.tools/v3/assert/cmp"
 )
 
+type fakeConfigProvider struct {
+	cfg *configfile.ConfigFile
+}
+
+func (f *fakeConfigProvider) ConfigFile() *configfile.ConfigFile {
+	return f.cfg
+}
+
 func TestGetNaiveFlags(t *testing.T) {
 	testCases := []struct {
 		args          []string
@@ -141,3 +152,71 @@ func TestAppendNextSteps(t *testing.T) {
 		})
 	}
 }
+
+func TestRunPluginHooksPassesErrorMessage(t *testing.T) {
+	cfg := configfile.New("")
+	cfg.Plugins = map[string]map[string]string{
+		"test-plugin": {"hooks": "build"},
+	}
+	provider := &fakeConfigProvider{cfg: cfg}
+	root := &cobra.Command{Use: "docker"}
+	sub := &cobra.Command{Use: "build"}
+	root.AddCommand(sub)
+
+	// Should not panic with empty error message (success case)
+	RunPluginHooks(context.Background(), provider, root, sub, []string{"build"}, "")
+
+	// Should not panic with non-empty error message (failure case)
+	RunPluginHooks(context.Background(), provider, root, sub, []string{"build"}, "exit status 1")
+}
+
+func TestInvokeAndCollectHooksForwardsErrorMessage(t *testing.T) {
+	cfg := configfile.New("")
+	cfg.Plugins = map[string]map[string]string{
+		"nonexistent": {"hooks": "build"},
+	}
+	root := &cobra.Command{Use: "docker"}
+	sub := &cobra.Command{Use: "build"}
+	root.AddCommand(sub)
+
+	// Plugin binary doesn't exist — invokeAndCollectHooks skips it
+	// gracefully and returns empty. Verifies the error message path
+	// doesn't cause issues when forwarded through the call chain.
+	result := invokeAndCollectHooks(
+		context.Background(), cfg, root, sub,
+		"build", map[string]string{}, "exit status 1",
+	)
+	assert.Check(t, is.Len(result, 0))
+}
+
+func TestInvokeAndCollectHooksNoPlugins(t *testing.T) {
+	cfg := configfile.New("")
+	root := &cobra.Command{Use: "docker"}
+	sub := &cobra.Command{Use: "build"}
+	root.AddCommand(sub)
+
+	result := invokeAndCollectHooks(
+		context.Background(), cfg, root, sub,
+		"build", map[string]string{}, "some error",
+	)
+	assert.Check(t, is.Len(result, 0))
+}
+
+func TestInvokeAndCollectHooksCancelledContext(t *testing.T) {
+	cfg := configfile.New("")
+	cfg.Plugins = map[string]map[string]string{
+		"test-plugin": {"hooks": "build"},
+	}
+	root := &cobra.Command{Use: "docker"}
+	sub := &cobra.Command{Use: "build"}
+	root.AddCommand(sub)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // cancel immediately
+
+	result := invokeAndCollectHooks(
+		ctx, cfg, root, sub,
+		"build", map[string]string{}, "exit status 1",
+	)
+	assert.Check(t, is.Nil(result))
+}

cmd/docker/docker.go

@@ -480,10 +480,14 @@ func runDocker(ctx context.Context, dockerCli *command.DockerCli) error {
 		subCommand = ccmd
 		if err != nil || pluginmanager.IsPluginCommand(ccmd) {
 			err := tryPluginRun(ctx, dockerCli, cmd, args[0], envs)
-			if err == nil {
-				if ccmd != nil && dockerCli.Out().IsTerminal() && dockerCli.HooksEnabled() {
-					pluginmanager.RunPluginHooks(ctx, dockerCli, cmd, ccmd, args)
+			if ccmd != nil && dockerCli.Out().IsTerminal() && dockerCli.HooksEnabled() {
+				var errMessage string
+				if err != nil {
+					errMessage = err.Error()
 				}
+				pluginmanager.RunPluginHooks(ctx, dockerCli, cmd, ccmd, args, errMessage)
+			}
+			if err == nil {
 				return nil
 			}
 			if !errdefs.IsNotFound(err) {