diff --git a/.gitignore b/.gitignore index 6ffd459..92bc81e 100644 --- a/.gitignore +++ b/.gitignore @@ -34,3 +34,17 @@ dist/ # Claude Code ctags index .claude/tags + +# Claude Code worktrees +.claude/worktrees/ + +# PR Review Fix Loop +.claude/pr-review-loop-report.local.md +.claude/pr-review-loop-stats.local.json +.claude/pr-review-loop-debug.local.log +.claude/pr-review-fix-loop.local.md +.codex-review.md +.codex-review.stderr + +# pr-review-fix-loop local artifacts +.claude/*.local.md diff --git a/CLAUDE.md b/CLAUDE.md index 7d95ecd..f73eea2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2,7 +2,7 @@ ## About the Project -**port-selector** — CLI utility in Go for automatic free port selection from a given range. Designed for AI-driven development with multiple parallel agents. +**port-selector** — CLI utility in Go for automatic free port selection from a given range. Designed for AI-driven development with multiple parallel agents. Supports named allocations, port locking, Docker container detection, and process discovery. ## Tech Stack @@ -15,16 +15,23 @@ ``` port-selector/ -├── cmd/port-selector/main.go # Entry point, argument parsing +├── cmd/port-selector/main.go # Entry point, argument parsing, CLI commands ├── internal/ │ ├── allocations/ # Port allocations with flock-based locking -│ │ ├── allocations.go # Store, Load, Save, WithStore +│ │ ├── allocations.go # Store, Load, Save, WithStore, CRUD operations │ │ ├── lock_unix.go # Unix flock implementation │ │ └── lock_windows.go # Windows stub (no locking) -│ ├── config/config.go # Read/create YAML config +│ ├── config/config.go # Read/create YAML config, duration parsing +│ ├── debug/debug.go # Debug logging (--verbose flag) +│ ├── docker/docker.go # Docker container detection and project directory resolution │ ├── logger/logger.go # Structured logging for state changes -│ └── port/checker.go # Port availability checking -├── .github/workflows/release.yml +│ ├── pathutil/pathutil.go # Path utilities (~ shortening) +│ └── port/ +│ ├── checker.go # Port availability checking, free port search +│ └── procinfo.go # Process discovery via /proc (Linux only) +├── .github/workflows/ +│ ├── ci.yml # Tests and linting on PRs +│ └── release.yml # Build and release on tags ├── .mise.toml ├── go.mod └── go.sum @@ -34,35 +41,186 @@ port-selector/ ### Functional -1. **No arguments** → outputs free port to STDOUT -2. **-h, --help** → help message -3. **-v, --version** → version (embedded at build via `-ldflags`) -4. **Config** in `~/.config/port-selector/config.yaml`: - ```yaml - portStart: 3000 - portEnd: 4000 - freezePeriod: 24h - # log: ~/.config/port-selector/port-selector.log - ``` -5. **Allocations** in `~/.config/port-selector/allocations.yaml`: - ```yaml - last_issued_port: 3005 - allocations: - 3000: - directory: /path/to/project - assigned_at: 2024-01-15T10:30:00Z - last_used_at: 2024-01-15T12:00:00Z - locked: false - ``` -6. **Wrap-around** — after reaching portEnd, start from portStart -7. **Error** to STDERR with exit code 1 if all ports are busy +#### Port Allocation (primary command) +1. **No arguments** → outputs free port to STDOUT for current directory +2. **`--name NAME`** → allocate a named port (default: "main"), allowing multiple ports per directory +3. Port is **stable per (directory, name)** — same directory+name always returns the same port +4. **Wrap-around** — after reaching portEnd, start from portStart +5. **Error** to STDERR with exit code 1 if all ports are busy or frozen + +#### Information Commands +6. **`-h, --help`** → help message +7. **`-v, --version`** → version (embedded at build via `-ldflags`) +8. **`-l, --list`** → show all allocations in table format (PORT, DIRECTORY, NAME, SOURCE, STATUS columns) +9. **`--verbose`** → enable debug output to STDERR (combinable with any command) + +#### Allocation Management +10. **`--forget`** → remove all allocations for current directory +11. **`--forget --name NAME`** → remove only the named allocation +12. **`--forget-all`** → remove all allocations globally +13. **`--scan`** → scan port range, detect busy ports, identify owning processes/containers +14. **`--refresh`** → remove stale external allocations (ports no longer in use) + +#### Port Locking +15. **`-c, --lock [PORT]`** → lock port for current directory and name (prevent reuse by others) +16. **`-u, --unlock [PORT]`** → unlock port +17. **`--force, -f`** → force lock a busy port or reassign a locked port from another directory + +#### Lock Decision Matrix (for `--lock PORT`) + +| Port State | Allocated To | Action | `--force` needed? | +|------------|-------------|--------|-------------------| +| Free | Current dir | Update lock status | No | +| Free | Other dir (unlocked) | Reassign to current dir | No | +| Free | Other dir (locked) | Reassign to current dir | **Yes** | +| Busy | Current dir | Lock (same dir owns it) | No | +| Busy | Other dir | **Block** (even with --force) | N/A | +| Busy | Not allocated (known process, same CWD) | Register + lock | No | +| Busy | Not allocated (known process, other CWD) | Register as external | No | +| Busy | Not allocated (unknown process) | Error | Yes (user takes responsibility) | +| Free | Not allocated | Allocate + lock | No | ### Non-functional -- Minimal dependencies (only Go stdlib + yaml parser) -- Fast startup (< 100ms) +- **Go module dependencies:** only `gopkg.in/yaml.v3` +- **Optional runtime dependency:** Docker CLI (for container detection in `--scan`) +- Fast startup (< 100ms for port allocation, `--scan` may be slower) - Flock-based file locking (to prevent race conditions on Unix) -- Platform support: Linux, macOS (Windows builds but without file locking) +- Platform support: Linux (full), macOS (port allocation works, process discovery limited), Windows (builds but no file locking) + +## Config + +**Location:** `~/.config/port-selector/config.yaml` + +```yaml +portStart: 3000 +portEnd: 4000 +freezePeriod: 24h +# allocationTTL: 30d +log: ~/.config/port-selector/port-selector.log +``` + +| Field | Default | Description | +|-------|---------|-------------| +| `portStart` | 3000 | Start of port range (1-65535) | +| `portEnd` | 4000 | End of port range (must be > portStart) | +| `freezePeriod` | 24h | Time to avoid reusing recently allocated ports (supports d/h/m/s) | +| `allocationTTL` | disabled | Auto-expire allocations after this duration (e.g., 30d, 720h) | +| `log` | ~/.config/port-selector/port-selector.log | Path to log file (empty to disable) | + +**Duration format:** supports `30d` (days), `720h` (hours), `30m` (minutes), standard Go duration. + +**freezePeriod vs allocationTTL:** +- `freezePeriod` — prevents reusing a port within this window (port stays "frozen" for other directories) +- `allocationTTL` — deletes the allocation record entirely after this period of inactivity +- Locked allocations are **never** expired by TTL + +**Legacy:** `freezePeriodMinutes` (integer) is supported for backward compatibility; `freezePeriod` takes precedence. + +## Allocations + +**Location:** `~/.config/port-selector/allocations.yaml` + +```yaml +last_issued_port: 3005 +allocations: + 3000: + directory: /home/user/project-a + name: main + assigned_at: 2024-01-15T10:30:00Z + last_used_at: 2024-01-15T12:00:00Z + locked: false + process_name: node + 3001: + directory: /home/user/project-a + name: api + assigned_at: 2024-01-15T11:00:00Z + last_used_at: 2024-01-15T12:00:00Z + locked: true + locked_at: 2024-01-15T11:00:00Z + 3002: + directory: /home/user/project-b + name: main + assigned_at: 2024-01-16T09:00:00Z + last_used_at: 2024-01-16T09:00:00Z + status: external + external_pid: 1234 + external_user: root + external_process_name: docker-proxy + container_id: abc123def456 +``` + +### AllocationInfo Fields + +| Field | Type | Description | +|-------|------|-------------| +| `directory` | string | Working directory of the project | +| `name` | string | Allocation name (default: "main") | +| `assigned_at` | time | When the allocation was created | +| `last_used_at` | time | Last time port was issued or updated | +| `locked` | bool | Whether port is locked (prevent reuse) | +| `locked_at` | time | When the port was locked | +| `process_name` | string | Name of the process using the port | +| `container_id` | string | Docker container ID (if applicable) | +| `status` | string | "" (normal) or "external" | +| `external_pid` | int | PID of external process | +| `external_user` | string | Username of external process owner | +| `external_process_name` | string | Name of external process | + +**Primary key:** port number (unique in the map). +**Lookup keys:** (directory), (directory, name), (port). +**Invariant:** at most one locked port per (directory, name) combination. + +## Named Allocations + +One directory can have multiple port allocations with different names: + +```bash +$ cd ~/project +$ port-selector --name web # → 3000 +$ port-selector --name api # → 3001 +$ port-selector --name db # → 3002 +$ port-selector --name web # → 3000 (stable) +``` + +- Default name is `"main"` (empty name normalizes to "main") +- `--forget` without `--name` removes **all** allocations for the directory +- `--forget --name api` removes only the "api" allocation +- Freeze period applies per-port, not per-name + +## External Allocations + +Ports used by processes not managed by port-selector: + +- **Created by:** `--scan` (discovers busy ports) or `--lock PORT` (when port is busy on another directory) +- **Status:** `status: external` in allocations.yaml +- **Stored info:** PID, username, process name, container ID +- **Cleanup:** `--refresh` removes stale external allocations (ports now free) +- **Display:** shown as `SOURCE=external` in `--list` + +## Docker Integration + +When a port is owned by `docker-proxy`, port-selector resolves the actual project directory: + +1. Find container by port: `docker ps --filter publish=PORT` +2. Try compose label: `com.docker.compose.project.working_dir` +3. Fallback to first bind mount source + +**Requires:** Docker CLI (`docker` command). Gracefully degrades if unavailable. + +## Process Discovery + +On Linux, `--scan` and `--lock` identify port owners via: + +1. Parse `/proc/net/tcp` and `/proc/net/tcp6` for listening sockets +2. Match socket inode to process via `/proc/*/fd/` +3. Read process name from `/proc/[pid]/comm`, CWD from `/proc/[pid]/cwd` +4. Resolve UID to username + +**Platform limitations:** +- **Linux:** full support (may need sudo for other users' processes) +- **macOS:** process discovery not available (`/proc` doesn't exist) +- **Windows:** process discovery not available ## Development Commands @@ -98,12 +256,20 @@ func IsPortFree(port int) bool { } ``` +**Known limitation (TOCTOU):** Port is checked as free, then released. Another process could take it between check and actual use. Callers should handle bind failures gracefully. + ### Config Handling - Use `os.UserConfigDir()` for cross-platform compatibility - Create directories with `os.MkdirAll(..., 0755)` - Use `gopkg.in/yaml.v3` for YAML +### Atomic Operations + +- `WithStore(configDir, fn)` — read-modify-write with flock (use for all mutations) +- `Load(configDir)` — read-only without lock (use for `--list`) +- `Save(configDir, store)` — atomic write via temp file + rename (no lock) + ### Error Handling ```go @@ -111,10 +277,17 @@ func IsPortFree(port int) bool { fmt.Fprintln(os.Stderr, "error: all ports in range are busy") os.Exit(1) -// Successful output to STDOUT (port only!) +// Successful port output to STDOUT (port only!) fmt.Println(port) + +// Informational output (--list, --forget, --lock, etc.) to STDOUT +fmt.Printf("Locked port %d for '%s' in %s\n", port, name, dir) ``` +**STDOUT rules:** +- Port allocation (no args, `--name`) → port number only +- Other commands (`--list`, `--forget`, `--lock`, etc.) → informational messages + ## Testing ### Unit Tests @@ -139,6 +312,12 @@ func TestFindFreePort(t *testing.T) { } ``` +### Platform-specific Tests + +- Process discovery tests (`procinfo_test.go`) skip on non-Linux with `runtime.GOOS` +- Docker tests gracefully handle missing Docker CLI +- Concurrent tests verify flock correctness with multiple goroutines + ## GitHub Actions ### Release Workflow @@ -169,6 +348,8 @@ logger.AllocLock // When allocation lock status changes logger.AllocDelete // When a single allocation is removed logger.AllocDeleteAll // When all allocations are cleared logger.AllocExpire // When allocation expires due to TTL +logger.AllocExternal // When registering an external port (from --scan or --lock) +logger.AllocRefresh // When refreshing external allocations (--refresh) ``` ### Usage Pattern @@ -177,7 +358,8 @@ logger.AllocExpire // When allocation expires due to TTL // Log state changes with relevant fields logger.Log(logger.AllocAdd, logger.Field("port", port), - logger.Field("dir", directory)) + logger.Field("dir", directory), + logger.Field("name", name)) // Field() auto-quotes values with spaces logger.Field("dir", "/path with spaces") // -> dir="/path with spaces" @@ -186,27 +368,45 @@ logger.Field("dir", "/path with spaces") // -> dir="/path with spaces" ### Requirements for New Code 1. **Any function that modifies allocations** must call `logger.Log()` with appropriate event type -2. **Include relevant context** using `logger.Field()` (port, directory, count, etc.) +2. **Include relevant context** using `logger.Field()` (port, directory, name, count, reason, etc.) 3. **Logger is safe when nil** — if logging is disabled, calls are no-op 4. **No need to check if logger is enabled** — just call `logger.Log()` ## Important Details -1. **STDOUT for port only** — no additional text -2. **File locking** — flock-based locking on Unix for concurrent access safety -3. **Graceful handling** — if no permissions for config, continue with defaults +1. **STDOUT for port only** (in port allocation mode) — no additional text. Other commands output informational messages. +2. **File locking** — flock-based locking on Unix for concurrent access safety. `WithStore` uses blocking `LOCK_EX`. Lock auto-releases on process exit. +3. **Graceful handling** — if no permissions for config, continue with defaults with warning to STDERR 4. **Don't block port** — only check and immediately close listener +5. **Directory-based persistence** — port is allocated per working directory. Same directory always returns the same port. +6. **Name normalization** — empty name normalizes to "main" for backward compatibility ## Usage Example ```bash -# Agent runs +# Basic: get a port for current directory $ port-selector 3000 -# Next agent -$ port-selector +# Named allocations for multiple services +$ port-selector --name web 3001 +$ port-selector --name api +3002 + +# Lock a port (prevent reuse) +$ port-selector --lock +Locked port 3000 for 'main' in ~/project + +# List all allocations +$ port-selector --list +PORT DIRECTORY NAME SOURCE STATUS +3000 ~/project-a main free locked +3001 ~/project-a api free +3002 ~/project-b main free + +# Scan for busy ports in range +$ port-selector --scan # Usage in script $ npm run dev -- --port $(port-selector) diff --git a/cmd/port-selector/main.go b/cmd/port-selector/main.go index 73dd6b8..29183a5 100644 --- a/cmd/port-selector/main.go +++ b/cmd/port-selector/main.go @@ -968,7 +968,7 @@ Port Locking: registered as an external allocation instead of failing. Configuration: - ~/.config/port-selector/default.yaml + ~/.config/port-selector/config.yaml Available options: portStart: 3000 # Start of port range @@ -1108,8 +1108,9 @@ func runRefresh() error { } fmt.Printf("Refreshing %d external allocation(s)...\n", totalCount) - removedCount = store.RefreshExternalAllocations(port.IsPortFree) - return nil + var refreshErr error + removedCount, refreshErr = store.RefreshExternalAllocations(port.IsPortFree) + return refreshErr }) if err != nil { diff --git a/cmd/port-selector/main_test.go b/cmd/port-selector/main_test.go index fe4fb49..3a85414 100644 --- a/cmd/port-selector/main_test.go +++ b/cmd/port-selector/main_test.go @@ -2,6 +2,7 @@ package main import ( "bytes" + "fmt" "net" "os" "os/exec" @@ -177,6 +178,7 @@ func TestLockAllocatesAndLocksFreePort(t *testing.T) { alloc := allocs.FindByPort(3500) if alloc == nil { t.Fatal("allocation for port 3500 was not created") + return // unreachable, but satisfies staticcheck SA5011 } if alloc.Directory != workDir { t.Errorf("expected directory %s, got %s", workDir, alloc.Directory) @@ -255,6 +257,7 @@ func TestLockPortWhenDirectoryAlreadyHasAllocation(t *testing.T) { alloc := allocs2.FindByPort(3500) if alloc == nil { t.Fatal("allocation for port 3500 was not created") + return // unreachable, but satisfies staticcheck SA5011 } if alloc.Directory != workDir { t.Errorf("expected directory %s, got %s", workDir, alloc.Directory) @@ -417,6 +420,7 @@ func TestLockPortFromAnotherDirectory_WithForce(t *testing.T) { alloc := store.FindByPort(3002) if alloc == nil { t.Fatal("expected allocation for port 3002") + return // unreachable, but satisfies staticcheck SA5011 } if alloc.Directory != workDir2 { t.Errorf("expected port to belong to %s, got %s", workDir2, alloc.Directory) @@ -442,24 +446,74 @@ func TestLockPortSameDirectory_NoError(t *testing.T) { env := append(os.Environ(), "XDG_CONFIG_HOME="+filepath.Join(tmpDir, ".config")) - // Step 1: Allocate port 3003 for project - cmd := exec.Command(binary, "--lock", "3003") - cmd.Dir = workDir - cmd.Env = env - if output, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("failed to lock port 3003: %v, output: %s", err, output) + // Find a free port and lock it in one loop, retrying on TOCTOU. + // This avoids skipping the entire test when a single port becomes busy + // between discovery and lock — instead we try the next free port. + var freePort int + for p := 3000; p <= 4000; p++ { + ln, err := net.Listen("tcp", fmt.Sprintf(":%d", p)) + if err != nil { + continue + } + ln.Close() + + portStr := fmt.Sprintf("%d", p) + cmd := exec.Command(binary, "--lock", portStr) + cmd.Dir = workDir + cmd.Env = env + output, err := cmd.CombinedOutput() + if err != nil { + outStr := string(output) + if strings.Contains(outStr, "is in use") || strings.Contains(outStr, "busy") { + continue // TOCTOU: port became busy, try next + } + t.Fatalf("failed to lock port %s: %v, output: %s", portStr, err, outStr) + } + if strings.Contains(string(output), "externally used") { + continue // TOCTOU: external process grabbed it, try next + } + freePort = p + break + } + if freePort == 0 { + t.Skipf("could not find and lock any free port in range 3000-4000") } + portStr := fmt.Sprintf("%d", freePort) - // Step 2: Lock port 3003 again from same directory (should succeed without --force) - cmd = exec.Command(binary, "--lock", "3003") + // Step 2: Lock same port again from same directory (should succeed without --force). + // No TOCTOU skip needed here: once Step 1 allocates the port to this directory, + // lockSpecificPort takes the alloc.Directory==cwd fast path (main.go:645-651) + // which updates lock status without checking port busyness. + cmd := exec.Command(binary, "--lock", portStr) cmd.Dir = workDir cmd.Env = env output, err := cmd.CombinedOutput() if err != nil { t.Fatalf("expected success, got error: %v, output: %s", err, output) } - if !strings.Contains(string(output), "Locked port 3003") { - t.Errorf("expected 'Locked port 3003' message, got: %s", output) + expectedMsg := fmt.Sprintf("Locked port %d", freePort) + if !strings.Contains(string(output), expectedMsg) { + t.Errorf("expected %q message, got: %s", expectedMsg, output) + } + + // Verify allocation state is correct after re-lock + store, err := allocations.Load(configDir) + if err != nil { + t.Fatalf("failed to load allocations: %v", err) + } + alloc := store.FindByPort(freePort) + if alloc == nil { + t.Fatalf("expected allocation for port %d", freePort) + return // unreachable, but satisfies staticcheck SA5011 + } + if alloc.Directory != workDir { + t.Errorf("expected port to belong to %s, got %s", workDir, alloc.Directory) + } + if alloc.Name != "main" { + t.Errorf("expected name 'main' preserved after re-lock, got %q", alloc.Name) + } + if !alloc.Locked { + t.Error("expected port to remain locked after re-lock") } } @@ -560,6 +614,7 @@ func TestScan_SkipsAlreadyAllocatedPorts(t *testing.T) { alloc := loaded.FindByPort(3501) if alloc == nil { t.Fatal("allocation for port 3501 disappeared") + return // unreachable, but satisfies staticcheck SA5011 } if alloc.Directory != existingDir { t.Errorf("expected directory %s to be preserved, got %s", existingDir, alloc.Directory) @@ -714,6 +769,7 @@ func TestLockPort_FreeUnlockedFromOtherDir_NoForceNeeded(t *testing.T) { alloc := loaded.FindByPort(3010) if alloc == nil { t.Fatal("expected allocation for port 3010") + return // unreachable, but satisfies staticcheck SA5011 } if alloc.Directory != workDir2 { t.Errorf("expected port to belong to %s, got %s", workDir2, alloc.Directory) @@ -817,6 +873,7 @@ func TestLockPort_BusyNotAllocated_RegistersAsExternal(t *testing.T) { alloc := loaded.FindByPort(3012) if alloc == nil { t.Fatal("expected allocation for port 3012") + return // unreachable, but satisfies staticcheck SA5011 } if alloc.Status != "external" { t.Errorf("expected status 'external', got %q", alloc.Status) @@ -862,6 +919,7 @@ func TestLockPort_UnlocksOldLockedPort(t *testing.T) { alloc3013 := loaded.FindByPort(3013) if alloc3013 == nil { t.Fatal("expected allocation for port 3013 to still exist") + return // unreachable, but satisfies staticcheck SA5011 } if alloc3013.Locked { t.Error("old port 3013 should be unlocked after locking new port") @@ -870,6 +928,7 @@ func TestLockPort_UnlocksOldLockedPort(t *testing.T) { alloc3014 := loaded.FindByPort(3014) if alloc3014 == nil { t.Fatal("expected allocation for port 3014") + return // unreachable, but satisfies staticcheck SA5011 } if !alloc3014.Locked { t.Error("new port 3014 should be locked") @@ -1001,6 +1060,7 @@ func TestLockPort_SameDirectoryDifferentName(t *testing.T) { alloc := loaded.FindByPort(3020) if alloc == nil { t.Fatal("expected allocation for port 3020") + return // unreachable, but satisfies staticcheck SA5011 } // Name should be preserved as "web" since we're locking an existing port if alloc.Name != "web" { @@ -1059,6 +1119,7 @@ func TestLockPort_SameDirectorySamePortIdempotent(t *testing.T) { alloc := loaded.FindByPort(3021) if alloc == nil { t.Fatal("expected allocation for port 3021") + return // unreachable, but satisfies staticcheck SA5011 } if !alloc.Locked { t.Error("expected port to remain locked") @@ -1226,6 +1287,13 @@ func TestPortSelector_ReturnsSamePortEvenWhenBusy(t *testing.T) { t.Logf("Initial port: %s", initialPort) // Step 2: Simulate user's service running on that port + portNum := 0 + if _, err := fmt.Sscanf(initialPort, "%d", &portNum); err != nil { + t.Fatalf("failed to parse port %q: %v", initialPort, err) + } + if portNum < 1 || portNum > 65535 { + t.Fatalf("port-selector returned invalid port number: %d (raw output: %q)", portNum, initialPort) + } ln, err := net.Listen("tcp", ":"+initialPort) if err != nil { t.Skipf("could not occupy port %s for test: %v", initialPort, err) diff --git a/internal/allocations/allocations.go b/internal/allocations/allocations.go index 7ba7a36..d8dea4d 100644 --- a/internal/allocations/allocations.go +++ b/internal/allocations/allocations.go @@ -941,11 +941,10 @@ func (s *Store) SetExternalAllocation(port int, pid int, user, processName, cwd // RefreshExternalAllocations removes stale external allocations (ports that are now free). // Updates LastUsedAt for allocations that are still active. -// Returns the count of removed allocations. -// Panics if isPortFree is nil (programming error). -func (s *Store) RefreshExternalAllocations(isPortFree PortChecker) int { +// Returns the count of removed allocations and an error if isPortFree is nil. +func (s *Store) RefreshExternalAllocations(isPortFree PortChecker) (int, error) { if isPortFree == nil { - panic("RefreshExternalAllocations: isPortFree function cannot be nil") + return 0, fmt.Errorf("refresh external allocations: isPortFree function cannot be nil") } var removedPorts []int @@ -991,5 +990,5 @@ func (s *Store) RefreshExternalAllocations(isPortFree PortChecker) int { logger.Field("updated", len(updatedPorts))) } - return len(removedPorts) + return len(removedPorts), nil } diff --git a/internal/allocations/allocations_test.go b/internal/allocations/allocations_test.go index 96f0f86..b115c13 100644 --- a/internal/allocations/allocations_test.go +++ b/internal/allocations/allocations_test.go @@ -2341,7 +2341,10 @@ func TestRefreshExternalAllocations_RemovesStale(t *testing.T) { return port == 3000 // 3000 is free, 3001 is busy } - removed := store.RefreshExternalAllocations(portChecker) + removed, err := store.RefreshExternalAllocations(portChecker) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if removed != 1 { t.Errorf("expected 1 removed, got %d", removed) @@ -2644,7 +2647,10 @@ func TestRefreshExternalAllocations_KeepsActive(t *testing.T) { // Port is still busy portChecker := func(port int) bool { return false } - removed := store.RefreshExternalAllocations(portChecker) + removed, err := store.RefreshExternalAllocations(portChecker) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if removed != 0 { t.Errorf("expected 0 removed, got %d", removed) @@ -2668,7 +2674,10 @@ func TestRefreshExternalAllocations_SkipsNonExternal(t *testing.T) { portChecker := func(port int) bool { return true } - removed := store.RefreshExternalAllocations(portChecker) + removed, err := store.RefreshExternalAllocations(portChecker) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } if removed != 0 { t.Errorf("expected 0 removed (non-external should be skipped), got %d", removed) @@ -2680,16 +2689,13 @@ func TestRefreshExternalAllocations_SkipsNonExternal(t *testing.T) { } } -func TestRefreshExternalAllocations_NilPortChecker_Panics(t *testing.T) { +func TestRefreshExternalAllocations_NilPortChecker_ReturnsError(t *testing.T) { store := NewStore() - defer func() { - if r := recover(); r == nil { - t.Error("expected panic with nil PortChecker, but did not panic") - } - }() - - store.RefreshExternalAllocations(nil) + _, err := store.RefreshExternalAllocations(nil) + if err == nil { + t.Error("expected error with nil PortChecker, but got nil") + } } func TestFindByPort_IncludesExternalFields(t *testing.T) {