feat: add Zero MCP client backend - #37
Merged
Merged
Conversation
Contributor
Zero automated PR reviewVerdict: No blockers found Blockers
Validation
ScopeHead: This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality. |
anandh8x
approved these changes
Jun 3, 2026
anandh8x
left a comment
anandh8x
left a comment
Collaborator
There was a problem hiding this comment.
Review: Zero MCP client backend
No blockers. Clean M4 foundational slice with good architecture.
What's good
- Separation of concerns —
config.ts(schema/identity),transport.ts(stdio/http/SSE),manager.ts(lifecycle),tools.ts(Zero adaptation). Each file is focused and testable. - Transport design —
ZeroMcpStdioTransportwith lazy spawn, newline-delimited JSON-RPC, per-request timeouts (5s default), stderr capture (last 4k chars), cleanrejectAllon process exit.ZeroMcpHttpTransportwith session header preservation, SSE response parsing, AbortController timeouts. - Identity stability —
computeZeroMcpServerIdentityhashes only transport-defining fields (type/command/args/url). Env and headers excluded — changing API keys doesn't change identity. toJSONSchemahook — MCP tools preserve their native input schema viatoJSONSchema()instead of Zod schema conversion. Agent loop checks for the hook before falling back toz.toJSONSchema. Zero-backward-compat cost.- Safety defaults — All MCP tools registered as
networkside-effect withpromptpermission (permission: 'prompt'). User approval required before first use per run. - Tool naming —
mcp__{server}__{tool}with sanitization. Server-scoped, no collisions within a server. - Config merging —
mergeLayersdoes shallow server-level merge. Same server name in multiple layers → later wins. Correct. - Lazy init — Child processes spawned / HTTP connections made on first request, not construction. No waste.
- Fake stdio server in tests — Clever approach: write a
.mjsscript that speaks MCP JSON-RPC over stdin/stdout. Covers discovery, call, permission gating, JSON schema passthrough, CLI output. - Test coverage — 284 lines: config loading, identity stability, stdio tool discovery/call, registry integration (granted/blocked), CLI
--jsonand--json --tools.
Observations (non-blocking)
- No HTTP transport test — HTTP/SSE transport is tested structurally (paths, headers, SSE parsing) but not integration-tested against a real HTTP MCP server. Acceptable for M4 slice 1.
createZeroMcpToolNamesanitization —[^a-z0-9_]+→_. Different server names likemy_serverandmy__serverwould collide. Server names are validated with a strict regex, so unlikely in practice.normalizeToolsListResultis strict — One unnamed tool throws for the entire server. Correct — a partially invalid response indicates a misbehaving server.- No auto-reconnect on crash — If stdio child exits,
rejectAllfires, but next request starts a fresh process. Simple and correct.
📝 WalkthroughWalkthroughThis PR introduces a complete Model Context Protocol (MCP) client subsystem that enables the Zero agent to discover and execute tools from external MCP servers. The implementation spans type definitions, transport protocols (stdio and HTTP), server coordination, tool integration, and system wiring, with comprehensive test coverage validating end-to-end functionality. ChangesMCP Client Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Collaborator
Blockers
Non-Blocking
Looks Good
Verdict: Changes Requested — Clean backend slice, but MCP registry name collisions must fail closed before external tools are dispatchable. |
Vasanthdev2004
requested changes
Jun 3, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Blockers
- MCP tool names are not collision-safe.
createZeroMcpToolName()normalizes distinct valid server/tool names to the same registry key (docs-prod+lookup-allanddocs_prod+lookup_allboth becomemcp__docs_prod__lookup_all), andToolRegistry.register()silently overwrites the earlier tool. That can dispatch a model-approved MCP call to the wrong server/tool once multiple MCP servers/tools are configured. Please preserve uniqueness (for example include the stable server identity and/or detect/reject duplicate generated names) and add a regression test.
Non-Blocking
- None.
Looks Good
- MCP config layering, stdio discovery/call flow, schema forwarding, and prompt-gated registry execution are covered well.
- Validation passed: frozen install, typecheck, 243/243 tests, build, smoke:build, CLI help/mcp list smokes, and both diff-check gates.
- Scope is contained to the M4 MCP backend slice: 1,249 additions / 3 deletions across 11 files.
Verdict: Changes Requested — Clean backend slice, but MCP registry name collisions must fail closed before external tools are dispatchable.
Collaborator
Author
|
Addressed the requested MCP registry collision fix.
Validation after the fix:
@Vasanthdev2004 @anandh8x ready for re-review. |
Collaborator
BlockersNone found. Non-Blocking
Looks Good
Verdict: Approve — Clean MCP client backend with the collision-safety blocker fixed. |
Vasanthdev2004
approved these changes
Jun 3, 2026
Vasanthdev2004
left a comment
Vasanthdev2004
left a comment
Collaborator
There was a problem hiding this comment.
Clean MCP client backend with the collision-safety blocker fixed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/agent/loop.ts (1)
90-102:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't rewrite tool-supplied JSON schemas.
toJSONSchema()is the escape hatch for forwarding MCP input schemas, but this block still mutates the returned object by deleting$schemaand addingadditionalProperties: false. That changes open-ended remote schemas into closed ones and breaks the "preserve the provider schema" contract.💡 Proposed fix
- const jsonSchema = typeof t.toJSONSchema === 'function' - ? t.toJSONSchema() as any - : z.toJSONSchema(t.parameters, { - target: 'draft-7', - }) as any; + const hasCustomSchema = typeof t.toJSONSchema === 'function'; + const jsonSchema = hasCustomSchema + ? structuredClone(t.toJSONSchema() as Record<string, unknown>) + : z.toJSONSchema(t.parameters, { + target: 'draft-7', + }) as any; // Remove $schema if present (some providers dislike it) delete jsonSchema.$schema; // Make it strict by default (good practice) - if (jsonSchema.type === 'object' && !('additionalProperties' in jsonSchema)) { + if (!hasCustomSchema && jsonSchema.type === 'object' && !('additionalProperties' in jsonSchema)) { jsonSchema.additionalProperties = false; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/agent/loop.ts` around lines 90 - 102, The code currently mutates whatever toJSONSchema returned (jsonSchema), which rewrites tool-supplied schemas from t.toJSONSchema and breaks the preservation contract; change the logic so that if typeof t.toJSONSchema === 'function' you accept the returned schema as-is (do not delete jsonSchema.$schema or set jsonSchema.additionalProperties), but if you generate the schema via z.toJSONSchema(t.parameters, ...) you may continue to strip $schema and add additionalProperties = false (or clone before mutating). Update the branches around t.toJSONSchema / z.toJSONSchema and the jsonSchema handling so tool-supplied schemas are preserved.
🧹 Nitpick comments (1)
tests/zero-mcp-client.test.ts (1)
223-228: ⚡ Quick winCover the schema pass-through contract in tests.
This only asserts a schema that is already strict, so it won't catch
runAgenttightening a tool-provided schema. Add a case wheretoJSONSchema()omitsadditionalProperties(and ideally includes$schema) and assert the provider receives it unchanged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/zero-mcp-client.test.ts` around lines 223 - 228, Add a test that verifies schema pass-through by stubbing a tool whose toJSONSchema() returns a schema that omits additionalProperties and includes a $schema field, then invoke the same code path that previously called tool?.toJSONSchema?.() (e.g., via runAgent or the MCP client setup used in this test) and assert the provider/consumer receives exactly that schema object unchanged (including the $schema key and no additionalProperties). Specifically, create a tool stub returning { $schema: '...', type: 'object', properties: { query: { type: 'string' } }, required: ['query'] } and add an expectation that the provider-received schema equals that object to ensure runAgent or related code does not inject/override additionalProperties.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/zero-mcp/tools.ts`:
- Around line 54-64: The current code registers each MCP tool into the registry
inside the loop before checking all tools for duplicates, causing partial state
on failure. To fix this in the function containing the listTools call and the
loop over tools, first check for duplicate names across new tools and existing
registry entries without mutating the registry. Only after validating that there
are no duplicates, register all tools in a separate step. This will ensure the
registration is atomic and leaves no partial state if a duplicate is found.
In `@src/zero-mcp/transport.ts`:
- Around line 269-281: The issue is that multiple concurrent calls to
ensureInitialized can pass the initialized check and send duplicate
initialization requests, causing race conditions. To fix this, introduce a
Promise or lock to serialize initialization calls in ensureInitialized, so that
only the first call performs the initialization sequence while others await its
completion and do not send duplicate requests. This ensures safe and atomic
initialization in the async method ensureInitialized.
- Around line 200-214: In handleMessage, guard the parsed JSON before using the
in-operator: after JSON.parse in ZeroMcpStdioTransport.handleMessage validate
the value with the existing isRecord() (or equivalent) and return/reject if it’s
not an object, then safely check ('id' in message) and lookup pending; this
prevents TypeError on primitives. In ZeroMcpHttpTransport.ensureInitialized
replace the boolean initialized flag with a shared initialization Promise/lock
(like the stdio transport’s pattern) — create an initializingPromise that is set
before starting the handshake, await it on concurrent callers, and set/clear it
on success/failure so only one handshake runs and others wait for its result.
---
Outside diff comments:
In `@src/agent/loop.ts`:
- Around line 90-102: The code currently mutates whatever toJSONSchema returned
(jsonSchema), which rewrites tool-supplied schemas from t.toJSONSchema and
breaks the preservation contract; change the logic so that if typeof
t.toJSONSchema === 'function' you accept the returned schema as-is (do not
delete jsonSchema.$schema or set jsonSchema.additionalProperties), but if you
generate the schema via z.toJSONSchema(t.parameters, ...) you may continue to
strip $schema and add additionalProperties = false (or clone before mutating).
Update the branches around t.toJSONSchema / z.toJSONSchema and the jsonSchema
handling so tool-supplied schemas are preserved.
---
Nitpick comments:
In `@tests/zero-mcp-client.test.ts`:
- Around line 223-228: Add a test that verifies schema pass-through by stubbing
a tool whose toJSONSchema() returns a schema that omits additionalProperties and
includes a $schema field, then invoke the same code path that previously called
tool?.toJSONSchema?.() (e.g., via runAgent or the MCP client setup used in this
test) and assert the provider/consumer receives exactly that schema object
unchanged (including the $schema key and no additionalProperties). Specifically,
create a tool stub returning { $schema: '...', type: 'object', properties: {
query: { type: 'string' } }, required: ['query'] } and add an expectation that
the provider-received schema equals that object to ensure runAgent or related
code does not inject/override additionalProperties.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9a55e56e-4a42-4264-9c28-5adbfc430fa1
📒 Files selected for processing (11)
src/agent/loop.tssrc/config/loader.tssrc/index.tssrc/tools/types.tssrc/zero-mcp/config.tssrc/zero-mcp/index.tssrc/zero-mcp/manager.tssrc/zero-mcp/tools.tssrc/zero-mcp/transport.tssrc/zero-mcp/types.tstests/zero-mcp-client.test.ts
Comment on lines
+54
to
+64
| const descriptors = await manager.listTools(options.serverName); | ||
| const tools = descriptors.map((descriptor) => createZeroMcpTool(descriptor, manager)); | ||
| const names = new Set<string>(); | ||
| for (const tool of tools) { | ||
| if (names.has(tool.name) || registry.get(tool.name)) { | ||
| throw new Error(`Duplicate MCP tool registry name "${tool.name}" was refused.`); | ||
| } | ||
| names.add(tool.name); | ||
| registry.register(tool); | ||
| } | ||
| return tools; |
There was a problem hiding this comment.
Make MCP registration atomic.
This loop mutates registry before the full batch is validated. If a later descriptor collides, the function throws with some MCP tools already registered, leaving partial state behind after a failed load.
Suggested fix
export async function registerZeroMcpTools(
registry: ToolRegistry,
manager: ZeroMcpClientManager,
options: { serverName?: string } = {}
): Promise<Tool[]> {
const descriptors = await manager.listTools(options.serverName);
const tools = descriptors.map((descriptor) => createZeroMcpTool(descriptor, manager));
const names = new Set<string>();
+
for (const tool of tools) {
if (names.has(tool.name) || registry.get(tool.name)) {
throw new Error(`Duplicate MCP tool registry name "${tool.name}" was refused.`);
}
names.add(tool.name);
+ }
+
+ for (const tool of tools) {
registry.register(tool);
}
return tools;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/zero-mcp/tools.ts` around lines 54 - 64, The current code registers each
MCP tool into the registry inside the loop before checking all tools for
duplicates, causing partial state on failure. To fix this in the function
containing the listTools call and the loop over tools, first check for duplicate
names across new tools and existing registry entries without mutating the
registry. Only after validating that there are no duplicates, register all tools
in a separate step. This will ensure the registration is atomic and leaves no
partial state if a duplicate is found.
Comment on lines
+200
to
+214
| private handleMessage(rawLine: string): void { | ||
| let message: ZeroMcpJsonRpcResponse; | ||
| try { | ||
| message = JSON.parse(rawLine) as ZeroMcpJsonRpcResponse; | ||
| } catch (err: unknown) { | ||
| this.rejectAll(new ZeroMcpError( | ||
| `MCP server "${this.serverName}" wrote invalid JSON to stdout.`, | ||
| { cause: err } | ||
| )); | ||
| return; | ||
| } | ||
|
|
||
| if (!('id' in message)) return; | ||
| const pending = this.pending.get(message.id ?? ''); | ||
| if (!pending) return; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Does JavaScript throw a TypeError when the inoperator is used on a primitive value, such as'id' in "hello"or'id' in 1?
💡 Result:
Yes, JavaScript throws a TypeError when the in operator is used on a primitive value as the right-hand operand [1][2]. The in operator is designed exclusively to check for the existence of a property within an object or its prototype chain [1][3]. When you use a primitive value—such as a string literal (e.g., 'id' in "hello"), a number (e.g., 'id' in 1), or other primitive types—the JavaScript engine throws a TypeError because these types are not objects [2][4]. Specific details regarding this behavior include: - Error Message: In most environments, this results in an error such as "TypeError: Cannot use 'in' operator to search for..." or "right-hand side of 'in' should be an object" [2][5]. - Why it happens: Unlike property access (e.g., "hello".length), which triggers "autoboxing" (the temporary creation of a wrapper object to allow method and property access), the in operator does not perform this automatic conversion [4]. It requires the right-hand side to be an object type [2][4]. - Workarounds: If you need to check if a primitive has properties, you must first convert it to an object (e.g., using Object(value)), though this is rarely the desired approach for simple checks [6]. For common tasks like checking if a substring exists in a string, you should use the String.prototype.includes() method instead [2][5].
Citations:
- 1: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/in
- 2: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/in_operator_no_object
- 3: https://github.com/mdn/content/blob/main/files/en-us/web/javascript/reference/operators/in/index.md
- 4: https://stackoverflow.com/questions/55522988/why-is-the-in-operator-throwing-an-error-with-a-string-literal-instead-of-logg
- 5: https://bobbyhadz.com/blog/javascript-cannot-use-in-operator-to-search-for
- 6: https://stackoverflow.com/questions/79060985/check-if-a-potentially-primitive-value-has-a-given-property
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and print the relevant regions around the reported lines.
FILE="src/zero-mcp/transport.ts"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE"
exit 1
fi
echo "== handleMessage area =="
sed -n '170,250p' "$FILE" | nl -ba -w2 -s': '
echo
echo "== ensureInitialized implementations =="
rg -n "ensureInitialized|ZeroMcpHttpTransport|ZeroMcpStdioTransport|handleMessage" "$FILE"
echo
echo "== isRecord / type guard helpers =="
rg -n "isRecord|is[A-Za-z]+Record|is[A-Za-z]+Object|type guard|Record<" "$FILE"Repository: Gitlawb/zero
Length of output: 143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/zero-mcp/transport.ts"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE"
exit 1
fi
echo "== handleMessage area (lines ~170-250) =="
sed -n '170,250p' "$FILE" | cat -n -w2 -s': '
echo
echo "== ensureInitialized + handleMessage occurrences =="
rg -n "ensureInitialized|ZeroMcpHttpTransport|ZeroMcpStdioTransport|handleMessage" "$FILE"
echo
echo "== type-guard helpers around parsed objects =="
rg -n "isRecord|type guard|Record<" "$FILE"Repository: Gitlawb/zero
Length of output: 182
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/zero-mcp/transport.ts"
if [ ! -f "$FILE" ]; then
echo "Missing $FILE"
exit 1
fi
echo "== transport.ts: handleMessage around line 200 =="
sed -n '185,235p' "$FILE"
echo
echo "== find ensureInitialized implementations =="
rg -n "class ZeroMcpStdioTransport|class ZeroMcpHttpTransport|ensureInitialized|handleMessage" "$FILE"
echo
echo "== search for record/type-guard helpers =="
rg -n "isRecord|type guard|is[A-Za-z]+Record|Record<|typeof .*=== 'object'|instanceof" "$FILE" || trueRepository: Gitlawb/zero
Length of output: 3077
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/zero-mcp/transport.ts"
echo "== isRecord definition (around line ~420) =="
sed -n '380,455p' "$FILE"Repository: Gitlawb/zero
Length of output: 1731
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/zero-mcp/transport.ts"
echo "== ZeroMcpStdioTransport.ensureInitialized (around line ~90-140) =="
sed -n '40,160p' "$FILE"
echo
echo "== ZeroMcpHttpTransport.ensureInitialized (around line ~240-310) =="
sed -n '220,330p' "$FILE"Repository: Gitlawb/zero
Length of output: 6905
Guard JSON-RPC payloads before using 'id' in ... (and de-race HTTP init).
ZeroMcpStdioTransport.handleMessage()parses JSON and then doesif (!('id' in message)) return;without checking that the parsed value is an object; usinginon a primitive throwsTypeError, turning bad server output into an uncaught handler failure instead of rejecting pending requests. Reuse the existingisRecord()guard intransport.tsbefore'id' in.ZeroMcpHttpTransport.ensureInitialized()uses a booleaninitializedflag; concurrent first-use calls can both run theinitializehandshake before either sets the flag. Use a shared Promise/lock like the stdio transport.
Suggested fix (stdio)
private handleMessage(rawLine: string): void {
- let message: ZeroMcpJsonRpcResponse;
+ let parsed: unknown;
try {
- message = JSON.parse(rawLine) as ZeroMcpJsonRpcResponse;
+ parsed = JSON.parse(rawLine);
} catch (err: unknown) {
this.rejectAll(new ZeroMcpError(
`MCP server "${this.serverName}" wrote invalid JSON to stdout.`,
{ cause: err }
));
return;
}
+ if (!isRecord(parsed)) {
+ this.rejectAll(new ZeroMcpError(
+ `MCP server "${this.serverName}" wrote a non-object JSON-RPC message to stdout.`
+ ));
+ return;
+ }
+
+ const message = parsed as ZeroMcpJsonRpcResponse;
if (!('id' in message)) return;
const pending = this.pending.get(message.id ?? '');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/zero-mcp/transport.ts` around lines 200 - 214, In handleMessage, guard
the parsed JSON before using the in-operator: after JSON.parse in
ZeroMcpStdioTransport.handleMessage validate the value with the existing
isRecord() (or equivalent) and return/reject if it’s not an object, then safely
check ('id' in message) and lookup pending; this prevents TypeError on
primitives. In ZeroMcpHttpTransport.ensureInitialized replace the boolean
initialized flag with a shared initialization Promise/lock (like the stdio
transport’s pattern) — create an initializingPromise that is set before starting
the handshake, await it on concurrent callers, and set/clear it on
success/failure so only one handshake runs and others wait for its result.
Comment on lines
+269
to
+281
| private async ensureInitialized(): Promise<void> { | ||
| if (this.initialized) return; | ||
|
|
||
| await this.request('initialize', { | ||
| protocolVersion: ZERO_MCP_PROTOCOL_VERSION, | ||
| capabilities: {}, | ||
| clientInfo: { | ||
| name: 'zero', | ||
| version: ZERO_VERSION, | ||
| }, | ||
| }, { skipInitialize: true }); | ||
| await this.notify('notifications/initialized', {}); | ||
| this.initialized = true; |
There was a problem hiding this comment.
Serialize HTTP transport initialization.
This handshake is only guarded by a boolean. Two concurrent first calls can both observe initialized === false and send duplicate initialize / notifications/initialized requests, which is enough to confuse strict MCP servers or race mcp-session-id establishment.
Suggested fix
export class ZeroMcpHttpTransport implements ZeroMcpTransportClient {
- private initialized = false;
+ private initialized: Promise<void> | undefined;
private sessionId: string | undefined;
private nextId = 1;
private readonly timeoutMs: number;
@@
private async ensureInitialized(): Promise<void> {
- if (this.initialized) return;
-
- await this.request('initialize', {
- protocolVersion: ZERO_MCP_PROTOCOL_VERSION,
- capabilities: {},
- clientInfo: {
- name: 'zero',
- version: ZERO_VERSION,
- },
- }, { skipInitialize: true });
- await this.notify('notifications/initialized', {});
- this.initialized = true;
+ if (!this.initialized) {
+ this.initialized = (async () => {
+ await this.request('initialize', {
+ protocolVersion: ZERO_MCP_PROTOCOL_VERSION,
+ capabilities: {},
+ clientInfo: {
+ name: 'zero',
+ version: ZERO_VERSION,
+ },
+ }, { skipInitialize: true });
+ await this.notify('notifications/initialized', {});
+ })().catch((err) => {
+ this.initialized = undefined;
+ throw err;
+ });
+ }
+
+ await this.initialized;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private async ensureInitialized(): Promise<void> { | |
| if (this.initialized) return; | |
| await this.request('initialize', { | |
| protocolVersion: ZERO_MCP_PROTOCOL_VERSION, | |
| capabilities: {}, | |
| clientInfo: { | |
| name: 'zero', | |
| version: ZERO_VERSION, | |
| }, | |
| }, { skipInitialize: true }); | |
| await this.notify('notifications/initialized', {}); | |
| this.initialized = true; | |
| private async ensureInitialized(): Promise<void> { | |
| if (!this.initialized) { | |
| this.initialized = (async () => { | |
| await this.request('initialize', { | |
| protocolVersion: ZERO_MCP_PROTOCOL_VERSION, | |
| capabilities: {}, | |
| clientInfo: { | |
| name: 'zero', | |
| version: ZERO_VERSION, | |
| }, | |
| }, { skipInitialize: true }); | |
| await this.notify('notifications/initialized', {}); | |
| })().catch((err) => { | |
| this.initialized = undefined; | |
| throw err; | |
| }); | |
| } | |
| await this.initialized; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/zero-mcp/transport.ts` around lines 269 - 281, The issue is that multiple
concurrent calls to ensureInitialized can pass the initialized check and send
duplicate initialization requests, causing race conditions. To fix this,
introduce a Promise or lock to serialize initialization calls in
ensureInitialized, so that only the first call performs the initialization
sequence while others await its completion and do not send duplicate requests.
This ensures safe and atomic initialization in the async method
ensureInitialized.
This was referenced Jun 3, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Summary
Adds the first M4 MCP backend slice for Zero. This PR introduces a typed MCP client layer that can read MCP server configuration, connect to servers, discover tools, and adapt those tools into Zero's existing tool registry shape.
What Changed
src/zero-mcpwith typed config, server identity hashing, transport clients, manager APIs, and tool registration helpers.mcp__<server>__<serverIdentity>__<tool>__<toolDigest>so external tools remain unique after sanitization.Tool.toJSONSchema()hook so remote schemas are sent to providers without losing their shape.mcp.serverssupport for stdio, HTTP, and SSE definitions.zero mcp listwith--jsonand--toolsfor backend inspection and local smoke usage.Behavior Notes
networktools by default.Validation
git diff --checknpx --yes bun install --frozen-lockfilenpx --yes bun run typechecknpx --yes bun test tests/zero-mcp-client.test.ts --timeout 15000— 7 passnpx --yes bun test ./tests --timeout 15000— 245 pass, 0 failnpx --yes bun run buildnpx --yes bun run smoke:build./zero --help./zero mcp --help./zero mcp list --help./zero mcp list --jsonReviewers: @Vasanthdev2004 @anandh8x