Fix WebUI model menu flickering and improve port scanning#17
Open
scouzi1966 wants to merge 1 commit intomainfrom
Open
Fix WebUI model menu flickering and improve port scanning#17scouzi1966 wants to merge 1 commit intomainfrom
scouzi1966 wants to merge 1 commit intomainfrom
Conversation
WebUI fixes: - Add _selectingModel flag to prevent re-entrant dropdown triggers - Add _modelRestorationAttempted flag to only restore preferred model once - Remove aggressive auto-switch logic that caused 2-second flickering Port scanning improvements: - Use lsof to get actually listening ports instead of TCP connect scanning - Add macOS ephemeral port range (49152-65535) to scan ranges - Much faster scanning since we only probe ports that are actually listening - Fix lsof output parsing to correctly extract port numbers Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reviewer's GuideSwitches backend discovery from TCP probing to lsof-based listening-port detection (including macOS ephemeral ports) and hardens the WebUI model dropdown auto-selection logic to prevent flicker and re-entrancy in gateway mode. Sequence diagram for lsof-based port scanning in scanOpenPortssequenceDiagram
actor User
participant BackendDiscoveryService
participant OS_lsof as lsof
participant BackendDefinition
participant Logger
User->>BackendDiscoveryService: scanOpenPorts()
BackendDiscoveryService->>BackendDefinition: read allKnown.defaultPort
BackendDefinition-->>BackendDiscoveryService: knownPorts
BackendDiscoveryService->>BackendDefinition: read blacklistedPorts
BackendDefinition-->>BackendDiscoveryService: blacklisted
BackendDiscoveryService->>OS_lsof: getListeningPorts() via lsof -iTCP -sTCP:LISTEN -nP
OS_lsof-->>BackendDiscoveryService: stdout with listening sockets
BackendDiscoveryService->>BackendDiscoveryService: parse NAME column to Set~Int~ listeningPorts
BackendDiscoveryService->>BackendDiscoveryService: filter listeningPorts by scanPortRanges, selfPort, knownPorts, blacklisted
BackendDiscoveryService->>Logger: log "Port scan found N listening port(s)"
BackendDiscoveryService->>BackendDiscoveryService: withTaskGroup for each open port
BackendDiscoveryService-->>User: discovered backends merged
Class diagram for BackendDiscoveryService and WebUI model selection flagsclassDiagram
class BackendDiscoveryService {
- static scanPortRanges : [ClosedRange~Int~]
- selfPort : Int
- logger : Logger
+ scanKnownBackends() async
+ scanOpenPorts() async
- getListeningPorts() Set~Int~
}
class BackendDefinition {
<<static>>
+ allKnown : [BackendDefinition]
+ blacklistedPorts : Set~Int~
+ defaultPort : Int
}
class DiscoveredBackend {
+ id : String
+ baseURL : URL
}
BackendDiscoveryService --> BackendDefinition : uses
BackendDiscoveryService --> DiscoveredBackend : discovers
class Server {
- _autoSelectDone : Bool
- _userClickedModel : Bool
- _isMultiModel : Bool
- _modelRestorationAttempted : Bool
- _lastModel : String
- _selectingModel : Bool
- _modelsCache : Any
- _preferredModel : String
+ autoSelectDefault()
+ selectModelByName(name, force)
+ updateInfoStrip()
}
Server ..> BackendDiscoveryService : frontends gateway
State diagram for WebUI model restoration and dropdown selection flagsstateDiagram-v2
[*] --> Initial
state Initial {
[*] --> Idle
Idle: _selectingModel=false
Idle: _modelRestorationAttempted=false
}
Idle --> NeedRestore : SPA_reset_detected_and_isMultiModel
NeedRestore: _selectingModel=false
NeedRestore: _modelRestorationAttempted=false
NeedRestore --> SelectingPreferredModel : selectModelByName_called
SelectingPreferredModel: _selectingModel=true
SelectingPreferredModel --> ModelRestored : preferred_model_found_and_clicked
SelectingPreferredModel --> ModelRestored : preferred_model_not_found_dropdown_closed
ModelRestored: _selectingModel=false
ModelRestored: _modelRestorationAttempted=true
ModelRestored --> StableSelected : info_strip_updated
StableSelected: _selectingModel=false
StableSelected: _modelRestorationAttempted=true
StableSelected --> StableSelected : user_clicks_model_dropdown_and_changes_model
Idle --> StableSelected : model_already_selected_on_load
StableSelected --> [*] : page_unload_or_navigation
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
getListeningPortsimplementation assumes/usr/sbin/lsofexists and succeeds; consider gating this logic by platform and/or falling back to the previous TCP probe approach when the binary is missing or exits non‑zero so discovery does not silently degrade to returning an empty set. getListeningPortsis a synchronous, nonisolated call that runsProcessandwaitUntilExit(); if invoked from latency‑sensitive paths, consider moving the lsof invocation to a separate async task or background queue to avoid blocking the actor’s caller thread.- The lsof parsing loop processes every line, including the header, and matches any column with a colon; to make this more robust, explicitly skip the first header line and narrow the match to the NAME column (e.g., by index) or a stricter regex to avoid accidentally interpreting unrelated fields as ports.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `getListeningPorts` implementation assumes `/usr/sbin/lsof` exists and succeeds; consider gating this logic by platform and/or falling back to the previous TCP probe approach when the binary is missing or exits non‑zero so discovery does not silently degrade to returning an empty set.
- `getListeningPorts` is a synchronous, nonisolated call that runs `Process` and `waitUntilExit()`; if invoked from latency‑sensitive paths, consider moving the lsof invocation to a separate async task or background queue to avoid blocking the actor’s caller thread.
- The lsof parsing loop processes every line, including the header, and matches any column with a colon; to make this more robust, explicitly skip the first header line and narrow the match to the NAME column (e.g., by index) or a stricter regex to avoid accidentally interpreting unrelated fields as ports.
## Individual Comments
### Comment 1
<location> `Sources/MacLocalAPI/Services/BackendDiscoveryService.swift:92-97` </location>
<code_context>
+ /// Get all TCP ports currently listening on localhost using lsof.
+ /// This is much faster than attempting TCP connections to thousands of ports.
+ private nonisolated func getListeningPorts() -> Set<Int> {
+ let process = Process()
+ process.executableURL = URL(fileURLWithPath: "/usr/sbin/lsof")
+ process.arguments = ["-iTCP", "-sTCP:LISTEN", "-nP"]
+
+ let pipe = Pipe()
+ process.standardOutput = pipe
+ process.standardError = FileHandle.nullDevice
+
+ do {
+ try process.run()
+ process.waitUntilExit()
+ } catch {
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider checking `terminationStatus` and early-returning on lsof failures before parsing stdout.
Currently we only handle the case where `process.run()` throws. If `lsof` runs but exits non‑zero (e.g. permission error, sandboxing, missing binary), we’ll still parse stdout and may treat partial/invalid output as real ports. Instead, after `waitUntilExit()` we should check `terminationStatus == 0` before reading/parsing the pipe, and return `[]` on failure to avoid using erroneous results.
```suggestion
do {
try process.run()
process.waitUntilExit()
// If lsof exited with a non-zero status, treat it as a failure and return no ports.
guard process.terminationStatus == 0 else {
return []
}
} catch {
return []
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+92
to
+97
| do { | ||
| try process.run() | ||
| process.waitUntilExit() | ||
| } catch { | ||
| return [] | ||
| } |
There was a problem hiding this comment.
suggestion (bug_risk): Consider checking terminationStatus and early-returning on lsof failures before parsing stdout.
Currently we only handle the case where process.run() throws. If lsof runs but exits non‑zero (e.g. permission error, sandboxing, missing binary), we’ll still parse stdout and may treat partial/invalid output as real ports. Instead, after waitUntilExit() we should check terminationStatus == 0 before reading/parsing the pipe, and return [] on failure to avoid using erroneous results.
Suggested change
| do { | |
| try process.run() | |
| process.waitUntilExit() | |
| } catch { | |
| return [] | |
| } | |
| do { | |
| try process.run() | |
| process.waitUntilExit() | |
| // If lsof exited with a non-zero status, treat it as a failure and return no ports. | |
| guard process.terminationStatus == 0 else { | |
| return [] | |
| } | |
| } catch { | |
| return [] | |
| } |
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
1 participant
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
-wg)lsoffor faster port scanning instead of TCP connect probesChanges
WebUI Model Menu Fix
_selectingModelflag to prevent re-entrant dropdown triggers_modelRestorationAttemptedflag to only restore preferred model once per page loadPort Scanning Improvements
lsof -iTCP -sTCP:LISTEN -nPto get actually listening ports (instant)Test plan
afm -wgand verify model dropdown no longer flickers🤖 Generated with Claude Code
Summary by Sourcery
Improve backend discovery performance and accuracy while stabilizing the WebUI model selection menu.
Bug Fixes:
Enhancements: