fix(desktop): cleanup stale packaged backend processes on startup#1479
fix(desktop): cleanup stale packaged backend processes on startup#1479futuremeng wants to merge 2 commits intoagentscope-ai:mainfrom
Conversation
When a packaged desktop app exits abnormally, the backend (uvicorn) process may survive as an orphan, causing port conflicts on the next launch. This commit adds a lightweight cleanup step at the start of the desktop command: it scans running processes for the packaged backend signature (.app/Contents/Resources/env/bin/python ... uvicorn copaw.app._app:app) and terminates any stale instances with SIGTERM, waiting up to 2 s for graceful shutdown before escalating to SIGKILL. Source-code dev processes (e.g. python -m copaw app --reload) are excluded from the match pattern and are never affected.
github-actions
bot
added
the
first-time-contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a crucial pre-launch cleanup step for the desktop application. It addresses an issue where abnormally terminated desktop apps could leave behind orphaned backend processes, leading to port conflicts and instability on subsequent launches. By proactively identifying and terminating these specific stale processes, the change ensures a clean and reliable startup experience for the desktop application. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a helpful cleanup mechanism for stale backend processes that might be left running after an abnormal application exit. The approach of identifying and terminating these processes on startup is sound. My review focuses on improving the robustness of the implementation by replacing broad exception handlers (except Exception) with more specific ones. This will make the code more maintainable and prevent potential bugs from being hidden.
| except Exception: | ||
| return [] |
There was a problem hiding this comment.
The except Exception: is too broad and can hide unexpected errors. It's better to catch specific exceptions that subprocess.check_output is known to raise, such as subprocess.CalledProcessError and FileNotFoundError. Also, consider logging the error for easier debugging, which will be helpful if process scanning fails for some reason.
| except Exception: | |
| return [] | |
| except (subprocess.CalledProcessError, FileNotFoundError) as e: | |
| _log_desktop(f'[desktop] Failed to scan processes while cleaning up: {e}') | |
| return [] |
| except ProcessLookupError: | ||
| continue | ||
| except Exception: | ||
| continue |
There was a problem hiding this comment.
Using except Exception: is too broad. It's better to catch specific exceptions. In this case, besides ProcessLookupError, os.kill might raise a PermissionError if the current user doesn't have the rights to terminate the process. It's good practice to handle these specific cases and let other unexpected exceptions propagate. You can combine the except blocks for cleaner code.
| except ProcessLookupError: | |
| continue | |
| except Exception: | |
| continue | |
| except (ProcessLookupError, PermissionError): | |
| continue |
src/copaw/cli/desktop_cmd.py
Outdated
| except Exception: | ||
| pass |
There was a problem hiding this comment.
The except Exception: is too broad and silently ignores any error that occurs when trying to SIGKILL a process. It's better to catch specific exceptions like ProcessLookupError (if the process died just before the kill attempt) or PermissionError.
| except Exception: | |
| pass | |
| except (ProcessLookupError, PermissionError): | |
| pass |
There was a problem hiding this comment.
Pull request overview
Adds a startup cleanup step to the desktop CLI command to prevent port/resource conflicts caused by orphaned backend (uvicorn/FastAPI) processes that can survive abnormal desktop-app exits.
Changes:
- Introduces process scanning via
psto find packaged-desktop backend candidates. - Sends
SIGTERM, waits briefly, then escalates toSIGKILLfor surviving processes. - Logs the list of cleaned PIDs before starting the new backend.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
src/copaw/cli/desktop_cmd.py
Outdated
| if ( | ||
| ".app/Contents/Resources/env/bin/python" in cmd | ||
| and "-m uvicorn" in cmd | ||
| and "copaw.app._app:app" in cmd |
src/copaw/cli/desktop_cmd.py
Outdated
| # Only clean stale desktop-packaged backend processes. | ||
| if ( | ||
| ".app/Contents/Resources/env/bin/python" in cmd | ||
| and "-m uvicorn" in cmd | ||
| and "copaw.app._app:app" in cmd | ||
| ): | ||
| stale_pids.append(pid) |
futuremeng
requested a deployment
to
maintainer-approved
GitHub Actions
Waiting
When a packaged desktop app exits abnormally, the backend (uvicorn) process
may survive as an orphan, causing port conflicts on the next launch.
This commit adds a lightweight cleanup step at the start of the desktop
command: it scans running processes for the packaged backend signature
(
.app/Contents/Resources/env/bin/python ... uvicorn copaw.app._app:app)and terminates any stale instances with SIGTERM, waiting up to 2 s for
graceful shutdown before escalating to SIGKILL.
Source-code dev processes (e.g.
python -m copaw app --reload) are excludedfrom the match pattern and are never affected.
Description
The packaged desktop app launches a backend server subprocess. If the app
crashes or is force-killed, the subprocess may remain alive. On the next
launch, the port-scan logic finds a free port successfully, but the orphan
process can interfere with shared resources. This PR adds a safe pre-launch
cleanup that only targets clearly-identified packaged backends.
Related Issue: N/A
Security Considerations: The cleanup uses
os.killwithSIGTERM/SIGKILLscoped strictly to processes matching the
.app-bundled backend path pattern.No shell injection is involved (args are passed as a list to
subprocess.check_output).Type of Change
Component(s) Affected
Checklist
pre-commit run --all-fileslocally and it passespytestor as relevant) and they passTesting
kill -9 <pid>).backend starts. Verify via
lsof -i :<port>that only one backend is listening.Local Verification Evidence
pytest tests/unit/providers/ # 14 passed