fix: clean up orphaned bridge and MCP child processes (#1724)#1725
fix: clean up orphaned bridge and MCP child processes (#1724)#1725Yeachan-Heo merged 1 commit intodevfrom
Conversation
|
This PR has 1597 additions and 563 deletions across 30 files. Consider breaking it into smaller PRs for easier review. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b4b7f86fd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| try { | ||
| await cleanupOwnedBridgeSessions(); | ||
| } catch { |
There was a problem hiding this comment.
Reorder shutdown steps to avoid 5s force-exit cutoff
gracefulShutdown now awaits cleanupOwnedBridgeSessions() before disconnectAllLsp() while a hard setTimeout(() => process.exit(1), 5000) is already running, so a single unresponsive bridge can consume the entire deadline (the bridge cleanup path escalates over multiple waits in killBridgeWithEscalation) and force-exit the process before LSP/server cleanup runs. In that scenario this change can still leave orphaned LSP children despite entering the shutdown handler.
Useful? React with 👍 / 👎.
| PARENT_WATCH_INTERVAL_S = max( | ||
| float(os.environ.get("OMC_PARENT_POLL_INTERVAL_MS", "1000")) / 1000.0, 0.25 | ||
| ) |
There was a problem hiding this comment.
Parse parent poll interval defensively
The module-level float(os.environ.get("OMC_PARENT_POLL_INTERVAL_MS", "1000")) conversion throws ValueError for any non-numeric env value (for example an empty string), which aborts bridge startup before request handling begins. Because this runs at import time, one malformed environment variable can break all Python bridge launches instead of falling back to the default interval.
Useful? React with 👍 / 👎.
Summary
omc hud --watchloops when shutdown/parent-exit is detected and add regression coverageTesting
npx tsc --noEmitnpx vitest run src/mcp/__tests__/standalone-shutdown.test.ts src/cli/__tests__/hud-watch.test.ts src/tools/python-repl/__tests__/bridge-manager-cleanup.test.ts src/cli/__tests__/launch.test.ts src/__tests__/standalone-server.test.ts src/hooks/session-end/__tests__/session-end-bridge-cleanup.test.ts src/hooks/session-end/__tests__/python-repl-cleanup.test.tsnpm run build