-
Notifications
You must be signed in to change notification settings - Fork 30
feat(stdio): add auto-start connections and orphan process cleanup #2169
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.
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
base: feat/stdio-connection-support
Are you sure you want to change the base?
feat(stdio): add auto-start connections and orphan process cleanup #2169
Conversation
🧪 BenchmarkShould we run the MCP Gateway benchmark for this PR? React with 👍 to run the benchmark.
Benchmark will run on the next push after you react. |
Release OptionsShould a new version be published when this PR is merged? React with an emoji to vote on the release type:
Current version: Deployment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 2 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/mesh/src/api/app.ts">
<violation number="1" location="apps/mesh/src/api/app.ts:191">
P2: The `.catch()` on `resetStdioConnectionPool()` converts rejected promises to resolved ones, so `await poolResetPromise` will succeed even if pool reset failed. This defeats the stated purpose of waiting for completion before auto-start. Consider re-throwing after logging, or handling the error case explicitly in the auto-start IIFE.</violation>
</file>
<file name="apps/mesh/src/stdio/stable-transport.ts">
<violation number="1" location="apps/mesh/src/stdio/stable-transport.ts:374">
P1: Killing any process on port 9999 is dangerous. Unlike the `pkill` patterns above that match specific command lines, this kills ANY process on port 9999 without verification. Consider checking the process command line before killing, or using a more specific approach like `lsof -t -i:9999 | xargs -I{} sh -c 'ps -p {} -o args= | grep -q mesh-bridge && kill -9 {}'`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| // Also kill anything listening on port 9999 (Bridge WebSocket) | ||
| try { | ||
| const { stdout } = await execAsync(`lsof -t -i:9999 2>/dev/null || true`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Killing any process on port 9999 is dangerous. Unlike the pkill patterns above that match specific command lines, this kills ANY process on port 9999 without verification. Consider checking the process command line before killing, or using a more specific approach like lsof -t -i:9999 | xargs -I{} sh -c 'ps -p {} -o args= | grep -q mesh-bridge && kill -9 {}'.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/stdio/stable-transport.ts, line 374:
<comment>Killing any process on port 9999 is dangerous. Unlike the `pkill` patterns above that match specific command lines, this kills ANY process on port 9999 without verification. Consider checking the process command line before killing, or using a more specific approach like `lsof -t -i:9999 | xargs -I{} sh -c 'ps -p {} -o args= | grep -q mesh-bridge && kill -9 {}'`.</comment>
<file context>
@@ -343,6 +343,51 @@ async function forceCloseAllStdioConnections(): Promise<void> {
+
+ // Also kill anything listening on port 9999 (Bridge WebSocket)
+ try {
+ const { stdout } = await execAsync(`lsof -t -i:9999 2>/dev/null || true`);
+ const pids = stdout.trim().split("\n").filter(Boolean);
+ for (const pid of pids) {
</file context>
✅ Addressed in c662c2a
| // Old processes have stale credentials, need fresh spawn with new tokens | ||
| resetStdioConnectionPool().catch((err) => { | ||
| // IMPORTANT: Track this promise so autoStart waits for it to complete | ||
| const poolResetPromise = resetStdioConnectionPool().catch((err) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: The .catch() on resetStdioConnectionPool() converts rejected promises to resolved ones, so await poolResetPromise will succeed even if pool reset failed. This defeats the stated purpose of waiting for completion before auto-start. Consider re-throwing after logging, or handling the error case explicitly in the auto-start IIFE.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/api/app.ts, line 191:
<comment>The `.catch()` on `resetStdioConnectionPool()` converts rejected promises to resolved ones, so `await poolResetPromise` will succeed even if pool reset failed. This defeats the stated purpose of waiting for completion before auto-start. Consider re-throwing after logging, or handling the error case explicitly in the auto-start IIFE.</comment>
<file context>
@@ -129,7 +187,8 @@ export function createApp(options: CreateAppOptions = {}) {
// Old processes have stale credentials, need fresh spawn with new tokens
- resetStdioConnectionPool().catch((err) => {
+ // IMPORTANT: Track this promise so autoStart waits for it to complete
+ const poolResetPromise = resetStdioConnectionPool().catch((err) => {
console.error("[StableStdio] Error resetting pool:", err);
});
</file context>
✅ Addressed in c662c2a
de8fa15 to
51e399a
Compare
196adcb to
ff65603
Compare
b58f7b7 to
ac0720f
Compare
1d3989b to
af27cb0
Compare
ac0720f to
9829cdf
Compare
…s logging - Added functionality to automatically start STDIO connections based on the AUTO_START_CONNECTIONS environment variable. - Introduced a new helper function, autoStartConnectionsByTitle, to manage the auto-start process. - Enhanced EventBus with additional logging for event processing and subscription matching. - Implemented a mechanism to kill orphaned STDIO processes to prevent stale connections. - Updated error handling in the proxy to avoid logging unnecessary "Method not found" errors. This commit improves the management of STDIO connections and provides better visibility into event bus operations.
9829cdf to
bf98523
Compare
…out prompts Error code -32601 is expected for MCPs that don't implement prompts/list. Don't log these as errors since they're not actionable.
Summary by cubic
Adds auto-start for STDIO connections on startup and cleans up orphaned processes. Adds a 5-minute cooldown after spawn failures, improves event bus delivery reliability, and enables gateway subscriptions.
New Features
Bug Fixes
Written for commit d6da145. Summary will update on new commits.