Add a local control socket and nebula ctl (#1975)

Add support for accessing the previously ssh-only debug commands via
`nebula ctl <command>.`

- unix socket (named pipe on windows) for communication
- sockets scoped to the user + an optional group
- in cmd/nebula and cmd/nebula-service, but not nebula.Main

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

commands/command.go

@@ -1,36 +1,73 @@
 package commands
 
 import (
+	"errors"
 	"flag"
+	"fmt"
+	"io"
+	"strings"
 )
 
 // CommandFlags is called before help or command execution to parse command line flags.
 // It should return a flag.FlagSet instance and a pointer to the struct that will contain parsed flags
+//
+// Every call must build a fresh FlagSet and a fresh struct. Nothing may be shared between
+// calls: a command runs on more than one surface and on more than one goroutine.
 type CommandFlags func() (*flag.FlagSet, any)
 
-// CommandCallback is called when your command should execute.
-// fs will be a pointer to the struct provided by Command.Flags callback, if there was one. -h and -help are reserved
-// and handled automatically for you.
-// a will be any unconsumed arguments, if no Command.Flags was available this will be all the flags passed in.
-// w is the writer to use when sending messages back to the client.
-// An error returned by the callback is handed back to whoever dispatched the command, which reports a failure: exit
-// status 1 over ssh. The error is not shown to the user. For a failure the user caused or can act on, write the
-// message and return it as a *Failure, which Fail does in one step; the surface then reports the failure without
-// logging what the user was already told. Any other error is logged by the surface as well.
-type CommandCallback func(fs any, a []string, w StringWriter) error
+// RunFunc computes a command's Result.
+//
+// fs is the pointer returned by Command.Flags, if there was one; -h, -help, -json and
+// -pretty are reserved and handled before Run is called. a is the operands: whatever
+// was not consumed as a flag. The Result is rendered by whichever surface asked: Value
+// for a command that reports, Did or Failed for one that acts, Nothing for one with
+// nothing to say. An error is for a failure of the command itself; a bad argument is
+// Failed, so the caller hears why in its own format.
+type RunFunc func(fs any, a []string) (Result, error)
+
+// TextFunc renders a Result's Value for a person. See Typed for declaring one against
+// the type the command actually returns.
+type TextFunc func(v any, w io.Writer) error
 
 type Command struct {
 	Name             Name
 	ShortDescription string
 	Help             string
 
-	Flags    CommandFlags
-	Callback CommandCallback
+	Flags CommandFlags
+
+	// Run produces the command's Result, leaving presentation to Render.
+	Run RunFunc
+
+	// Text renders a Result's Value for a person. Both surfaces call it, so a command
+	// reads the same over ssh as it does through the control socket. It never sees an
+	// outcome: Render prints those itself, the same way for every command. Leaving it
+	// nil declares that no human rendering of the Value exists yet, which JSONOnly says
+	// up front; Render then refuses text rather than inventing an interim form for
+	// people to script against.
+	Text TextFunc
+
+	// JSONOnly declares that the command's Value has no human rendering yet, so a surface
+	// whose default is text asks for JSON instead, and the schema says so to a client. It
+	// is a statement that no such form exists, not a fallback. A refusal is still
+	// rendered as a sentence, since that always has a human form.
+	JSONOnly bool
+
+	// Then, when set, is the second half of a command that acts: Run says what is about
+	// to happen and Then does it and says how it went, with its Result rendered after
+	// Run's and deciding the outcome. A Run that refused is the outcome, and Then does
+	// not run. It exists for a command whose action can cut off
+	// its own reply: reload restarts the ssh server when the sshd keys changed, which
+	// ends the session that asked, so Run's line has to be on the wire before the reload
+	// begins. A surface that answers in one piece, the control socket, runs Then before
+	// it answers and carries both.
+	Then func() Result
 }
 
-// Failure is the error a command returns for a failure it has already explained to the user:
-// the message it wrote is the whole story. A surface reports it as a failure and does not
-// log it.
+// Failure is the error a surface gets for a refusal the person has already been shown:
+// a Run that returned Failed, a command that is not there, a line that would not parse.
+// The message it rendered is the whole story, so the surface reports the failure, exit
+// status 1 over ssh, and does not log it.
 type Failure struct {
 	Msg string
 }
@@ -39,45 +76,225 @@ func (f *Failure) Error() string {
 	return f.Msg
 }
 
-// Fail writes msg to w and returns it as a *Failure, so a command reports a failure to the
-// user and to its surface in one line. If the write itself fails, that error is returned
-// instead: the user was not told anything.
-func Fail(w StringWriter, msg string) error {
-	if err := w.WriteLine(msg); err != nil {
-		return err
-	}
-	return &Failure{Msg: msg}
-}
-
-func execCommand(c *Command, args []string, w StringWriter) error {
-	var (
-		fl *flag.FlagSet
-		fs any
-	)
-
+// commandFlags returns the FlagSet a command is driven through, on any surface: the
+// command's own flags, if it declares any, with the two presentation flags added. own
+// reports whether the command declared any of its own, which is what tells a caller
+// whether to say a parameter is unknown or that the command takes none at all.
+//
+// The presentation flags are defined here rather than by each command, because how a
+// result is displayed is not a property of the command that produced it. This is also
+// the one definition of them: the ssh line parses into it, nebula ctl parses its line
+// against the schema derived from it, and help renders from that schema. They are
+// command line flags only: a socket request chooses its rendering with the envelope's
+// format field, and Invoke refuses them as parameters.
+func commandFlags(c *Command) (fl *flag.FlagSet, fs any, own bool) {
 	if c.Flags != nil {
 		fl, fs = c.Flags()
-		if fl != nil {
-			// SetOutput() here in case fl.Parse dumps usage.
-			fl.SetOutput(w.GetWriter())
-			err := fl.Parse(args)
-			if err != nil {
-				// fl.Parse has dumped error information to the user via the w writer.
-				return err
+	}
+	own = fl != nil
+	if fl == nil {
+		fl = flag.NewFlagSet(string(c.Name), flag.ContinueOnError)
+	}
+
+	addPresentationFlags(fl)
+
+	// What Parse prints after a bad flag, and for -h: the command's usage as help renders
+	// it, so a line that went wrong is answered with the same text as `help <command>`.
+	fl.Usage = func() { _ = WriteCommandHelp(schemaOf(c, fl), fl.Output()) }
+
+	return fl, fs, own
+}
+
+// addPresentationFlags defines the two presentation flags on fl.
+func addPresentationFlags(fl *flag.FlagSet) {
+	fl.Bool(FlagJSON, false, "outputs as json")
+	fl.Bool(FlagPretty, false, "pretty prints json, assumes -json")
+}
+
+// Flagged fills in c's Flags and Run as a pair: define declares the flags on a fresh F,
+// and run is handed that F. The struct the flags fill is the struct Run reads by
+// construction, so no Run has to assert the type it was given or decide what to do
+// when the assertion fails.
+func Flagged[F any](c *Command, define func(*flag.FlagSet, *F), run func(*F, []string) (Result, error)) *Command {
+	c.Flags = func() (*flag.FlagSet, any) {
+		fl := flag.NewFlagSet("", flag.ContinueOnError)
+		s := new(F)
+		define(fl, s)
+		return fl, s
+	}
+	c.Run = func(fs any, a []string) (Result, error) {
+		s, ok := fs.(*F)
+		if !ok {
+			return Result{}, fmt.Errorf("%s was handed %T for its flags, not %T", c.Name, fs, s)
+		}
+		return run(s, a)
+	}
+	return c
+}
+
+// isPresentationFlag reports whether name is one of the two flags commandFlags adds.
+func isPresentationFlag(name string) bool {
+	return name == FlagJSON || name == FlagPretty
+}
+
+// execLine runs an ssh command line against c, reporting problems to w. The line is
+// parsed into the command's own FlagSet and Run reads it directly, so a flag.Value whose
+// String does not re-parse, flag.Func above all, arrives as the line said. Invoke fills
+// the same FlagSet through Set.
+func (r *Registry) execLine(c *Command, args []string, w Output) error {
+	fl, fs, _ := commandFlags(c)
+
+	// Parse reports a bad flag to this writer, with the command's usage after it. -h,
+	// -help and --help take the same path with nothing wrong: the usage is the whole
+	// answer, and Parse says so with ErrHelp. A -h that is a flag's value or follows --
+	// is read as the operand it is, since ParseArgs is the one reading of the line.
+	fl.SetOutput(w.Err)
+	operands, err := ParseArgs(fl, args)
+	if err != nil {
+		if errors.Is(err, flag.ErrHelp) {
+			return nil
+		}
+		return err
+	}
+
+	format := presentationFormat(fl)
+
+	// An error from Run is something the command could not do rather than a refusal it
+	// chose, but the person at the prompt still hears it; the surface logs it too.
+	res, err := c.Run(fs, operands)
+	if err != nil {
+		_, _ = fmt.Fprintln(w.Err, err.Error())
+		return err
+	}
+
+	// Run's reply goes out first, and then the action, if the command has one and Run
+	// did not refuse, whose Result is the outcome. In JSON only the outcome travels:
+	// the promise is for a person to read, and a program expects one document.
+	if c.Then == nil || !res.OK || r.formatFor(c, res, format) == FormatText {
+		if err := r.renderLine(c, res, format, w); err != nil {
+			return err
+		}
+	}
+	if c.Then != nil && res.OK {
+		res = c.Then()
+		if err := r.renderLine(c, res, format, w); err != nil {
+			return err
+		}
+	}
+
+	if !res.OK {
+		return &Failure{Msg: res.Reason}
+	}
+	return nil
+}
+
+// formatFor is the format a Result is rendered in. The line wins when it asked for one;
+// otherwise the surface that owns the registry decides, from the command and its
+// Result, so each surface can keep its own default.
+func (r *Registry) formatFor(c *Command, res Result, format Format) Format {
+	if format == FormatUnset {
+		return r.defaultFormatFor(c, res)
+	}
+	return format
+}
+
+// renderLine writes one Result of a command line to w; see formatFor. An empty Result
+// writes nothing.
+func (r *Registry) renderLine(c *Command, res Result, format Format, w Output) error {
+	if res.Empty() {
+		return nil
+	}
+
+	if err := Render(c, res, r.formatFor(c, res, format), w.Out); err != nil {
+		if errors.Is(err, ErrNoTextRendering) {
+			_, _ = fmt.Fprintln(w.Err, err.Error())
+		}
+		return err
+	}
+	return nil
+}
+
+// ParseArgs parses args with fl and returns the operands.
+//
+// It differs from fl.Parse in two ways, and both surfaces parse through here so a line
+// means the same typed at an ssh prompt or to nebula ctl.
+//
+// Flags and operands may come in any order, which is the one way it differs from
+// fl.Parse. Parse stops at the first operand, so `print-tunnel 10.0.0.1 -json` would
+// reach the command with "-json" as a second operand and the flag silently dropped. Here
+// an operand ends nothing: parsing resumes after it, and only "--" ends the flags, with
+// everything after it an operand. What counts as a flag is Parse's rule: every argument
+// that starts with "-", a negative number included, so one that is meant as an operand
+// goes after "--".
+func ParseArgs(fl *flag.FlagSet, args []string) ([]string, error) {
+	operands := make([]string, 0, len(args))
+
+	for len(args) > 0 {
+		// Walk the flags the way Parse will, to the first argument it would not read as a
+		// flag: an operand or the terminator. Everything before it is Parse's.
+		end := len(args)
+		for i := 0; i < len(args); {
+			a := args[i]
+			if a == "--" {
+				if err := fl.Parse(args[:i]); err != nil {
+					return nil, err
+				}
+				return append(operands, args[i+1:]...), nil
+			}
+
+			if len(a) < 2 || a[0] != '-' {
+				end = i
+				break
+			}
+
+			f, hasValue := LookupArg(fl, a)
+			if f == nil {
+				// An undefined flag: leave it for Parse to report.
+				i++
+				continue
+			}
+
+			if hasValue || IsBoolFlag(f) {
+				i++
+			} else {
+				// The flag's value is the next argument, whatever it looks like.
+				i += 2
 			}
-			args = fl.Args()
 		}
+
+		if err := fl.Parse(args[:end]); err != nil {
+			return nil, err
+		}
+
+		if end == len(args) {
+			break
+		}
+
+		operands = append(operands, args[end])
+		args = args[end+1:]
 	}
 
-	return c.Callback(fs, args, w)
+	return operands, nil
 }
 
-func checkHelpArgs(args []string) bool {
-	for _, a := range args {
-		if a == "-h" || a == "-help" {
-			return true
-		}
+// LookupArg reads arg the way Parse does: one or two leading dashes, the flag's name, and
+// an optional "=value". f is the flag of fl by that name, nil when fl has none, and
+// hasValue reports that the value came with the argument rather than in the one after it.
+// It is the one reading of an argument's shape: ParseArgs walks a line with it, and
+// nebula ctl lifts its own flags out of a line with it, so the two cannot disagree about
+// which word is a flag's value.
+func LookupArg(fl *flag.FlagSet, arg string) (f *flag.Flag, hasValue bool) {
+	name := strings.TrimPrefix(strings.TrimPrefix(arg, "-"), "-")
+	if eq := strings.IndexByte(name, '='); eq >= 0 {
+		name, hasValue = name[:eq], true
 	}
+	return fl.Lookup(name), hasValue
+}
 
-	return false
+// IsBoolFlag reports whether f is satisfied by its presence alone and never reads the
+// argument after it as its value. The flag package marks these with an unexported
+// interface, which is the only way to tell.
+func IsBoolFlag(f *flag.Flag) bool {
+	bf, ok := f.Value.(interface{ IsBoolFlag() bool })
+	return ok && bf.IsBoolFlag()
 }

commands/command_test.go

@@ -0,0 +1,101 @@
+package commands
+
+import (
+	"bytes"
+	"flag"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestParseArgsAllowsFlagsAfterOperands pins the one way ParseArgs departs from fl.Parse,
+// that a flag may follow an operand, and that it departs in no other: a negative number
+// is a flag, as Parse reads it, and goes after "--" when it is meant as an operand.
+func TestParseArgsAllowsFlagsAfterOperands(t *testing.T) {
+	cases := map[string]struct {
+		args     []string
+		wantOps  []string
+		wantN    string
+		wantV    bool
+		wantJSON bool
+		wantErr  string
+	}{
+		"flag after operand":  {args: []string{"x", "-json"}, wantOps: []string{"x"}, wantJSON: true},
+		"flags around":        {args: []string{"-v", "x", "-n", "1", "y"}, wantOps: []string{"x", "y"}, wantV: true, wantN: "1"},
+		"value after operand": {args: []string{"x", "-n", "-1"}, wantOps: []string{"x"}, wantN: "-1"},
+		"equals value":        {args: []string{"-n=-1", "x"}, wantOps: []string{"x"}, wantN: "-1"},
+		"terminator after":    {args: []string{"x", "--", "-json", "-1"}, wantOps: []string{"x", "-json", "-1"}},
+		"terminator":          {args: []string{"--", "-json"}, wantOps: []string{"-json"}},
+		"defined numeric":     {args: []string{"-2", "x"}, wantOps: []string{"x"}, wantV: true},
+		"negative number":     {args: []string{"-1"}, wantErr: "flag provided but not defined: -1"},
+		"bad flag after":      {args: []string{"x", "-nope"}, wantErr: "flag provided but not defined: -nope"},
+		"undefined flag":      {args: []string{"-nope", "x"}, wantErr: "flag provided but not defined: -nope"},
+		"missing value":       {args: []string{"-n"}, wantErr: "flag needs an argument: -n"},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			fl := flag.NewFlagSet("t", flag.ContinueOnError)
+			fl.SetOutput(&bytes.Buffer{})
+			n := fl.String("n", "", "")
+			v := fl.Bool("v", false, "")
+			two := fl.Bool("2", false, "a flag that happens to be a number")
+			asJSON := fl.Bool(FlagJSON, false, "")
+
+			ops, err := ParseArgs(fl, tc.args)
+			if tc.wantErr != "" {
+				require.ErrorContains(t, err, tc.wantErr)
+				return
+			}
+
+			require.NoError(t, err)
+			assert.Equal(t, tc.wantOps, ops)
+			assert.Equal(t, tc.wantN, *n)
+			assert.Equal(t, tc.wantV, *v || *two)
+			assert.Equal(t, tc.wantJSON, *asJSON)
+		})
+	}
+}
+
+// TestDispatchFlagAfterOperand: the same through a registered command with no Flags of
+// its own, which only has the presentation flags to find.
+func TestDispatchFlagAfterOperand(t *testing.T) {
+	out := &bytes.Buffer{}
+	require.NoError(t, testRegistry().DispatchArgs([]string{"served", "x", "-json"}, MergedOutput(out)))
+	assert.Equal(t, "\"read x\"\n", out.String())
+}
+
+// TestLookupArg pins the reading of an argument's shape that ParseArgs and nebula ctl
+// share.
+func TestLookupArg(t *testing.T) {
+	fl := flag.NewFlagSet("t", flag.ContinueOnError)
+	fl.String("n", "", "")
+	fl.Bool("v", false, "")
+
+	for _, tc := range []struct {
+		arg      string
+		name     string
+		hasValue bool
+	}{
+		{"-n", "n", false},
+		{"--n", "n", false},
+		{"-n=1", "n", true},
+		{"--n=", "n", true},
+		{"-v", "v", false},
+		{"-nope", "", false},
+		{"-1", "", false},
+	} {
+		f, hasValue := LookupArg(fl, tc.arg)
+		if tc.name == "" {
+			assert.Nil(t, f, tc.arg)
+		} else {
+			require.NotNil(t, f, tc.arg)
+			assert.Equal(t, tc.name, f.Name, tc.arg)
+		}
+		assert.Equal(t, tc.hasValue, hasValue, tc.arg)
+	}
+
+	assert.True(t, IsBoolFlag(fl.Lookup("v")))
+	assert.False(t, IsBoolFlag(fl.Lookup("n")))
+}

commands/registry.go

@@ -2,7 +2,9 @@ package commands
 
 import (
 	"cmp"
+	"flag"
 	"fmt"
+	"io"
 	"maps"
 	"slices"
 	"strings"
@@ -11,14 +13,38 @@ import (
 	"github.com/anmitsu/go-shlex"
 )
 
-// Registry holds a set of commands. New is the only way to make one.
+// Registry holds a set of commands.
 //
-// It is safe for concurrent use. Commands may be registered while a surface is serving
-// from the registry: the ssh server clones it per session, and a clone may overlap a
-// Register.
+// It is safe for concurrent use. Commands may be registered while surfaces are serving
+// from the registry: the ssh server clones it per session and the control socket filters
+// it per request, and either may overlap a Register.
 type Registry struct {
 	mu       sync.RWMutex
 	commands map[Name]*Command
+
+	// full is set on a filtered registry and points at the registry it was filtered
+	// from, so a command that's filtered can be reported as such.
+	full *Registry
+
+	// reason explains, in the terms an operator cares about, why this registry withholds
+	// a command they can name. It is supplied by whoever built the filter, because why a
+	// surface declines a command is that surface's business, not the registry's.
+	reason string
+
+	// DefaultFormat decides how a result is rendered when the command line named no
+	// format. It sees the command and its Result, and applies to values and refusals
+	// alike: the registry does not special-case either. The renderer is shared between
+	// surfaces; what each asks for by default is not, so the surface that owns this
+	// registry sets it. Nil means text.
+	DefaultFormat func(c *Command, r Result) Format
+}
+
+// defaultFormatFor resolves the surface's default for c's Result.
+func (r *Registry) defaultFormatFor(c *Command, res Result) Format {
+	if r.DefaultFormat == nil {
+		return FormatText
+	}
+	return r.DefaultFormat(c, res)
 }
 
 // New returns an empty Registry with only `help` available.
@@ -32,15 +58,111 @@ func (r *Registry) registerHelp() {
 	r.Register(&Command{
 		Name:             Help,
 		ShortDescription: "prints available commands or help <command> for specific usage info",
-		Callback: func(a any, args []string, w StringWriter) error {
-			return r.help(args, w)
+		Run: func(a any, args []string) (Result, error) {
+			if len(args) == 0 {
+				return Value(r.Schema()), nil
+			}
+
+			if c := r.Get(Name(args[0])); c != nil {
+				return Value(r.schemaFor(c)), nil
+			}
+
+			if err := r.unavailable(Name(args[0])); err != nil {
+				return Failed("%s", err), nil
+			}
+
+			return Failed("Command not available %s", args[0]), nil
 		},
+		Text: renderHelp,
 	})
 }
 
-// Register adds a command that can be run by a user. A command registered under a name
-// already held replaces it; see Clone for what that means for Help.
+// renderHelp is help's human rendering: the command list, or one command's usage.
+func renderHelp(v any, w io.Writer) error {
+	switch h := v.(type) {
+	case []CommandSchema:
+		return writeCommandList(h, w)
+	case CommandSchema:
+		return WriteCommandHelp(h, w)
+	default:
+		return fmt.Errorf("expected a command schema, got %T", v)
+	}
+}
+
+func writeCommandList(cmds []CommandSchema, w io.Writer) error {
+	if _, err := io.WriteString(w, "Available commands:\n"); err != nil {
+		return err
+	}
+
+	// In the order the schema came, which is the display order; see Registry.All.
+	lines := make([]string, 0, len(cmds))
+	for _, c := range cmds {
+		lines = append(lines, fmt.Sprintf("%s - %s", c.Name, c.Short))
+	}
+
+	_, err := io.WriteString(w, strings.Join(lines, "\n")+"\n\n")
+	return err
+}
+
+// WriteCommandHelp renders one command's usage. It is the single rendering of a
+// CommandSchema for a person: the daemon uses it for `help <command>` on both surfaces,
+// and a client that already holds the schema uses it for `-h`, so the two cannot drift.
+func WriteCommandHelp(c CommandSchema, w io.Writer) error {
+	if _, err := fmt.Fprintf(w, "%s - %s\n", c.Name, c.Short); err != nil {
+		return err
+	}
+
+	if c.Help != "" {
+		if _, err := fmt.Fprintf(w, "  %s\n", c.Help); err != nil {
+			return err
+		}
+	}
+
+	// Same layout as flag.PrintDefaults, including its habit of leaving out a default
+	// that is the zero value.
+	for _, f := range c.Flags {
+		kind := ""
+		if !f.Bool {
+			kind = " string"
+		}
+		def := ""
+		if f.Default != "" && f.Default != "false" {
+			def = fmt.Sprintf(" (default %s)", f.Default)
+		}
+		if _, err := fmt.Fprintf(w, "  -%s%s\n    \t%s%s\n", f.Name, kind, f.Usage, def); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+// Register adds a command that can be run by a user.
+//
+// It panics if the command declares a reserved flag. That is a programming error, and
+// surfacing it here, at startup, is better than at the moment an operator first types
+// the command.
 func (r *Registry) Register(c *Command) {
+	if c.Run == nil {
+		panic(fmt.Sprintf("commands: %s has no Run", c.Name))
+	}
+
+	if c.Flags != nil {
+		if fl, _ := c.Flags(); fl != nil {
+			for _, name := range reservedFlags {
+				if fl.Lookup(name) != nil {
+					panic(fmt.Sprintf("commands: %s declares reserved flag -%s", c.Name, name))
+				}
+			}
+		}
+	}
+
+	r.insert(c)
+}
+
+// insert adds c without the reserved flag check, for commands that have already passed
+// it on another registry.
+func (r *Registry) insert(c *Command) {
 	r.mu.Lock()
 	defer r.mu.Unlock()
 	r.commands[c.Name] = c
@@ -68,7 +190,8 @@ func (r *Registry) Match(prefix string) []string {
 	return cmds
 }
 
-// All returns every registered command sorted by name, which is the order help lists them.
+// All returns every registered command sorted by name, which is the order help lists
+// them and the schema describes them.
 func (r *Registry) All() []*Command {
 	r.mu.RLock()
 	defer r.mu.RUnlock()
@@ -78,104 +201,228 @@ func (r *Registry) All() []*Command {
 	return cmds
 }
 
-// Clone returns a copy holding the same commands, with help bound to the copy: a
-// command registered on the clone alone is listed and described there, and one the
-// clone withholds is not. That rebinding replaces whatever was registered under Help on
-// the source, built-in or not.
+// Clone returns a copy holding the same commands.
+//
+// help is re-registered on the copy rather than carried over, because the source's help
+// is bound to the source and would list its commands, not the copy's, once a caller
+// registers something on the copy.
 func (r *Registry) Clone() *Registry {
 	r.mu.RLock()
-	c := &Registry{commands: maps.Clone(r.commands)}
+	out := &Registry{
+		commands:      maps.Clone(r.commands),
+		full:          r.full,
+		reason:        r.reason,
+		DefaultFormat: r.DefaultFormat,
+	}
 	r.mu.RUnlock()
-	c.registerHelp()
-	return c
+
+	out.registerHelp()
+	return out
+}
+
+// Filter returns a filtered copy of the registry.
+//
+// Keep should return true for commands that should be included in the copy.
+//
+// reason says, to an operator who names a command the copy left out, why this surface
+// does not serve it; "" leaves the registry to say only that it does not.
+func (r *Registry) Filter(keep func(*Command) bool, reason string) *Registry {
+	out := &Registry{commands: map[Name]*Command{}, full: r, reason: reason, DefaultFormat: r.DefaultFormat}
+
+	for _, cmd := range r.All() {
+		if keep(cmd) {
+			out.insert(cmd)
+		}
+	}
+
+	// Bind help to the filtered set so it only advertises what this surface serves.
+	out.registerHelp()
+	return out
 }
 
 // Dispatch parses a command line and runs it, reporting problems to w. The error is for
-// the caller to log, not to show; see CommandCallback.
-func (r *Registry) Dispatch(line string, w StringWriter) error {
-	// A line shlex cannot split, an unbalanced quote.
+// the caller to log, not to show; see RunFunc and Failure.
+func (r *Registry) Dispatch(line string, w Output) error {
+	// A line shlex cannot split, an unbalanced quote. Reported on Err so the output
+	// stream stays parseable.
 	args, err := shlex.Split(line, true)
 	if err != nil {
-		return Fail(w, "could not parse the command line: "+err.Error())
+		msg := "could not parse the command line: " + err.Error()
+		if _, err := fmt.Fprintln(w.Err, msg); err != nil {
+			return err
+		}
+		return &Failure{Msg: msg}
 	}
 
 	return r.DispatchArgs(args, w)
 }
 
 // DispatchArgs runs an already tokenized command line. The error is as for Dispatch.
-func (r *Registry) DispatchArgs(args []string, w StringWriter) error {
+//
+// An empty line and an unknown command both end up running `help`, the first with the
+// arguments help itself would take; `<command> -h` is answered by the command's own
+// FlagSet, with the same rendering, in execLine.
+func (r *Registry) DispatchArgs(args []string, w Output) error {
 	if len(args) == 0 {
-		r.dumpCommands(w)
-		return nil
+		args = []string{"help"}
 	}
 
 	c := r.Get(Name(args[0]))
 	if c == nil {
-		// Only the first word was looked up, so only it is named.
-		msg := fmt.Sprintf("did not understand: %s", args[0])
-		if err := w.WriteLine(msg); err != nil {
+		if err := r.unavailable(Name(args[0])); err != nil {
+			// Saying why, rather than "did not understand", keeps the rule legible and
+			// points at a surface that will actually run the command.
+			_, _ = fmt.Fprintln(w.Err, err)
 			return err
 		}
 
-		r.dumpCommands(w)
+		// Only the first word was looked up, so only it is named.
+		msg := fmt.Sprintf("did not understand: %s", args[0])
+		if _, err := fmt.Fprintln(w.Out, msg); err != nil {
+			return err
+		}
+
+		if err := r.DispatchArgs([]string{"help"}, w); err != nil {
+			return err
+		}
 		return &Failure{Msg: msg}
 	}
 
-	if checkHelpArgs(args) {
-		// Through the registered help command, so -h and `help <name>` are one path.
-		return execCommand(r.Get(Help), []string{string(c.Name)}, w)
-	}
-
-	return execCommand(c, args[1:], w)
+	return r.execLine(c, args[1:], w)
 }
 
-// dumpCommands writes the list of available commands to w.
-func (r *Registry) dumpCommands(w StringWriter) {
-	err := w.WriteLine("Available commands:")
-	if err != nil {
-		return
-	}
-
-	// In the order All gives, which is the display order.
-	cmds := make([]string, 0)
-	for _, l := range r.All() {
-		cmds = append(cmds, fmt.Sprintf("%s - %s", l.Name, l.ShortDescription))
-	}
-
-	_ = w.Write(strings.Join(cmds, "\n") + "\n\n")
-}
-
-// help writes the command list, or usage for the single command named in a.
-func (r *Registry) help(a []string, w StringWriter) error {
-	// Just typed help
-	if len(a) == 0 {
-		r.dumpCommands(w)
+// unavailable explains why name cannot be run here when this registry was filtered from
+// one that holds it: the command exists, and this surface withholds it, for the reason
+// whoever built the filter gave. It is nil when name is simply unknown, so the two cases
+// read differently to an operator.
+func (r *Registry) unavailable(name Name) error {
+	if r.full == nil {
 		return nil
 	}
 
-	// We are printing a specific commands help text
-	cmd := r.Get(Name(a[0]))
-	if cmd == nil {
-		return Fail(w, "Command not available "+a[0])
+	c := r.full.Get(name)
+	if c == nil {
+		return nil
 	}
 
-	if err := w.WriteLine(fmt.Sprintf("%s - %s", cmd.Name, cmd.ShortDescription)); err != nil {
-		return err
+	reason := r.reason
+	if reason == "" {
+		reason = "this surface does not serve it"
 	}
 
-	if cmd.Help != "" {
-		if err := w.WriteLine(fmt.Sprintf("  %s", cmd.Help)); err != nil {
-			return err
+	return fmt.Errorf("%s is not available on this surface: %s", c.Name, reason)
+}
+
+// Resolve returns the command registered under name, or says why there is none: the
+// filter's reason for one this surface withholds, or unknown for a name that does not
+// exist.
+func (r *Registry) Resolve(name Name) (*Command, error) {
+	if c := r.Get(name); c != nil {
+		return c, nil
+	}
+
+	if err := r.unavailable(name); err != nil {
+		return nil, err
+	}
+
+	return nil, fmt.Errorf("unknown command %q", name)
+}
+
+// setParams sets each named parameter on fl, in the order of names, and stops at the
+// first that is unknown or does not parse. own says whether the command declared flags
+// of its own, which decides between "takes no parameters" and "has no parameter".
+//
+// The presentation flags are on fl, since commandFlags put them there, but they are not
+// parameters: a request has one place to choose its rendering, the envelope's format
+// field, and a client that sends them here is told where that is rather than having a
+// second, weaker way to say the same thing.
+func setParams(fl *flag.FlagSet, name string, names []string, params map[string]string, own bool) error {
+	for _, k := range names {
+		if isPresentationFlag(k) {
+			return fmt.Errorf("%s is a command line flag, not a parameter; choose the rendering with the request's format field", k)
 		}
-	}
 
-	if cmd.Flags != nil {
-		fs, _ := cmd.Flags()
-		if fs != nil {
-			fs.SetOutput(w.GetWriter())
-			fs.PrintDefaults()
+		f := fl.Lookup(k)
+		if f == nil {
+			if !own {
+				return fmt.Errorf("%s takes no parameters", name)
+			}
+			return fmt.Errorf("%s has no parameter %q", name, k)
+		}
+
+		v := params[k]
+		if err := fl.Set(k, v); err != nil {
+			if IsBoolFlag(f) {
+				// The flag package's own words for this are "parse error".
+				return fmt.Errorf("%s: expected a boolean, got %q", k, v)
+			}
+			return fmt.Errorf("%s: %w", k, err)
 		}
 	}
 
 	return nil
 }
+
+// Schema describes every command in the registry, so a client can offer them without
+// having been built against this daemon.
+func (r *Registry) Schema() []CommandSchema {
+	all := r.All()
+	out := make([]CommandSchema, 0, len(all))
+	for _, c := range all {
+		out = append(out, r.schemaFor(c))
+	}
+	return out
+}
+
+// schemaFor describes one command. It is the one derivation, for the whole schema and
+// for `help <command>` alike.
+func (r *Registry) schemaFor(c *Command) CommandSchema {
+	fl, _, _ := commandFlags(c)
+	return schemaOf(c, fl)
+}
+
+// schemaOf describes c as driven through fl, the FlagSet commandFlags built for it. The
+// presentation flags are included because a command line client parses its line
+// against this and help renders from it; they are listed after the command's own,
+// which is where readers expect them. Invoke refuses them as parameters.
+func schemaOf(c *Command, fl *flag.FlagSet) CommandSchema {
+	cs := CommandSchema{Name: c.Name, Short: c.ShortDescription, Help: c.Help}
+
+	var own, shared []FlagSchema
+	fl.VisitAll(func(f *flag.Flag) {
+		fs := FlagSchema{
+			Name:    f.Name,
+			Bool:    IsBoolFlag(f),
+			Default: f.DefValue,
+			Usage:   f.Usage,
+		}
+		if isPresentationFlag(f.Name) {
+			shared = append(shared, fs)
+			return
+		}
+		own = append(own, fs)
+	})
+	cs.Flags = append(own, shared...)
+
+	return cs
+}
+
+// CommandSchema describes one command well enough for a client to offer it.
+type CommandSchema struct {
+	Name  Name         `json:"name"`
+	Short string       `json:"short,omitempty"`
+	Help  string       `json:"help,omitempty"`
+	Flags []FlagSchema `json:"flags,omitempty"`
+}
+
+// FlagSchema describes one of a command's named parameters.
+type FlagSchema struct {
+	Name string `json:"name"`
+
+	// Bool reports a parameter that takes no value, which is what tells a command line
+	// client whether `-pretty` swallows the word after it.
+	Bool bool `json:"bool,omitempty"`
+
+	Default string `json:"default,omitempty"`
+	Usage   string `json:"usage,omitempty"`
+}

commands/registry_test.go

@@ -4,173 +4,417 @@ import (
 	"bytes"
 	"errors"
 	"flag"
+	"io"
 	"strings"
+	"sync"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+// notWithheld is a stand in for a surface's own policy, which the registry knows nothing
+// of; see Command.Identity for the one the info API applies.
+func notWithheld(c *Command) bool { return c.Name != "withheld" }
+
+const testReason = "this surface does not run commands that write"
+
+// textLine is a Text renderer for test commands whose result is one string.
+func textLine(v any, w io.Writer) error {
+	_, err := io.WriteString(w, v.(string)+"\n")
+	return err
+}
+
 func testRegistry() *Registry {
 	r := New()
 	r.Register(&Command{
 		Name:             "served",
 		ShortDescription: "a command this surface serves",
-		Callback: func(fs any, a []string, w StringWriter) error {
-			return w.WriteLine("read " + strings.Join(a, ","))
+		Run: func(fs any, a []string) (Result, error) {
+			return Value("read " + strings.Join(a, ",")), nil
 		},
+		Text: textLine,
 	})
 	r.Register(&Command{
 		Name:             "withheld",
 		ShortDescription: "a command this surface withholds",
-		Callback: func(fs any, a []string, w StringWriter) error {
-			return w.WriteLine("wrote")
+		Run: func(fs any, a []string) (Result, error) {
+			return Value("wrote"), nil
 		},
+		Text: textLine,
 	})
 	return r
 }
 
-// TestFlagUsageGoesToTheWriter pins that a parse failure's usage lands on the command's
-// writer.
-func TestFlagUsageGoesToTheWriter(t *testing.T) {
+// TestFilterKeepsWhatKeepSays: the copy holds the commands keep accepted, and help.
+func TestFilterKeepsWhatKeepSays(t *testing.T) {
+	ro := testRegistry().Filter(notWithheld, testReason)
+
+	names := make([]string, 0)
+	for _, c := range ro.All() {
+		names = append(names, string(c.Name))
+	}
+
+	assert.ElementsMatch(t, []string{"help", "served"}, names)
+}
+
+// TestRefusesWithheldByName covers a client that knows a command exists and asks for it
+// anyway. The filter lives on the serving side, so the answer must not depend on what
+// the client was told was available.
+func TestRefusesWithheldByName(t *testing.T) {
+	out, errOut := &bytes.Buffer{}, &bytes.Buffer{}
+	w := Output{Out: out, Err: errOut}
+
+	err := testRegistry().Filter(notWithheld, testReason).DispatchArgs([]string{"withheld"}, w)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "withheld is not available on this surface")
+	assert.Empty(t, out.String(), "a withheld command must produce no output")
+	assert.Contains(t, errOut.String(), "withheld is not available on this surface")
+	assert.Contains(t, errOut.String(), testReason,
+		"the registry should render the explanation the filter supplied, not one of its own")
+}
+
+// TestWithheldIsNotReportedAsUnknown keeps the two cases distinct: an operator who
+// mistypes and one who asked for a real command should not get the same answer.
+func TestWithheldIsNotReportedAsUnknown(t *testing.T) {
+	out, errOut := &bytes.Buffer{}, &bytes.Buffer{}
+	err := testRegistry().Filter(notWithheld, testReason).DispatchArgs([]string{"no-such-command"}, Output{Out: out, Err: errOut})
+
+	var f *Failure
+	require.ErrorAs(t, err, &f, "an unknown command is a failure to the surface")
+	assert.Contains(t, out.String(), "did not understand: no-such-command")
+}
+
+// paramRegistry holds one command with a bool flag and a duration flag, and reports what
+// the command saw when it ran.
+func paramRegistry(t *testing.T) (*Registry, *[]paramsSeen) {
+	t.Helper()
+
+	var seen []paramsSeen
 	r := New()
 	r.Register(&Command{
-		Name:             "emit",
-		ShortDescription: "emits",
+		Name:             "set",
+		ShortDescription: "sets things",
 		Flags: func() (*flag.FlagSet, any) {
 			fl := flag.NewFlagSet("", flag.ContinueOnError)
-			fl.Bool("json", false, "emit json")
-			return fl, nil
+			s := paramsSeen{}
+			fl.BoolVar(&s.Verbose, "verbose", false, "say more")
+			fl.DurationVar(&s.Wait, "wait", 0, "how long")
+			return fl, &s
+		},
+		Run: func(fs any, a []string) (Result, error) {
+			seen = append(seen, *fs.(*paramsSeen))
+			return Value("ok"), nil
 		},
-		Callback: func(fs any, a []string, w StringWriter) error { return nil },
 	})
 
-	merged := &bytes.Buffer{}
-	require.Error(t, r.DispatchArgs([]string{"emit", "-nope"}, NewStringWriter(merged)))
-	assert.Contains(t, merged.String(), "flag provided but not defined")
+	return r, &seen
 }
 
-// TestDispatchReportsUnparseableLine: a line shlex rejects, an unbalanced quote, is a
-// failure the user is told about.
+type paramsSeen struct {
+	Verbose bool
+	Wait    time.Duration
+}
+
+// TestInvokeReportsParametersInNameOrder: parameters arrive as an unordered object, so
+// with two bad ones the error must not depend on map iteration. Name order decides.
+func TestInvokeReportsParametersInNameOrder(t *testing.T) {
+	r, _ := paramRegistry(t)
+
+	cases := map[string]struct {
+		params map[string]string
+		want   string
+	}{
+		"two unknown":               {map[string]string{"zed": "1", "alpha": "1"}, `set has no parameter "alpha"`},
+		"unknown before bad bool":   {map[string]string{"aaa": "1", "verbose": "maybe"}, `set has no parameter "aaa"`},
+		"bad bool before unknown":   {map[string]string{"verbose": "maybe", "zzz": "1"}, `verbose: expected a boolean, got "maybe"`},
+		"bad value before bad bool": {map[string]string{"wait": "soon", "verbose": "maybe"}, `verbose: expected a boolean`},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			// Enough runs that a map-order dependence would show.
+			for i := 0; i < 50; i++ {
+				_, err := Invoke(Request{Command: r.Get("set"), Params: tc.params})
+				require.Error(t, err)
+				assert.ErrorContains(t, err, tc.want)
+			}
+		})
+	}
+}
+
+// TestInvokeWithABadParameterSetsNothing: a request with any bad parameter must not run
+// the command, and must leave nothing behind for the next request to see. The good
+// parameter sorts before the bad one, so a one-pass Invoke would have applied it.
+func TestInvokeWithABadParameterSetsNothing(t *testing.T) {
+	r, seen := paramRegistry(t)
+
+	_, err := Invoke(Request{Command: r.Get("set"), Params: map[string]string{"verbose": "true", "wait": "soon"}})
+	require.ErrorContains(t, err, "wait:")
+	assert.Empty(t, *seen, "the command must not run with a partial parameter set")
+
+	v, err := Invoke(Request{Command: r.Get("set")})
+	require.NoError(t, err)
+	assert.Equal(t, "ok", v.Value)
+	require.Len(t, *seen, 1)
+	assert.Equal(t, paramsSeen{}, (*seen)[0], "nothing from the failed request carried over")
+
+	_, err = Invoke(Request{Command: r.Get("set"), Params: map[string]string{"verbose": "true", "wait": "2s"}})
+	require.NoError(t, err)
+	require.Len(t, *seen, 2)
+	assert.Equal(t, paramsSeen{Verbose: true, Wait: 2 * time.Second}, (*seen)[1])
+}
+
+func TestHelpListsOnlyServedCommands(t *testing.T) {
+	out := &bytes.Buffer{}
+	require.NoError(t, testRegistry().Filter(notWithheld, testReason).DispatchArgs(nil, MergedOutput(out)))
+
+	assert.Contains(t, out.String(), "served")
+	assert.NotContains(t, out.String(), "withheld")
+}
+
+// TestDispatchReportsUnparseableLine pins that a line shlex rejects says so on Err rather
+// than coming back silent, so an operator with an unbalanced quote learns the quote is the
+// problem instead of assuming the command is broken.
 func TestDispatchReportsUnparseableLine(t *testing.T) {
-	out := &bytes.Buffer{}
-	err := testRegistry().Dispatch(`served "unterminated`, NewStringWriter(out))
-
-	var f *Failure
-	require.ErrorAs(t, err, &f)
-	assert.Equal(t, f.Msg+"\n", out.String())
-	assert.Contains(t, out.String(), "could not parse the command line: ")
+	out, errOut := &bytes.Buffer{}, &bytes.Buffer{}
+	testRegistry().Dispatch(`served "unterminated`, Output{Out: out, Err: errOut})
+	assert.Empty(t, out.String())
+	assert.Contains(t, errOut.String(), "could not parse the command line")
 }
 
-// TestFailTellsTheUserAndTheSurface: Fail writes the message and returns it as a Failure;
-// when the write fails, the user heard nothing, so the write error is what comes back.
-func TestFailTellsTheUserAndTheSurface(t *testing.T) {
-	out := &bytes.Buffer{}
-	err := Fail(NewStringWriter(out), "no such tunnel")
-	var f *Failure
-	require.ErrorAs(t, err, &f)
-	assert.Equal(t, "no such tunnel", f.Msg)
-	assert.Equal(t, "no such tunnel\n", out.String())
-
-	err = Fail(NewStringWriter(failingWriter{}), "no such tunnel")
-	require.EqualError(t, err, "gone")
-	assert.NotErrorAs(t, err, &f)
+// TestRegisterRejectsReservedFlags: a command that declares a reserved flag is refused
+// at Register rather than when an operator first runs it; see reservedFlags.
+func TestRegisterRejectsReservedFlags(t *testing.T) {
+	for _, name := range []string{"json", "pretty", "h", "help", "config", "socket"} {
+		t.Run(name, func(t *testing.T) {
+			r := New()
+			assert.PanicsWithValue(t, "commands: clash declares reserved flag -"+name, func() {
+				r.Register(&Command{
+					Name: "clash",
+					Flags: func() (*flag.FlagSet, any) {
+						fl := flag.NewFlagSet("clash", flag.ContinueOnError)
+						fl.Bool(name, false, "")
+						return fl, nil
+					},
+					Run: func(fs any, a []string) (Result, error) { return Nothing(), nil },
+				})
+			})
+		})
+	}
 }
 
-type failingWriter struct{}
+// TestRegistryIsSafeForConcurrentUse runs the operations the surfaces perform, filtering
+// and cloning per request or session, against a stream of registrations. It exists to
+// be run under the race detector: the map underneath is not goroutine safe.
+func TestRegistryIsSafeForConcurrentUse(t *testing.T) {
+	r := testRegistry()
 
-func (failingWriter) Write([]byte) (int, error) { return 0, errors.New("gone") }
-
-// TestAllSortsByName: registration order does not matter.
-func TestAllSortsByName(t *testing.T) {
-	r := New()
-	for _, n := range []Name{"zz", Version, "aa", ChangeRemote} {
-		r.Register(&Command{Name: n})
+	var wg sync.WaitGroup
+	for i := 0; i < 8; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			for j := 0; j < 50; j++ {
+				r.Register(&Command{
+					Name:             Name("late-" + strings.Repeat("a", i) + "-" + strings.Repeat("b", j)),
+					ShortDescription: "registered after the surfaces started",
+					Run:              func(fs any, a []string) (Result, error) { return Nothing(), nil },
+				})
+			}
+		}(i)
 	}
 
-	var got []Name
-	for _, c := range r.All() {
-		got = append(got, c.Name)
+	for i := 0; i < 8; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for j := 0; j < 50; j++ {
+				f := r.Filter(notWithheld, testReason)
+				_ = f.Get("served")
+				_ = f.Schema()
+				_ = r.Clone().Match("late-")
+			}
+		}()
 	}
-	assert.Equal(t, []Name{"aa", ChangeRemote, Help, Version, "zz"}, got)
+
+	wg.Wait()
+
+	f := r.Filter(notWithheld, testReason)
+	assert.Len(t, f.Match("late-"), 8*50, "every late registration is visible once the writers finish")
 }
 
-// TestHelpFlagAgreesWithHelpCommand pins that -h answers through the registered help
-// command, and that help on a clone describes the clone: a command registered only there
-// is known to both -h and `help <name>`, and one withheld from it is known to neither.
-func TestHelpFlagAgreesWithHelpCommand(t *testing.T) {
+// TestCloneHelpSeesCloneCommands: the ssh session registers logout on its clone, and
+// the source does not learn of it. Help on the clone answers for the clone, and -h on a
+// command prints what `help <name>` does, on a clone as on the source: both render
+// through WriteCommandHelp, and Clone rebinds help to the copy.
+func TestCloneHelpSeesCloneCommands(t *testing.T) {
 	r := testRegistry()
 	c := r.Clone()
 	c.Register(&Command{
 		Name:             Logout,
 		ShortDescription: "ends the session",
-		Callback:         func(fs any, a []string, w StringWriter) error { return nil },
+		Run:              func(fs any, a []string) (Result, error) { return Nothing(), nil },
 	})
-
-	flag := &bytes.Buffer{}
-	c.Dispatch("logout -h", NewStringWriter(flag))
-	cmd := &bytes.Buffer{}
-	c.Dispatch("help logout", NewStringWriter(cmd))
-	assert.Equal(t, cmd.String(), flag.String())
-	assert.Equal(t, "logout - ends the session\n", flag.String())
-
-	// The source never learns of it.
-	src := &bytes.Buffer{}
-	r.Dispatch("help logout", NewStringWriter(src))
-	assert.Equal(t, "Command not available logout\n", src.String())
-
-	// And a command both hold is described either way.
-	flag.Reset()
-	c.Dispatch("served -h", NewStringWriter(flag))
-	assert.Equal(t, "served - a command this surface serves\n", flag.String())
-}
-
-// TestCloneHelpListsTheClone: the command list help prints is the clone's, so a clone
-// that withholds a command does not name it, and the source still does.
-func TestCloneHelpListsTheClone(t *testing.T) {
-	r := testRegistry()
-	c := New()
-	c.Register(r.Get("served"))
-	c = c.Clone()
+	assert.Nil(t, r.Get(Logout), "the source is not affected")
 
 	out := &bytes.Buffer{}
-	c.Dispatch("help", NewStringWriter(out))
-	assert.Equal(t, "Available commands:\n"+
-		"help - prints available commands or help <command> for specific usage info\n"+
-		"served - a command this surface serves\n\n", out.String())
+	require.NoError(t, c.DispatchArgs([]string{"help"}, MergedOutput(out)))
+	assert.Contains(t, out.String(), "logout")
 
-	out.Reset()
-	c.Dispatch("help withheld", NewStringWriter(out))
-	assert.Equal(t, "Command not available withheld\n", out.String())
-
-	out.Reset()
-	r.Dispatch("help withheld", NewStringWriter(out))
-	assert.Equal(t, "withheld - a command this surface withholds\n", out.String())
+	for _, name := range []string{"logout", "served"} {
+		flag := &bytes.Buffer{}
+		c.Dispatch(name+" -h", MergedOutput(flag))
+		cmd := &bytes.Buffer{}
+		c.Dispatch("help "+name, MergedOutput(cmd))
+		assert.Equal(t, cmd.String(), flag.String(), name)
+		assert.Contains(t, flag.String(), name+" - ", name)
+	}
 }
 
-// TestDispatchReturnsTheCommandsError: what the callback returns reaches the caller, so
-// a surface can log it.
-func TestDispatchReturnsTheCommandsError(t *testing.T) {
+// TestThenRunsAfterTheReply pins the order Then exists for: Run's line is on the writer
+// before the action begins, Then's Result follows it, and Then's outcome is the
+// command's.
+func TestThenRunsAfterTheReply(t *testing.T) {
+	out := &bytes.Buffer{}
+	seen := "never ran"
+	outcome := Did("acted")
+
 	r := New()
 	r.Register(&Command{
-		Name: "fails",
-		Callback: func(fs any, a []string, w StringWriter) error {
-			return errors.New("boom")
+		Name:             "act",
+		ShortDescription: "replies, then acts",
+		Run: func(fs any, a []string) (Result, error) {
+			return Did("about to act"), nil
+		},
+		Then: func() Result {
+			seen = out.String()
+			return outcome
 		},
 	})
 
-	out := &bytes.Buffer{}
-	require.EqualError(t, r.Dispatch("fails", NewStringWriter(out)), "boom")
-	assert.Empty(t, out.String(), "a command's own error is not shown; the command speaks for itself")
+	require.NoError(t, r.DispatchArgs([]string{"act"}, MergedOutput(out)))
+	assert.Equal(t, "about to act\n", seen, "the line was written before the action")
+	assert.Equal(t, "about to act\nacted\n", out.String())
+
+	out.Reset()
+	outcome = Failed("could not act")
+	var f *Failure
+	require.ErrorAs(t, r.DispatchArgs([]string{"act"}, MergedOutput(out)), &f)
+	assert.Equal(t, "about to act\ncould not act\n", out.String())
+
+	// In JSON only the outcome travels: one document for the program that asked.
+	out.Reset()
+	require.ErrorAs(t, r.DispatchArgs([]string{"act", "-json"}, MergedOutput(out)), &f)
+	assert.JSONEq(t, `{"ok":false,"reason":"could not act"}`, out.String())
+
+	// A Run that refused is the outcome; the action does not happen.
+	ran := false
+	r.Register(&Command{
+		Name:             "refuse",
+		ShortDescription: "refuses, so never acts",
+		Run: func(fs any, a []string) (Result, error) {
+			return Failed("not like that"), nil
+		},
+		Then: func() Result {
+			ran = true
+			return Did("acted anyway")
+		},
+	})
+	out.Reset()
+	require.ErrorAs(t, r.DispatchArgs([]string{"refuse"}, MergedOutput(out)), &f)
+	assert.Equal(t, "not like that\n", out.String())
+	assert.False(t, ran)
+	out.Reset()
+	require.ErrorAs(t, r.DispatchArgs([]string{"refuse", "-json"}, MergedOutput(out)), &f)
+	assert.JSONEq(t, `{"ok":false,"reason":"not like that"}`, out.String())
+	assert.False(t, ran)
 }
 
-// TestUnknownCommandNamesTheWord: only the first word is looked up, so only it is
-// reported, followed by the command list, and the line is a failure to the surface.
-func TestUnknownCommandNamesTheWord(t *testing.T) {
+// TestWriteCommandHelpIsTheOneRendering pins the layout both `help <command>` on the
+// daemon and `-h` in the client print, so the two cannot drift apart again.
+func TestWriteCommandHelpIsTheOneRendering(t *testing.T) {
+	cs := CommandSchema{
+		Name:  "change-remote",
+		Short: "changes a remote",
+		Help:  "Give it an address.",
+		Flags: []FlagSchema{
+			{Name: "address", Usage: "the new remote", Default: "1.2.3.4:4242"},
+			{Name: "by-index", Bool: true, Usage: "match by index"},
+			{Name: "json", Bool: true, Usage: "outputs as json"},
+		},
+	}
+
 	out := &bytes.Buffer{}
-	err := testRegistry().Dispatch(`nope "with args"`, NewStringWriter(out))
-	var f *Failure
-	require.ErrorAs(t, err, &f)
-	assert.Equal(t, "did not understand: nope", f.Msg)
-	assert.True(t, strings.HasPrefix(out.String(), "did not understand: nope\nAvailable commands:\n"), out.String())
+	require.NoError(t, WriteCommandHelp(cs, out))
+
+	assert.Equal(t, "change-remote - changes a remote\n"+
+		"  Give it an address.\n"+
+		"  -address string\n    \tthe new remote (default 1.2.3.4:4242)\n"+
+		"  -by-index\n    \tmatch by index\n"+
+		"  -json\n    \toutputs as json\n", out.String())
+}
+
+// addrFlags is the flag struct of a test command that takes -address.
+type addrFlags struct{ Address string }
+
+// TestHelpFlagIsReadWhereParseReadsIt: -h asks for help only where the line's parser
+// reads it as a flag. As a flag's value or after -- it reaches the command as the
+// operand it is, and help is rendered by the same function `help <command>` uses.
+func TestHelpFlagIsReadWhereParseReadsIt(t *testing.T) {
+	var seen []string
+	r := New()
+	r.Register(Flagged(&Command{Name: "x", ShortDescription: "takes an address"},
+		func(fl *flag.FlagSet, s *addrFlags) { fl.StringVar(&s.Address, "address", "", "an address") },
+		func(s *addrFlags, a []string) (Result, error) {
+			seen = append(seen, s.Address+"|"+strings.Join(a, ","))
+			return Nothing(), nil
+		}))
+
+	for _, line := range []string{"x -h", "x -help", "x --help", "x 10.0.0.1 -h", "x -address=1 -h"} {
+		out := &bytes.Buffer{}
+		r.Dispatch(line, MergedOutput(out))
+		assert.Contains(t, out.String(), "x - takes an address\n", line)
+		assert.Contains(t, out.String(), "-address string", line)
+	}
+	assert.Empty(t, seen, "asking for help must not run the command")
+
+	for _, line := range []string{"x -address -h", "x -- -h"} {
+		out := &bytes.Buffer{}
+		r.Dispatch(line, MergedOutput(out))
+		assert.Empty(t, out.String(), line)
+	}
+	assert.Equal(t, []string{"-h|", "|-h"}, seen)
+}
+
+// TestBadFlagIsAnsweredWithTheCommandsHelp: a flag the command does not have is reported
+// on the error stream followed by the command's usage, in the rendering help uses.
+func TestBadFlagIsAnsweredWithTheCommandsHelp(t *testing.T) {
+	out, errOut := &bytes.Buffer{}, &bytes.Buffer{}
+	r := New()
+	r.Register(&Command{Name: "x", ShortDescription: "has no flags", Run: func(fs any, a []string) (Result, error) { return Nothing(), nil }})
+
+	require.Error(t, r.DispatchArgs([]string{"x", "-nope"}, Output{Out: out, Err: errOut}))
+	assert.Empty(t, out.String())
+	assert.Equal(t, "flag provided but not defined: -nope\nx - has no flags\n  -json\n    \toutputs as json\n  -pretty\n    \tpretty prints json, assumes -json\n", errOut.String())
+}
+
+// TestRunErrorIsShown: an error Run returns reaches the person at the prompt on the
+// error stream, as well as the surface that logs it.
+func TestRunErrorIsShown(t *testing.T) {
+	r := New()
+	r.Register(&Command{
+		Name:             "broken",
+		ShortDescription: "cannot do what it is asked",
+		Run: func(fs any, a []string) (Result, error) {
+			return Result{}, errors.New("no certificate is loaded")
+		},
+	})
+
+	out, errOut := &bytes.Buffer{}, &bytes.Buffer{}
+	require.EqualError(t, r.DispatchArgs([]string{"broken"}, Output{Out: out, Err: errOut}), "no certificate is loaded")
+	assert.Empty(t, out.String())
+	assert.Equal(t, "no certificate is loaded\n", errOut.String())
 }

commands/render.go

@@ -0,0 +1,179 @@
+package commands
+
+import (
+	"encoding/json"
+	"errors"
+	"fmt"
+	"io"
+)
+
+// ErrNoTextRendering is returned by Render when a command's Value has no human rendering
+// and text was asked for anyway. A command declares this by leaving Text nil and saying
+// so with JSONOnly; it is not a fallback situation but a statement that no such form
+// exists yet, so nobody builds a script around an interim one. Each surface's default
+// policy avoids ever asking.
+var ErrNoTextRendering = errors.New("no human readable output yet; pass -json or -pretty")
+
+// Format is how a result should be presented.
+type Format int
+
+const (
+	// FormatUnset is a request that expressed no preference. A surface resolves it with
+	// its own default before rendering; Render itself treats it as FormatText.
+	FormatUnset Format = iota
+
+	// FormatText renders for a person. It is what an operator gets by default on every
+	// surface with a human at the other end.
+	FormatText
+
+	// FormatJSON renders the result as compact JSON.
+	FormatJSON
+
+	// FormatPrettyJSON renders the result as indented JSON.
+	FormatPrettyJSON
+)
+
+// Flag names for the two presentation flags. They are offered on every command rather
+// than declared by each one, because how a result is displayed is not a property of the
+// command that produced it.
+const (
+	FlagJSON   = "json"
+	FlagPretty = "pretty"
+)
+
+// Flag names nebula ctl takes for itself, anywhere on its line, to say which daemon to
+// talk to; see reservedFlags.
+const (
+	FlagConfig = "config"
+	FlagSocket = "socket"
+)
+
+// reservedFlags are handled before a command's Run is called, so a command may not
+// declare them itself. -h and -help are intercepted as a request for usage; -json and
+// -pretty are added to every command's FlagSet by commandFlags, on every surface, so a
+// command that defined either would make that definition panic with "flag redefined"
+// on whichever goroutine ran the command; and -config and -socket are nebula ctl's own,
+// taken off the line before the client knows which command it names, so a command
+// that declared either would never receive it there.
+var reservedFlags = []string{"h", "help", FlagJSON, FlagPretty, FlagConfig, FlagSocket}
+
+// Render writes r in the requested format. Every surface calls it, so a hostmap looks
+// the same over ssh as it does through the control socket, and a change to either
+// rendering happens once.
+func Render(c *Command, r Result, f Format, w io.Writer) error {
+	switch f {
+	case FormatJSON:
+		return renderJSON(r.Payload(), w, false)
+
+	case FormatPrettyJSON:
+		return renderJSON(r.Payload(), w, true)
+
+	default:
+		// An outcome reads the same for every command: its reason, as a sentence. Only a
+		// Value needs the command's own rendering, so Text never sees anything else.
+		if r.Value == nil {
+			_, err := io.WriteString(w, r.Reason+"\n")
+			return err
+		}
+
+		if c != nil && c.Text != nil {
+			return c.Text(r.Value, w)
+		}
+
+		name := "this command"
+		if c != nil {
+			name = string(c.Name)
+		}
+		return fmt.Errorf("%s has %w", name, ErrNoTextRendering)
+	}
+}
+
+func renderJSON(v any, w io.Writer, indent bool) error {
+	enc := json.NewEncoder(w)
+	if indent {
+		enc.SetIndent("", "    ")
+	}
+
+	return enc.Encode(v)
+}
+
+// Result is what every Run returns: whether the command worked, a sentence saying so,
+// and, for a command that reports rather than acts, the value it produced.
+//
+// Value and Reason are alternatives. A command that reports returns Value(v), and the
+// surfaces render v, as JSON or through the command's Text. A command that acts, or any
+// command refusing its arguments, returns Did or Failed, and the surfaces render the
+// Outcome: its Reason is the sentence these commands have always printed. Nothing is for
+// a command with nothing to say.
+//
+// The envelope itself never reaches a caller. Payload is what JSON carries, and Render
+// hands Text only the Value, so a command's renderer sees the one type it returns on
+// success and is never asked to recognise an Outcome.
+type Result struct {
+	Outcome
+	Value any
+}
+
+// Outcome is the part of a Result that says whether the command worked and, in words a
+// caller can show someone, why. It is what a program receives from a command that acts
+// rather than reports: closing a tunnel, changing a remote, reloading config.
+//
+// One shape serves all of them. Designing a payload per outcome would be work without a
+// reader. Reason is set either way, so a success says what it did rather than leaving
+// the caller to infer it from a bare true.
+type Outcome struct {
+	OK     bool   `json:"ok"`
+	Reason string `json:"reason"`
+}
+
+// Value is the Result of a command that reports: v is what it has to show, and it must
+// marshal to JSON.
+func Value(v any) Result {
+	return Result{Outcome: Outcome{OK: true}, Value: v}
+}
+
+// Failed is a convenience for the common "it did not work, here is why" case.
+func Failed(format string, args ...any) Result {
+	return Result{Outcome: Outcome{Reason: fmt.Sprintf(format, args...)}}
+}
+
+// Did is a convenience for the common "it worked, here is what happened" case.
+func Did(format string, args ...any) Result {
+	return Result{Outcome: Outcome{OK: true, Reason: fmt.Sprintf(format, args...)}}
+}
+
+// Nothing is the Result of a command with nothing to say: it did its work, and the
+// surface sends an empty, successful reply without rendering anything.
+func Nothing() Result {
+	return Result{Outcome: Outcome{OK: true}}
+}
+
+// Empty reports a Result with nothing to render: no Value and no Reason. The zero Result
+// counts, so a Run that returns Result{} says nothing rather than failing silently.
+func (r Result) Empty() bool {
+	return r.Value == nil && r.Reason == ""
+}
+
+// Payload is what a program receives: the Value when there is one, otherwise the
+// Outcome. No surface sends the envelope itself, so print-tunnel -json emits the tunnel
+// and close-tunnel -json emits {ok, reason}.
+func (r Result) Payload() any {
+	if r.Value != nil {
+		return r.Value
+	}
+	return r.Outcome
+}
+
+// Typed adapts a renderer for one result type to the TextFunc a Command declares. The
+// value Render passes is the command's own, see Result, so all that is left is to name
+// its type once here instead of asserting it at the top of every renderer. A renderer
+// paired with a command that returns something else fails here, saying what arrived.
+func Typed[T any](render func(T, io.Writer) error) TextFunc {
+	return func(v any, w io.Writer) error {
+		t, ok := v.(T)
+		if !ok {
+			return fmt.Errorf("expected %T, got %T", t, v)
+		}
+		return render(t, w)
+	}
+}

commands/render_test.go

@@ -0,0 +1,105 @@
+package commands
+
+import (
+	"bytes"
+	"io"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestRenderPrintsAnOutcomeWithoutText: a command that acts declares no Text. Its
+// outcome renders as the reason, the same for every command, and as {ok, reason} in
+// JSON, which is the shape these commands have always emitted.
+func TestRenderPrintsAnOutcomeWithoutText(t *testing.T) {
+	c := &Command{Name: "act"}
+
+	out := &bytes.Buffer{}
+	require.NoError(t, Render(c, Did("Closed"), FormatText, out))
+	assert.Equal(t, "Closed\n", out.String())
+
+	out.Reset()
+	require.NoError(t, Render(c, Failed("No vpn address was provided"), FormatText, out))
+	assert.Equal(t, "No vpn address was provided\n", out.String())
+
+	out.Reset()
+	require.NoError(t, Render(c, Failed("nope"), FormatJSON, out))
+	assert.JSONEq(t, `{"ok":false,"reason":"nope"}`, out.String())
+}
+
+// TestRenderHandsTextOnlyTheValue pins the guarantee Result exists for: a command's
+// renderer sees the type it returns on success and never the envelope or an outcome, so
+// a report command that refuses its arguments needs nothing in its Text to cope.
+func TestRenderHandsTextOnlyTheValue(t *testing.T) {
+	var got any
+	c := &Command{
+		Name: "report",
+		Text: func(v any, w io.Writer) error {
+			got = v
+			_, err := io.WriteString(w, "rendered\n")
+			return err
+		},
+	}
+
+	out := &bytes.Buffer{}
+	require.NoError(t, Render(c, Value("data"), FormatText, out))
+	assert.Equal(t, "data", got)
+	assert.Equal(t, "rendered\n", out.String())
+
+	got, out = nil, &bytes.Buffer{}
+	require.NoError(t, Render(c, Failed("refused"), FormatText, out))
+	assert.Nil(t, got, "Text must not be called for an outcome")
+	assert.Equal(t, "refused\n", out.String())
+
+	out.Reset()
+	require.NoError(t, Render(c, Value("data"), FormatJSON, out))
+	assert.JSONEq(t, `"data"`, out.String(), "JSON carries the value, never the envelope")
+}
+
+// TestRenderRefusesTextForAValueWithoutText: leaving Text nil is a statement that no
+// human form of the value exists yet, and asking for one is an error that says so.
+func TestRenderRefusesTextForAValueWithoutText(t *testing.T) {
+	out := &bytes.Buffer{}
+	err := Render(&Command{Name: "print-tunnel"}, Value(map[string]int{"index": 7}), FormatText, out)
+	require.ErrorIs(t, err, ErrNoTextRendering)
+	require.ErrorContains(t, err, "print-tunnel has no human readable output yet")
+	assert.Empty(t, out.String())
+}
+
+// TestTypedNamesTheTypeOnce: Typed replaces the assertion at the top of every renderer.
+// The right type reaches the renderer; the wrong one is reported, naming both.
+func TestTypedNamesTheTypeOnce(t *testing.T) {
+	type info struct{ Name string }
+	render := Typed(func(i info, w io.Writer) error {
+		_, err := io.WriteString(w, i.Name+"\n")
+		return err
+	})
+
+	out := &bytes.Buffer{}
+	require.NoError(t, render(info{Name: "tun0"}, out))
+	assert.Equal(t, "tun0\n", out.String())
+
+	err := render("not an info", out)
+	require.Error(t, err)
+	assert.ErrorContains(t, err, "expected commands.info, got string")
+}
+
+// TestResultShapes pins what each constructor means to a surface: Empty is the one that
+// renders nothing, Payload is what a program receives, and OK is set wherever a reason
+// or value says the command worked.
+func TestResultShapes(t *testing.T) {
+	assert.True(t, Nothing().Empty())
+	assert.True(t, Result{}.Empty(), "the zero Result says nothing rather than failing silently")
+	assert.False(t, Did("done").Empty())
+	assert.False(t, Failed("no").Empty())
+	assert.False(t, Value("x").Empty())
+
+	assert.True(t, Value("x").OK)
+	assert.True(t, Did("done").OK)
+	assert.False(t, Failed("no").OK)
+
+	assert.Equal(t, "x", Value("x").Payload())
+	assert.Equal(t, Outcome{OK: true, Reason: "done"}, Did("done").Payload())
+	assert.Equal(t, Outcome{Reason: "no"}, Failed("no").Payload())
+}

commands/request.go

@@ -0,0 +1,119 @@
+package commands
+
+import (
+	"flag"
+	"maps"
+	"slices"
+)
+
+// Request is one command to run, as every surface hands it to Invoke: the command, its
+// options by name in the form their flags parse, its operands, and the rendering the
+// line asked for, if any.
+//
+// nebula ctl builds one with ParseLine and it travels as a JSON object to the control
+// socket. An ssh line never becomes one: it is parsed straight into the command's own
+// FlagSet; see execLine.
+type Request struct {
+	// Command is the command to run, resolved by the surface at its edge. See
+	// Registry.Resolve for the one that tells an unknown name from a withheld one.
+	Command *Command
+
+	// Params carries the options the line set, by flag name, each in the form its flag
+	// parses from. An option left alone is absent, so the command's default applies.
+	Params map[string]string
+
+	// Args carries the operands: whatever the line did not consume as a flag.
+	Args []string
+
+	// Format is the rendering the line asked for with -json or -pretty, or FormatUnset
+	// when it expressed no preference. It is ParseLine's reading of the line for the
+	// client that sends the Request; Invoke does not read it, since the surface that
+	// runs a Request renders with the format it agreed with its caller.
+	Format Format
+}
+
+// ParseLine reads a FlagSet that ParseArgs has filled, and the operands it returned,
+// into a Request to send over the wire. Command is left for the caller to set.
+//
+// Every flag the line set other than the presentation flags travels by name, as its
+// flag's String, which is what the daemon's Set parses. That round trip is exact for
+// the bool and string flags a client builds from the schema; it is the client's to make,
+// and the ssh path does not make it, since it runs the FlagSet it parsed.
+func ParseLine(fl *flag.FlagSet, operands []string) Request {
+	req := Request{Args: operands, Format: presentationFormat(fl)}
+
+	fl.Visit(func(f *flag.Flag) {
+		if isPresentationFlag(f.Name) {
+			return
+		}
+		if req.Params == nil {
+			req.Params = map[string]string{}
+		}
+		req.Params[f.Name] = f.Value.String()
+	})
+
+	return req
+}
+
+// presentationFormat is the one reading of -json and -pretty from a parsed line, so a
+// line means the same on every surface: only a flag set to true asks for JSON, since one
+// set to false expresses no preference, and -pretty implies -json. FormatUnset means the
+// line expressed no preference and the surface's own default applies.
+func presentationFormat(fl *flag.FlagSet) Format {
+	var asJSON, pretty bool
+	fl.Visit(func(f *flag.Flag) {
+		switch f.Name {
+		case FlagJSON:
+			asJSON = flagIsTrue(f)
+		case FlagPretty:
+			pretty = flagIsTrue(f)
+		}
+	})
+
+	switch {
+	case pretty:
+		return FormatPrettyJSON
+	case asJSON:
+		return FormatJSON
+	default:
+		return FormatUnset
+	}
+}
+
+// flagIsTrue reports whether a bool flag was set to true. The flag package's bool value
+// is a flag.Getter whose Get returns the bool, so this never falls back to parsing text.
+func flagIsTrue(f *flag.Flag) bool {
+	g, ok := f.Value.(flag.Getter)
+	if !ok {
+		return false
+	}
+	b, _ := g.Get().(bool)
+	return b
+}
+
+// Invoke runs one Request and returns the command's Result, unrendered.
+//
+// The parameters are applied by flag name, in name order: a request from the wire is a
+// JSON object with no order of its own, and which of two bad parameters is reported must
+// not change from one request to the next. An unknown parameter is an error rather than
+// something quietly ignored, because a caller that misspells one deserves to hear about
+// it instead of getting a default. A bad parameter means Run is never called, and the
+// FlagSet is fresh for this call, so nothing set before it is seen by anyone.
+//
+// Nothing here writes anywhere and nothing here renders. What a surface does with the
+// Result, marshal its Payload or run the command's own text rendering through Render,
+// is the surface's decision, and keeping that out of here is what lets a caller tell a
+// refusal from a success after the text has been produced. The presentation flags are
+// not parameters: a Request says how it wants rendering in Format, and sending them as
+// parameters is refused rather than read as a preference.
+func Invoke(req Request) (Result, error) {
+	c := req.Command
+	names := slices.Sorted(maps.Keys(req.Params))
+
+	fl, fs, own := commandFlags(c)
+	if err := setParams(fl, string(c.Name), names, req.Params, own); err != nil {
+		return Result{}, err
+	}
+
+	return c.Run(fs, req.Args)
+}

commands/request_test.go

@@ -0,0 +1,115 @@
+package commands
+
+import (
+	"flag"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func lineFlags() *flag.FlagSet {
+	fl := flag.NewFlagSet("set", flag.ContinueOnError)
+	fl.Bool("verbose", false, "")
+	fl.Duration("wait", 0, "")
+	fl.String("label", "none", "")
+	fl.Bool(FlagJSON, false, "")
+	fl.Bool(FlagPretty, false, "")
+	return fl
+}
+
+// TestParseLineReadsThePresentationFlagsOnce: presentationFormat is the one reading of
+// -json and -pretty, for every surface, and ParseLine carries its answer. A flag set to
+// false expresses no preference, and neither flag ever travels as a parameter.
+func TestParseLineReadsThePresentationFlagsOnce(t *testing.T) {
+	for _, tc := range []struct {
+		args []string
+		want Format
+	}{
+		{nil, FormatUnset},
+		{[]string{"-json=false"}, FormatUnset},
+		{[]string{"-pretty=false"}, FormatUnset},
+		{[]string{"-json"}, FormatJSON},
+		{[]string{"-pretty"}, FormatPrettyJSON},
+		{[]string{"-pretty", "-json=false"}, FormatPrettyJSON},
+	} {
+		fl := lineFlags()
+		operands, err := ParseArgs(fl, tc.args)
+		require.NoError(t, err)
+
+		req := ParseLine(fl, operands)
+		assert.Equal(t, tc.want, req.Format, "%v", tc.args)
+		assert.Nil(t, req.Params, "%v: presentation flags are not parameters", tc.args)
+	}
+}
+
+// TestParseLineCarriesWhatWasSetInParseableForm: only the flags the line set travel,
+// each as the string its flag parses from, so the daemon's Invoke can apply them to a
+// fresh FlagSet and get the same values back. Operands come through untouched, those
+// after "--" included.
+func TestParseLineCarriesWhatWasSetInParseableForm(t *testing.T) {
+	fl := lineFlags()
+	operands, err := ParseArgs(fl, []string{"-verbose", "-wait", "2s", "x", "--", "-1"})
+	require.NoError(t, err)
+
+	req := ParseLine(fl, operands)
+	assert.Equal(t, map[string]string{"verbose": "true", "wait": "2s"}, req.Params, "label was left alone and must be absent")
+	assert.Equal(t, []string{"x", "-1"}, req.Args)
+	assert.Equal(t, FormatUnset, req.Format)
+
+	// The round trip a client makes: what ParseLine produced is what Invoke applies.
+	r, seen := paramRegistry(t)
+	req.Command = r.Get("set")
+	_, err = Invoke(req)
+	require.NoError(t, err)
+	require.Len(t, *seen, 1)
+	assert.Equal(t, paramsSeen{Verbose: true, Wait: 2 * time.Second}, (*seen)[0])
+}
+
+// TestResolveTellsUnknownFromWithheld: the two cases read differently to an operator,
+// and Resolve is the edge that decides which one a name from the wire is.
+func TestResolveTellsUnknownFromWithheld(t *testing.T) {
+	r := testRegistry().Filter(notWithheld, testReason)
+
+	c, err := r.Resolve("served")
+	require.NoError(t, err)
+	assert.Equal(t, Name("served"), c.Name)
+
+	_, err = r.Resolve("withheld")
+	require.ErrorContains(t, err, "withheld is not available on this surface")
+
+	_, err = r.Resolve("no-such-command")
+	require.ErrorContains(t, err, `unknown command "no-such-command"`)
+}
+
+// TestLineRunsTheFlagSetItParsed: the ssh path hands Run the FlagSet the line was parsed
+// into, with no trip through a Request. A flag.Func has no String worth re-parsing, so a
+// round trip would hand Run an empty value and no error; the line's value must arrive.
+func TestLineRunsTheFlagSetItParsed(t *testing.T) {
+	var seen []string
+	r := New()
+	r.Register(&Command{
+		Name: "mode",
+		Flags: func() (*flag.FlagSet, any) {
+			fl := flag.NewFlagSet("", flag.ContinueOnError)
+			s := new(string)
+			fl.Func("mode", "the mode", func(v string) error {
+				*s = "set:" + v
+				return nil
+			})
+			return fl, s
+		},
+		Run: func(fs any, a []string) (Result, error) {
+			seen = append(seen, *fs.(*string))
+			return Value("ok"), nil
+		},
+	})
+
+	var out, errw strings.Builder
+	r.Dispatch("mode -json -mode fast", Output{Out: &out, Err: &errw})
+	require.Equal(t, []string{"set:fast"}, seen)
+	assert.Empty(t, errw.String())
+	assert.Equal(t, "\"ok\"\n", out.String(), "-json was read from the same FlagSet")
+}

commands/writer.go

@@ -2,38 +2,22 @@ package commands
 
 import "io"
 
-// StringWriter is how a command talks back to whoever invoked it: one stream, which the
-// ssh surface points at its channel.
-type StringWriter interface {
-	WriteLine(string) error
-	Write(string) error
-	WriteBytes([]byte) error
-	GetWriter() io.Writer
+// Output is where a command line surface's text goes.
+//
+// Out and Err are distinct so a command's primary output can stay machine readable on
+// a surface that separates the two. The ssh surface points both at the same channel and
+// sees a single merged stream. The local
+// control socket does not use one at all: it invokes by named parameter and carries the
+// returned value, so there is no stream for a command to write to.
+type Output struct {
+	// Out receives what a command produces.
+	Out io.Writer
+
+	// Err receives usage text, refusals and errors.
+	Err io.Writer
 }
 
-type stringWriter struct {
-	w io.Writer
-}
-
-// NewStringWriter returns a StringWriter that writes to w.
-func NewStringWriter(w io.Writer) StringWriter {
-	return &stringWriter{w: w}
-}
-
-func (w *stringWriter) WriteLine(s string) error {
-	return w.Write(s + "\n")
-}
-
-func (w *stringWriter) Write(s string) error {
-	_, err := w.w.Write([]byte(s))
-	return err
-}
-
-func (w *stringWriter) WriteBytes(b []byte) error {
-	_, err := w.w.Write(b)
-	return err
-}
-
-func (w *stringWriter) GetWriter() io.Writer {
-	return w.w
+// MergedOutput sends output and errors to the same writer.
+func MergedOutput(w io.Writer) Output {
+	return Output{Out: w, Err: w}
 }