cli-plugins: Add generic hook messages

Allow plugins to return informational messages without the next-steps heading
or indentation. Invoke each hook once and render generic messages before the
shared next-steps block.

Centralize grouping and formatting in PrintMessages, with a callback for
template evaluation and error handling. This avoids the need for an
intermediate EvaluatedMessage type and simplifies hook collection to return
the original responses.

Skip messages containing only whitespace while preserving empty lines
within non-empty messages. Keep PrintNextSteps as a deprecated wrapper
to preserve existing callers and output.

Update the printer tests and remove the obsolete appendMessages helper
and its tests.

Co-authored-by: Nicolas Beck <nicolas.beck@docker.com>
Signed-off-by: Nicolas Beck <nicolas.beck@docker.com>
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

cli-plugins/hooks/hook_types.go

@@ -30,7 +30,11 @@ package hooks
 type ResponseType int
 
 const (
+	// NextSteps renders suggestions under a shared "What's next:" header.
 	NextSteps ResponseType = 0
+	// GenericMessage renders before NextSteps, without a header or indentation.
+	// CLI versions that do not recognize this type ignore the response.
+	GenericMessage ResponseType = 1
 )
 
 // Request is the type representing the information

cli-plugins/hooks/printer.go

@@ -7,17 +7,42 @@ const (
 	indent    = "    "
 )
 
-// PrintNextSteps renders list of [NextSteps] messages and writes them
-// to out. It is a no-op if messages is empty.
-func PrintNextSteps(out io.Writer, messages []string) {
-	if len(messages) == 0 {
-		return
+// PrintMessages groups messages by type and writes them to out. Generic messages
+// appear first, without a header or indentation, followed by next steps under a
+// shared "What's next:" header. Order within each type is preserved.
+// Messages with no lines or an unknown type produce no output.
+func PrintMessages(out io.Writer, messages []Response, evaluate func(Response) []string) {
+	grouped := make(map[ResponseType][]string)
+	for _, message := range messages {
+		lines := evaluate(message)
+		grouped[message.Type] = append(grouped[message.Type], lines...)
 	}
+	for _, responseType := range []ResponseType{GenericMessage, NextSteps} {
+		lines := grouped[responseType]
+		if len(lines) == 0 {
+			continue
+		}
 
-	_, _ = io.WriteString(out, whatsNext)
-	for _, msg := range messages {
-		_, _ = io.WriteString(out, indent)
-		_, _ = io.WriteString(out, msg)
-		_, _ = io.WriteString(out, "\n")
+		header, prefix := "\n", ""
+		if responseType == NextSteps {
+			header, prefix = whatsNext, indent
+		}
+
+		_, _ = io.WriteString(out, header)
+		for _, line := range lines {
+			_, _ = io.WriteString(out, prefix)
+			_, _ = io.WriteString(out, line)
+			_, _ = io.WriteString(out, "\n")
+		}
 	}
 }
+
+// PrintNextSteps renders list of [NextSteps] messages and writes them
+// to out. It is a no-op if messages is empty.
+//
+// Deprecated: use [PrintMessages] instead.
+func PrintNextSteps(out io.Writer, messages []string) {
+	PrintMessages(out, []Response{{Type: NextSteps}}, func(Response) []string {
+		return messages
+	})
+}

cli-plugins/hooks/printer_test.go

@@ -8,7 +8,85 @@ import (
 	"gotest.tools/v3/assert"
 )
 
-func TestPrintHookMessages(t *testing.T) {
+func TestPrintMessages(t *testing.T) {
+	const header = "\n\x1b[1mWhat's next:\x1b[0m\n"
+
+	for _, tc := range []struct {
+		doc      string
+		messages []hooks.Response
+		expected string
+	}{
+		{doc: "no messages"},
+		{
+			doc: "no lines",
+			messages: []hooks.Response{
+				{Type: hooks.GenericMessage},
+				{Type: hooks.NextSteps},
+			},
+		},
+		{
+			doc: "generic message",
+			messages: []hooks.Response{
+				{Type: hooks.GenericMessage, Template: "Session active"},
+			},
+			expected: "\nSession active\n",
+		},
+		{
+			doc: "preserve message formatting",
+			messages: []hooks.Response{
+				{Type: hooks.GenericMessage, Template: "Session active\n\n  Details"},
+			},
+			expected: "\nSession active\n\n  Details\n",
+		},
+		{
+			doc: "next steps",
+			messages: []hooks.Response{
+				{Type: hooks.NextSteps, Template: "Try another command"},
+			},
+			expected: header + "    Try another command\n",
+		},
+		{
+			doc: "group interleaved types and preserve order within each type",
+			messages: []hooks.Response{
+				{Type: hooks.NextSteps, Template: "First suggestion"},
+				{Type: hooks.GenericMessage, Template: "Session active"},
+				{Type: hooks.NextSteps, Template: "Second suggestion\nThird suggestion"},
+				{Type: hooks.GenericMessage, Template: "Another status"},
+			},
+			expected: "\nSession active\nAnother status\n" + header + "    First suggestion\n    Second suggestion\n    Third suggestion\n",
+		},
+		{
+			doc: "unknown type",
+			messages: []hooks.Response{
+				{Type: hooks.ResponseType(99), Template: "Ignored"},
+			},
+		},
+		{
+			doc: "unknown type does not affect known messages",
+			messages: []hooks.Response{
+				{Type: hooks.ResponseType(99), Template: "Ignored"},
+				{Type: hooks.GenericMessage, Template: "Session active"},
+				{Type: hooks.NextSteps, Template: "Try another command"},
+			},
+			expected: "\nSession active\n" + header + "    Try another command\n",
+		},
+	} {
+		t.Run(tc.doc, func(t *testing.T) {
+			var w strings.Builder
+			hooks.PrintMessages(&w, tc.messages, func(message hooks.Response) []string {
+				if message.Template == "" {
+					return nil
+				}
+				lines, err := hooks.ParseTemplate(message.Template, nil)
+				assert.NilError(t, err)
+				return lines
+			})
+			assert.Equal(t, w.String(), tc.expected)
+		})
+	}
+}
+
+func TestPrintNextSteps(t *testing.T) {
 	const header = "\n\x1b[1mWhat's next:\x1b[0m\n"
 
 	tests := []struct {
@@ -34,6 +112,11 @@ func TestPrintHookMessages(t *testing.T) {
 				"    Foo\n" +
 				"    bar\n",
 		},
+		{
+			doc:            "preserve empty and indented lines",
+			messages:       []string{"", "  Details", ""},
+			expectedOutput: header + "    \n      Details\n    \n",
+		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.doc, func(t *testing.T) {

cli-plugins/hooks/template.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"errors"
 	"fmt"
+	"slices"
 	"strings"
 	"text/template"
 
@@ -43,7 +44,16 @@ func ParseTemplate(hookTemplate string, cmd *cobra.Command) ([]string, error) {
 	if n := strings.Count(out, "\n"); n > maxMessages {
 		return nil, fmt.Errorf("hook template contains too many messages (%d): maximum is %d", n, maxMessages)
 	}
-	return strings.SplitN(out, "\n", maxMessages), nil
+	lines := strings.SplitN(out, "\n", maxMessages)
+
+	// Skip messages containing only whitespace, but preserve empty lines
+	// in messages that have content (e.g., for paragraph spacing).
+	if !slices.ContainsFunc(lines, func(line string) bool {
+		return strings.TrimSpace(line) != ""
+	}) {
+		return nil, nil
+	}
+	return lines, nil
 }
 
 var ErrHookTemplateParse = errors.New("failed to parse hook template")

cli-plugins/hooks/template_test.go

@@ -33,7 +33,17 @@ func TestParseTemplate(t *testing.T) {
 		{
 			doc:            "empty template",
 			template:       "",
-			expectedOutput: []string{""},
+			expectedOutput: nil,
+		},
+		{
+			doc:            "only empty lines",
+			template:       "\n\n",
+			expectedOutput: nil,
+		},
+		{
+			doc:            "whitespace-only lines",
+			template:       " \n\t\n  ",
+			expectedOutput: nil,
 		},
 		{
 			doc:            "plain message",

cli-plugins/manager/hooks.go

@@ -49,11 +49,18 @@ func RunPluginHooks(ctx context.Context, dockerCLI config.Provider, rootCmd, sub
 }
 
 func runHooks(ctx context.Context, cfg *configfile.ConfigFile, rootCmd, subCommand *cobra.Command, invokedCommand string, flags map[string]string, cmdErrorMessage string) {
-	nextSteps := invokeAndCollectHooks(ctx, cfg, rootCmd, subCommand, invokedCommand, flags, cmdErrorMessage)
-	hooks.PrintNextSteps(subCommand.ErrOrStderr(), nextSteps)
+	messages := invokeAndCollectHooks(ctx, cfg, rootCmd, invokedCommand, flags, cmdErrorMessage)
+	hooks.PrintMessages(subCommand.ErrOrStderr(), messages, func(message hooks.Response) []string {
+		lines, err := hooks.ParseTemplate(message.Template, subCommand)
+		if err != nil {
+			logrus.WithError(err).Debug("Failed to evaluate hook template")
+			return nil
+		}
+		return lines
+	})
 }
 
-func invokeAndCollectHooks(ctx context.Context, cfg *configfile.ConfigFile, rootCmd, subCmd *cobra.Command, subCmdStr string, flags map[string]string, cmdErrorMessage string) []string {
+func invokeAndCollectHooks(ctx context.Context, cfg *configfile.ConfigFile, rootCmd *cobra.Command, subCmdStr string, flags map[string]string, cmdErrorMessage string) []hooks.Response {
 	if ctx.Err() != nil {
 		return nil
 	}
@@ -64,17 +71,17 @@ func invokeAndCollectHooks(ctx context.Context, cfg *configfile.ConfigFile, root
 	}
 
 	pluginDirs := getPluginDirs(cfg)
-	nextSteps := make([]string, 0, len(pluginsCfg))
+	var collected []hooks.Response
 
-	tryInvokeHook := func(pluginName string, pluginCfg map[string]string) (messages []string, ok bool, err error) {
+	tryInvokeHook := func(pluginName string, pluginCfg map[string]string) (hooks.Response, error) {
 		match, matched := pluginMatch(pluginCfg, subCmdStr, cmdErrorMessage)
 		if !matched {
-			return nil, false, nil
+			return hooks.Response{}, nil
 		}
 
 		p, err := getPlugin(pluginName, pluginDirs, rootCmd)
 		if err != nil {
-			return nil, false, err
+			return hooks.Response{}, err
 		}
 
 		resp, err := p.RunHook(ctx, hooks.Request{
@@ -83,29 +90,23 @@ func invokeAndCollectHooks(ctx context.Context, cfg *configfile.ConfigFile, root
 			CommandError: cmdErrorMessage,
 		})
 		if err != nil {
-			return nil, false, err
+			return hooks.Response{}, err
 		}
 
 		var message hooks.Response
 		if err := json.Unmarshal(resp, &message); err != nil {
-			return nil, false, fmt.Errorf("failed to unmarshal hook response (%q): %w", string(resp), err)
+			return hooks.Response{}, fmt.Errorf("failed to unmarshal hook response (%q): %w", string(resp), err)
 		}
 
-		// currently the only hook type
-		if message.Type != hooks.NextSteps {
-			return nil, false, errors.New("unexpected hook response type: " + strconv.Itoa(int(message.Type)))
+		if message.Type != hooks.NextSteps && message.Type != hooks.GenericMessage {
+			return hooks.Response{}, errors.New("unexpected hook response type: " + strconv.Itoa(int(message.Type)))
 		}
 
-		messages, err = hooks.ParseTemplate(message.Template, subCmd)
-		if err != nil {
-			return nil, false, err
-		}
-
-		return messages, true, nil
+		return message, nil
 	}
 
 	for pluginName, pluginCfg := range pluginsCfg {
-		messages, ok, err := tryInvokeHook(pluginName, pluginCfg)
+		message, err := tryInvokeHook(pluginName, pluginCfg)
 		if err != nil {
 			// skip misbehaving plugins, but don't halt execution
 			logrus.WithFields(logrus.Fields{
@@ -114,38 +115,13 @@ func invokeAndCollectHooks(ctx context.Context, cfg *configfile.ConfigFile, root
 			}).Debug("Plugin hook invocation failed")
 			continue
 		}
-		if !ok {
+		if message.Template == "" {
 			continue
 		}
 
-		var appended bool
-		nextSteps, appended = appendNextSteps(nextSteps, messages)
-		if !appended {
-			logrus.WithFields(logrus.Fields{
-				"plugin": pluginName,
-			}).Debug("Plugin responded with an empty hook message; ignoring")
-		}
+		collected = append(collected, message)
 	}
-	return nextSteps
-}
-
-// appendNextSteps appends the processed hook output to the nextSteps slice.
-// If the processed hook output is empty, it is not appended.
-// Empty lines are not stripped if there's at least one non-empty line.
-func appendNextSteps(nextSteps []string, processed []string) ([]string, bool) {
-	empty := true
-	for _, l := range processed {
-		if strings.TrimSpace(l) != "" {
-			empty = false
-			break
-		}
-	}
-
-	if empty {
-		return nextSteps, false
-	}
-
-	return append(nextSteps, processed...), true
+	return collected
 }
 
 // pluginMatch takes a plugin configuration and a string representing the

cli-plugins/manager/hooks_test.go

@@ -2,6 +2,11 @@ package manager
 
 import (
 	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
 	"testing"
 
 	"github.com/docker/cli/cli/config/configfile"
@@ -258,38 +263,6 @@ func TestMatchHookConfig(t *testing.T) {
 	}
 }
 
-func TestAppendNextSteps(t *testing.T) {
-	testCases := []struct {
-		processed   []string
-		expectedOut []string
-	}{
-		{
-			processed:   []string{},
-			expectedOut: []string{},
-		},
-		{
-			processed:   []string{"", ""},
-			expectedOut: []string{},
-		},
-		{
-			processed:   []string{"Some hint", "", "Some other hint"},
-			expectedOut: []string{"Some hint", "", "Some other hint"},
-		},
-		{
-			processed:   []string{"Hint 1", "Hint 2"},
-			expectedOut: []string{"Hint 1", "Hint 2"},
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run("", func(t *testing.T) {
-			got, appended := appendNextSteps([]string{}, tc.processed)
-			assert.Check(t, is.DeepEqual(got, tc.expectedOut))
-			assert.Check(t, is.Equal(appended, len(got) > 0))
-		})
-	}
-}
-
 func TestRunPluginHooksPassesErrorMessage(t *testing.T) {
 	cfg := configfile.New("")
 	cfg.Plugins = map[string]map[string]string{
@@ -336,7 +309,7 @@ func TestInvokeAndCollectHooksErrorHooksSkippedOnSuccess(t *testing.T) {
 	// On success, error-hooks should not match, so the plugin
 	// binary is never looked up and no results are returned.
 	result := invokeAndCollectHooks(
-		context.Background(), cfg, root, sub,
+		context.Background(), cfg, root,
 		"build", map[string]string{}, "",
 	)
 	assert.Check(t, is.Len(result, 0))
@@ -349,7 +322,7 @@ func TestInvokeAndCollectHooksNoPlugins(t *testing.T) {
 	root.AddCommand(sub)
 
 	result := invokeAndCollectHooks(
-		context.Background(), cfg, root, sub,
+		context.Background(), cfg, root,
 		"build", map[string]string{}, "some error",
 	)
 	assert.Check(t, is.Len(result, 0))
@@ -368,8 +341,108 @@ func TestInvokeAndCollectHooksCancelledContext(t *testing.T) {
 	cancel() // cancel immediately
 
 	result := invokeAndCollectHooks(
-		ctx, cfg, root, sub,
+		ctx, cfg, root,
 		"build", map[string]string{}, "exit status 1",
 	)
 	assert.Check(t, is.Nil(result))
 }
+
+func TestRunHooksMessages(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("test plugins require a POSIX shell")
+	}
+
+	const header = "\n\x1b[1mWhat's next:\x1b[0m\n"
+	for _, tc := range []struct {
+		doc       string
+		responses map[string]string
+		expected  string
+	}{
+		{
+			doc: "generic message preserves formatting and expands templates",
+			responses: map[string]string{
+				"status": `{"Type":1,"Template":"Status for {{command}}\n\n  Details"}`,
+			},
+			expected: "\nStatus for build\n\n  Details\n",
+		},
+		{
+			doc: "omitted type defaults to next steps",
+			responses: map[string]string{
+				"suggestion": `{"Template":"Try another command"}`,
+			},
+			expected: header + "    Try another command\n",
+		},
+		{
+			doc: "generic messages precede shared next steps header",
+			responses: map[string]string{
+				"statusone":     `{"Type":1,"Template":"Session active"}`,
+				"statustwo":     `{"Type":1,"Template":"Session active"}`,
+				"suggestionone": `{"Type":0,"Template":"Try another command"}`,
+				"suggestiontwo": `{"Type":0,"Template":"Try another command"}`,
+			},
+			expected: "\nSession active\nSession active\n" + header + "    Try another command\n    Try another command\n",
+		},
+		{
+			doc: "empty responses produce no output",
+			responses: map[string]string{
+				"status":     `{"Type":1,"Template":" \n\t"}`,
+				"suggestion": `{"Type":0,"Template":""}`,
+			},
+		},
+		{
+			doc: "empty generic message does not affect next steps",
+			responses: map[string]string{
+				"status":     `{"Type":1,"Template":""}`,
+				"suggestion": `{"Type":0,"Template":"Try another command"}`,
+			},
+			expected: header + "    Try another command\n",
+		},
+		{
+			doc: "invalid responses do not affect other plugins",
+			responses: map[string]string{
+				"unknown":    `{"Type":99,"Template":"Ignored"}`,
+				"invalid":    `{"Type":1,"Template":"{{"}`,
+				"status":     `{"Type":1,"Template":"Session active"}`,
+				"suggestion": `{"Type":0,"Template":"Try another command"}`,
+			},
+			expected: "\nSession active\n" + header + "    Try another command\n",
+		},
+	} {
+		t.Run(tc.doc, func(t *testing.T) {
+			pluginDir := t.TempDir()
+			cfg := configfile.New("")
+			cfg.CLIPluginsExtraDirs = []string{pluginDir}
+			cfg.Plugins = make(map[string]map[string]string)
+			for name, response := range tc.responses {
+				plugin := fmt.Sprintf(`#!/bin/sh
+case "$1" in
+docker-cli-plugin-metadata)
+    printf '%%s\n' '{"SchemaVersion":"0.1.0","Vendor":"test"}'
+    ;;
+*)
+    test "$2" = docker-cli-plugin-hooks || exit 1
+    printf 'called\n' >> "$0.calls"
+    printf '%%s\n' '%s'
+    ;;
+esac
+`, response)
+				assert.NilError(t, os.WriteFile(filepath.Join(pluginDir, "docker-"+name), []byte(plugin), 0o755))
+				cfg.Plugins[name] = map[string]string{"hooks": "build"}
+			}
+
+			root := &cobra.Command{Use: "docker"}
+			sub := &cobra.Command{Use: "build"}
+			root.AddCommand(sub)
+			var output strings.Builder
+			sub.SetErr(&output)
+
+			runHooks(context.Background(), cfg, root, sub, "build", nil, "")
+			assert.Equal(t, output.String(), tc.expected)
+			for name := range tc.responses {
+				calls, err := os.ReadFile(filepath.Join(pluginDir, "docker-"+name+".calls"))
+				assert.NilError(t, err)
+				assert.Equal(t, string(calls), "called\n", "each plugin hook must run exactly once")
+			}
+		})
+	}
+}