Drop the tests that tested the same thing twice

The endpoint is one type under both servers, so the info API's copies of
the rebind, failed-rebind and cancellation tests ran the control socket's
code a second time; in-place access changes were pinned four times, and
the registry and ctl tests had grown several that a later one subsumed.

- info API: the three endpoint tests, the mode test and the wire test's mode step go
- the group hand-over test runs under a tight umask and stands for both servers
- the two umask tests loop over the umask; the stale-socket and bounded-clients tests go, platform and slow-command tests cover them
- registry: one Resolve test with both the typed and the worded assertions, one clone test, no separate -h or writer tests
- ctl: the format table keeps one off, one json and one pretty case
- two control socket tests renamed for what they now check

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

cmd/internal/ctl/ctl_test.go

@@ -128,23 +128,6 @@ func TestParseCommandReportsWhatFlagSaid(t *testing.T) {
 	}
 }
 
-// TestParseCommandSendsATextlessCommandAsText: whether a command without a human form
-// can answer in text depends on what it returns, which the client cannot know until it
-// asks. A refusal always has a human form, so the request goes out as text and the
-// daemon decides.
-func TestParseCommandSendsATextlessCommandAsText(t *testing.T) {
-	schema := []commands.CommandSchema{{Name: "print-tunnel"}}
-
-	req, printedHelp, err := parseCommand(schema, []string{"print-tunnel"})
-	require.NoError(t, err)
-	assert.False(t, printedHelp)
-	assert.Equal(t, commands.PrintTunnel, req.name)
-	assert.Nil(t, req.Params)
-	assert.Empty(t, req.Args)
-	assert.Equal(t, commands.FormatUnset, req.Format)
-	assert.Equal(t, "text", wireFormat(req.Format))
-}
-
 // TestParseCommandOffPresentationFlagIsNoPreference: the daemon treats -json=false as
 // no preference, so nebula ctl must too, or the same line renders differently on the
 // two surfaces. Only a true value asks for JSON, and -pretty implies it.
@@ -164,12 +147,8 @@ func TestParseCommandOffPresentationFlagIsNoPreference(t *testing.T) {
 	}{
 		{[]string{"device-info"}, commands.FormatUnset, "text"},
 		{[]string{"device-info", "-json=false"}, commands.FormatUnset, "text"},
-		{[]string{"device-info", "-pretty=false"}, commands.FormatUnset, "text"},
-		{[]string{"device-info", "-json=false", "-pretty=false"}, commands.FormatUnset, "text"},
 		{[]string{"device-info", "-json"}, commands.FormatJSON, "json"},
-		{[]string{"device-info", "-json=true"}, commands.FormatJSON, "json"},
 		{[]string{"device-info", "-pretty"}, commands.FormatPrettyJSON, "json"},
-		{[]string{"device-info", "-pretty", "-json=false"}, commands.FormatPrettyJSON, "json"},
 	} {
 		t.Run(strings.Join(tc.argv[1:], " "), func(t *testing.T) {
 			req, printedHelp, err := parseCommand(schema, tc.argv)

commands/registry_test.go

@@ -87,66 +87,27 @@ func TestWithheldIsNotReportedAsUnknown(t *testing.T) {
 	assert.Contains(t, out.String(), "did not understand: no-such-command")
 }
 
-// TestResolveTypesTheWithheldRefusal: a surface with status codes tells a withheld
-// command from an unknown one by the error's type, never by asking the unfiltered
-// registry, which is the one thing a filtered surface must not consult.
-func TestResolveTypesTheWithheldRefusal(t *testing.T) {
+// TestResolveTellsUnknownFromWithheld: the two cases read differently to an operator,
+// and a surface with status codes tells them apart by the error's type, never by asking
+// the unfiltered registry, which is the one thing a filtered surface must not consult.
+func TestResolveTellsUnknownFromWithheld(t *testing.T) {
 	r := testRegistry().Filter(notWithheld, testReason)
 
-	_, err := r.Resolve("withheld")
+	c, err := r.Resolve("served")
+	require.NoError(t, err)
+	assert.Equal(t, Name("served"), c.Name)
+
+	_, err = r.Resolve("withheld")
 	var withheld *WithheldError
 	require.ErrorAs(t, err, &withheld)
-	assert.Contains(t, err.Error(), testReason)
+	require.ErrorContains(t, err, "withheld is not available on this surface")
+	require.ErrorContains(t, err, testReason)
 
 	_, err = r.Resolve("no-such-command")
-	require.Error(t, err)
+	require.ErrorContains(t, err, `unknown command "no-such-command"`)
 	assert.NotErrorAs(t, err, &withheld, "an unknown name is not a withheld one")
 }
 
-// TestSplitWriterKeepsOutputParseable is why the writer grew Err(): usage text on the
-// same stream as -json output would corrupt it.
-func TestSplitWriterKeepsOutputParseable(t *testing.T) {
-	r := New()
-	r.Register(&Command{
-		Name:             "emit",
-		ShortDescription: "emits json",
-		Flags: func() (*flag.FlagSet, any) {
-			fl := flag.NewFlagSet("", flag.ContinueOnError)
-			fl.Bool("verbose", false, "say more")
-			return fl, nil
-		},
-		Run: func(fs any, a []string) (Result, error) {
-			return Value(map[string]bool{"ok": true}), nil
-		},
-	})
-
-	out, errOut := &bytes.Buffer{}, &bytes.Buffer{}
-	require.Error(t, r.DispatchArgs([]string{"emit", "-nope"}, Output{Out: out, Err: errOut}))
-
-	assert.Empty(t, out.String(), "flag errors must not land on the output stream")
-	assert.Contains(t, errOut.String(), "flag provided but not defined")
-}
-
-// TestMergedWriterPreservesSshBehavior: the ssh surface points both streams at one
-// channel, and must keep seeing a single merged stream.
-func TestMergedWriterPreservesSshBehavior(t *testing.T) {
-	r := New()
-	r.Register(&Command{
-		Name:             "emit",
-		ShortDescription: "emits",
-		Flags: func() (*flag.FlagSet, any) {
-			fl := flag.NewFlagSet("", flag.ContinueOnError)
-			fl.Bool("verbose", false, "say more")
-			return fl, nil
-		},
-		Run: func(fs any, a []string) (Result, error) { return Nothing(), nil },
-	})
-
-	merged := &bytes.Buffer{}
-	require.Error(t, r.DispatchArgs([]string{"emit", "-nope"}, MergedOutput(merged)))
-	assert.Contains(t, merged.String(), "flag provided but not defined")
-}
-
 // 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) {
@@ -235,12 +196,6 @@ func TestHelpListsOnlyServedCommands(t *testing.T) {
 	assert.NotContains(t, out.String(), "withheld")
 }
 
-func TestDispatchHelpFlag(t *testing.T) {
-	out := &bytes.Buffer{}
-	testRegistry().Dispatch("served -h", MergedOutput(out))
-	assert.Contains(t, out.String(), "served - a command this surface serves")
-}
-
 // 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.
@@ -276,14 +231,6 @@ func TestRegisterRejectsReservedFlags(t *testing.T) {
 	}
 }
 
-func TestCloneDoesNotAffectOriginal(t *testing.T) {
-	r := testRegistry()
-	c := r.Clone()
-	c.Register(&Command{Name: "logout", ShortDescription: "ends the session", Run: func(fs any, a []string) (Result, error) { return Nothing(), nil }})
-
-	assert.Nil(t, r.Get("logout"))
-}
-
 // 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.
@@ -324,16 +271,23 @@ func TestRegistryIsSafeForConcurrentUse(t *testing.T) {
 	assert.Len(t, f.Match("late-"), 8*50, "every late registration is visible once the writers finish")
 }
 
-// TestHelpFlagAgreesWithHelpCommand pins that -h on a command and `help <name>` print the
-// same text, on a clone as on the source: both render through WriteCommandHelp, and Clone
-// rebinds help to the copy, so a command registered on the clone is found either way.
-func TestHelpFlagAgreesWithHelpCommand(t *testing.T) {
-	c := testRegistry().Clone()
+// 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",
 		Run:              func(fs any, a []string) (Result, error) { return Nothing(), nil },
 	})
+	assert.Nil(t, r.Get(Logout), "the source is not affected")
+
+	out := &bytes.Buffer{}
+	require.NoError(t, c.DispatchArgs([]string{"help"}, MergedOutput(out)))
+	assert.Contains(t, out.String(), "logout")
 
 	for _, name := range []string{"logout", "served"} {
 		flag := &bytes.Buffer{}
@@ -345,21 +299,6 @@ func TestHelpFlagAgreesWithHelpCommand(t *testing.T) {
 	}
 }
 
-// TestCloneHelpSeesCloneCommands: help on a clone answers for the clone. The ssh session
-// registers logout on its clone, and `help logout` has to find it.
-func TestCloneHelpSeesCloneCommands(t *testing.T) {
-	c := testRegistry().Clone()
-	c.Register(&Command{Name: "logout", ShortDescription: "ends the session", Run: func(fs any, a []string) (Result, error) { return Nothing(), nil }})
-
-	out := &bytes.Buffer{}
-	require.NoError(t, c.DispatchArgs([]string{"help", "logout"}, MergedOutput(out)))
-	assert.Contains(t, out.String(), "ends the session")
-
-	out.Reset()
-	require.NoError(t, c.DispatchArgs([]string{"help"}, MergedOutput(out)))
-	assert.Contains(t, out.String(), "logout")
-}
-
 // 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.

commands/request_test.go

@@ -68,22 +68,6 @@ func TestParseLineCarriesWhatWasSetInParseableForm(t *testing.T) {
 	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.

localapi/access_unix_test.go

@@ -4,7 +4,6 @@ package localapi
 
 import (
 	"context"
-	"net/http"
 	"os"
 	"os/user"
 	"strconv"
@@ -15,6 +14,7 @@ import (
 	"github.com/slackhq/nebula/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
 )
 
 // ownGroup returns a group this process may hand a socket to: its own effective group,
@@ -86,9 +86,12 @@ func TestControlAccessFromConfig(t *testing.T) {
 }
 
 // TestControlSocketIsHandedToTheGroup: the socket is created with the group's gid and
-// 0660, and a reload that takes the group away tightens the socket in place, without
-// moving it.
+// 0660, whatever the umask, and a reload that takes the group away tightens the socket
+// in place, without moving it. The endpoint is shared, so this covers the info API too.
 func TestControlSocketIsHandedToTheGroup(t *testing.T) {
+	old := unix.Umask(0077)
+	defer unix.Umask(old)
+
 	name, gid := ownGroup(t)
 	want, err := strconv.ParseUint(gid, 10, 32)
 	require.NoError(t, err)
@@ -175,32 +178,6 @@ func TestInfoAccessFromConfig(t *testing.T) {
 	assert.Nil(t, cfg.Access.Group)
 }
 
-// TestSocketIsHandedToTheGroup: the socket is created with the group's gid and the
-// mode, and a changed group or mode on a reload is applied to the socket in place.
-func TestSocketIsHandedToTheGroup(t *testing.T) {
-	name, gid := ownGroup(t)
-	group := &Group{Name: name, ID: gid}
-	want, err := strconv.ParseUint(gid, 10, 32)
-	require.NoError(t, err)
-
-	s, path := serveInfo(t, infoRegistry(), 0600)
-	require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: 0660, Group: group}))
-	fi, err := os.Lstat(path)
-	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0660), fi.Mode().Perm())
-	assert.Equal(t, uint32(want), socketGroup(t, path))
-
-	status, _ := test.HTTPGet(t, path, "/v1/whoami")
-	assert.Equal(t, http.StatusOK, status, "the same listener is still serving")
-
-	// The mode is the operator's to widen further.
-	require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: 0664, Group: group}))
-	fi, err = os.Lstat(path)
-	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0664), fi.Mode().Perm())
-	assert.Equal(t, path, s.Path())
-}
-
 // TestRemovingTheGroupHandsTheSocketBack: a reload that drops the group key leaves the
 // mode the operator set but returns the socket to the daemon's own group, so the old
 // group's members are no longer admitted. The socket is first handed to a group this

localapi/info_test.go

@@ -23,7 +23,6 @@ import (
 	"github.com/slackhq/nebula/test"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-	"golang.org/x/sys/unix"
 )
 
 type whoamiFlags struct {
@@ -233,28 +232,6 @@ func TestInfoPresentationFlags(t *testing.T) {
 	assert.Equal(t, compact, off)
 }
 
-// TestInfoSocketMode: the mode is the operator's, pinned regardless of the umask, and a
-// changed mode on a reload is applied in place rather than by rebinding.
-func TestInfoSocketMode(t *testing.T) {
-	old := unix.Umask(0077)
-	defer unix.Umask(old)
-
-	s, path := serveInfo(t, infoRegistry(), 0660)
-
-	fi, err := os.Stat(path)
-	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0660), fi.Mode().Perm())
-
-	require.NoError(t, s.Rebind(context.Background(), path, Access{Mode: 0640}))
-	fi, err = os.Stat(path)
-	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0640), fi.Mode().Perm())
-	assert.Equal(t, path, s.Path())
-
-	status, _ := test.HTTPGet(t, path, "/v1/whoami")
-	assert.Equal(t, http.StatusOK, status, "the same listener is still serving")
-}
-
 // TestInfoModeChangeRefusesASwappedSocket: whoever can write the directory can replace
 // the node with a link to a file of their choosing; the daemon must not set the
 // operator's mode on that file. The node is seen to be gone, the link is refused by the
@@ -276,54 +253,6 @@ func TestInfoModeChangeRefusesASwappedSocket(t *testing.T) {
 	assert.Empty(t, s.Path(), "nothing of ours answers at the path, so nothing is served")
 }
 
-func TestInfoRebindMovesAndStopRemoves(t *testing.T) {
-	s, first := serveInfo(t, infoRegistry(), 0600)
-
-	second := testSocket(t)
-	require.NoError(t, s.Rebind(context.Background(), second, Access{Mode: 0600}))
-	assert.Equal(t, second, s.Path())
-	_, err := os.Stat(first)
-	assert.True(t, os.IsNotExist(err), "the old socket is removed")
-
-	status, body := test.HTTPGet(t, second, "/v1/whoami?name=alice")
-	assert.Equal(t, http.StatusOK, status)
-	assert.JSONEq(t, `{"name":"alice"}`, body)
-
-	s.Stop()
-	assert.Empty(t, s.Path())
-	_, err = os.Stat(second)
-	assert.True(t, os.IsNotExist(err), "Stop removes the socket")
-}
-
-func TestInfoRebindFailureKeepsTheOldSocket(t *testing.T) {
-	s, first := serveInfo(t, infoRegistry(), 0600)
-
-	err := s.Rebind(context.Background(), filepath.Join(privateBase(t), "missing", "n", "s.sock"), Access{Mode: 0600})
-	require.Error(t, err)
-	assert.Equal(t, first, s.Path())
-
-	status, _ := test.HTTPGet(t, first, "/v1/whoami")
-	assert.Equal(t, http.StatusOK, status)
-}
-
-func TestInfoContextCancellationStops(t *testing.T) {
-	ctx, cancel := context.WithCancel(context.Background())
-	path := testSocket(t)
-	s := NewInfoServer(test.NewLogger(), infoRegistry())
-	require.NoError(t, s.Rebind(ctx, path, Access{Mode: 0600}))
-	t.Cleanup(s.Stop)
-
-	cancel()
-	require.Eventually(t, func() bool { return s.Path() == "" }, time.Second, 10*time.Millisecond)
-	_, err := os.Stat(path)
-	assert.True(t, os.IsNotExist(err))
-
-	// And a reload that lands during shutdown must not bring it back.
-	err = s.Rebind(ctx, path, Access{Mode: 0600})
-	require.Error(t, err)
-	assert.ErrorContains(t, err, "shutting down")
-}
-
 func TestInfoConfig(t *testing.T) {
 	load := func(t *testing.T, yaml string) (infoConfig, error) {
 		t.Helper()
@@ -380,11 +309,8 @@ func TestWireInfoFollowsConfig(t *testing.T) {
 
 	ctlPath := testSocket(t)
 	infoPath := testSocket(t)
-	sectionWithMode := func(enabled bool, socket, mode string) string {
-		return fmt.Sprintf("local_api:\n  control:\n    socket: %s\n  info:\n    enabled: %t\n    socket: %s\n    socket_mode: \"%s\"\n", ctlPath, enabled, socket, mode)
-	}
 	section := func(enabled bool, socket string) string {
-		return sectionWithMode(enabled, socket, "0600")
+		return fmt.Sprintf("local_api:\n  control:\n    socket: %s\n  info:\n    enabled: %t\n    socket: %s\n", ctlPath, enabled, socket)
 	}
 
 	c := config.NewC(test.NewLogger())
@@ -406,18 +332,11 @@ func TestWireInfoFollowsConfig(t *testing.T) {
 	require.NoError(t, c.ReloadConfigString(section(true, infoPath)).Err)
 	assert.Equal(t, infoPath, eps.Info.Path())
 
-	// A reload that changes only the mode applies it in place.
-	require.NoError(t, c.ReloadConfigString(sectionWithMode(true, infoPath, "0660")).Err)
-	assert.Equal(t, infoPath, eps.Info.Path())
-	fi, err := os.Stat(infoPath)
-	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0660), fi.Mode().Perm())
-
 	// A reload that moves the socket moves it, and removes the old one.
 	moved := testSocket(t)
 	require.NoError(t, c.ReloadConfigString(section(true, moved)).Err)
 	assert.Equal(t, moved, eps.Info.Path())
-	_, err = os.Stat(infoPath)
+	_, err := os.Stat(infoPath)
 	assert.True(t, os.IsNotExist(err))
 	status, body = test.HTTPGet(t, moved, "/v1/whoami?name=bob")
 	assert.Equal(t, http.StatusOK, status)

localapi/server_test.go

@@ -126,65 +126,26 @@ func TestPanicInCommandDoesNotKillTheDaemon(t *testing.T) {
 	assert.Empty(t, errOut)
 }
 
+// TestSocketPermissions: the modes are pinned, not left to the umask. A 077 umask would
+// make the directory 0700 and hide a root daemon's socket from an unprivileged nebula
+// ctl.
 func TestSocketPermissions(t *testing.T) {
-	old := unix.Umask(0)
-	defer unix.Umask(old)
+	for _, umask := range []int{0, 0077} {
+		t.Run(fmt.Sprintf("umask %04o", umask), func(t *testing.T) {
+			old := unix.Umask(umask)
+			defer unix.Umask(old)
 
-	_, path := serve(t, basicRegistry())
-
-	fi, err := os.Stat(path)
-	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0600), fi.Mode().Perm(), "socket must not be reachable by other users")
-
-	di, err := os.Stat(filepath.Dir(path))
-	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0755), di.Mode().Perm(), "a directory we create ourselves is searchable, so ServedByRoot can see into it")
-}
-
-// TestSocketPermissionsUnderTightUmask: the modes are pinned, not left to the umask. A
-// 077 umask would make the directory 0700 and hide a root daemon's socket from an
-// unprivileged nebula ctl.
-func TestSocketPermissionsUnderTightUmask(t *testing.T) {
-	old := unix.Umask(0077)
-	defer unix.Umask(old)
-
-	_, path := serve(t, basicRegistry())
-
-	fi, err := os.Stat(path)
-	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0600), fi.Mode().Perm())
-
-	di, err := os.Stat(filepath.Dir(path))
-	require.NoError(t, err)
-	assert.Equal(t, os.FileMode(0755), di.Mode().Perm(), "the umask must not take the directory's searchability back")
-}
-
-func TestStaleSocketIsReplaced(t *testing.T) {
-	path := testSocket(t)
-	require.NoError(t, os.Mkdir(filepath.Dir(path), 0700))
-
-	// A hard kill leaves a socket file with nothing behind it.
-	ln, err := net.Listen("unix", path)
-	require.NoError(t, err)
-	require.NoError(t, ln.Close())
-	require.NoError(t, os.WriteFile(path, nil, 0600))
-	_ = os.Remove(path)
-	ln, err = net.Listen("unix", path)
-	require.NoError(t, err)
-	fl, ok := ln.(*net.UnixListener)
-	require.True(t, ok)
-	fl.SetUnlinkOnClose(false)
-	require.NoError(t, ln.Close())
-
-	_, err = os.Stat(path)
-	require.NoError(t, err, "test setup should leave a stale socket behind")
+			_, path := serve(t, basicRegistry())
 
-	s := NewServer(test.NewLogger(), basicRegistry())
-	require.NoError(t, s.Rebind(context.Background(), path, ownerOnly()), "a stale socket must not block startup")
-	defer s.Stop()
+			fi, err := os.Stat(path)
+			require.NoError(t, err)
+			assert.Equal(t, os.FileMode(0600), fi.Mode().Perm(), "socket must not be reachable by other users")
 
-	out, _ := run(t, path, "echo", "ok")
-	assert.Equal(t, "[ok]", out)
+			di, err := os.Stat(filepath.Dir(path))
+			require.NoError(t, err)
+			assert.Equal(t, os.FileMode(0755), di.Mode().Perm(), "a directory we create ourselves is searchable, so ServedByRoot can see into it")
+		})
+	}
 }
 
 func TestLiveSocketIsNotStolen(t *testing.T) {
@@ -346,44 +307,6 @@ func TestOversizedRequestIsRejected(t *testing.T) {
 
 // TestConcurrentClientsAreBounded checks that going over the connection cap is answered
 // rather than queued, so a burst of clients cannot pile up against the packet path.
-func TestConcurrentClientsAreBounded(t *testing.T) {
-	reg := commands.New()
-	release := make(chan struct{})
-	reg.Register(&commands.Command{
-		Name:             "block",
-		ShortDescription: "blocks until released",
-		Run: func(fs any, a []string) (commands.Result, error) {
-			<-release
-			return commands.Value("done"), nil
-		},
-	})
-
-	s := NewServer(test.NewLogger(), reg)
-	s.maxConns = 2
-	path := testSocket(t)
-	require.NoError(t, s.Rebind(context.Background(), path, ownerOnly()))
-	defer s.Stop()
-
-	var wg sync.WaitGroup
-	for i := 0; i < 2; i++ {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			_, _ = (&Client{Path: path}).Invoke("block", nil, nil, "")
-		}()
-	}
-
-	// Give the two blockers time to occupy the slots.
-	time.Sleep(200 * time.Millisecond)
-
-	res, err := (&Client{Path: path}).Invoke("block", nil, nil, "")
-	require.NoError(t, err)
-	assert.Contains(t, res.Error, "busy")
-
-	close(release)
-	wg.Wait()
-}
-
 // TestSlowCommandIsNamedInTheLog: a command that does not return holds its slot for
 // good, and enough of them leave the socket answering only "busy". Both are logged, the
 // command by name once it has run past slowAfter and each refused connection as it
@@ -673,8 +596,9 @@ func TestJSONOutputIsAJSONValue(t *testing.T) {
 	assert.Equal(t, []int{1, 2, 3}, decoded.Hosts)
 }
 
-// TestTextOutputIsAJSONString: everything else round trips as a string, unchanged.
-func TestTextOutputIsAJSONString(t *testing.T) {
+// TestStringValueIsAJSONString: a command whose value is a string answers a JSON request
+// with a JSON string, quotes and all.
+func TestStringValueIsAJSONString(t *testing.T) {
 	_, path := serve(t, basicRegistry())
 
 	res, err := (&Client{Path: path}).Invoke("echo", nil, []string{"hi"}, "")
@@ -685,7 +609,7 @@ func TestTextOutputIsAJSONString(t *testing.T) {
 	assert.Equal(t, "[hi]", s)
 }
 
-func TestDescribeReportsFlagTypes(t *testing.T) {
+func TestDescribeListsTheCommands(t *testing.T) {
 	_, path := serve(t, basicRegistry())
 
 	schema, err := (&Client{Path: path}).Describe()
@@ -696,8 +620,7 @@ func TestDescribeReportsFlagTypes(t *testing.T) {
 		names = append(names, string(c.Name))
 	}
 
-	assert.Contains(t, names, "echo")
-	assert.NotContains(t, names, "withheld", "the schema advertises only what this surface serves")
+	assert.ElementsMatch(t, []string{"help", "echo", "boom"}, names)
 }
 
 // TestPresentationParametersAreRefused: the schema lists json and pretty on every