diff --git a/cli/src/cmd/codingbooth/help.go b/cli/src/cmd/codingbooth/help.go index 648d9543..c6dac35d 100644 --- a/cli/src/cmd/codingbooth/help.go +++ b/cli/src/cmd/codingbooth/help.go @@ -413,16 +413,21 @@ func showHelpShell() { USAGE: %s shell [options] [name] OPTIONS: - --name Container name - --shell Shell to launch (default: bash) - --dir Starting directory (default: /home/coder/code) - --run Run the booth first if it is not already running - --keep-alive With --run, leave the booth running afterwards - -e Set environment variable (repeatable) - --envfile Load environment variables from a file + --name Container name + --shell Shell to launch (default: bash) + --dir Starting directory (default: /home/coder/code) + --run Run the booth first if it is not already running + --keep-alive With --run, leave the booth running afterwards + --port + With --run, host port when creating a missing booth + --accept-existing Connect even if create flags (e.g. --port) do not match + -e Set environment variable (repeatable) + --envfile Load environment variables from a file A booth brought up by --run is stopped again when you disconnect, unless --keep-alive is given. A booth that was already running is never stopped. +Create-intent flags like --port apply only when a booth is created; against an +existing booth a mismatch fails unless --accept-existing is set. EXAMPLES: %s shell myproject @@ -431,7 +436,9 @@ EXAMPLES: %s shell myproject -e DEBUG=1 %s shell myproject --run %s shell myproject --run --keep-alive -`, s, s, s, s, s, s, s, s) + %s shell myproject --run --port 9000 + %s shell myproject --port 9000 --accept-existing +`, s, s, s, s, s, s, s, s, s, s) } func showHelpExec() { @@ -441,17 +448,22 @@ func showHelpExec() { USAGE: %s exec [options] [name] -- OPTIONS: - --name Container name - --dir Working directory (default: /home/coder/code) - -it Force interactive mode with TTY - --run Run the booth first if it is not already running - --keep-alive With --run, leave the booth running afterwards - -e Set environment variable (repeatable) - --envfile Load environment variables from a file + --name Container name + --dir Working directory (default: /home/coder/code) + -it Force interactive mode with TTY + --run Run the booth first if it is not already running + --keep-alive With --run, leave the booth running afterwards + --port + With --run, host port when creating a missing booth + --accept-existing Connect even if create flags (e.g. --port) do not match + -e Set environment variable (repeatable) + --envfile Load environment variables from a file The exit code of the executed command is forwarded to the caller. A booth brought up by --run is stopped again when the command finishes, unless --keep-alive is given. A booth that was already running is never stopped. +Create-intent flags like --port apply only when a booth is created; against an +existing booth a mismatch fails unless --accept-existing is set. EXAMPLES: %s exec myproject -- make test @@ -459,7 +471,9 @@ EXAMPLES: %s exec myproject --dir /tmp -- ls %s exec myproject --run -- make test %s exec myproject --run --keep-alive -- make test -`, s, s, s, s, s, s, s) + %s exec myproject --run --port 9000 -- make test + %s exec myproject --port 9000 --accept-existing -- make test +`, s, s, s, s, s, s, s, s, s) } func showHelpEmitDockerfile() { diff --git a/cli/src/pkg/lifecycle/connect.go b/cli/src/pkg/lifecycle/connect.go index 927d3896..d584349e 100644 --- a/cli/src/pkg/lifecycle/connect.go +++ b/cli/src/pkg/lifecycle/connect.go @@ -34,6 +34,14 @@ func (f *stringSliceFlag) Set(value string) error { return nil } +// connectCreateOpts holds create-time run overrides accepted by shell/exec. +// These apply only when --run creates a missing booth; against an existing booth +// they are asserted (fail by default) unless --accept-existing is set. +type connectCreateOpts struct { + port string // --port (empty if unset) + acceptExisting bool // --accept-existing: connect despite create-intent mismatch +} + // Shell opens a new interactive shell inside a running booth container. func Shell(args []string, stderr io.Writer) error { positional, flags := extractPositionalAndFlags(args) @@ -45,6 +53,8 @@ func Shell(args []string, stderr io.Writer) error { envfile := flagSet.String("envfile", "", "Load environment variables from a file") run := flagSet.Bool("run", false, "Run the booth first if it is not already running") keepAlive := flagSet.Bool("keep-alive", false, "Keep a booth started by --run running afterwards") + port := flagSet.String("port", "", "With --run, host port for a newly created booth (number, NEXT, or RANDOM)") + acceptExisting := flagSet.Bool("accept-existing", false, "Connect to an existing booth even if create flags (e.g. --port) do not match") var envVars stringSliceFlag flagSet.Var(&envVars, "e", "Set environment variable (repeatable)") flagSet.SetOutput(stderr) @@ -53,7 +63,8 @@ func Shell(args []string, stderr io.Writer) error { return commandExit(2, "") } - target, cleanup, err := resolveConnectTarget(*name, positional, *run, *keepAlive, stderr) + create := connectCreateOpts{port: *port, acceptExisting: *acceptExisting} + target, cleanup, err := resolveConnectTarget(*name, positional, *run, *keepAlive, create, stderr) if err != nil { return err } @@ -94,6 +105,8 @@ func Exec(args []string, stderr io.Writer) error { envfile := flagSet.String("envfile", "", "Load environment variables from a file") run := flagSet.Bool("run", false, "Run the booth first if it is not already running") keepAlive := flagSet.Bool("keep-alive", false, "Keep a booth started by --run running afterwards") + port := flagSet.String("port", "", "With --run, host port for a newly created booth (number, NEXT, or RANDOM)") + acceptExisting := flagSet.Bool("accept-existing", false, "Connect to an existing booth even if create flags (e.g. --port) do not match") var envVars stringSliceFlag flagSet.Var(&envVars, "e", "Set environment variable (repeatable)") flagSet.SetOutput(stderr) @@ -106,7 +119,8 @@ func Exec(args []string, stderr io.Writer) error { return commandExit(1, "Error: no command specified. Usage: booth exec -- ") } - target, cleanup, err := resolveConnectTarget(*name, positional, *run, *keepAlive, stderr) + create := connectCreateOpts{port: *port, acceptExisting: *acceptExisting} + target, cleanup, err := resolveConnectTarget(*name, positional, *run, *keepAlive, create, stderr) if err != nil { return err } @@ -147,13 +161,17 @@ const ( // run=true a stopped booth is started and a missing booth is created with // `booth run` (in daemon mode) before connecting. // +// Create-intent flags (e.g. --port) are applied only when a new booth is created. +// Against an existing booth they are asserted: a mismatch fails by default, or +// connects with a warning when --accept-existing is set. +// // When --run had to change the booth's state to connect (started a stopped one // or created a new one) and keepAlive is false, the cleanup stops the booth // again so the session leaves no trace: a freshly-created `--rm` booth is // removed, and a pre-existing keep-alive booth returns to stopped. A booth that // was already running, or keepAlive=true, yields a no-op cleanup. The returned // error is already a commandError. -func resolveConnectTarget(name string, positional []string, run, keepAlive bool, stderr io.Writer) (managedContainer, func(), error) { +func resolveConnectTarget(name string, positional []string, run, keepAlive bool, create connectCreateOpts, stderr io.Writer) (managedContainer, func(), error) { noCleanup := func() {} containers, err := managedContainers(false) @@ -166,6 +184,14 @@ func resolveConnectTarget(name string, positional []string, run, keepAlive bool, return managedContainer{}, noCleanup, commandExit(1, err.Error()) } + // Create-intent flags only reconfigure a missing booth. Against an existing + // one they are a contract: refuse on mismatch unless the user opts out. + if action == connectUse || action == connectStart { + if err := checkCreateIntentAgainstExisting(target, create, stderr); err != nil { + return managedContainer{}, noCleanup, err + } + } + switch action { case connectUse: // Already running. We did not bring it up, but it may be an ephemeral @@ -190,7 +216,7 @@ func resolveConnectTarget(name string, positional []string, run, keepAlive bool, } case connectRun: - created, err := runConnectTarget(name, positional, keepAlive, stderr) + created, err := runConnectTarget(name, positional, keepAlive, create, stderr) if err != nil { return managedContainer{}, noCleanup, err } @@ -204,6 +230,62 @@ func resolveConnectTarget(name string, positional []string, run, keepAlive bool, return target, cleanup, nil } +// checkCreateIntentAgainstExisting compares explicit create-intent flags against +// an already-existing booth. Returns a commandError on mismatch unless +// accept-existing is set (then warns and returns nil). Non-comparable values +// such as --port NEXT/RANDOM are not asserted. +func checkCreateIntentAgainstExisting(target managedContainer, create connectCreateOpts, stderr io.Writer) error { + mismatches := createIntentMismatches(target, create) + if len(mismatches) == 0 { + return nil + } + + detail := strings.Join(mismatches, "; ") + if create.acceptExisting { + fmt.Fprintf(stderr, "Warning: booth %q does not match create flags (%s); connecting anyway (--accept-existing).\n", target.Name, detail) + return nil + } + + return commandExit(1, fmt.Sprintf( + "Error: booth %q does not match create flags (%s).\n"+ + " Refusing to connect so the command does not run against the wrong environment.\n"+ + " Use --accept-existing to connect anyway, or remove the booth and re-run with the desired flags.", + target.Name, detail, + )) +} + +// createIntentMismatches returns human-readable mismatch descriptions for +// explicit create-intent flags that disagree with the existing booth. Pure for +// unit tests. +func createIntentMismatches(target managedContainer, create connectCreateOpts) []string { + var mismatches []string + + if create.port != "" && isComparablePort(create.port) { + actual := strings.TrimSpace(target.Port) + if actual == "" { + mismatches = append(mismatches, fmt.Sprintf("--port %s requested but booth has no published host port", create.port)) + } else if actual != create.port { + mismatches = append(mismatches, fmt.Sprintf("--port %s requested but booth is on port %s", create.port, actual)) + } + } + + return mismatches +} + +// isComparablePort reports whether a --port value can be asserted against a +// live booth. Symbolic NEXT/RANDOM only apply on create. +func isComparablePort(port string) bool { + if port == "" { + return false + } + upper := strings.ToUpper(port) + if upper == "NEXT" || upper == "RANDOM" { + return false + } + // Numeric (and any other explicit token users might pass) is comparable. + return true +} + // registerConnectSession wires this shell/exec session into the booth's // connection count and returns the cleanup to defer. // @@ -291,7 +373,9 @@ func connectPlan(containers []managedContainer, name string, positional []string // container. It shells out to the same executable so the full run pipeline // (config, image build, ports, …) is reused verbatim. When keepAlive is set the // new booth is created with --keep-alive so it persists across a later stop. -func runConnectTarget(name string, positional []string, keepAlive bool, stderr io.Writer) (managedContainer, error) { +// Create-intent flags (e.g. --port) are forwarded so a shell/exec --run create +// matches an equivalent booth run. +func runConnectTarget(name string, positional []string, keepAlive bool, create connectCreateOpts, stderr io.Writer) (managedContainer, error) { self, err := os.Executable() if err != nil { return managedContainer{}, commandExit(1, fmt.Sprintf("Error: failed to locate booth executable: %v", err)) @@ -302,13 +386,7 @@ func runConnectTarget(name string, positional []string, keepAlive bool, stderr i explicitName = positional[0] } - runArgs := []string{"run", "--daemon"} - if keepAlive { - runArgs = append(runArgs, "--keep-alive") - } - if explicitName != "" { - runArgs = append(runArgs, "--name", explicitName) - } + runArgs := buildConnectRunArgs(explicitName, keepAlive, create) fmt.Fprintf(stderr, "No running booth found; starting one with 'booth run'...\n") @@ -541,6 +619,23 @@ func buildExecFlags(interactive bool, dir string, envVars stringSliceFlag, envfi return execArgs } +// buildConnectRunArgs builds the argv for `booth run` when shell/exec creates a +// missing booth. Always daemon mode so the process returns and shell/exec can +// attach. Pure for unit tests. +func buildConnectRunArgs(explicitName string, keepAlive bool, create connectCreateOpts) []string { + runArgs := []string{"run", "--daemon"} + if keepAlive { + runArgs = append(runArgs, "--keep-alive") + } + if explicitName != "" { + runArgs = append(runArgs, "--name", explicitName) + } + if create.port != "" { + runArgs = append(runArgs, "--port", create.port) + } + return runArgs +} + // extractPositionalAndFlags separates positional arguments from flags. // Go's flag.Parse stops at the first positional arg, so flags after a positional // arg are never parsed. This function pulls out non-flag args (positional) and @@ -549,7 +644,7 @@ func buildExecFlags(interactive bool, dir string, envVars stringSliceFlag, envfi func extractPositionalAndFlags(args []string) (positional []string, flags []string) { knownValueFlags := map[string]bool{ "-e": true, "--name": true, "--shell": true, - "--dir": true, "--envfile": true, + "--dir": true, "--envfile": true, "--port": true, } for i := 0; i < len(args); i++ { @@ -557,7 +652,8 @@ func extractPositionalAndFlags(args []string) (positional []string, flags []stri if strings.HasPrefix(arg, "-") { flags = append(flags, arg) // If this flag takes a value, consume the next arg too - if knownValueFlags[arg] && i+1 < len(args) { + // (including --flag=value forms, which are a single token). + if !strings.Contains(arg, "=") && knownValueFlags[arg] && i+1 < len(args) { i++ flags = append(flags, args[i]) } diff --git a/cli/src/pkg/lifecycle/lifecycle.go b/cli/src/pkg/lifecycle/lifecycle.go index 0afa4a3e..339163d4 100644 --- a/cli/src/pkg/lifecycle/lifecycle.go +++ b/cli/src/pkg/lifecycle/lifecycle.go @@ -47,6 +47,13 @@ type inspectData struct { HostPort string `json:"HostPort"` } `json:"Ports"` } `json:"NetworkSettings"` + // HostConfig.PortBindings retains the configured host ports even when the + // container is stopped (NetworkSettings.Ports is often empty then). + HostConfig struct { + PortBindings map[string][]struct { + HostPort string `json:"HostPort"` + } `json:"PortBindings"` + } `json:"HostConfig"` } type commandError struct { @@ -436,10 +443,7 @@ func inspectManagedContainer(name string, flags docker.DockerFlags) (managedCont labels = map[string]string{} } - port := "" - if bindings, found := data.NetworkSettings.Ports["10000/tcp"]; found && len(bindings) > 0 { - port = bindings[0].HostPort - } + port := hostPortFromInspect(data) containerName := strings.TrimPrefix(data.Name, "/") if containerName == "" { @@ -463,6 +467,27 @@ func inspectManagedContainer(name string, flags docker.DockerFlags) (managedCont }, nil } +// hostPortFromInspect returns the host port mapped to the booth UI container +// port. Prefers live NetworkSettings, then HostConfig bindings (works for +// stopped containers). Checks both 10000/tcp (default) and 10443/tcp (public TLS). +func hostPortFromInspect(data inspectData) string { + for _, containerPort := range []string{"10000/tcp", "10443/tcp"} { + if bindings, found := data.NetworkSettings.Ports[containerPort]; found && len(bindings) > 0 { + if p := strings.TrimSpace(bindings[0].HostPort); p != "" { + return p + } + } + } + for _, containerPort := range []string{"10000/tcp", "10443/tcp"} { + if bindings, found := data.HostConfig.PortBindings[containerPort]; found && len(bindings) > 0 { + if p := strings.TrimSpace(bindings[0].HostPort); p != "" { + return p + } + } + } + return "" +} + func filterByState(containers []managedContainer, runningOnly bool, stoppedOnly bool) []managedContainer { if !runningOnly && !stoppedOnly { return containers diff --git a/cli/src/pkg/lifecycle/lifecycle_test.go b/cli/src/pkg/lifecycle/lifecycle_test.go index e710e9e5..9bac632d 100644 --- a/cli/src/pkg/lifecycle/lifecycle_test.go +++ b/cli/src/pkg/lifecycle/lifecycle_test.go @@ -193,3 +193,166 @@ func TestExtractPositionalAndFlagsRun(t *testing.T) { } } } + +func TestExtractPositionalAndFlagsPort(t *testing.T) { + // --port takes a value and must not swallow a later positional. + cases := []struct { + args []string + positional []string + flags []string + }{ + {[]string{"myproject", "--port", "9000"}, []string{"myproject"}, []string{"--port", "9000"}}, + {[]string{"--port", "9000", "myproject"}, []string{"myproject"}, []string{"--port", "9000"}}, + {[]string{"--port", "NEXT", "--run", "myproject"}, []string{"myproject"}, []string{"--port", "NEXT", "--run"}}, + {[]string{"--accept-existing", "myproject"}, []string{"myproject"}, []string{"--accept-existing"}}, + } + + for _, tc := range cases { + positional, flags := extractPositionalAndFlags(tc.args) + if len(positional) != len(tc.positional) || (len(positional) > 0 && positional[0] != tc.positional[0]) { + t.Fatalf("extractPositionalAndFlags(%v) positional = %v, want %v", tc.args, positional, tc.positional) + } + if strings.Join(flags, " ") != strings.Join(tc.flags, " ") { + t.Fatalf("extractPositionalAndFlags(%v) flags = %v, want %v", tc.args, flags, tc.flags) + } + } +} + +func TestIsComparablePort(t *testing.T) { + if !isComparablePort("9000") { + t.Fatal("numeric port should be comparable") + } + if isComparablePort("NEXT") || isComparablePort("next") || isComparablePort("RANDOM") { + t.Fatal("NEXT/RANDOM should not be comparable") + } + if isComparablePort("") { + t.Fatal("empty port should not be comparable") + } +} + +func TestCreateIntentMismatches(t *testing.T) { + target := managedContainer{Name: "demo", Port: "8080"} + + // No create flags → no mismatch + if got := createIntentMismatches(target, connectCreateOpts{}); len(got) != 0 { + t.Fatalf("no flags: mismatches = %v, want none", got) + } + + // Matching port → no mismatch + if got := createIntentMismatches(target, connectCreateOpts{port: "8080"}); len(got) != 0 { + t.Fatalf("matching port: mismatches = %v, want none", got) + } + + // NEXT/RANDOM → not asserted + if got := createIntentMismatches(target, connectCreateOpts{port: "NEXT"}); len(got) != 0 { + t.Fatalf("NEXT: mismatches = %v, want none", got) + } + if got := createIntentMismatches(target, connectCreateOpts{port: "RANDOM"}); len(got) != 0 { + t.Fatalf("RANDOM: mismatches = %v, want none", got) + } + + // Wrong port → mismatch + got := createIntentMismatches(target, connectCreateOpts{port: "9000"}) + if len(got) != 1 || !strings.Contains(got[0], "9000") || !strings.Contains(got[0], "8080") { + t.Fatalf("wrong port: mismatches = %v", got) + } + + // No published port on booth but explicit numeric request → mismatch + noPort := managedContainer{Name: "term"} + got = createIntentMismatches(noPort, connectCreateOpts{port: "9000"}) + if len(got) != 1 || !strings.Contains(got[0], "no published host port") { + t.Fatalf("empty port: mismatches = %v", got) + } +} + +func TestCheckCreateIntentAgainstExisting(t *testing.T) { + target := managedContainer{Name: "demo", Port: "8080"} + var stderr strings.Builder + + // Fail by default on mismatch + err := checkCreateIntentAgainstExisting(target, connectCreateOpts{port: "9000"}, &stderr) + if err == nil { + t.Fatal("expected mismatch error") + } + if code := ExitCode(err); code != 1 { + t.Fatalf("ExitCode = %d, want 1", code) + } + if !strings.Contains(err.Error(), "--accept-existing") { + t.Fatalf("error should mention --accept-existing: %v", err) + } + + // --accept-existing: warn and continue + stderr.Reset() + err = checkCreateIntentAgainstExisting(target, connectCreateOpts{port: "9000", acceptExisting: true}, &stderr) + if err != nil { + t.Fatalf("accept-existing should not error: %v", err) + } + if !strings.Contains(stderr.String(), "Warning:") || !strings.Contains(stderr.String(), "accept-existing") { + t.Fatalf("expected warning on stderr, got %q", stderr.String()) + } + + // Match: quiet success + stderr.Reset() + err = checkCreateIntentAgainstExisting(target, connectCreateOpts{port: "8080"}, &stderr) + if err != nil { + t.Fatalf("matching port should not error: %v", err) + } + if stderr.Len() != 0 { + t.Fatalf("matching port should be quiet, got %q", stderr.String()) + } +} + +func TestBuildConnectRunArgs(t *testing.T) { + got := buildConnectRunArgs("demo", true, connectCreateOpts{port: "9000"}) + want := []string{"run", "--daemon", "--keep-alive", "--name", "demo", "--port", "9000"} + if strings.Join(got, " ") != strings.Join(want, " ") { + t.Fatalf("buildConnectRunArgs = %v, want %v", got, want) + } + + got = buildConnectRunArgs("", false, connectCreateOpts{}) + want = []string{"run", "--daemon"} + if strings.Join(got, " ") != strings.Join(want, " ") { + t.Fatalf("minimal buildConnectRunArgs = %v, want %v", got, want) + } + + got = buildConnectRunArgs("", false, connectCreateOpts{port: "NEXT"}) + want = []string{"run", "--daemon", "--port", "NEXT"} + if strings.Join(got, " ") != strings.Join(want, " ") { + t.Fatalf("NEXT port buildConnectRunArgs = %v, want %v", got, want) + } +} + +func TestHostPortFromInspect(t *testing.T) { + // Live NetworkSettings wins + data := inspectData{} + data.NetworkSettings.Ports = map[string][]struct { + HostPort string `json:"HostPort"` + }{ + "10000/tcp": {{HostPort: "9000"}}, + } + if got := hostPortFromInspect(data); got != "9000" { + t.Fatalf("NetworkSettings port = %q, want 9000", got) + } + + // Stopped: fall back to HostConfig + data = inspectData{} + data.HostConfig.PortBindings = map[string][]struct { + HostPort string `json:"HostPort"` + }{ + "10000/tcp": {{HostPort: "8080"}}, + } + if got := hostPortFromInspect(data); got != "8080" { + t.Fatalf("HostConfig port = %q, want 8080", got) + } + + // Public TLS container port + data = inspectData{} + data.NetworkSettings.Ports = map[string][]struct { + HostPort string `json:"HostPort"` + }{ + "10443/tcp": {{HostPort: "443"}}, + } + if got := hostPortFromInspect(data); got != "443" { + t.Fatalf("public TLS port = %q, want 443", got) + } +} diff --git a/docs/BOOTH_CONNECT.md b/docs/BOOTH_CONNECT.md index 3888f4fe..21b9f644 100644 --- a/docs/BOOTH_CONNECT.md +++ b/docs/BOOTH_CONNECT.md @@ -22,6 +22,9 @@ Back to [README](../README.md) - [exec](#exec) - [Target Resolution](#target-resolution) - [Run the booth if it is not running](#run-the-booth-if-it-is-not-running) + - [Cleanup](#cleanup-the-booth-is-brought-back-down-afterwards) + - [Multiple connections](#multiple-connections-are-reference-counted) +- [Create flags vs an existing booth](#create-flags-vs-an-existing-booth) - [Common Workflows](#common-workflows) - [Differences from docker exec](#differences-from-docker-exec) @@ -29,20 +32,22 @@ Back to [README](../README.md) ## Overview -Both commands operate on a **running** booth container. Under the hood they use `docker exec`, so there is nothing to install and no port to expose. +Both commands connect into a booth with `docker exec` — nothing to install, no SSH, no extra ports. | Command | Purpose | Interactive | Requires `--` | |---------|--------------------------------------|:--------------:|:-------------:| | `shell` | Open a new interactive shell session | Yes | No | | `exec` | Run a one-off command | No (by default)| Yes | -If the target booth is not running, both commands error by default. Pass `--run` to bring it up first — see [Run the booth if it is not running](#run-the-booth-if-it-is-not-running). +By default the target booth must already be **running**. Pass **`--run`** to start or create it first — see [Run the booth if it is not running](#run-the-booth-if-it-is-not-running). + +When creating a booth, create-time flags such as **`--port`** are forwarded to `booth run`. Against an existing booth those flags are a **contract**: a mismatch fails unless you pass **`--accept-existing`** — see [Create flags vs an existing booth](#create-flags-vs-an-existing-booth). --- ## `shell` -Open a new interactive shell inside a running booth. +Open a new interactive shell inside a booth. ```bash ./booth shell myproject @@ -53,36 +58,40 @@ The shell launched is the default shell configured for the `coder` user inside t ### Options ```bash -./booth shell myproject --shell zsh # use a specific shell -./booth shell myproject --dir /tmp # start in a specific directory -./booth shell myproject -e DEBUG=1 # set an environment variable -./booth shell myproject --envfile .env # load variables from a file -./booth shell myproject --run # run the booth first if not running -./booth shell myproject --run --keep-alive # ...and leave it running afterwards +./booth shell myproject --shell zsh # use a specific shell +./booth shell myproject --dir /tmp # start in a specific directory +./booth shell myproject -e DEBUG=1 # set an environment variable +./booth shell myproject --envfile .env # load variables from a file +./booth shell myproject --run # run the booth first if not running +./booth shell myproject --run --keep-alive # ...and leave it running afterwards +./booth shell myproject --run --port 9000 # create (if needed) on host port 9000 +./booth shell myproject --port 9000 --accept-existing # attach even if port differs ``` -| Flag | Description | -|--------------------|-----------------------------------------------------------------------| -| `--shell ` | Shell to launch (default: container's default shell) | -| `--dir ` | Starting directory inside the container (default: `/home/coder/code`) | -| `--run` | Run the booth first if it is not already running | -| `--keep-alive` | With `--run`, leave the booth running after you disconnect | -| `-e ` | Set environment variable for the session | -| `--envfile ` | Load environment variables from a file | -| `--name ` | Target container by name | +| Flag | Description | +|------------------------------|-----------------------------------------------------------------------------| +| `--shell ` | Shell to launch (default: container's default shell) | +| `--dir ` | Starting directory inside the container (default: `/home/coder/code`) | +| `--run` | Run the booth first if it is not already running | +| `--keep-alive` | With `--run`, leave the booth running after you disconnect | +| `--port ` | Host port when **creating** a missing booth; asserted against existing ones | +| `--accept-existing` | Connect even if create flags (e.g. `--port`) do not match the booth | +| `-e ` | Set environment variable for the session | +| `--envfile ` | Load environment variables from a file | +| `--name ` | Target container by name | ### What you get - A fully interactive terminal session with TTY and stdin attached. - The session runs as the `coder` user, in the `/home/coder/code` directory — the same context as the original terminal. - Environment variables, installed tools, and filesystem state are shared with the running container. -- Exiting the shell (Ctrl+D or `exit`) closes only that session; the booth keeps running. +- Exiting the shell (Ctrl+D or `exit`) closes only that session; the booth keeps running (unless this session brought up an ephemeral booth with `--run` and no `--keep-alive` — see [Cleanup](#cleanup-the-booth-is-brought-back-down-afterwards)). --- ## `exec` -Run a command inside a running booth and return the result. +Run a command inside a booth and return the result. ```bash ./booth exec myproject -- make test @@ -95,23 +104,27 @@ Everything after `--` is executed inside the container. The exit code is forward ### Options ```bash -./booth exec myproject -it -- bash # force interactive + TTY -./booth exec myproject -e FOO=bar -- env # set an environment variable -./booth exec myproject --envfile .env -- env # load variables from a file -./booth exec myproject --dir /tmp -- ls # run command in a specific directory -./booth exec myproject --run -- make test # run the booth first if not running -./booth exec myproject --run --keep-alive -- make test # ...and leave it running +./booth exec myproject -it -- bash # force interactive + TTY +./booth exec myproject -e FOO=bar -- env # set an environment variable +./booth exec myproject --envfile .env -- env # load variables from a file +./booth exec myproject --dir /tmp -- ls # run command in a specific directory +./booth exec myproject --run -- make test # run the booth first if not running +./booth exec myproject --run --keep-alive -- make test # ...and leave it running +./booth exec myproject --run --port 9000 -- make test # create (if needed) on port 9000 +./booth exec myproject --port 9000 --accept-existing -- make test ``` -| Flag | Description | -|-------------------|----------------------------------------------------------------------| -| `-it` | Force interactive mode with TTY (default: non-interactive) | -| `--run` | Run the booth first if it is not already running | -| `--keep-alive` | With `--run`, leave the booth running after the command finishes | -| `-e ` | Set environment variable for the command | -| `--envfile `| Load environment variables from a file | -| `--dir ` | Working directory inside the container (default: `/home/coder/code`) | -| `--name ` | Target container by name | +| Flag | Description | +|------------------------------|-----------------------------------------------------------------------------| +| `-it` | Force interactive mode with TTY (default: non-interactive) | +| `--run` | Run the booth first if it is not already running | +| `--keep-alive` | With `--run`, leave the booth running after the command finishes | +| `--port ` | Host port when **creating** a missing booth; asserted against existing ones | +| `--accept-existing` | Connect even if create flags (e.g. `--port`) do not match the booth | +| `-e ` | Set environment variable for the command | +| `--envfile ` | Load environment variables from a file | +| `--dir ` | Working directory inside the container (default: `/home/coder/code`) | +| `--name ` | Target container by name | ### Exit codes @@ -140,7 +153,7 @@ Both `shell` and `exec` resolve the target container using the same priority as cd ~/projects/app && ./booth shell # default from current directory ``` -If the target container is not running, the command exits with an error and suggests using `booth run` first — unless you pass `--run`. +If the target container is not running, the command exits with an error — unless you pass `--run`. --- @@ -149,23 +162,30 @@ If the target container is not running, the command exits with an error and sugg By default `shell` and `exec` require the booth to already be running. Pass `--run` to bring it up automatically before connecting: ```bash -./booth shell myproject --run # run (if needed), then open a shell +./booth shell myproject --run # run (if needed), then open a shell ./booth exec myproject --run -- make test # run (if needed), then run a command +./booth exec myproject --run --port 9000 -- make test ``` When `--run` is given, the booth is made available in whatever way is needed: -- If the booth is **already running**, it is used as-is — nothing is restarted. -- If a **stopped** container exists (e.g. a `--keep-alive` booth that was stopped), it is started (equivalent to `booth start`). -- If **no container exists**, a new booth is created from the current workspace with `booth run` in daemon mode, exactly as if you had run `booth` here yourself. +| Booth state | What happens | +|-------------|--------------| +| **Already running** | Used as-is — nothing is restarted | +| **Stopped** (e.g. a `--keep-alive` booth that was stopped) | Started (equivalent to `booth start`) | +| **Does not exist** | Created from the current workspace with `booth run --daemon` | + +On create, create-time flags such as **`--port`** and **`--name`** are forwarded to that run (along with **`--keep-alive`** when set), so the booth matches an equivalent `booth run` invocation. -In every case a short note is printed to **stderr** and the new booth's startup output is also sent to stderr, so `exec`'s **stdout stays clean** for scripting. `shell`/`exec` then wait for the booth's `coder` user alignment to finish before connecting, so the first command never races container startup. +A short note is printed to **stderr**, and the new booth's startup output also goes to stderr, so `exec`'s **stdout stays clean** for scripting. `shell`/`exec` then wait for the booth's `coder` user alignment to finish before connecting, so the first command never races container startup. > **Why this matters:** a normal booth that is stopped is *removed* (only `--keep-alive` booths persist as stopped containers). So "the booth is not running" usually means "there is no container" — and `--run` recreates it from the workspace config rather than failing. +Because running a booth is a side effect (it can build an image and allocate ports), `--run` is opt-in: omit it and a non-running booth remains an error, which keeps `booth exec` predictable in scripts and CI. + ### Cleanup: the booth is brought back down afterwards -A booth that `--run` had to bring up does **not** outlive your session. When you disconnect (the shell exits, or the command finishes), the booth is returned to the state it was in before — so a `--run` session leaves no trace: +A booth that `--run` had to bring up does **not** outlive your session. When you disconnect (the shell exits, or the command finishes), the booth is returned to the state it was in before — so a default `--run` session leaves no trace: | Before connecting | After disconnecting (default) | |--------------------------|----------------------------------------| @@ -182,11 +202,51 @@ Pass **`--keep-alive`** to opt out and leave the booth running after you disconn A booth created with `--run --keep-alive` is created as a `--keep-alive` booth, so it persists across a later `booth stop` just like one you launched directly. -#### Multiple connections are reference-counted +### Multiple connections are reference-counted If you open several `--run` sessions on the same booth (e.g. two `booth shell --run` in different terminals), the booth is only brought down when the **last** one disconnects. An earlier session exiting will not pull the booth out from under the others. Passing `--keep-alive` from any session promotes the booth to persistent, so none of the sessions will stop it. -Because running a booth is a side effect (it can build an image and allocate ports), `--run` is opt-in: omit it and a non-running booth remains an error, which keeps `booth exec` predictable in scripts and CI. +--- + +## Create flags vs an existing booth + +Flags that configure a **new** booth only reconfigure when no container exists. Against a **running or stopped** booth they are a **contract** — fail by default so scripts do not run against the wrong environment. + +Today the create flag on `shell` / `exec` is: + +| Flag | On create (`--run`, no container) | Against an existing booth | +|------|----------------------------------|---------------------------| +| `--port ` | Forwarded to `booth run` | Must match the booth's published host port | +| `--port NEXT` / `RANDOM` | Forwarded to `booth run` | **Not compared** (only meaningful on create) | +| `--name` / positional | Name used for create and lookup | Target identity (not a mismatch check) | +| `--keep-alive` | Creates a keep-alive booth | Session policy only — never a mismatch | + +### Mismatch policy + +| Situation | Default | With `--accept-existing` | +|-----------|---------|--------------------------| +| Explicit create flag **matches** the booth | Connect | Connect | +| Explicit create flag **mismatches** (e.g. `--port 9000` but booth is on `8080`) | **Error** — refuse to connect | Connect with a **warning** on stderr | +| Symbolic port (`NEXT` / `RANDOM`) | Not compared | Not compared | +| No create flags | Connect | Connect | + +Mismatch checks apply whenever you connect to an **existing** booth — with or without `--run`. They do not reconfigure a live container; they only decide whether connecting is safe. + +```bash +# Create on 9000 if missing; fail if myproject already runs on another port +./booth exec myproject --run --port 9000 -- make test + +# Attach anyway when the existing booth differs +./booth exec myproject --port 9000 --accept-existing -- make test +``` + +Example error (stderr, exit code 1): + +```text +Error: booth "myproject" does not match create flags (--port 9000 requested but booth is on port 8080). + Refusing to connect so the command does not run against the wrong environment. + Use --accept-existing to connect anyway, or remove the booth and re-run with the desired flags. +``` --- @@ -222,6 +282,28 @@ make build-all # takes a while... ./booth exec myproject -- python3 --version ``` +### One-shot: run if needed, then exec + +```bash +# From the project workspace — create/start as needed, tear down after +./booth exec --run -- make test + +# Leave the booth up for later shells +./booth exec --run --keep-alive -- make test +./booth shell --run --keep-alive +``` + +### Create on a fixed port (or refuse a mismatch) + +```bash +# Prefer host port 9000 when this command creates the booth +./booth exec myproject --run --port 9000 -- make test + +# Scripts that must not silently use the wrong port fail by default; +# opt in only when attaching to whatever is already there is intentional: +./booth exec myproject --port 9000 --accept-existing -- make test +``` + ### Use with daemon booths ```bash @@ -249,5 +331,6 @@ make build-all # takes a while... | Interactive shell | `docker exec -it bash` | `booth shell ` | | Environment | Manual `-e` flags | Inherits booth environment | | Env file | `--env-file ` | `--envfile ` | +| Auto start/create | Not available | `--run` (+ optional create flags / cleanup) | > **Tip:** If you need raw `docker exec` capabilities not exposed by these commands, you can always fall back to Docker directly. Use `booth list --name-only` to get the container name. diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 4fe1c1d7..8a4e358e 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -4,6 +4,8 @@ This file contains a list of changes for each released version. ## Unreleased +- **`shell` / `exec` accept create-time `--port` and fail on mismatch by default.** With `--run`, a missing booth is created via `booth run --daemon`, and `--port` (number, `NEXT`, or `RANDOM`) is forwarded so the new booth gets the host UI port you asked for — same for `--name` / `--keep-alive` as before. Against an **existing** booth, an explicit numeric `--port` is a contract: if it does not match the published host port, the command **refuses to connect** (so scripts do not run against the wrong environment). Pass **`--accept-existing`** to connect anyway with a warning on stderr. Symbolic `--port NEXT` / `RANDOM` are only applied on create and are not compared. Unit tests cover the mismatch rules and run-arg construction; complex test `tests/complex/test-connect-run-port/` exercises create, match, refuse, accept-existing, `NEXT`, and ephemeral teardown. See `docs/BOOTH_CONNECT.md`. + - **A booth no longer generates a config docker refuses to start.** Docker cannot bind one host port twice — it fails the container with `address already in use` — and run-args grew duplicates in two ordinary ways. A template's short-form `-p` plus the user's long-form `--publish` for the diff --git a/tests/complex/test-connect-run-port/test--connect-run-port.sh b/tests/complex/test-connect-run-port/test--connect-run-port.sh new file mode 100755 index 00000000..5bf432a7 --- /dev/null +++ b/tests/complex/test-connect-run-port/test--connect-run-port.sh @@ -0,0 +1,226 @@ +#!/bin/bash +# Copyright 2025-2026 : Nawa Manusitthipol +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. + +# ----------------------------------------------------------------------------- +# Test: shell/exec --run create flags (--port) and mismatch policy +# +# Covers: +# - exec --run --port creates a booth with the requested host UI port +# - matching --port against an existing booth connects +# - mismatched numeric --port refuses by default +# - --accept-existing connects despite mismatch (with a warning on stderr) +# - symbolic --port NEXT is not compared against an existing booth +# - ephemeral exec --run --port tears the booth down afterwards +# ----------------------------------------------------------------------------- + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +cd "$SCRIPT_DIR" + +source ../../common--source.sh + +FAILED=0 +NAME="connect-port-$RANDOM-$RANDOM" + +cleanup() { + docker rm -f "$NAME" >/dev/null 2>&1 || true +} +trap cleanup EXIT + +is_port_free() { + local port="$1" + if command -v lsof >/dev/null 2>&1; then + ! lsof -iTCP:"$port" -sTCP:LISTEN -Pn 2>/dev/null | grep -q . + elif command -v ss >/dev/null 2>&1; then + ! ss -ltn "( sport = :$port )" 2>/dev/null | grep -q ":$port" + else + ! (command -v nc >/dev/null 2>&1 && nc -z 127.0.0.1 "$port" >/dev/null 2>&1) + fi +} + +random_free_port() { + local p + for _ in {1..100}; do + p=$((50000 + RANDOM % 10000)) + if is_port_free "$p"; then + echo "$p" + return 0 + fi + done + return 1 +} + +host_port_10000() { + docker inspect -f '{{(index (index .HostConfig.PortBindings "10000/tcp") 0).HostPort}}' "$1" 2>/dev/null || true +} + +state_of() { + local s + s="$(docker inspect -f '{{.State.Status}}' "$1" 2>/dev/null)" || s="" + s="$(printf '%s' "$s" | tr -d '[:space:]')" + if [[ -n "$s" ]]; then + printf '%s\n' "$s" + else + printf '%s\n' "missing" + fi +} + +wait_coder_ready() { + local name="$1" + local i + for i in {1..60}; do + if docker inspect --format '{{.State.Running}}' "$name" 2>/dev/null | grep -q true; then + if docker exec "$name" id coder >/dev/null 2>&1; then + return 0 + fi + fi + sleep 1 + done + return 1 +} + +PORT_A="$(random_free_port)" +PORT_B="$(random_free_port)" +while [[ "$PORT_B" == "$PORT_A" ]]; do + PORT_B="$(random_free_port)" +done + +# --------------------------------------------------------------------------- +# 1) exec --run --keep-alive --port creates a booth on that host port +# --------------------------------------------------------------------------- +# Tear down any leftover with the same name so --run takes the create path. +docker rm -f "$NAME" >/dev/null 2>&1 || true + +set +e +CREATE_OUT="$(run_coding_booth exec --name "$NAME" --run --keep-alive --port "$PORT_A" -- whoami 2>&1)" +CREATE_EXIT=$? +set -e + +if [[ $CREATE_EXIT -eq 0 ]] \ + && [[ "$CREATE_OUT" == *"coder"* ]] \ + && [[ "$(state_of "$NAME")" == "running" ]] \ + && [[ "$(host_port_10000 "$NAME")" == "$PORT_A" ]]; then + print_test_result "true" "$0" "1" "exec --run --port creates booth on requested host port" +else + print_test_result "false" "$0" "1" "exec --run --port should create on port $PORT_A" + echo " exit=$CREATE_EXIT state=$(state_of "$NAME") port=$(host_port_10000 "$NAME")" + echo " output: $CREATE_OUT" + FAILED=$((FAILED + 1)) +fi + +# Ensure coder is ready for subsequent execs (create path already waited, but be safe). +if [[ "$(state_of "$NAME")" == "running" ]]; then + wait_coder_ready "$NAME" || true +fi + +# --------------------------------------------------------------------------- +# 2) matching --port against existing booth connects +# --------------------------------------------------------------------------- +set +e +MATCH_OUT="$(run_coding_booth exec --name "$NAME" --port "$PORT_A" -- whoami 2>&1)" +MATCH_EXIT=$? +set -e + +if [[ $MATCH_EXIT -eq 0 ]] && [[ "$MATCH_OUT" == *"coder"* ]]; then + print_test_result "true" "$0" "2" "matching --port connects to existing booth" +else + print_test_result "false" "$0" "2" "matching --port should connect (exit=$MATCH_EXIT out=$MATCH_OUT)" + FAILED=$((FAILED + 1)) +fi + +# --------------------------------------------------------------------------- +# 3) mismatched numeric --port refuses by default +# --------------------------------------------------------------------------- +set +e +MISMATCH_ERR="$(run_coding_booth exec --name "$NAME" --port "$PORT_B" -- whoami 2>&1)" +MISMATCH_EXIT=$? +set -e + +if [[ $MISMATCH_EXIT -ne 0 ]] \ + && grep -q "does not match create flags" <<<"$MISMATCH_ERR" \ + && grep -q -- "--accept-existing" <<<"$MISMATCH_ERR"; then + print_test_result "true" "$0" "3" "mismatched --port refuses without --accept-existing" +else + print_test_result "false" "$0" "3" "mismatched --port should fail with accept-existing hint" + echo " exit=$MISMATCH_EXIT" + echo " output: $MISMATCH_ERR" + FAILED=$((FAILED + 1)) +fi + +# Booth must still be running after the refused connect. +if [[ "$(state_of "$NAME")" == "running" ]]; then + print_test_result "true" "$0" "4" "refused mismatch leaves existing booth running" +else + print_test_result "false" "$0" "4" "booth should still be running after refused mismatch" + FAILED=$((FAILED + 1)) +fi + +# --------------------------------------------------------------------------- +# 5) --accept-existing connects despite mismatch (warn on stderr) +# --------------------------------------------------------------------------- +set +e +# Capture stderr separately so we can assert the warning; stdout is the command. +ACCEPT_STDOUT="$(run_coding_booth exec --name "$NAME" --port "$PORT_B" --accept-existing -- whoami 2>/tmp/cb-connect-accept-$$.err)" +ACCEPT_EXIT=$? +ACCEPT_STDERR="$(cat /tmp/cb-connect-accept-$$.err 2>/dev/null || true)" +rm -f /tmp/cb-connect-accept-$$.err +set -e + +if [[ $ACCEPT_EXIT -eq 0 ]] \ + && [[ "$ACCEPT_STDOUT" == *"coder"* ]] \ + && grep -qi "warning" <<<"$ACCEPT_STDERR" \ + && grep -q "accept-existing" <<<"$ACCEPT_STDERR"; then + print_test_result "true" "$0" "5" "--accept-existing connects with mismatch warning" +else + print_test_result "false" "$0" "5" "--accept-existing should warn and connect" + echo " exit=$ACCEPT_EXIT stdout=$ACCEPT_STDOUT" + echo " stderr=$ACCEPT_STDERR" + FAILED=$((FAILED + 1)) +fi + +# --------------------------------------------------------------------------- +# 6) symbolic --port NEXT is not asserted against existing booth +# --------------------------------------------------------------------------- +set +e +NEXT_OUT="$(run_coding_booth exec --name "$NAME" --port NEXT -- whoami 2>&1)" +NEXT_EXIT=$? +set -e + +if [[ $NEXT_EXIT -eq 0 ]] && [[ "$NEXT_OUT" == *"coder"* ]]; then + print_test_result "true" "$0" "6" "--port NEXT against existing booth connects (not compared)" +else + print_test_result "false" "$0" "6" "--port NEXT should connect without mismatch (exit=$NEXT_EXIT out=$NEXT_OUT)" + FAILED=$((FAILED + 1)) +fi + +# --------------------------------------------------------------------------- +# 7) ephemeral exec --run --port: create, run, remove (no keep-alive) +# --------------------------------------------------------------------------- +docker rm -f "$NAME" >/dev/null 2>&1 || true + +set +e +EPH_OUT="$(run_coding_booth exec --name "$NAME" --run --port "$PORT_B" -- whoami 2>&1)" +EPH_EXIT=$? +set -e +EPH_STATE="$(state_of "$NAME")" +# Prefer "does the container still exist?" over status string — stop may race +# inspect with a blank line on some Docker versions. +EPH_GONE=false +if ! docker inspect "$NAME" >/dev/null 2>&1; then + EPH_GONE=true +fi + +if [[ $EPH_EXIT -eq 0 ]] \ + && [[ "$EPH_OUT" == *"coder"* ]] \ + && [[ "$EPH_GONE" == true ]]; then + print_test_result "true" "$0" "7" "ephemeral exec --run --port removes booth afterwards" +else + print_test_result "false" "$0" "7" "ephemeral --run should run then remove (exit=$EPH_EXIT state=$EPH_STATE gone=$EPH_GONE)" + echo " output: $EPH_OUT" + FAILED=$((FAILED + 1)) +fi + +exit $FAILED