diff --git a/CHANGELOG.md b/CHANGELOG.md index e343675..ba1bd35 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added +- `--lock PORT` now shows directory in success message (#77) + - New format: `Locked port 3001 for 'main' in ~/project` + +### Changed +- Smart `--force` logic for `--lock PORT` (#77) + - Free + unlocked port from another directory: allowed without `--force` (abandoned allocation) + - Free + locked port from another directory: requires `--force` + - Busy port from another directory: blocked completely (stop the service first) + - Busy unallocated port: requires `--force` (user takes responsibility) + +### Fixed +- Locked+busy port now correctly returned by `port-selector` (#77) + - Previously created new allocation instead of returning user's running service + - New priority: locked+free > locked+busy > unlocked+free > unlocked+busy(skip) +- Locking new port now correctly unlocks old locked port for same directory+name (#77) + - Invariant: at most one locked port per directory+name combination + - Old allocation is preserved (only unlocked), not deleted + ## [0.9.5] - 2026-02-02 ### Added diff --git a/README.md b/README.md index 8be181f..ce73216 100644 --- a/README.md +++ b/README.md @@ -319,18 +319,30 @@ port-selector --unlock 3005 When using `--lock ` with a specific port number: - If the port is not allocated, it will be allocated to the current directory AND locked - This is useful when you want a specific port for a new project -- The port must be free and within the configured range -- If the port is allocated to another directory, an error is shown with hint to use `--force` +- The port must be within the configured range + +Smart `--force` behavior when the port belongs to another directory: +- **Free + unlocked**: reassigned without `--force` (abandoned allocation) +- **Free + locked**: requires `--force` to reassign +- **Busy (any)**: blocked completely — stop the service first + +When locking an unallocated busy port: +- Requires `--force` (you take responsibility for the conflict) ```bash +# Port locked by another directory - requires --force: port-selector --lock 3006 -# error: port 3006 is allocated to ~/code/other-project +# error: port 3006 is locked by ~/code/other-project # use --lock 3006 --force to reassign it to current directory -# Force reassign from another directory: +# Force reassign locked port: port-selector --lock 3006 --force # warning: port 3006 was allocated to ~/code/other-project -# Reassigned and locked port 3006 for 'main' +# Reassigned and locked port 3006 for 'main' in ~/current-project + +# Port busy on another directory - cannot reassign: +port-selector --lock 3006 --force +# error: port 3006 is in use by ~/code/other-project; stop the service first ``` When a port is locked: @@ -406,7 +418,7 @@ Options: -l, --list List all port allocations -c, --lock [PORT] Lock port for current directory and name (or specified port) -u, --unlock [PORT] Unlock port for current directory and name (or specified port) - --force, -f Force reassign port from another directory (use with --lock PORT) + --force, -f Force lock a busy port or locked port from another directory --forget Clear all port allocations for current directory --forget --name NAME Clear port allocation for current directory with specific name --forget-all Clear all port allocations diff --git a/README.ru.md b/README.ru.md index f6dabf4..53983b1 100644 --- a/README.ru.md +++ b/README.ru.md @@ -319,18 +319,30 @@ port-selector --unlock 3005 При использовании `--lock ` с конкретным номером порта: - Если порт не выделен, он будет выделен текущей директории И заблокирован - Это удобно, когда вы хотите конкретный порт для нового проекта -- Порт должен быть свободен и находиться в настроенном диапазоне -- Если порт уже выделен другой директории, выдаётся ошибка с подсказкой использовать `--force` +- Порт должен находиться в настроенном диапазоне + +Умная логика `--force`, когда порт принадлежит другой директории: +- **Свободен + разблокирован**: переназначается без `--force` (заброшенная аллокация) +- **Свободен + заблокирован**: требуется `--force` +- **Занят (любой)**: блокируется полностью — сначала остановите сервис + +При блокировке занятого порта без аллокации: +- Требуется `--force` (вы берёте ответственность за конфликт) ```bash +# Порт заблокирован другой директорией - нужен --force: port-selector --lock 3006 -# error: port 3006 is allocated to ~/code/other-project +# error: port 3006 is locked by ~/code/other-project # use --lock 3006 --force to reassign it to current directory -# Принудительное переназначение из другой директории: +# Принудительное переназначение заблокированного порта: port-selector --lock 3006 --force # warning: port 3006 was allocated to ~/code/other-project -# Reassigned and locked port 3006 for 'main' +# Reassigned and locked port 3006 for 'main' in ~/current-project + +# Порт занят другой директорией - переназначить нельзя: +port-selector --lock 3006 --force +# error: port 3006 is in use by ~/code/other-project; stop the service first ``` Когда порт заблокирован: @@ -406,7 +418,7 @@ Options: -l, --list Показать все аллокации портов -c, --lock [PORT] Заблокировать порт для текущей директории и имени (или указанный порт) -u, --unlock [PORT] Разблокировать порт для текущей директории и имени (или указанный порт) - --force, -f Принудительно переназначить порт из другой директории (с --lock PORT) + --force, -f Принудительно заблокировать занятый или чужой заблокированный порт --forget Удалить все аллокации для текущей директории --forget --name NAME Удалить аллокацию с указанным именем для текущей директории --forget-all Удалить все аллокации diff --git a/cmd/port-selector/main.go b/cmd/port-selector/main.go index 55d964e..04030f8 100644 --- a/cmd/port-selector/main.go +++ b/cmd/port-selector/main.go @@ -369,19 +369,27 @@ func runWithName(name string) error { } // Check if current directory already has an allocated port for this name - if existing := store.FindByDirectoryAndName(cwd, name); existing != nil { - debug.Printf("main", "found existing allocation for name %s: port %d", name, existing.Port) - // Check if the previously allocated port is free - if port.IsPortFree(existing.Port) { - debug.Printf("main", "existing port %d is free, reusing", existing.Port) + // Uses priority: locked+free > locked+busy > unlocked+free > unlocked+busy(skip) + if existing := store.FindByDirectoryAndNameWithPriority(cwd, name, port.IsPortFree); existing != nil { + debug.Printf("main", "found existing allocation for name %s: port %d (locked=%v)", name, existing.Port, existing.Locked) + // If locked+busy, the user's service is already running - return this port + // If free (locked or not), return this port for reuse + isFree := port.IsPortFree(existing.Port) + if isFree || existing.Locked { + if isFree { + debug.Printf("main", "existing port %d is free, reusing", existing.Port) + } else { + debug.Printf("main", "existing port %d is busy but locked (user's service running), returning it", existing.Port) + } // Update last_used timestamp for the specific port being issued if !store.UpdateLastUsedByPort(existing.Port) { debug.Printf("main", "warning: UpdateLastUsedByPort failed for port %d", existing.Port) + fmt.Fprintf(os.Stderr, "warning: failed to update timestamp for port %d\n", existing.Port) } resultPort = existing.Port return nil } - debug.Printf("main", "existing port %d is busy, need new allocation", existing.Port) + debug.Printf("main", "existing port %d is busy and unlocked, need new allocation", existing.Port) } // Get last used port for round-robin behavior @@ -595,43 +603,65 @@ func runSetLocked(name string, portArg int, locked bool, force bool) error { // Print warning if port was reassigned from another directory if reassignedFrom != "" { fmt.Fprintf(os.Stderr, "warning: port %d was allocated to %s\n", targetPort, pathutil.ShortenHomePath(reassignedFrom)) - fmt.Printf("Reassigned and locked port %d for '%s'\n", targetPort, name) + fmt.Printf("Reassigned and locked port %d for '%s' in %s\n", targetPort, name, pathutil.ShortenHomePath(cwd)) } else { action := "Locked" if !locked { action = "Unlocked" } - fmt.Printf("%s port %d for '%s'\n", action, targetPort, name) + fmt.Printf("%s port %d for '%s' in %s\n", action, targetPort, name, pathutil.ShortenHomePath(cwd)) } return nil } // lockSpecificPort handles locking/unlocking a specific port number. // Returns the port, the old directory (if reassigned), and any error. +// +// Decision Matrix for --lock PORT: +// - Require --force if: port is locked for another directory +// - Block completely (even with --force) if: port is busy on another directory +// - Allow without --force if: port not allocated, or allocated but free and unlocked +// - Special case: port busy but not in allocations — without --force error, with --force create allocation func lockSpecificPort(store *allocations.Store, name string, portArg int, cwd string, locked bool, force bool) (int, string, error) { + isBusy := !port.IsPortFree(portArg) alloc := store.FindByPort(portArg) + if alloc != nil { - // Port already allocated - check if it belongs to current directory - if alloc.Directory != cwd { - // Port belongs to another directory + // Port already allocated + if alloc.Directory == cwd { + // Port belongs to current directory - just update lock status + if !store.SetLockedByPort(portArg, locked) { + return 0, "", fmt.Errorf("internal error: allocation for port %d disappeared unexpectedly", portArg) + } + return portArg, "", nil + } + + // Port belongs to another directory + if isBusy { + // Port is busy on another directory — block completely (even with --force) + return 0, "", fmt.Errorf("port %d is in use by %s; stop the service first", + portArg, pathutil.ShortenHomePath(alloc.Directory)) + } + + // Port is free — check if it's locked + if alloc.Locked { + // Require --force to reassign locked port if !force { - return 0, "", fmt.Errorf("port %d is allocated to %s\n use --lock %d --force to reassign it to current directory", + return 0, "", fmt.Errorf("port %d is locked by %s\n use --lock %d --force to reassign it to current directory", portArg, pathutil.ShortenHomePath(alloc.Directory), portArg) } - // --force: reassign port to current directory - oldDir := alloc.Directory - store.RemoveByPort(portArg) - store.SetAllocationWithName(cwd, portArg, name) - if !store.SetLockedByPort(portArg, true) { - return 0, "", fmt.Errorf("internal error: failed to lock port %d after reassignment", portArg) - } - return portArg, oldDir, nil } - // Port belongs to current directory - just update lock status - if !store.SetLockedByPort(portArg, locked) { - return 0, "", fmt.Errorf("internal error: allocation for port %d disappeared unexpectedly", portArg) + // Port is free and (unlocked OR --force provided) — allow reassignment + oldDir := alloc.Directory + store.RemoveByPort(portArg) + store.SetAllocationWithName(cwd, portArg, name) + if !store.SetLockedByPort(portArg, true) { + return 0, "", fmt.Errorf("internal error: failed to lock port %d after reassignment", portArg) } - return portArg, "", nil + // Unlock any previously locked ports for this directory+name (invariant: at most one locked) + // This is done AFTER locking the new port so old locked ports are preserved during SetAllocation + store.UnlockOtherLockedPorts(cwd, name, portArg) + return portArg, oldDir, nil } // Port not allocated yet @@ -649,20 +679,28 @@ func lockSpecificPort(store *allocations.Store, name string, portArg int, cwd st return 0, "", fmt.Errorf("port %d is outside configured range %d-%d", portArg, cfg.PortStart, cfg.PortEnd) } - if !port.IsPortFree(portArg) { - if procInfo := port.GetPortProcess(portArg); procInfo != nil { - return 0, "", fmt.Errorf("port %d is in use by another process (%s)", portArg, procInfo) + if isBusy { + // Port busy but NOT in allocations + if !force { + if procInfo := port.GetPortProcess(portArg); procInfo != nil { + return 0, "", fmt.Errorf("port %d is in use by another process (%s)", portArg, procInfo) + } + return 0, "", fmt.Errorf("port %d is in use", portArg) } - return 0, "", fmt.Errorf("port %d is in use by another process", portArg) + // With --force: create allocation even though port is busy (user takes responsibility) } // Allocate and lock the port for this directory and name - // This will replace any existing allocation for the same name + // SetAllocationWithName preserves locked ports (they won't be deleted) store.SetAllocationWithName(cwd, portArg, name) if !store.SetLockedByPort(portArg, true) { return 0, "", fmt.Errorf("internal error: failed to lock port %d after allocation", portArg) } + // Unlock any previously locked ports for this directory+name (invariant: at most one locked) + // This is done AFTER locking the new port so old locked ports are preserved during SetAllocation + store.UnlockOtherLockedPorts(cwd, name, portArg) + return portArg, "", nil } @@ -835,7 +873,7 @@ Options: -l, --list List all port allocations -c, --lock [PORT] Lock port for current directory and name (or specified port) -u, --unlock [PORT] Unlock port for current directory and name (or specified port) - --force, -f Force reassign port from another directory (use with --lock PORT) + --force, -f Force lock a busy port or locked port from another directory --forget Clear all port allocations for current directory --forget --name NAME Clear port allocation for current directory with specific name --forget-all Clear all port allocations @@ -864,10 +902,15 @@ Port Locking: Use this for long-running services. Using --lock with a port number will allocate AND lock that port - to the current directory/name in one step (if the port is free). + to the current directory/name in one step. + + When --lock PORT targets another directory's port: + - Free + unlocked: reassigned without --force (abandoned allocation) + - Free + locked: requires --force to reassign + - Busy (any): blocked completely — stop the service first - If the port is already allocated to another directory, an error is shown. - Use --force to reassign the port to the current directory. + When --lock PORT targets a busy unallocated port: + - Requires --force (you take responsibility for the conflict) Configuration: ~/.config/port-selector/default.yaml diff --git a/cmd/port-selector/main_test.go b/cmd/port-selector/main_test.go index 35a9a5d..ae036fb 100644 --- a/cmd/port-selector/main_test.go +++ b/cmd/port-selector/main_test.go @@ -1,6 +1,7 @@ package main import ( + "bytes" "net" "os" "os/exec" @@ -332,6 +333,7 @@ func TestLockPortFromAnotherDirectory_Error(t *testing.T) { } // Step 2: Try to lock port 3001 from project2 (should fail without --force) + // Port is now locked by project1, so error is "is locked by" cmd = exec.Command(binary, "--lock", "3001") cmd.Dir = workDir2 cmd.Env = env @@ -339,8 +341,8 @@ func TestLockPortFromAnotherDirectory_Error(t *testing.T) { if err == nil { t.Fatalf("expected error when locking port from another directory, got success: %s", output) } - if !strings.Contains(string(output), "is allocated to") { - t.Errorf("expected 'is allocated to' error, got: %s", output) + if !strings.Contains(string(output), "is locked by") { + t.Errorf("expected 'is locked by' error, got: %s", output) } if !strings.Contains(string(output), "--force") { t.Errorf("expected '--force' hint in error, got: %s", output) @@ -650,3 +652,400 @@ func TestLockedPortExcludedFromAllocation(t *testing.T) { t.Error("port 3000 should NOT be excluded for project-a (its own port)") } } + +// Tests for issue #77: Smart --force logic + +func TestLockPort_FreeUnlockedFromOtherDir_NoForceNeeded(t *testing.T) { + binary := buildBinary(t) + + tmpDir := t.TempDir() + configDir := filepath.Join(tmpDir, ".config", "port-selector") + if err := os.MkdirAll(configDir, 0755); err != nil { + t.Fatal(err) + } + + workDir1 := filepath.Join(tmpDir, "project1") + workDir2 := filepath.Join(tmpDir, "project2") + if err := os.MkdirAll(workDir1, 0755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(workDir2, 0755); err != nil { + t.Fatal(err) + } + + env := append(os.Environ(), "XDG_CONFIG_HOME="+filepath.Join(tmpDir, ".config")) + + // Create allocation for project1 (free and unlocked - abandoned) + store := allocations.NewStore() + store.SetAllocationWithName(workDir1, 3010, "main") + // NOT locked, so it's "abandoned" + if err := allocations.Save(configDir, store); err != nil { + t.Fatal(err) + } + + // Try to lock from project2 without --force (should succeed because port is free and unlocked) + cmd := exec.Command(binary, "--lock", "3010") + cmd.Dir = workDir2 + cmd.Env = env + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("expected success (free+unlocked allows reassignment), got error: %v, output: %s", err, output) + } + + // Verify port is now allocated to project2 + loaded, _ := allocations.Load(configDir) + alloc := loaded.FindByPort(3010) + if alloc == nil { + t.Fatal("expected allocation for port 3010") + } + if alloc.Directory != workDir2 { + t.Errorf("expected port to belong to %s, got %s", workDir2, alloc.Directory) + } +} + +func TestLockPort_BusyFromOtherDir_BlocksEvenWithForce(t *testing.T) { + binary := buildBinary(t) + + tmpDir := t.TempDir() + configDir := filepath.Join(tmpDir, ".config", "port-selector") + if err := os.MkdirAll(configDir, 0755); err != nil { + t.Fatal(err) + } + + workDir1 := filepath.Join(tmpDir, "project1") + workDir2 := filepath.Join(tmpDir, "project2") + if err := os.MkdirAll(workDir1, 0755); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(workDir2, 0755); err != nil { + t.Fatal(err) + } + + // Occupy port to simulate busy port + ln, err := net.Listen("tcp", ":3011") + if err != nil { + t.Skipf("could not occupy port 3011 for test: %v", err) + } + defer ln.Close() + + env := append(os.Environ(), "XDG_CONFIG_HOME="+filepath.Join(tmpDir, ".config")) + + // Create allocation for project1 (busy) + store := allocations.NewStore() + store.SetAllocationWithName(workDir1, 3011, "main") + if err := allocations.Save(configDir, store); err != nil { + t.Fatal(err) + } + + // Try to lock from project2 with --force (should fail because port is busy on another dir) + cmd := exec.Command(binary, "--lock", "--force", "3011") + cmd.Dir = workDir2 + cmd.Env = env + output, err := cmd.CombinedOutput() + if err == nil { + t.Fatalf("expected error (busy port on another dir), got success: %s", output) + } + if !strings.Contains(string(output), "in use by") { + t.Errorf("expected 'in use by' error, got: %s", output) + } + if !strings.Contains(string(output), "stop the service") { + t.Errorf("expected 'stop the service' hint, got: %s", output) + } +} + +func TestLockPort_BusyNotAllocated_RequiresForce(t *testing.T) { + binary := buildBinary(t) + + tmpDir := t.TempDir() + configDir := filepath.Join(tmpDir, ".config", "port-selector") + if err := os.MkdirAll(configDir, 0755); err != nil { + t.Fatal(err) + } + + workDir := filepath.Join(tmpDir, "project") + if err := os.MkdirAll(workDir, 0755); err != nil { + t.Fatal(err) + } + + // Occupy port to simulate busy port + ln, err := net.Listen("tcp", ":3012") + if err != nil { + t.Skipf("could not occupy port 3012 for test: %v", err) + } + defer ln.Close() + + env := append(os.Environ(), "XDG_CONFIG_HOME="+filepath.Join(tmpDir, ".config")) + + // Try to lock without --force (should fail) + cmd := exec.Command(binary, "--lock", "3012") + cmd.Dir = workDir + cmd.Env = env + output, err := cmd.CombinedOutput() + if err == nil { + t.Fatalf("expected error (busy port not allocated), got success: %s", output) + } + if !strings.Contains(string(output), "in use") { + t.Errorf("expected 'in use' error, got: %s", output) + } + + // Try with --force (should succeed - user takes responsibility) + cmd = exec.Command(binary, "--lock", "--force", "3012") + cmd.Dir = workDir + cmd.Env = env + output, err = cmd.CombinedOutput() + if err != nil { + t.Fatalf("expected success with --force, got error: %v, output: %s", err, output) + } + + // Verify allocation was created + loaded, _ := allocations.Load(configDir) + alloc := loaded.FindByPort(3012) + if alloc == nil { + t.Fatal("expected allocation for port 3012") + } + if alloc.Directory != workDir { + t.Errorf("expected port to belong to %s, got %s", workDir, alloc.Directory) + } +} + +func TestLockPort_UnlocksOldLockedPort(t *testing.T) { + binary := buildBinary(t) + + tmpDir := t.TempDir() + configDir := filepath.Join(tmpDir, ".config", "port-selector") + if err := os.MkdirAll(configDir, 0755); err != nil { + t.Fatal(err) + } + + workDir := filepath.Join(tmpDir, "project") + if err := os.MkdirAll(workDir, 0755); err != nil { + t.Fatal(err) + } + + env := append(os.Environ(), "XDG_CONFIG_HOME="+filepath.Join(tmpDir, ".config")) + + // Create allocation for project with locked port 3013 + store := allocations.NewStore() + store.SetAllocationWithName(workDir, 3013, "main") + store.SetLockedByPort(3013, true) + if err := allocations.Save(configDir, store); err != nil { + t.Fatal(err) + } + + // Lock new port 3014 for same directory+name + cmd := exec.Command(binary, "--lock", "3014") + cmd.Dir = workDir + cmd.Env = env + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("expected success, got error: %v, output: %s", err, output) + } + + // Verify old port 3013 is unlocked, new port 3014 is locked + loaded, _ := allocations.Load(configDir) + + alloc3013 := loaded.FindByPort(3013) + if alloc3013 == nil { + t.Fatal("expected allocation for port 3013 to still exist") + } + if alloc3013.Locked { + t.Error("old port 3013 should be unlocked after locking new port") + } + + alloc3014 := loaded.FindByPort(3014) + if alloc3014 == nil { + t.Fatal("expected allocation for port 3014") + } + if !alloc3014.Locked { + t.Error("new port 3014 should be locked") + } +} + +func TestLockMessage_ShowsDirectory(t *testing.T) { + binary := buildBinary(t) + + tmpDir := t.TempDir() + configDir := filepath.Join(tmpDir, ".config", "port-selector") + if err := os.MkdirAll(configDir, 0755); err != nil { + t.Fatal(err) + } + + workDir := filepath.Join(tmpDir, "project") + if err := os.MkdirAll(workDir, 0755); err != nil { + t.Fatal(err) + } + + env := append(os.Environ(), "XDG_CONFIG_HOME="+filepath.Join(tmpDir, ".config")) + + // Lock a port + cmd := exec.Command(binary, "--lock", "3015") + cmd.Dir = workDir + cmd.Env = env + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("expected success, got error: %v, output: %s", err, output) + } + + // Verify message shows directory + if !strings.Contains(string(output), "in ") { + t.Errorf("expected 'in ' in message, got: %s", output) + } + if !strings.Contains(string(output), "project") { + t.Errorf("expected directory path in message, got: %s", output) + } +} + +func TestPortSelector_ReturnsLockedBusyPort(t *testing.T) { + binary := buildBinary(t) + + tmpDir := t.TempDir() + configDir := filepath.Join(tmpDir, ".config", "port-selector") + if err := os.MkdirAll(configDir, 0755); err != nil { + t.Fatal(err) + } + + workDir := filepath.Join(tmpDir, "project") + if err := os.MkdirAll(workDir, 0755); err != nil { + t.Fatal(err) + } + + // Occupy port to simulate user's service running + ln, err := net.Listen("tcp", ":3016") + if err != nil { + t.Skipf("could not occupy port 3016 for test: %v", err) + } + defer ln.Close() + + env := append(os.Environ(), "XDG_CONFIG_HOME="+filepath.Join(tmpDir, ".config")) + + // Create locked allocation for this directory + store := allocations.NewStore() + store.SetAllocationWithName(workDir, 3016, "main") + store.SetLockedByPort(3016, true) + if err := allocations.Save(configDir, store); err != nil { + t.Fatal(err) + } + + // Run port-selector - should return locked+busy port (user's service already running) + cmd := exec.Command(binary) + cmd.Dir = workDir + cmd.Env = env + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err = cmd.Run() + if err != nil { + t.Fatalf("expected success, got error: %v, stderr: %s", err, stderr.String()) + } + + port := strings.TrimSpace(stdout.String()) + if port != "3016" { + t.Errorf("expected port 3016 (locked+busy), got: %s (stderr: %s)", port, stderr.String()) + } +} + +func TestLockPort_SameDirectoryDifferentName(t *testing.T) { + binary := buildBinary(t) + + tmpDir := t.TempDir() + configDir := filepath.Join(tmpDir, ".config", "port-selector") + if err := os.MkdirAll(configDir, 0755); err != nil { + t.Fatal(err) + } + + workDir := filepath.Join(tmpDir, "project") + if err := os.MkdirAll(workDir, 0755); err != nil { + t.Fatal(err) + } + + env := append(os.Environ(), "XDG_CONFIG_HOME="+filepath.Join(tmpDir, ".config")) + + // Create allocation for "web" name + store := allocations.NewStore() + store.SetAllocationWithName(workDir, 3020, "web") + if err := allocations.Save(configDir, store); err != nil { + t.Fatal(err) + } + + // Lock same port from same dir but default name "main" + // This should lock the port but keep the existing name "web" + // (user is locking a specific port, not changing its name) + cmd := exec.Command(binary, "--lock", "3020") + cmd.Dir = workDir + cmd.Env = env + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("expected success, got error: %v, output: %s", err, output) + } + + // Verify port is locked but name is preserved + loaded, err := allocations.Load(configDir) + if err != nil { + t.Fatalf("failed to load allocations: %v", err) + } + alloc := loaded.FindByPort(3020) + if alloc == nil { + t.Fatal("expected allocation for port 3020") + } + // Name should be preserved as "web" since we're locking an existing port + if alloc.Name != "web" { + t.Errorf("expected name 'web' (preserved), got %q", alloc.Name) + } + if !alloc.Locked { + t.Error("expected port to be locked") + } +} + +func TestLockPort_SameDirectorySamePortIdempotent(t *testing.T) { + binary := buildBinary(t) + + tmpDir := t.TempDir() + configDir := filepath.Join(tmpDir, ".config", "port-selector") + if err := os.MkdirAll(configDir, 0755); err != nil { + t.Fatal(err) + } + + workDir := filepath.Join(tmpDir, "project") + if err := os.MkdirAll(workDir, 0755); err != nil { + t.Fatal(err) + } + + // Occupy port to simulate service running + ln, err := net.Listen("tcp", ":3021") + if err != nil { + t.Skipf("could not occupy port 3021 for test: %v", err) + } + defer ln.Close() + + env := append(os.Environ(), "XDG_CONFIG_HOME="+filepath.Join(tmpDir, ".config")) + + // Create locked allocation for same directory + store := allocations.NewStore() + store.SetAllocationWithName(workDir, 3021, "main") + store.SetLockedByPort(3021, true) + if err := allocations.Save(configDir, store); err != nil { + t.Fatal(err) + } + + // Lock same port again (idempotent operation) + cmd := exec.Command(binary, "--lock", "3021") + cmd.Dir = workDir + cmd.Env = env + output, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("expected success (idempotent lock), got error: %v, output: %s", err, output) + } + + // Should still be locked + loaded, err := allocations.Load(configDir) + if err != nil { + t.Fatalf("failed to load allocations: %v", err) + } + alloc := loaded.FindByPort(3021) + if alloc == nil { + t.Fatal("expected allocation for port 3021") + } + if !alloc.Locked { + t.Error("expected port to remain locked") + } +} diff --git a/internal/allocations/allocations.go b/internal/allocations/allocations.go index a0a0491..25a27ce 100644 --- a/internal/allocations/allocations.go +++ b/internal/allocations/allocations.go @@ -729,6 +729,8 @@ func normalizeName(name string) string { // FindByDirectoryAndName returns the allocation for a given directory and name, or nil if not found. // When multiple ports are allocated to the same directory/name, returns the most recently used one. +// Note: This method does not check port availability. Use FindByDirectoryAndNameWithPriority +// for smart selection that considers locked status and port availability. func (s *Store) FindByDirectoryAndName(dir string, name string) *Allocation { dir = filepath.Clean(dir) name = normalizeName(name) @@ -771,6 +773,112 @@ func (s *Store) FindByDirectoryAndName(dir string, name string) *Allocation { } } +// FindByDirectoryAndNameWithPriority returns the best allocation for a given directory and name. +// Uses smart priority based on locked status and port availability: +// 1. Locked + free → return (reserved port is available) +// 2. Locked + busy → return (user's service is already running on their reserved port) +// 3. Unlocked + free → return (can reuse this port) +// 4. Unlocked + busy → skip (port in use, find another) +// +// Returns nil if no suitable allocation found (all are unlocked+busy or none exist). +func (s *Store) FindByDirectoryAndNameWithPriority(dir string, name string, isPortFree PortChecker) *Allocation { + dir = filepath.Clean(dir) + name = normalizeName(name) + + // Collect all matching allocations + type candidate struct { + port int + info *AllocationInfo + free bool + } + var candidates []candidate + + for port, info := range s.Allocations { + if info == nil || info.Directory != dir || info.Name != name { + continue + } + free := isPortFree != nil && isPortFree(port) + candidates = append(candidates, candidate{port: port, info: info, free: free}) + } + + if len(candidates) == 0 { + return nil + } + + // Priority selection: + // 1. Locked + free + // 2. Locked + busy + // 3. Unlocked + free + // 4. Unlocked + busy (skip) + + var best *candidate + + for i := range candidates { + c := &candidates[i] + + // Skip unlocked + busy (priority 4) + if !c.info.Locked && !c.free { + continue + } + + if best == nil { + best = c + continue + } + + // Compare priorities + bestPriority := getPriority(best.info.Locked, best.free) + cPriority := getPriority(c.info.Locked, c.free) + + if cPriority < bestPriority { + // Lower priority number = better + best = c + } else if cPriority == bestPriority { + // Same priority - use most recent time, then lower port as tiebreaker + bestTime := best.info.LastUsedAt + if bestTime.IsZero() { + bestTime = best.info.AssignedAt + } + cTime := c.info.LastUsedAt + if cTime.IsZero() { + cTime = c.info.AssignedAt + } + if cTime.After(bestTime) || (cTime.Equal(bestTime) && c.port < best.port) { + best = c + } + } + } + + if best == nil { + return nil + } + + return &Allocation{ + Port: best.port, + Directory: best.info.Directory, + AssignedAt: best.info.AssignedAt, + LastUsedAt: best.info.LastUsedAt, + Locked: best.info.Locked, + ProcessName: best.info.ProcessName, + ContainerID: best.info.ContainerID, + Name: best.info.Name, + } +} + +// getPriority returns priority number for locked/free combination (lower = better). +func getPriority(locked, free bool) int { + if locked && free { + return 1 + } + if locked && !free { + return 2 + } + if !locked && free { + return 3 + } + return 4 // unlocked + busy (should be skipped) +} + // RemoveByDirectoryAndName removes the allocation for a given directory and name. // Returns the removed allocation and true if found, nil and false otherwise. func (s *Store) RemoveByDirectoryAndName(dir string, name string) (*Allocation, bool) { @@ -857,3 +965,29 @@ func (s *Store) SetLockedByPortAndName(port int, name string, locked bool) bool logger.Log(logger.AllocLock, logger.Field("port", port), logger.Field("locked", locked), logger.Field("name", name)) return true } + +// UnlockOtherLockedPorts unlocks all locked ports for the given directory and name, +// except the specified port. This ensures the invariant: at most one locked port +// per directory+name combination. +// Returns the count of ports that were unlocked. +func (s *Store) UnlockOtherLockedPorts(dir string, name string, exceptPort int) int { + dir = filepath.Clean(dir) + name = normalizeName(name) + debug.Printf("allocations", "UnlockOtherLockedPorts: dir=%s name=%s exceptPort=%d", dir, name, exceptPort) + count := 0 + for port, info := range s.Allocations { + if info == nil || port == exceptPort { + continue + } + if info.Directory == dir && info.Name == name && info.Locked { + info.Locked = false + logger.Log(logger.AllocLock, + logger.Field("port", port), + logger.Field("locked", false), + logger.Field("name", name), + logger.Field("reason", "new_lock_for_same_name")) + count++ + } + } + return count +} diff --git a/internal/allocations/allocations_test.go b/internal/allocations/allocations_test.go index 6c3f5d6..a9414b4 100644 --- a/internal/allocations/allocations_test.go +++ b/internal/allocations/allocations_test.go @@ -2409,3 +2409,290 @@ func TestSetAllocation_PreservesLockedPorts(t *testing.T) { t.Errorf("expected 2 allocations (locked + new), got %d", len(store.Allocations)) } } + +// Tests for issue #77: FindByDirectoryAndNameWithPriority + +func TestFindByDirectoryAndNameWithPriority_LockedFreeFirst(t *testing.T) { + now := time.Now() + store := NewStore() + + // Unlocked + free (would be selected by time in old logic) + store.Allocations[3000] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: now, + LastUsedAt: now, + Locked: false, + } + // Locked + free (should be selected by priority) + store.Allocations[3001] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: now.Add(-1 * time.Hour), // older + LastUsedAt: now.Add(-1 * time.Hour), + Locked: true, + } + + allFree := func(port int) bool { return true } + result := store.FindByDirectoryAndNameWithPriority("/home/user/project", "main", allFree) + + if result == nil { + t.Fatal("expected allocation, got nil") + } + if result.Port != 3001 { + t.Errorf("expected locked port 3001, got %d", result.Port) + } +} + +func TestFindByDirectoryAndNameWithPriority_LockedBusyReturned(t *testing.T) { + now := time.Now() + store := NewStore() + + // Unlocked + free + store.Allocations[3000] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: now, + LastUsedAt: now, + Locked: false, + } + // Locked + busy (should be returned - user's service is running) + store.Allocations[3001] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: now.Add(-1 * time.Hour), + LastUsedAt: now.Add(-1 * time.Hour), + Locked: true, + } + + // 3001 is busy, 3000 is free + checker := func(port int) bool { return port == 3000 } + result := store.FindByDirectoryAndNameWithPriority("/home/user/project", "main", checker) + + if result == nil { + t.Fatal("expected allocation, got nil") + } + // Locked+busy (priority 2) beats unlocked+free (priority 3) + if result.Port != 3001 { + t.Errorf("expected locked+busy port 3001 (user's service running), got %d", result.Port) + } +} + +func TestFindByDirectoryAndNameWithPriority_SkipsUnlockedBusy(t *testing.T) { + now := time.Now() + store := NewStore() + + // Unlocked + busy (should be skipped) + store.Allocations[3000] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: now, + LastUsedAt: now, + Locked: false, + } + // Unlocked + free (should be returned) + store.Allocations[3001] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: now.Add(-1 * time.Hour), // older but free + LastUsedAt: now.Add(-1 * time.Hour), + Locked: false, + } + + // 3000 is busy, 3001 is free + checker := func(port int) bool { return port == 3001 } + result := store.FindByDirectoryAndNameWithPriority("/home/user/project", "main", checker) + + if result == nil { + t.Fatal("expected allocation, got nil") + } + if result.Port != 3001 { + t.Errorf("expected free port 3001, got %d (unlocked+busy should be skipped)", result.Port) + } +} + +func TestFindByDirectoryAndNameWithPriority_AllUnlockedBusyReturnsNil(t *testing.T) { + now := time.Now() + store := NewStore() + + // All unlocked + busy + store.Allocations[3000] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: now, + LastUsedAt: now, + Locked: false, + } + store.Allocations[3001] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: now.Add(-1 * time.Hour), + LastUsedAt: now.Add(-1 * time.Hour), + Locked: false, + } + + allBusy := func(port int) bool { return false } + result := store.FindByDirectoryAndNameWithPriority("/home/user/project", "main", allBusy) + + if result != nil { + t.Errorf("expected nil (all unlocked+busy should be skipped), got port %d", result.Port) + } +} + +// Tests for UnlockOtherLockedPorts + +func TestUnlockOtherLockedPorts(t *testing.T) { + store := NewStore() + + // Two locked ports for same directory+name + store.Allocations[3000] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + Locked: true, + } + store.Allocations[3001] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + Locked: true, + } + // Different name - should not be unlocked + store.Allocations[3002] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "web", + Locked: true, + } + // Different directory - should not be unlocked + store.Allocations[3003] = &AllocationInfo{ + Directory: "/home/user/other", + Name: "main", + Locked: true, + } + + // Unlock all locked ports for main except 3001 + count := store.UnlockOtherLockedPorts("/home/user/project", "main", 3001) + + if count != 1 { + t.Errorf("expected 1 port unlocked, got %d", count) + } + + // 3000 should be unlocked + if store.Allocations[3000].Locked { + t.Error("port 3000 should be unlocked") + } + // 3001 should remain locked (it's the except port) + if !store.Allocations[3001].Locked { + t.Error("port 3001 should remain locked") + } + // 3002 should remain locked (different name) + if !store.Allocations[3002].Locked { + t.Error("port 3002 should remain locked (different name)") + } + // 3003 should remain locked (different directory) + if !store.Allocations[3003].Locked { + t.Error("port 3003 should remain locked (different directory)") + } +} + +func TestUnlockOtherLockedPorts_NoOtherLocked(t *testing.T) { + store := NewStore() + + // Only one locked port + store.Allocations[3000] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + Locked: true, + } + + // Try to unlock others except 3000 + count := store.UnlockOtherLockedPorts("/home/user/project", "main", 3000) + + if count != 0 { + t.Errorf("expected 0 ports unlocked, got %d", count) + } + if !store.Allocations[3000].Locked { + t.Error("port 3000 should remain locked") + } +} + +func TestUnlockOtherLockedPorts_EmptyStore(t *testing.T) { + store := NewStore() + count := store.UnlockOtherLockedPorts("/home/user/project", "main", 3000) + if count != 0 { + t.Errorf("expected 0 for empty store, got %d", count) + } +} + +func TestFindByDirectoryAndNameWithPriority_NilPortChecker(t *testing.T) { + now := time.Now() + store := NewStore() + + // Locked port + store.Allocations[3000] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: now, + Locked: true, + } + // Unlocked port + store.Allocations[3001] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: now, + Locked: false, + } + + // nil checker - all ports considered busy + result := store.FindByDirectoryAndNameWithPriority("/home/user/project", "main", nil) + + // Should return locked port (locked+busy has priority 2, unlocked+busy is skipped) + if result == nil { + t.Fatal("expected allocation, got nil") + } + if result.Port != 3000 { + t.Errorf("expected locked port 3000, got %d", result.Port) + } +} + +func TestFindByDirectoryAndNameWithPriority_TieBreakByPort(t *testing.T) { + sameTime := time.Now() + store := NewStore() + + // Two locked+free ports with same time + store.Allocations[3002] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: sameTime, + LastUsedAt: sameTime, + Locked: true, + } + store.Allocations[3000] = &AllocationInfo{ + Directory: "/home/user/project", + Name: "main", + AssignedAt: sameTime, + LastUsedAt: sameTime, + Locked: true, + } + + allFree := func(port int) bool { return true } + + // Should be deterministic - lower port wins as tiebreaker + for i := 0; i < 10; i++ { + result := store.FindByDirectoryAndNameWithPriority("/home/user/project", "main", allFree) + if result.Port != 3000 { + t.Errorf("iteration %d: expected port 3000 (tiebreaker), got %d", i, result.Port) + } + } +} + +func TestFindByDirectoryAndNameWithPriority_NoMatchingAllocations(t *testing.T) { + store := NewStore() + store.Allocations[3000] = &AllocationInfo{ + Directory: "/home/user/other", + Name: "main", + } + + result := store.FindByDirectoryAndNameWithPriority("/home/user/project", "main", func(int) bool { return true }) + if result != nil { + t.Errorf("expected nil for non-matching dir, got port %d", result.Port) + } +}