amadan

eleven / messenger Public

server: an unknown `lchat` command prints usage instead of starting a server (#1225)

main()'s dispatch falls through to "run a single instance" for anything the
switch doesn't recognise, because a bare `lchat` plus flags is a supported
invocation. That's right for flags and wrong for words: a typo like
`lchat routes` — or `lchat help`, which was never a case — booted a full
messenger server. Usually it died on a port clash and read as a confusing
error; when the port was free it just kept running. That is how a stray
`lchat help` process ended up sitting on a production box for five days.

Now a first argument starting with "-" falls through exactly as before, and
anything else must name a subcommand or we print the command list and exit 2.
`lchat help` prints the same list on stdout and exits 0.

The list lives beside the guard in subcommands.go, and a test parses main.go's
dispatch switch and pins the two together — so a new subcommand with no help
entry, or a help entry that dispatches nowhere, fails the tests rather than
drifting quietly.
Co-authored-by: Claude <noreply@anthropic.com>
James Adam 6dc543298878e592c89612f79d9395e6f8f36d1d parent 4595dbd
4 files changed, +352 −0
  • docs/planet-scale-guide/reference.html +2 −0
  • main.go +5 −0
  • subcommands.go +133 −0
  • subcommands_test.go +212 −0
diff --git a/docs/planet-scale-guide/reference.html b/docs/planet-scale-guide/reference.html
index 884f78b..3845e8e 100644
--- a/docs/planet-scale-guide/reference.html
+++ b/docs/planet-scale-guide/reference.html
@@ -81,6 +81,8 @@
<dd>Run a bot sidecar (with no arguments, configuration comes from the unit&rsquo;s environment file) / claim an agent invite into a session file. <a href="processes.html#bot-sidecars">Part 03</a>.</dd>
<dt><code>lchat release keygen|sign|verify|payload</code></dt>
<dd>The release-feed signing plumbing — used by the publishing pipeline, not in box operation. <a href="deploys.html#signing">Part 04</a>.</dd>
+ <dt><code>lchat help</code></dt>
+ <dd>Prints every command on this page, one line each — the fastest way to check a verb&rsquo;s exact name on a box. Note that an <em>unrecognised</em> command (a typo like <code>lchat routes</code>) now prints this same list to stderr and exits 2. It used to fall through and start a messenger server instead, which either died on a port clash or, if the port was free, kept running unnoticed.</dd>
</dl>
<h2 id="env">Environment variable reference</h2>
diff --git a/main.go b/main.go
index b27269c..017d7f5 100644
--- a/main.go
+++ b/main.go
@@ -260,6 +260,11 @@ func main() {
runTunnelCmd(os.Args[2:])
return
}
+ // No case matched. A flag falls through to the single-instance form
+ // below (unchanged); a word does not — it is a typo, and starting a
+ // server for it is how a stray `lchat help` ran for days on a live
+ // box. See subcommands.go.
+ checkSubcommand(os.Args[1])
}
addr := flag.String("addr", envOr("ADDR", ":1747"), "listen address (env: ADDR)")
diff --git a/subcommands.go b/subcommands.go
new file mode 100644
index 0000000..06d67b6
--- /dev/null
+++ b/subcommands.go
@@ -0,0 +1,133 @@
+// 🤖 The `lchat` command surface: the subcommand list `lchat help` prints, and
+// the guard that stops an unrecognised word from silently starting a server.
+//
+// Why the guard exists: main()'s dispatch falls through to "run a single
+// instance" for anything it doesn't recognise, because a bare `lchat` plus
+// flags is a valid, supported invocation. That is right for flags and wrong
+// for words — a typo (`lchat routes`, `lchat help`) used to boot a full
+// messenger server rather than say "no such command". Usually it died on a
+// port clash and looked like a confusing error; when the port was free it kept
+// running, which is how a stray `lchat help` process sat on a production box
+// for five days.
+//
+// So: a first argument starting with "-" is a flag (fall through, unchanged),
+// and anything else must name a subcommand or we print usage and exit 2.
+package main
+
+import (
+ "fmt"
+ "io"
+ "os"
+ "strings"
+)
+
+// subcommand is one row of `lchat help`. The blurb is one line, imperative,
+// and describes the verb — flags belong to `lchat <name> -h`.
+type subcommand struct{ name, blurb string }
+
+// subcommands is what `lchat help` prints, in the order it prints them —
+// grouped by what an operator is trying to do rather than alphabetically,
+// since the list is long enough that alphabetical hides the shape.
+//
+// main()'s switch is the REAL dispatcher; this is only the human-facing list.
+// TestUsageCoversEverySubcommand reads main.go and pins the two together, so a
+// new `case` that nobody documents (or a documented verb that doesn't
+// dispatch) fails the tests rather than drifting quietly.
+var subcommands = []subcommand{
+ // Long-running processes: the front door and the sibling services.
+ {"router", "run the front door (TLS termination + host routing)"},
+ {"controld", "run the local control daemon (root-only provisioning API)"},
+ {"cloud", "run the public self-serve space provisioner"},
+ {"tunnel", "run the trial-tunnel gateway that fronts `lchat try`"},
+ {"links", "run the short-link redirector (`links run`)"},
+ {"hooks", "run the webhook-ingest sidecar (`hooks run`)"},
+ {"bot", "run the bot sidecar (`bot run`)"},
+
+ // Standing a box up, and putting things on it.
+ {"bootstrap", "install/refresh this box's systemd units, /data, and the router"},
+ {"add-instance", "create a chat instance and route it"},
+ {"add-service", "front an already-running backend on a unix socket"},
+ {"add-bot", "provision an instance's bot sidecar"},
+ {"add-hooks", "provision an instance's webhook-ingest sidecar"},
+
+ // Shipping code: the versioned, side-by-side deploy path.
+ {"stage", "register a built binary in the version store"},
+ {"install", "put a staged build onto one instance without restarting it"},
+ {"deploy", "repoint an instance at a version and restart (health-checked)"},
+ {"rollback", "step an instance back to its previous version"},
+ {"init-versions", "one-time adoption of the version store on an old box"},
+ {"update", "the release-channel auto-updater (`update status`)"},
+ {"release", "the signing side of the release feed (keygen/sign/verify)"},
+
+ // Looking after what's already running.
+ {"instances", "list every instance and service on this box"},
+ {"remove", "remove an instance (deliberately leaves /data behind)"},
+ {"reset", "archive a canary's data (canary-flag gated)"},
+ {"canary", "set/clear an instance's throwaway-canary flag"},
+ {"hibernate", "flip an instance between always-on and hibernating"},
+ {"replication", "read or set one instance's replication class"},
+ {"relocate", "move an instance to another region"},
+ {"mesh-seed", "bulk-replicate every space from a peer box"},
+ {"admin-link", "print an instance's admin link"},
+ {"setup-invite", "mint an instance's one-time setup link"},
+
+ // Local development.
+ {"try", "expose a local server at a real public origin"},
+ {"relay", "run a space through the tunnel gateway"},
+ {"version", "print this binary's build id"},
+}
+
+// isSubcommand reports whether arg names a subcommand. Kept separate from the
+// dispatch switch so it is testable without running main().
+func isSubcommand(arg string) bool {
+ for _, c := range subcommands {
+ if c.name == arg {
+ return true
+ }
+ }
+ return false
+}
+
+// looksLikeFlag reports whether arg is a flag rather than a mistyped
+// subcommand. A bare "-" is not a flag — nothing here reads stdin, so it is a
+// typo, and naming it as one beats letting the flag package complain.
+func looksLikeFlag(arg string) bool { return strings.HasPrefix(arg, "-") && arg != "-" }
+
+// writeUsage prints the command surface. Instance flags are deliberately NOT
+// listed: they belong to the no-subcommand form, and `lchat -h` still prints
+// them via the flag package exactly as it always has.
+func writeUsage(w io.Writer) {
+ fmt.Fprint(w, "lchat — the Eleven messenger server, router, and operator CLI.\n\n"+
+ "Usage:\n"+
+ " lchat <command> [flags] run a command (see below)\n"+
+ " lchat [flags] run a single instance directly (`lchat -h` for its flags)\n\n"+
+ "Commands:\n")
+ width := 0
+ for _, c := range subcommands {
+ if len(c.name) > width {
+ width = len(c.name)
+ }
+ }
+ for _, c := range subcommands {
+ fmt.Fprintf(w, " %-*s %s\n", width, c.name, c.blurb)
+ }
+ fmt.Fprint(w, "\nRun `lchat <command> -h` for a command's own flags.\n")
+}
+
+// checkSubcommand guards main()'s dispatch. It is called only when the switch
+// found no match, so reaching here with a non-flag word means the word names
+// nothing — print usage and exit rather than falling through and starting a
+// server. `help` is handled here too (it is a word, not a flag, so it used to
+// boot a server as well).
+func checkSubcommand(arg string) {
+ if arg == "help" {
+ writeUsage(os.Stdout)
+ os.Exit(0)
+ }
+ if looksLikeFlag(arg) {
+ return // a flag: fall through to the single-instance form, unchanged
+ }
+ fmt.Fprintf(os.Stderr, "lchat: unknown command %q\n\n", arg)
+ writeUsage(os.Stderr)
+ os.Exit(2)
+}
diff --git a/subcommands_test.go b/subcommands_test.go
new file mode 100644
index 0000000..ab75467
--- /dev/null
+++ b/subcommands_test.go
@@ -0,0 +1,212 @@
+package main
+
+import (
+ "go/ast"
+ "go/parser"
+ "go/token"
+ "os"
+ "os/exec"
+ "strconv"
+ "strings"
+ "testing"
+)
+
+// dispatchedSubcommands reads main.go and returns the case strings of the
+// switch that dispatches os.Args[1]. Reading the source (rather than keeping a
+// second hand-written list) is the point: this is what makes the drift test
+// below authoritative instead of just another copy to forget.
+func dispatchedSubcommands(t *testing.T) []string {
+ t.Helper()
+ f, err := parser.ParseFile(token.NewFileSet(), "main.go", nil, 0)
+ if err != nil {
+ t.Fatalf("parse main.go: %v", err)
+ }
+ var found []string
+ ast.Inspect(f, func(n ast.Node) bool {
+ fn, ok := n.(*ast.FuncDecl)
+ if !ok || fn.Name.Name != "main" {
+ return true
+ }
+ ast.Inspect(fn, func(n ast.Node) bool {
+ sw, ok := n.(*ast.SwitchStmt)
+ // Only the switch on os.Args[1] — main() contains others.
+ if !ok || !strings.Contains(exprText(sw.Tag), "os.Args[1]") {
+ return true
+ }
+ for _, stmt := range sw.Body.List {
+ cc, ok := stmt.(*ast.CaseClause)
+ if !ok {
+ continue
+ }
+ for _, e := range cc.List {
+ if lit, ok := e.(*ast.BasicLit); ok && lit.Kind == token.STRING {
+ if s, err := strconv.Unquote(lit.Value); err == nil {
+ found = append(found, s)
+ }
+ }
+ }
+ }
+ return false
+ })
+ return false
+ })
+ if len(found) == 0 {
+ t.Fatal("found no dispatch cases in main() — has the switch moved? this test must be updated with it")
+ }
+ return found
+}
+
+// exprText renders an expression enough to recognise os.Args[1].
+func exprText(e ast.Expr) string {
+ idx, ok := e.(*ast.IndexExpr)
+ if !ok {
+ return ""
+ }
+ sel, ok := idx.X.(*ast.SelectorExpr)
+ if !ok {
+ return ""
+ }
+ pkg, ok := sel.X.(*ast.Ident)
+ if !ok {
+ return ""
+ }
+ lit, ok := idx.Index.(*ast.BasicLit)
+ if !ok {
+ return ""
+ }
+ return pkg.Name + "." + sel.Sel.Name + "[" + lit.Value + "]"
+}
+
+// TestUsageCoversEverySubcommand is the contract: `lchat help` lists exactly
+// what main() dispatches. A new subcommand with no help entry is a silent
+// feature; a help entry that dispatches nowhere is a lie in the CLI's own
+// documentation. Both fail here.
+func TestUsageCoversEverySubcommand(t *testing.T) {
+ documented := map[string]bool{}
+ for _, c := range subcommands {
+ if documented[c.name] {
+ t.Errorf("subcommands lists %q twice", c.name)
+ }
+ documented[c.name] = true
+ }
+ dispatched := map[string]bool{}
+ for _, name := range dispatchedSubcommands(t) {
+ dispatched[name] = true
+ if !documented[name] {
+ t.Errorf("main() dispatches %q but `lchat help` does not list it — add it to subcommands", name)
+ }
+ }
+ for _, c := range subcommands {
+ if !dispatched[c.name] {
+ t.Errorf("`lchat help` lists %q but main() does not dispatch it", c.name)
+ }
+ }
+}
+
+func TestLooksLikeFlag(t *testing.T) {
+ for _, tc := range []struct {
+ arg string
+ flag bool
+ }{
+ {"-db", true},
+ {"--db", true},
+ {"-h", true},
+ {"-addr=:1747", true},
+ {"-", false}, // not a flag: a typo we should name, not pass to flag parsing
+ {"router", false},
+ {"routes", false},
+ {"", false},
+ } {
+ if got := looksLikeFlag(tc.arg); got != tc.flag {
+ t.Errorf("looksLikeFlag(%q) = %v, want %v", tc.arg, got, tc.flag)
+ }
+ }
+}
+
+func TestIsSubcommand(t *testing.T) {
+ for _, name := range []string{"router", "bootstrap", "links", "version"} {
+ if !isSubcommand(name) {
+ t.Errorf("isSubcommand(%q) = false, want true", name)
+ }
+ }
+ // The words that used to boot a server instead of erroring.
+ for _, name := range []string{"routes", "help", "", "Router", "add_instance"} {
+ if isSubcommand(name) {
+ t.Errorf("isSubcommand(%q) = true, want false", name)
+ }
+ }
+}
+
+func TestWriteUsageListsEveryCommand(t *testing.T) {
+ var sb strings.Builder
+ writeUsage(&sb)
+ out := sb.String()
+ for _, c := range subcommands {
+ if !strings.Contains(out, c.name) {
+ t.Errorf("usage output omits %q", c.name)
+ }
+ if c.blurb == "" {
+ t.Errorf("%q has no blurb", c.name)
+ }
+ }
+ if !strings.Contains(out, "lchat <command>") {
+ t.Error("usage output should show the command form")
+ }
+}
+
+// TestCheckSubcommandExits covers the behaviour that matters — the exit code
+// and stream — by re-running this test binary as a helper, since
+// checkSubcommand calls os.Exit. `help` must succeed on stdout; an unknown
+// word must fail on stderr; a flag must return so main() falls through to the
+// single-instance form.
+func TestCheckSubcommandExits(t *testing.T) {
+ for _, tc := range []struct {
+ name, arg string
+ wantCode int
+ wantStderr bool
+ wantText string
+ }{
+ {"help", "help", 0, false, "Commands:"},
+ {"unknown word", "routes", 2, true, `unknown command "routes"`},
+ {"bare dash", "-", 2, true, "unknown command"},
+ {"flag falls through", "-db", 0, false, "FELL-THROUGH"},
+ } {
+ t.Run(tc.name, func(t *testing.T) {
+ cmd := exec.Command(os.Args[0], "-test.run=TestCheckSubcommandHelper")
+ cmd.Env = append(os.Environ(), "LCHAT_SUBCOMMAND_HELPER="+tc.arg)
+ var stdout, stderr strings.Builder
+ cmd.Stdout, cmd.Stderr = &stdout, &stderr
+ err := cmd.Run()
+
+ code := 0
+ if ee, ok := err.(*exec.ExitError); ok {
+ code = ee.ExitCode()
+ } else if err != nil {
+ t.Fatalf("run helper: %v", err)
+ }
+ if code != tc.wantCode {
+ t.Errorf("exit code = %d, want %d (stderr: %s)", code, tc.wantCode, stderr.String())
+ }
+ stream, where := stdout.String(), "stdout"
+ if tc.wantStderr {
+ stream, where = stderr.String(), "stderr"
+ }
+ if !strings.Contains(stream, tc.wantText) {
+ t.Errorf("%s = %q, want it to contain %q", where, stream, tc.wantText)
+ }
+ })
+ }
+}
+
+// TestCheckSubcommandHelper is not a real test: it is the subprocess body for
+// TestCheckSubcommandExits, and does nothing unless that env var is set.
+func TestCheckSubcommandHelper(t *testing.T) {
+ arg, ok := os.LookupEnv("LCHAT_SUBCOMMAND_HELPER")
+ if !ok {
+ t.Skip("helper process only")
+ }
+ checkSubcommand(arg)
+ // Only reached for a flag — the case that must still start an instance.
+ os.Stdout.WriteString("FELL-THROUGH\n")
+ os.Exit(0)
+}