Skip to content

Commit 655bd65

Browse files
committed
chore: simplify Phase 2 code and bump version to v0.2.0
- Fix compile error: mgr.List() → mgr.List(ListOptions{All: true}) in integration test - Add identity source constants (SourceTailscale, SourceEnv) replacing raw strings - Use config.DefaultWorkspacesRoot constant in integration tests - Collapse double-scan in port registry Allocate into single pass - Move os.MkdirAll to NewFileRegistry constructor, use json.Marshal for state file - Replace O(k) hyphen-collapse loop with single regex pass in identity.Sanitize - Combine two SSH calls into one in ServerResources - Extract portListenerCount helper to deduplicate AssertPortListening/AssertPortFree - Update CLAUDE.md architecture section with Phase 2 packages - Bump version to v0.2.0
1 parent 1fc6df9 commit 655bd65

8 files changed

Lines changed: 61 additions & 45 deletions

File tree

CLAUDE.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,19 @@ Go CLI tool that turns any Linux machine into a ready-to-use dev environment in
33

44
## Architecture
55
- `cmd/devbox/` — CLI entry point, all cobra commands defined in main.go
6-
- `internal/config/` — devbox.yaml parsing and validation
7-
- `internal/workspace/` — Workspace model and Manager interface (lifecycle: create/start/stop/destroy/list/ssh)
6+
- `internal/config/` — devbox.yaml parsing, validation, Resources, PortRangeConfig
7+
- `internal/identity/` — User identity resolution (Tailscale login → username, DEVBOX_USER fallback)
8+
- `internal/port/` — Port auto-allocation registry with conflict detection
9+
- `internal/server/` — Server pool management (add/remove/list/health), least-loaded selector
10+
- `internal/workspace/` — Workspace model, Manager interface, resource limits, user-scoped naming
811
- `internal/tailscale/` — Tailscale Manager interface (serve/unserve/status)
12+
- `internal/integration/` — Multi-user integration tests (build tag: integration)
13+
- `internal/testutil/` — Shared test helpers for SSH, Docker, assertions
914
- `.claude/specs/` — Product vision and design documents
1015

1116
## Key Patterns
1217
- **Cobra CLI**: All commands defined as funcs returning `*cobra.Command`, wired in `main()`
13-
- **Interface-driven**: `workspace.Manager` and `tailscale.Manager` define contracts; implementations are TBD (Phase 0 skeleton)
18+
- **Interface-driven**: `workspace.Manager`, `tailscale.Manager`, `identity.Resolver`, `port.Registry`, `server.Pool` define contracts
1419
- **Config**: Per-project `devbox.yaml` parsed into `DevboxConfig` struct with yaml tags; `name` and `server` are required fields
1520
- **Error wrapping**: `fmt.Errorf("context: %w", err)` for all error propagation
1621
- **Single binary**: No runtime dependencies, cross-compile with `GOOS`/`GOARCH`
@@ -25,6 +30,7 @@ Go CLI tool that turns any Linux machine into a ready-to-use dev environment in
2530
go build -o devbox ./cmd/devbox/ # Build binary
2631
go test ./... # Run all tests
2732
go vet ./... # Lint
28-
./devbox up [project] # Start workspace
29-
./devbox stop|list|destroy|ssh # Other commands (stubs in Phase 0)
33+
./devbox up [project] # Start workspace (auto-selects server, resolves user)
34+
./devbox stop|list|destroy|ssh # Workspace lifecycle commands
35+
./devbox server add|remove|list # Server pool management
3036
```

cmd/devbox/main.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ import (
2525
)
2626

2727
var (
28-
version = "0.1.0"
28+
version = "0.2.0"
2929
verbose bool
3030
noColor bool
3131
)

internal/identity/identity.go

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,16 @@ import (
99
"github.com/junixlabs/devbox/internal/tailscale"
1010
)
1111

12+
// Source constants for identity resolution.
13+
const (
14+
SourceTailscale = "tailscale"
15+
SourceEnv = "env"
16+
)
17+
1218
// Identity represents a resolved user identity.
1319
type Identity struct {
1420
Username string
15-
Source string // "tailscale" or "env"
21+
Source string
1622
}
1723

1824
// Resolver resolves the current user identity.
@@ -32,15 +38,16 @@ func NewResolver(ts tailscale.Manager) Resolver {
3238
// unsafeChars matches characters not in [a-z0-9-].
3339
var unsafeChars = regexp.MustCompile(`[^a-z0-9-]`)
3440

41+
// multiHyphen collapses consecutive hyphens into one.
42+
var multiHyphen = regexp.MustCompile(`-{2,}`)
43+
3544
// Sanitize normalizes a username to lowercase alphanumeric + hyphens only.
3645
func Sanitize(username string) string {
3746
s := strings.ToLower(username)
3847
s = strings.ReplaceAll(s, ".", "-")
3948
s = strings.ReplaceAll(s, "_", "-")
4049
s = unsafeChars.ReplaceAllString(s, "")
41-
for strings.Contains(s, "--") {
42-
s = strings.ReplaceAll(s, "--", "-")
43-
}
50+
s = multiHyphen.ReplaceAllString(s, "-")
4451
s = strings.Trim(s, "-")
4552
return s
4653
}
@@ -60,15 +67,15 @@ func (r *resolver) Current() (*Identity, error) {
6067
if err == nil && status.UserLogin != "" {
6168
username := UsernameFromLogin(status.UserLogin)
6269
if username != "" {
63-
return &Identity{Username: username, Source: "tailscale"}, nil
70+
return &Identity{Username: username, Source: SourceTailscale}, nil
6471
}
6572
}
6673
}
6774

6875
if envUser := os.Getenv("DEVBOX_USER"); envUser != "" {
6976
username := Sanitize(envUser)
7077
if username != "" {
71-
return &Identity{Username: username, Source: "env"}, nil
78+
return &Identity{Username: username, Source: SourceEnv}, nil
7279
}
7380
}
7481

internal/identity/identity_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,8 @@ func TestResolver_Tailscale(t *testing.T) {
7070
if id.Username != "dev" {
7171
t.Errorf("Username = %q, want %q", id.Username, "dev")
7272
}
73-
if id.Source != "tailscale" {
74-
t.Errorf("Source = %q, want %q", id.Source, "tailscale")
73+
if id.Source != SourceTailscale {
74+
t.Errorf("Source = %q, want %q", id.Source, SourceTailscale)
7575
}
7676
}
7777

@@ -86,8 +86,8 @@ func TestResolver_EnvFallback(t *testing.T) {
8686
if id.Username != "envuser" {
8787
t.Errorf("Username = %q, want %q", id.Username, "envuser")
8888
}
89-
if id.Source != "env" {
90-
t.Errorf("Source = %q, want %q", id.Source, "env")
89+
if id.Source != SourceEnv {
90+
t.Errorf("Source = %q, want %q", id.Source, SourceEnv)
9191
}
9292
}
9393

@@ -101,8 +101,8 @@ func TestResolver_NilTailscale_EnvFallback(t *testing.T) {
101101
if id.Username != "testuser" {
102102
t.Errorf("Username = %q, want %q", id.Username, "testuser")
103103
}
104-
if id.Source != "env" {
105-
t.Errorf("Source = %q, want %q", id.Source, "env")
104+
if id.Source != SourceEnv {
105+
t.Errorf("Source = %q, want %q", id.Source, SourceEnv)
106106
}
107107
}
108108

internal/integration/multi_workspace_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ func TestConcurrentWorkspaceCreation(t *testing.T) {
9797
}
9898

9999
// Verify list returns all 3.
100-
list, err := mgr.List()
100+
list, err := mgr.List(workspace.ListOptions{All: true})
101101
if err != nil {
102102
t.Fatalf("List(): %v", err)
103103
}
@@ -211,8 +211,8 @@ func TestUserIsolation(t *testing.T) {
211211
testutil.WaitForContainer(t, server, cB, 60*time.Second)
212212

213213
// Verify workspace directories are separate.
214-
testutil.AssertDirExists(t, server, "/workspaces/"+wsA.Name)
215-
testutil.AssertDirExists(t, server, "/workspaces/"+wsB.Name)
214+
testutil.AssertDirExists(t, server, config.DefaultWorkspacesRoot+"/"+wsA.Name)
215+
testutil.AssertDirExists(t, server, config.DefaultWorkspacesRoot+"/"+wsB.Name)
216216

217217
// Verify containers run in separate Docker Compose projects.
218218
projectA := testutil.DockerInspect(t, server, cA, "{{index .Config.Labels \"com.docker.compose.project\"}}")
@@ -275,7 +275,7 @@ func TestCleanupAfterDestroy(t *testing.T) {
275275
t.Fatalf("Create: %v", err)
276276
}
277277
testutil.WaitForContainer(t, server, containerName(ws.Name), 60*time.Second)
278-
testutil.AssertDirExists(t, server, "/workspaces/"+ws.Name)
278+
testutil.AssertDirExists(t, server, config.DefaultWorkspacesRoot+"/"+ws.Name)
279279

280280
// Destroy the workspace.
281281
if err := mgr.Destroy(ws.Name); err != nil {
@@ -295,7 +295,7 @@ func TestCleanupAfterDestroy(t *testing.T) {
295295
testutil.AssertPortFree(t, server, port)
296296

297297
// Verify workspace directory is gone.
298-
testutil.AssertDirNotExists(t, server, "/workspaces/"+ws.Name)
298+
testutil.AssertDirNotExists(t, server, config.DefaultWorkspacesRoot+"/"+ws.Name)
299299

300300
// Verify workspace is removed from state.
301301
_, err = mgr.Get(ws.Name)

internal/port/registry.go

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ type fileRegistry struct {
1919
// TODO: In-process mutex only protects single-process access. For concurrent
2020
// devbox processes, file-level locking (flock) would be needed.
2121
func NewFileRegistry(path string, portRange PortRange) Registry {
22+
// Ensure state directory exists once at creation time.
23+
_ = os.MkdirAll(filepath.Dir(path), 0o755)
2224
return &fileRegistry{
2325
path: path,
2426
portRange: portRange,
@@ -34,15 +36,12 @@ func (r *fileRegistry) Allocate(workspace, service string, override *int) (int,
3436
return 0, err
3537
}
3638

37-
// Check if this workspace+service already has an allocation.
39+
// Single pass: check for existing allocation and build used-ports map.
40+
usedPorts := make(map[int]string, len(allocs)) // port → workspace
3841
for _, a := range allocs {
3942
if a.WorkspaceName == workspace && a.ServiceName == service {
4043
return a.Port, nil
4144
}
42-
}
43-
44-
usedPorts := make(map[int]string) // port → workspace
45-
for _, a := range allocs {
4645
usedPorts[a.Port] = a.WorkspaceName
4746
}
4847

@@ -186,11 +185,7 @@ func (r *fileRegistry) load() ([]Allocation, error) {
186185

187186
// save writes allocations to the state file, creating directories if needed.
188187
func (r *fileRegistry) save(allocs []Allocation) error {
189-
if err := os.MkdirAll(filepath.Dir(r.path), 0o755); err != nil {
190-
return fmt.Errorf("creating port state directory: %w", err)
191-
}
192-
193-
data, err := json.MarshalIndent(allocs, "", " ")
188+
data, err := json.Marshal(allocs)
194189
if err != nil {
195190
return fmt.Errorf("marshalling port state: %w", err)
196191
}

internal/testutil/testutil.go

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,28 +104,36 @@ func WaitForContainer(t *testing.T, host, containerName string, timeout time.Dur
104104
t.Fatalf("container %s not running after %v", containerName, timeout)
105105
}
106106

107+
// portListenerCount returns the number of listeners on a port via ss.
108+
func portListenerCount(host string, port int) (string, error) {
109+
cmd := fmt.Sprintf("ss -tlnp 2>/dev/null | grep -c ':%d ' || true", port)
110+
out, err := SSHRunE(host, cmd)
111+
if err != nil {
112+
return "", err
113+
}
114+
return strings.TrimSpace(out), nil
115+
}
116+
107117
// AssertPortListening verifies that a port is bound on the remote host.
108118
func AssertPortListening(t *testing.T, host string, port int) {
109119
t.Helper()
110-
cmd := fmt.Sprintf("ss -tlnp 2>/dev/null | grep -c ':%d ' || true", port)
111-
out, err := SSHRunE(host, cmd)
120+
count, err := portListenerCount(host, port)
112121
if err != nil {
113122
t.Fatalf("AssertPortListening: %v", err)
114123
}
115-
if strings.TrimSpace(out) == "0" {
124+
if count == "0" {
116125
t.Errorf("expected port %d to be listening on %s", port, host)
117126
}
118127
}
119128

120129
// AssertPortFree verifies that a port is NOT bound on the remote host.
121130
func AssertPortFree(t *testing.T, host string, port int) {
122131
t.Helper()
123-
cmd := fmt.Sprintf("ss -tlnp 2>/dev/null | grep -c ':%d ' || true", port)
124-
out, err := SSHRunE(host, cmd)
132+
count, err := portListenerCount(host, port)
125133
if err != nil {
126134
t.Fatalf("AssertPortFree: %v", err)
127135
}
128-
if strings.TrimSpace(out) != "0" {
136+
if count != "0" {
129137
t.Errorf("expected port %d to be free on %s, but it's in use", port, host)
130138
}
131139
}

internal/workspace/manager.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -327,15 +327,15 @@ func (m *remoteManager) ServerResources(host string) (*ServerResourceInfo, error
327327
defer sshExec.Close()
328328

329329
ctx := context.Background()
330-
cpuOut, _, err := sshExec.Run(ctx, host, "nproc")
330+
combined, _, err := sshExec.Run(ctx, host, "nproc && echo '---SEPARATOR---' && cat /proc/meminfo")
331331
if err != nil {
332-
return nil, fmt.Errorf("running nproc on %s: %w", host, err)
332+
return nil, fmt.Errorf("querying server resources on %s: %w", host, err)
333333
}
334-
memOut, _, err := sshExec.Run(ctx, host, "cat /proc/meminfo")
335-
if err != nil {
336-
return nil, fmt.Errorf("reading /proc/meminfo on %s: %w", host, err)
334+
parts := strings.SplitN(combined, "---SEPARATOR---", 2)
335+
if len(parts) != 2 {
336+
return nil, fmt.Errorf("unexpected server resources output from %s", host)
337337
}
338-
return ParseServerResources(cpuOut, memOut)
338+
return ParseServerResources(strings.TrimSpace(parts[0]), strings.TrimSpace(parts[1]))
339339
}
340340

341341
// mustGet returns a workspace by name, or a WorkspaceError if not found.

0 commit comments

Comments
 (0)