fix: improve --lock PORT behavior and fix locked ports handling (#77)#79
Merged
fix: improve --lock PORT behavior and fix locked ports handling (#77)#79
Conversation
## Summary Improve `--lock PORT` command with smart --force logic and fix bugs where locked ports were incorrectly handled. ## Changes ### Part 1: --lock PORT improvements - Show directory in lock success message: "Locked port 3001 for 'main' in ~/project" - Smart --force logic: - Free + unlocked port from another dir: allowed without --force (abandoned) - Free + locked port from another dir: requires --force - Busy port from another dir: blocked completely (stop service first) - Busy unallocated port: requires --force (user takes responsibility) - Unlock old locked port when locking new one for same directory+name ### Part 2: Bug fix - locked+busy port ignored - Changed FindByDirectoryAndNameWithPriority to use priority: 1. Locked + free → return 2. Locked + busy → return (user's service already running) 3. Unlocked + free → return 4. Unlocked + busy → skip, find another ### Part 3: Bug fix - multiple locked ports - UnlockOtherLockedPorts ensures at most one locked port per directory+name - Called after setting new lock to maintain invariant ## Tests - Added unit tests for FindByDirectoryAndNameWithPriority - Added unit tests for UnlockOtherLockedPorts - Added integration tests for all Decision Matrix scenarios Refs #77 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Improvements based on code review feedback: - Add warning to stderr when UpdateLastUsedByPort fails (MEDIUM fix) - Add debug logging to UnlockOtherLockedPorts - Add TestFindByDirectoryAndNameWithPriority_NilPortChecker - Add TestFindByDirectoryAndNameWithPriority_TieBreakByPort - Add TestFindByDirectoryAndNameWithPriority_NoMatchingAllocations - Add TestUnlockOtherLockedPorts_EmptyStore - Add TestLockPort_SameDirectoryDifferentName - Add TestLockPort_SameDirectorySamePortIdempotent Total: +191 lines of tests and improvements Refs #77 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
9de5bf1 to
0609f57
Compare
…Port The test was using CombinedOutput() which mixed stdout and stderr. In CI, logger warning goes to stderr, causing test to fail when comparing port number output. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Improve
--lock PORTcommand with smart --force logic and fix bugs where locked ports were incorrectly handled.Part 1:
--lock PORTimprovements--forcelogic per Decision MatrixPart 2: Bug fix - locked+busy port ignored
port-selectornow correctly returns locked+busy port (user's service already running)Part 3: Bug fix - multiple locked ports
Test plan
FindByDirectoryAndNameWithPriorityUnlockOtherLockedPorts--lockbehaviorCloses #77
🤖 Generated with Claude Code