Skip to content

Improve stale entry cleanup for cross-platform compatibility #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
daanzu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve stale entry cleanup for cross-platform compatibility #38

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 13 additions & 4 deletions scripts/discover-servers.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,19 @@ for f in "$servers_dir"/*.json; do

pid=$(jq -r '.pid' "$f" 2>/dev/null) || continue

# Clean up stale entries
if ! kill -0 "$pid" 2>/dev/null; then
rm -f "$f"
continue
# Clean up stale entries. Under MSYS/Cygwin, accept either a Windows-native
# process lookup or `kill -0` so we handle both Windows and Bash-owned PIDs.
if [[ "$OSTYPE" == msys* || "$OSTYPE" == cygwin* ]]; then
if ! powershell.exe -NoProfile -Command "Get-Process -Id $pid -ErrorAction SilentlyContinue | Out-Null; if (\$?) { exit 0 } else { exit 1 }" \
&& ! kill -0 "$pid" 2>/dev/null; then
Comment on lines +27 to +29

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If powershell.exe is unavailable on a given MSYS/Cygwin setup, this will emit a "command not found" error to stderr on every iteration (even though the kill -0 fallback still works). Consider checking command -v powershell.exe first (or redirecting its stderr) to keep output clean and avoid noisy logs.

Copilot uses AI. Check for mistakes.

@daanzu daanzu Apr 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Comment on lines +27 to +29

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$pid is interpolated directly into the PowerShell -Command string. Since the PID is read from a JSON file in the registry directory, a malformed or tampered .pid value could break the PowerShell command or be used for command injection. Please validate that pid is strictly numeric before using it, and pass it to PowerShell as an argument/parameter (instead of string interpolation) so it can’t alter the command text.

Copilot uses AI. Check for mistakes.

@daanzu daanzu Apr 22, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

rm -f "$f"
continue
fi
else
if ! kill -0 "$pid" 2>/dev/null; then
Comment thread
daanzu marked this conversation as resolved.
rm -f "$f"
continue
fi
fi

entry=$(jq '.' "$f" 2>/dev/null) || continue
Expand Down
Loading