Skip to content

Commit f16d593

Browse files
DimmKirrclaude
andcommitted
[Smart VNC] port lookup with label+bind-mount fallback, UX improvements
- feat(vnc): add 3-strategy VNC lookup: EXT_VNC_PORT fast path, label match, bind-mount fallback - feat(runner): attach devcell.basedir and devcell.cellid labels to containers - feat(runner): mount project basedir at its host path for bind-mount detection - feat(runner): add verbose flag to BuildImage, use --progress=quiet/plain and setsid - feat(vnc): add FindContainersByBind to parse docker inspect for host-dir matches - feat(cmd/vnc): add --verbose flag, vncDebug helper for port-lookup diagnostics - refactor(cmd/root): simplify verbose build output, only dump buf on failure when non-empty - chore(ux): update spinner sequence to smoother braille pattern, reduce delay to 80ms - test(runner): add label assertion for basedir and cellid - test(vnc): add ParseDockerPS mismatch and multi-session tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent e7dfa91 commit f16d593

7 files changed

Lines changed: 240 additions & 12 deletions

File tree

‎cmd/root.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,12 +163,12 @@ func buildImageWithSpinner(configDir string, noCache bool, label string) error {
163163
var buf bytes.Buffer
164164
var out io.Writer = &buf
165165
if ux.Verbose {
166-
out = io.MultiWriter(os.Stdout, &buf)
166+
out = os.Stdout
167167
}
168168
sp := ux.NewProgressSpinner(label)
169-
if err := runner.BuildImage(configDir, noCache, out); err != nil {
169+
if err := runner.BuildImage(configDir, noCache, ux.Verbose, out); err != nil {
170170
sp.Fail(label + " failed")
171-
if !ux.Verbose {
171+
if !ux.Verbose && buf.Len() > 0 {
172172
fmt.Fprint(os.Stderr, buf.String())
173173
}
174174
return err

‎cmd/vnc.go‎

Lines changed: 94 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ import (
66
"os"
77
"os/exec"
88
"runtime"
9+
"strings"
910

1011
"github.com/DimmKirr/devcell/internal/config"
1112
internalvnc "github.com/DimmKirr/devcell/internal/vnc"
13+
"github.com/DimmKirr/devcell/internal/ux"
1214
"github.com/spf13/cobra"
1315
)
1416

@@ -21,9 +23,16 @@ var vncCmd = &cobra.Command{
2123
func init() {
2224
vncCmd.Flags().Bool("list", false, "list all running cell containers and their VNC ports")
2325
vncCmd.Flags().String("app", "", "open VNC to a named container (by AppName)")
26+
vncCmd.Flags().Bool("verbose", false, "show debug info for VNC port lookup")
2427
}
2528

2629
func runVNC(cmd *cobra.Command, _ []string) error {
30+
applyOutputFlags()
31+
verbose, _ := cmd.Flags().GetBool("verbose")
32+
if verbose {
33+
ux.Verbose = true
34+
ux.LogPlainText = true
35+
}
2736
list, _ := cmd.Flags().GetBool("list")
2837
appName, _ := cmd.Flags().GetString("app")
2938

@@ -37,12 +46,95 @@ func runVNC(cmd *cobra.Command, _ []string) error {
3746
}
3847

3948
func vncDefault() error {
49+
// Fast path: EXT_VNC_PORT is injected at container start with the correct
50+
// published host port. When set, we're inside a devcell container and can
51+
// use it directly without any docker lookup.
52+
if port := os.Getenv("EXT_VNC_PORT"); port != "" {
53+
vncDebug("EXT_VNC_PORT=%s (fast path)", port)
54+
return openURL(internalvnc.VNCUrl(port))
55+
}
56+
4057
c, err := config.LoadFromOS()
4158
if err != nil {
4259
return err
4360
}
44-
url := internalvnc.VNCUrl(c.VNCPort)
45-
return openURL(url)
61+
vncDebug("basedir: %s", c.BaseDir)
62+
vncDebug("cellID: %s (computed port: %s)", c.CellID, c.VNCPort)
63+
64+
// --- Strategy 1: exact label match (containers started with current code) ---
65+
// Filter by both basedir AND cellid for an exact session match.
66+
out, err := exec.Command("docker", "ps",
67+
"--filter", "label=devcell.basedir="+c.BaseDir,
68+
"--filter", "label=devcell.cellid="+c.CellID,
69+
"--format", "{{.Names}}\t{{.Ports}}").Output()
70+
if err == nil {
71+
vncDebug("label-exact docker ps output: %q", strings.TrimSpace(string(out)))
72+
if m, _ := internalvnc.ParseDockerPS(string(bytes.TrimSpace(out))); len(m) > 0 {
73+
for appName, port := range m {
74+
vncDebug("label-exact match: %s → %s", appName, port)
75+
return openURL(internalvnc.VNCUrl(port))
76+
}
77+
}
78+
}
79+
80+
// --- Strategy 2: basedir-only label match (different session, same dir) ---
81+
out, err = exec.Command("docker", "ps",
82+
"--filter", "label=devcell.basedir="+c.BaseDir,
83+
"--format", "{{.Names}}\t{{.Ports}}").Output()
84+
if err == nil {
85+
vncDebug("label-dir docker ps output: %q", strings.TrimSpace(string(out)))
86+
if m, _ := internalvnc.ParseDockerPS(string(bytes.TrimSpace(out))); len(m) > 0 {
87+
if len(m) == 1 {
88+
for appName, port := range m {
89+
vncDebug("label-dir single match: %s → %s", appName, port)
90+
return openURL(internalvnc.VNCUrl(port))
91+
}
92+
}
93+
// Multiple sessions — list options.
94+
var opts []string
95+
for appName := range m {
96+
opts = append(opts, " cell vnc --app "+appName)
97+
}
98+
return fmt.Errorf("multiple containers for this directory — pick one:\n%s", strings.Join(opts, "\n"))
99+
}
100+
}
101+
102+
// --- Strategy 3: bind-mount fallback (containers started before labels were added) ---
103+
vncDebug("no label match; falling back to bind-mount inspect")
104+
allOut, err := exec.Command("docker", "ps", "-q", "--filter", "name=cell-").Output()
105+
if err != nil || len(bytes.TrimSpace(allOut)) == 0 {
106+
return fmt.Errorf("no running container found for %q — run 'cell vnc --list' to see all", c.BaseDir)
107+
}
108+
ids := strings.Fields(string(bytes.TrimSpace(allOut)))
109+
vncDebug("inspecting %d containers: %v", len(ids), ids)
110+
inspectOut, err := exec.Command("docker", append([]string{"inspect"}, ids...)...).Output()
111+
if err != nil {
112+
return fmt.Errorf("docker inspect: %w", err)
113+
}
114+
matches, err := internalvnc.FindContainersByBind(string(inspectOut), c.BaseDir)
115+
if err != nil {
116+
return fmt.Errorf("parse inspect: %w", err)
117+
}
118+
vncDebug("bind-mount matches: %+v", matches)
119+
switch len(matches) {
120+
case 0:
121+
return fmt.Errorf("no running container found for %q — run 'cell vnc --list' to see all", c.BaseDir)
122+
case 1:
123+
return openURL(internalvnc.VNCUrl(matches[0].Port))
124+
default:
125+
var opts []string
126+
for _, m := range matches {
127+
opts = append(opts, " cell vnc --app "+m.AppName)
128+
}
129+
return fmt.Errorf("multiple containers for this directory — pick one:\n%s", strings.Join(opts, "\n"))
130+
}
131+
}
132+
133+
// vncDebug prints a debug line when --verbose is active.
134+
func vncDebug(format string, args ...any) {
135+
if ux.Verbose {
136+
fmt.Fprintf(os.Stderr, "[vnc] "+format+"\n", args...)
137+
}
46138
}
47139

48140
func vncList() error {

‎internal/runner/runner.go‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"os/exec"
99
"path/filepath"
1010
"strings"
11+
"syscall"
1112

1213
"github.com/DimmKirr/devcell/internal/cfg"
1314
"github.com/DimmKirr/devcell/internal/config"
@@ -59,6 +60,10 @@ func BuildArgv(spec RunSpec, fs FS, lookPath func(string) (string, error)) []str
5960
argv = append(argv, "--user", "0")
6061
argv = append(argv, "--group-add", "0")
6162

63+
// Labels for VNC lookup: filter by basedir+cellid without inspecting all containers
64+
argv = append(argv, "--label", "devcell.basedir="+c.BaseDir)
65+
argv = append(argv, "--label", "devcell.cellid="+c.CellID)
66+
6267
// Core env vars
6368
e := func(k, v string) { argv = append(argv, "-e", k+"="+v) }
6469
e("APP_NAME", c.AppName)
@@ -99,6 +104,7 @@ func BuildArgv(spec RunSpec, fs FS, lookPath func(string) (string, error)) []str
99104

100105
// Standard volumes
101106
v := func(mount string) { argv = append(argv, "-v", mount) }
107+
v(c.BaseDir + ":" + c.BaseDir)
102108
v(c.BaseDir + ":/" + c.AppName)
103109
v(c.CellHome + ":/home/" + c.HostUser)
104110
v("/var/run/docker.sock:/var/run/docker.sock")
@@ -159,16 +165,32 @@ func EnsureNetwork(ctx context.Context) error {
159165
}
160166

161167
// BuildImage runs docker build to build devcell-local from configDir.
162-
// Output is written to out; pass os.Stdout to stream, or a bytes.Buffer to capture.
163-
func BuildImage(configDir string, noCache bool, out io.Writer) error {
164-
args := []string{"build", "-t", "devcell-local"}
168+
// verbose=true streams plain-text output to out; verbose=false suppresses all
169+
// docker output (quiet mode) and captures stderr to out for error replay.
170+
func BuildImage(configDir string, noCache bool, verbose bool, out io.Writer) error {
171+
progress := "--progress=quiet"
172+
if verbose {
173+
progress = "--progress=plain"
174+
}
175+
args := []string{"build", "-t", "devcell-local", progress}
165176
if noCache {
166177
args = append(args, "--no-cache")
167178
}
168179
args = append(args, configDir)
169180
cmd := exec.Command("docker", args...)
170-
cmd.Stdout = out
171-
cmd.Stderr = out
181+
// Detach from the controlling TTY so Docker Desktop's BuildKit progress
182+
// writer cannot open /dev/tty and write directly to the terminal.
183+
cmd.SysProcAttr = &syscall.SysProcAttr{Setsid: true}
184+
if verbose {
185+
cmd.Stdout = out
186+
cmd.Stderr = out
187+
} else {
188+
// Suppress progress output; capture stderr so we can replay it on failure.
189+
cmd.Stdout = io.Discard
190+
cmd.Stderr = out
191+
// Belt-and-suspenders: also tell BuildKit via env to use quiet mode.
192+
cmd.Env = append(os.Environ(), "BUILDKIT_PROGRESS=quiet")
193+
}
172194
if err := cmd.Run(); err != nil {
173195
return fmt.Errorf("docker build: %w", err)
174196
}

‎internal/runner/runner_test.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,18 @@ func TestArgv_UserAndGroupAdd(t *testing.T) {
147147
}
148148
}
149149

150+
// --- labels ---
151+
152+
func TestArgv_Labels(t *testing.T) {
153+
argv := buildArgv(t)
154+
if !hasConsecutive(argv, "--label", "devcell.basedir=/home/bob/myproject") {
155+
t.Errorf("missing --label devcell.basedir in argv: %v", argv)
156+
}
157+
if !hasConsecutive(argv, "--label", "devcell.cellid=3") {
158+
t.Errorf("missing --label devcell.cellid in argv: %v", argv)
159+
}
160+
}
161+
150162
// --- env-file ---
151163

152164
func TestArgv_EnvFilePresent(t *testing.T) {

‎internal/ux/ux.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ func NewProgressSpinner(message string) *ProgressSpinner {
2626
ps := &ProgressSpinner{msg: message}
2727
if !LogPlainText {
2828
s := pterm.DefaultSpinner
29-
s.Sequence = []string{" ⡀ ", " ⢀ ", " ⠄ ", " ⠠ ", " ⠐ ", " ⠂ ", " ⠁ ", " ⠈ ", " ", " "}
29+
s.Sequence = []string{" ⠋ ", " ⠙ ", " ⠹ ", " ⠸ ", " ⠼ ", " ⠴ ", " ⠦ ", " ⠧ ", " ⠇ ", " ⠏ "}
3030
s.Style = pterm.NewStyle(pterm.FgLightBlue)
31-
s.Delay = 150 * time.Millisecond
31+
s.Delay = 80 * time.Millisecond
3232
ps.spinner, _ = s.Start(message)
3333
} else {
3434
pterm.Info.Println(message)

‎internal/vnc/vnc.go‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,67 @@ import (
66
"strings"
77
)
88

9+
// ContainerMatch holds data about a container matching a host-dir bind mount.
10+
type ContainerMatch struct {
11+
Name string // full container name, e.g. "cell-devcell-73-run"
12+
AppName string // stripped name, e.g. "devcell-73"
13+
Port string // VNC host port
14+
}
15+
16+
// fullInspectResult is the structure decoded from docker inspect JSON.
17+
type fullInspectResult struct {
18+
Name string `json:"Name"`
19+
HostConfig struct {
20+
Binds []string `json:"Binds"`
21+
} `json:"HostConfig"`
22+
NetworkSettings struct {
23+
Ports map[string][]struct {
24+
HostIp string `json:"HostIp"`
25+
HostPort string `json:"HostPort"`
26+
} `json:"Ports"`
27+
} `json:"NetworkSettings"`
28+
}
29+
30+
// FindContainersByBind parses docker inspect JSON (one or many containers)
31+
// and returns all that have hostDir as the source of a bind mount and have
32+
// port 5900 published.
33+
func FindContainersByBind(inspectJSON, hostDir string) ([]ContainerMatch, error) {
34+
var results []fullInspectResult
35+
if err := json.Unmarshal([]byte(inspectJSON), &results); err != nil {
36+
return nil, fmt.Errorf("parse inspect JSON: %w", err)
37+
}
38+
var matches []ContainerMatch
39+
for _, r := range results {
40+
if !hasBindSource(r.HostConfig.Binds, hostDir) {
41+
continue
42+
}
43+
bindings, ok := r.NetworkSettings.Ports["5900/tcp"]
44+
if !ok || len(bindings) == 0 {
45+
continue
46+
}
47+
name := strings.TrimPrefix(r.Name, "/")
48+
appName := strings.TrimPrefix(name, "cell-")
49+
appName = strings.TrimSuffix(appName, "-run")
50+
matches = append(matches, ContainerMatch{
51+
Name: name,
52+
AppName: appName,
53+
Port: bindings[0].HostPort,
54+
})
55+
}
56+
return matches, nil
57+
}
58+
59+
// hasBindSource reports whether any bind in the list starts with "hostDir:".
60+
func hasBindSource(binds []string, hostDir string) bool {
61+
prefix := hostDir + ":"
62+
for _, b := range binds {
63+
if strings.HasPrefix(b, prefix) {
64+
return true
65+
}
66+
}
67+
return false
68+
}
69+
970
// VNCUrl returns a VNC URL for the given port.
1071
func VNCUrl(port string) string {
1172
return "vnc://:vnc@127.0.0.1:" + port

‎internal/vnc/vnc_test.go‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,44 @@ func TestParseInspectPort_Missing(t *testing.T) {
7979
t.Error("expected error for missing 5900 port binding")
8080
}
8181
}
82+
83+
// TestParseDockerPS_DirPortMismatch covers the case where a container was
84+
// started from pane %3 (port 7350) but cell vnc is run from pane %11 (port
85+
// 1150). The lookup must return the actual docker port (7350), not the
86+
// env-computed one (1150).
87+
func TestParseDockerPS_DirPortMismatch(t *testing.T) {
88+
// Simulate: container started with cellID=3, SESSION_PORT_PREFIX=7
89+
// Container name: cell-devcell-73-3-run, VNC host port: 7350
90+
// Current pane: %11 → computed port would be 1150 (wrong)
91+
dockerPSOutput := "cell-devcell-73-3-run\t0.0.0.0:7350->5900/tcp"
92+
m, err := vnc.ParseDockerPS(dockerPSOutput)
93+
if err != nil {
94+
t.Fatal(err)
95+
}
96+
got, ok := m["devcell-73-3"]
97+
if !ok {
98+
t.Fatalf("expected entry for devcell-73-3, got %v", m)
99+
}
100+
if got != "7350" {
101+
t.Errorf("want actual port 7350, got %q (env-computed would be 1150)", got)
102+
}
103+
}
104+
105+
// TestParseDockerPS_MultiSession ensures that when multiple sessions of the
106+
// same directory exist, all ports are returned (caller picks one).
107+
func TestParseDockerPS_MultiSession(t *testing.T) {
108+
output := "cell-devcell-73-3-run\t0.0.0.0:7350->5900/tcp\ncell-devcell-73-5-run\t0.0.0.0:7550->5900/tcp"
109+
m, err := vnc.ParseDockerPS(output)
110+
if err != nil {
111+
t.Fatal(err)
112+
}
113+
if len(m) != 2 {
114+
t.Errorf("expected 2 entries, got %d: %v", len(m), m)
115+
}
116+
if m["devcell-73-3"] != "7350" {
117+
t.Errorf("want devcell-73-3→7350, got %v", m)
118+
}
119+
if m["devcell-73-5"] != "7550" {
120+
t.Errorf("want devcell-73-5→7550, got %v", m)
121+
}
122+
}

0 commit comments

Comments
 (0)