Add mesh token to on config #2125
Open
Conversation
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
Contributor
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
|
Contributor
🧪 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. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
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/web/components/details/connection/settings-tab/index.tsx">
<violation number="1" location="apps/mesh/src/web/components/details/connection/settings-tab/index.tsx:497">
P2: The `connectionActions.update.mutateAsync` call is wrapped with redundant error handling. The underlying hook already provides `onError` toast notifications, so this catch block will cause duplicate error toasts when failures occur. Consider using try/finally instead to maintain the cleanup logic.
(Based on your team's feedback about centralized error handling in mutation hooks.) [FEEDBACK_USED]</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| data: {}, | ||
| }); | ||
| toast.success("Tools refreshed successfully"); | ||
| } catch (error) { |
There was a problem hiding this comment.
P2: The connectionActions.update.mutateAsync call is wrapped with redundant error handling. The underlying hook already provides onError toast notifications, so this catch block will cause duplicate error toasts when failures occur. Consider using try/finally instead to maintain the cleanup logic.
(Based on your team's feedback about centralized error handling in mutation hooks.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/web/components/details/connection/settings-tab/index.tsx, line 497:
<comment>The `connectionActions.update.mutateAsync` call is wrapped with redundant error handling. The underlying hook already provides `onError` toast notifications, so this catch block will cause duplicate error toasts when failures occur. Consider using try/finally instead to maintain the cleanup logic.
(Based on your team's feedback about centralized error handling in mutation hooks.) </comment>
<file context>
@@ -484,9 +485,39 @@ function SettingsTabContentImpl(props: SettingsTabContentImplProps) {
+ data: {},
+ });
+ toast.success("Tools refreshed successfully");
+ } catch (error) {
+ toast.error("Failed to refresh tools");
+ } finally {
</file context>
✅ Addressed in 1fd3047
There was a problem hiding this comment.
1 issue found across 5 files (changes from recent commits).
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/docs/client/src/content/pt-br/mcp-mesh/mcp-servers.mdx">
<violation number="1" location="apps/docs/client/src/content/pt-br/mcp-mesh/mcp-servers.mdx:64">
P2: The example uses `connectionId` which is never defined. Consider either defining it (e.g., from `state` or as a placeholder) or replacing it with a string literal to make the example self-contained.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const state = process.env.MESH_STATE ? JSON.parse(process.env.MESH_STATE) : {}; | ||
|
|
||
| // Usar token para chamadas à API mesh | ||
| const response = await fetch(`${meshUrl}/mcp/${connectionId}`, { |
There was a problem hiding this comment.
P2: The example uses connectionId which is never defined. Consider either defining it (e.g., from state or as a placeholder) or replacing it with a string literal to make the example self-contained.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/docs/client/src/content/pt-br/mcp-mesh/mcp-servers.mdx, line 64:
<comment>The example uses `connectionId` which is never defined. Consider either defining it (e.g., from `state` or as a placeholder) or replacing it with a string literal to make the example self-contained.</comment>
<file context>
@@ -36,16 +36,36 @@ Conexões STDIO iniciam um processo local (como `npx` ou scripts) e comunicam vi
+const state = process.env.MESH_STATE ? JSON.parse(process.env.MESH_STATE) : {};
+
+// Usar token para chamadas à API mesh
+const response = await fetch(`${meshUrl}/mcp/${connectionId}`, {
+ headers: { Authorization: `Bearer ${meshToken}` },
+ // ...
</file context>
✅ Addressed in f79d6b2
- Added documentation requirements to AGENTS.md for maintaining accurate documentation alongside code changes. - Updated README.md to include details on STDIO connections, including environment variable usage for credentials. - Expanded mcp-servers documentation in both English and Portuguese to clarify connection types and credential handling. - Modified proxy.ts to support infinite JWT tokens for STDIO connections and ensure environment variables are passed correctly. - Updated JWT handling to allow for no-expiration tokens for STDIO connections. - Improved logging in fetch-tools.ts for better debugging of STDIO tool fetch operations. - Added a refresh button in the settings tab to allow users to refresh tools from the MCP server.
vibegui
force-pushed
the
fix/stdio-bindings
branch
from
a27746b to
f79d6b2
Compare
- Updated tests in jwt.test.ts to issue tokens using an options object for expiration instead of a string.
- Changed expiration parameter from string format (e.g., "1h") to an object format (e.g., { expiresIn: "1h" }).
- Ensured consistency in token expiration handling across multiple test cases.
- Adjusted constants.ts to correct the draft version from "draft-07" to "draft-7".
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
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/web/utils/constants.ts">
<violation number="1" location="apps/mesh/src/web/utils/constants.ts:27">
P2: Invalid JSON Schema target format. The correct value is `"draft-07"` (with leading zero), not `"draft-7"`. Standard JSON Schema draft naming uses two-digit format (e.g., `draft-07`, `draft-2019-09`). Using `"draft-7"` may cause this option to be ignored or result in an error.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| export const BaseCollectionJsonSchema: JsonSchema = z.toJSONSchema( | ||
| BaseCollectionEntitySchema, | ||
| { target: "draft-07" }, | ||
| { target: "draft-7" }, |
There was a problem hiding this comment.
P2: Invalid JSON Schema target format. The correct value is "draft-07" (with leading zero), not "draft-7". Standard JSON Schema draft naming uses two-digit format (e.g., draft-07, draft-2019-09). Using "draft-7" may cause this option to be ignored or result in an error.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/web/utils/constants.ts, line 27:
<comment>Invalid JSON Schema target format. The correct value is `"draft-07"` (with leading zero), not `"draft-7"`. Standard JSON Schema draft naming uses two-digit format (e.g., `draft-07`, `draft-2019-09`). Using `"draft-7"` may cause this option to be ignored or result in an error.</comment>
<file context>
@@ -24,5 +24,5 @@ export type JsonSchema = {
export const BaseCollectionJsonSchema: JsonSchema = z.toJSONSchema(
BaseCollectionEntitySchema,
- { target: "draft-07" },
+ { target: "draft-7" },
) as JsonSchema;
</file context>
Suggested change
| { target: "draft-7" }, | |
| { target: "draft-07" }, |
During development with bun --watch, hot module reload (HMR) would cause STDIO connections (like mesh-bridge) to lose their credentials because: 1. JWT secret was regenerated on each module reload, invalidating tokens 2. ContextFactory was reset, causing undefined errors 3. Old STDIO processes weren't fully killed before new ones spawned 4. Auto-start ran before the connection pool finished resetting This commit fixes all four issues: - Store JWT secret in globalThis to survive module reloads - Persist ContextFactory function in globalThis for HMR stability - Add killProcessTree() to recursively terminate bun --watch children - Await pool reset promise before auto-starting new connections Now developers can hot-reload the mesh server without breaking the bridge.
The event-driven architecture for mesh-bridge requires reliable event delivery. This commit improves the event bus for production use: - Add EVENT_ACK tool for explicit event acknowledgment Subscribers can now acknowledge events after processing, enabling at-least-once delivery guarantees with manual control - Add ACK to EVENT_BUS_BINDING capabilities Connections with this binding can now use the acknowledge flow - Improve worker startup with better error handling Prevents silent failures during event bus initialization - Add conditional debug logging for event bus operations Helps troubleshoot delivery issues without flooding production logs - Log subscription details in proxy for debugging Shows which connections subscribe to which event types This enables mesh-bridge to reliably receive agent.response.ready events from Pilot and acknowledge them after sending to WhatsApp.
There was a problem hiding this comment.
2 issues found across 9 files (changes from recent commits).
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/tools/eventbus/subscribe.ts">
<violation number="1" location="apps/mesh/src/tools/eventbus/subscribe.ts:27">
P1: Potential authorization bypass: `subscriberId` allows any authenticated caller to create subscriptions for arbitrary connections without validation. The access check runs before `connectionId` is resolved, so it doesn't verify whether the caller is authorized to act on behalf of the provided `subscriberId`. Consider validating that the caller has permission to use `subscriberId` (e.g., check if the caller is a trusted gateway or has an appropriate scope in their token).</violation>
</file>
<file name="apps/mesh/src/stdio/stable-transport.ts">
<violation number="1" location="apps/mesh/src/stdio/stable-transport.ts:103">
P1: Health check `listTools()` call has no timeout protection. If the subprocess is hung but not dead, this will block indefinitely. Consider adding a timeout similar to the connection timeout pattern used below:
```typescript
const timeoutPromise = new Promise((_, reject) =>
setTimeout(() => reject(new Error('Health check timeout')), 5000)
);
await Promise.race([existing.stableClient.listTools(), timeoutPromise]);
```</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| const connectionId = ctx.connectionId; | ||
| // Get the subscriber connection ID | ||
| // Use explicit subscriberId if provided (for calls via gateway), otherwise use caller's connection | ||
| const connectionId = input.subscriberId || ctx.connectionId; |
There was a problem hiding this comment.
P1: Potential authorization bypass: subscriberId allows any authenticated caller to create subscriptions for arbitrary connections without validation. The access check runs before connectionId is resolved, so it doesn't verify whether the caller is authorized to act on behalf of the provided subscriberId. Consider validating that the caller has permission to use subscriberId (e.g., check if the caller is a trusted gateway or has an appropriate scope in their token).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/mesh/src/tools/eventbus/subscribe.ts, line 27:
<comment>Potential authorization bypass: `subscriberId` allows any authenticated caller to create subscriptions for arbitrary connections without validation. The access check runs before `connectionId` is resolved, so it doesn't verify whether the caller is authorized to act on behalf of the provided `subscriberId`. Consider validating that the caller has permission to use `subscriberId` (e.g., check if the caller is a trusted gateway or has an appropriate scope in their token).</comment>
<file context>
@@ -22,11 +22,12 @@ export const EVENT_SUBSCRIBE = defineTool({
- const connectionId = ctx.connectionId;
+ // Get the subscriber connection ID
+ // Use explicit subscriberId if provided (for calls via gateway), otherwise use caller's connection
+ const connectionId = input.subscriberId || ctx.connectionId;
if (!connectionId) {
throw new Error(
</file context>
| return existing.stableClient; | ||
| try { | ||
| // Quick ping to verify connection is alive (listTools has low overhead) | ||
| await existing.stableClient.listTools(); |
There was a problem hiding this comment.
P1: Health check listTools() call has no timeout protection. If the subprocess is hung but not dead, this will block indefinitely. Consider adding a timeout similar to the connection timeout pattern used below:
const timeoutPromise = new Promise((_, reject) =>
setTimeout(() => reject(new Error('Health check timeout')), 5000)
);
await Promise.race([existing.stableClient.listTools(), timeoutPromise]);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 103:
<comment>Health check `listTools()` call has no timeout protection. If the subprocess is hung but not dead, this will block indefinitely. Consider adding a timeout similar to the connection timeout pattern used below:
```typescript
const timeoutPromise = new Promise((_, reject) =>
setTimeout(() => reject(new Error('Health check timeout')), 5000)
);
await Promise.race([existing.stableClient.listTools(), timeoutPromise]);
```</comment>
<file context>
@@ -94,9 +96,18 @@ export async function getStableStdioClient(
- return existing.stableClient;
+ try {
+ // Quick ping to verify connection is alive (listTools has low overhead)
+ await existing.stableClient.listTools();
+ return existing.stableClient;
+ } catch {
</file context>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What is this contribution about?
Screenshots/Demonstration
Review Checklist
Summary by cubic
Expose Mesh credentials (JWT and URL) to MCPs via the proxy and as env vars for STDIO processes. STDIO MCPs get non-expiring tokens, credentials persist across restarts, and auto-start is supported; configuration updates include meshToken and meshUrl when needed.
New Features
Refactors
Written for commit 5ed918f. Summary will update on new commits.