Open
Conversation
Contributor
WalkthroughAdds a new first-party package Changes
Sequence Diagram(s)sequenceDiagram
participant Client as MCP Client
participant HTTP as HTTP Adapter
participant Module as PlatformMcpModule
participant Transport as StreamableHTTPServerTransport
participant Server as McpServer
participant DI as DI Container
participant Handler as Tool/Resource/Prompt Handler
Client->>HTTP: POST /mcp (MCP request)
HTTP->>Module: incoming request -> dispatch(ctx)
Module->>Transport: new StreamableHTTPServerTransport(...)
Module->>Server: transport.connect(server)
Module->>Transport: transport.handleRequest(request)
Transport->>Server: forward MCP message
Server->>DI: resolve handler token
DI->>Handler: invoke handler (function or class method)
Handler-->>Server: return result / throw error
Server->>Transport: send response
Transport->>HTTP: write response
HTTP->>Client: HTTP response (MCP payload)
Module->>Transport: transport.close()
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 Fix all issues with AI agents
In `@docs/docs/mcp.md`:
- Around line 66-69: Fix the awkward mid-phrase line break that leaves an orphan
"and" in the sentence describing DI usage: reflow the paragraph so the phrase
reads naturally (e.g., combine the broken lines into a single sentence) where
the documentation references `@Module`({providers: [...]}) and the functional
helpers defineTool, defineResource, definePrompt that wrap handlers in a Ts.ED
DIContext to allow injecting other services like a controller method.
- Around line 27-38: Add a short clarifying note in the docs explaining the two
MCP setup patterns: the side-effect import "import \"@tsed/platform-mcp\"" (used
for automatic/default registration of the MCP platform with no extra
configuration) versus the explicit module import via imports:
[PlatformMcpModule] (used when you need to customize or control module
configuration); mention that the side-effect import will auto-register the
module in typical cases and the explicit import is required only for advanced
customization or when you need to provide custom providers/configuration.
In `@openspec/changes/add-mcp-package/design.md`:
- Around line 1-3: Add a top-level level-1 heading to the design document to
satisfy MD041: insert a leading line like "# MCP package design" (or another
appropriate main title) at the very start of the file before the "## Context"
section so the doc begins with a single H1 heading; ensure the new H1 is
followed by a blank line and leave the existing "## Context" and subsequent
content unchanged.
In `@openspec/changes/add-mcp-package/proposal.md`:
- Around line 25-29: Update the "Impact" section to use the canonical spec path
and include the related PR link: replace the `mcp-endpoint` token with
`specs/mcp-endpoint` in the "Affected specs" line, ensure the "Affected code"
entries (`packages/platform/platform-mcp`, `.cli-mcp`, Ts.ED platform wiring via
`@tsed/platform-http`) remain accurate, and add a "Related PR" bullet linking to
the PR that introduces this change; keep formatting consistent with other
proposals (use backticks for code paths and a single bullet for the PR link).
In `@packages/platform/platform-mcp/package.json`:
- Line 27: The package.json currently pins the tslib dependency exactly as
"tslib": "2.7.0"; change this to a caret range (e.g., "tslib": "^2.7.0") to
allow compatible patch/minor updates and avoid duplicate installs across Ts.ED
packages—update the "tslib" entry in package.json accordingly and run install to
verify there are no CI failures.
In `@packages/platform/platform-mcp/src/decorators/prompt.spec.ts`:
- Around line 6-12: The exported test class TestPrompt in prompt.spec.ts
violates the noExportsInTest lint rule; remove the export modifier from the
class declaration (leave "class TestPrompt" with its decorators `@Prompt`, `@Title`,
`@Description` and method prompt()) so the spec file no longer exports symbols and
satisfies the Vitest/Biome linting rule.
In `@packages/platform/platform-mcp/src/decorators/resource.spec.ts`:
- Around line 6-13: The test file exports a class (TestResource) which violates
the noExportsInTest lint rule; remove the export from the TestResource
declaration so the class is local to the spec (i.e., change "export class
TestResource" to a non-exported class declaration) and keep the decorators
(`@Resource`, `@Title`, `@Description`, `@ContentType`) and method names unchanged.
In `@packages/platform/platform-mcp/src/fn/defineResource.ts`:
- Around line 7-12: The ResourceBaseProps type currently allows propertyKey
without token which causes runtime errors in inject() and
JsonEntityStore.fromMethod(); update the constructor/validator that consumes
ResourceBaseProps (or tighten the type) to enforce that if propertyKey is
present then token must be provided—e.g., add a runtime guard in the function
that creates/uses ResourceBaseProps to throw a clear error when propertyKey is
set but token is undefined, and/or change ResourceBaseProps to make token
required when propertyKey is present (use a discriminated union or conditional
types) so callers of ReadResourceCallback cannot pass propertyKey without token;
reference ResourceBaseProps, propertyKey, token, inject(), and
JsonEntityStore.fromMethod() when implementing the guard.
In `@packages/platform/platform-mcp/src/fn/defineTool.ts`:
- Around line 27-33: ClassToolProps allows name to be undefined which causes
defineTool to register class tools under the same DI token (MCP:TOOL:undefined);
update defineTool (and related registration code that reads ClassToolProps) to
ensure a deterministic name: if ClassToolProps.name is missing, derive a name
from propertyKey (e.g., String(propertyKey)) before building the DI token or
throw a descriptive error early; reference ClassToolProps, ToolProps, defineTool
and propertyKey when locating the code to change and ensure the resulting DI
token never contains "undefined".
In `@packages/platform/platform-mcp/src/services/McpServerFactory.ts`:
- Around line 32-37: The loop registering tools uses a non-null assertion on
name (in the collectTokens/MCP_PROVIDER_TYPES.TOOL -> inject(...) result) which
can register an undefined name; update the registration in McpServerFactory to
validate the injected definition from inject<ToolProps...>(token): check that
definition.name is a non-empty string before calling server.registerTool(name,
opts, handler), and if missing either throw a clear error or log and skip the
token; ensure you reference the symbols inject, ToolProps, server.registerTool,
handler and opts when locating the change and remove the unsafe non-null
assertion on name!.
- Around line 46-51: The prompt registration loop in McpServerFactory uses
collectTokens(MCP_PROVIDER_TYPES.PROMPT) and inject<PromptsSettings>(token) then
calls server.registerPrompt(name, opts, handler) without validating the
definition; add a check after creating const {name, handler, ...opts} =
definition to ensure name is a non-empty string (if missing, log an error or
throw and skip registration) so malformed prompt definitions from
settings.prompts are not silently ignored or cause confusing behavior; keep the
rest of the registration flow (server.registerPrompt) unchanged.
- Around line 21-30: The code uses a redundant fallback where
constant<PlatformMcpSettings>("mcp", {}) || {} is used; remove the extra "|| {}"
so MCP_SERVER.factory uses the value returned by constant("mcp", {}) directly.
Update the factory to assign settings = constant<PlatformMcpSettings>("mcp", {})
and leave the rest (name/version and new McpServer instantiation) unchanged,
ensuring references to PlatformMcpSettings, constant, MCP_SERVER, and McpServer
remain intact.
- Around line 39-44: The resource registration loop using
collectTokens(MCP_PROVIDER_TYPES.RESOURCE) does not validate that each injected
ResourceProps (from inject<ResourceProps & {uri?: string; template?:
ResourceTemplate}>(token)) contains either uri or template before calling
server.registerResource; update the loop in McpServerFactory so for each
definition (destructured as {name, handler, uri, template, ...opts}) you check
if (!uri && !template) and throw or log a clear error (including the resource
name or token) rather than passing (uri || template)! as any to
server.registerResource, and only call server.registerResource(name, uri ||
template, opts, handler) when the check passes.
In `@packages/platform/platform-mcp/src/services/PlatformMcpModule.spec.ts`:
- Around line 32-37: The test setup for PlatformTest.create relies on an
implicit default for MCP enablement; update the test configuration passed to
PlatformTest.create to explicitly set mcp.enabled to true (e.g., include mcp: {
path: "/ai/mcp", enabled: true }) so PlatformMcpModule registers routes as
expected—modify the beforeEach that calls PlatformTest.create to include this
explicit flag to avoid brittle defaults.
In `@packages/platform/platform-mcp/src/services/PlatformMcpModule.ts`:
- Around line 15-35: The checks for this.settings?.enabled currently treat
undefined as false so MCP is disabled by default; change both $onRoutesInit and
$logRoutes to treat enabled as true when undefined (e.g. use
this.settings?.enabled ?? true or explicit !== false) and keep using
this.settings?.path || "/mcp" and the existing dispatch handler; update the
conditional registrations in $onRoutesInit and the route entry in $logRoutes to
use that defaulted enabled value so the POST /mcp route and
PlatformMcpModule.dispatch() appear unless enabled is explicitly false.
In `@packages/platform/platform-mcp/src/utils/toZod.ts`:
- Around line 24-26: The branch handling JSON Schema numeric types in toZod
currently maps both "number" and "integer" to z.number(), allowing decimals for
integers; update the switch/case in toZod so that the "number" case returns
z.number() and the "integer" case returns z.number().int(), keeping other logic
intact and ensuring any pattern that grouped "number"/"integer" is split into
two distinct cases (look for the case labels "number" and "integer" in function
toZod).
- Line 2: The shape typing is too narrow: change usages that type shapes as
AnyZodObject to ZodTypeAny so buildShape/toZod accept any Zod schema (including
optional types returned by createZodType); update the function signatures and
any local variables or parameters that currently use AnyZodObject (e.g.,
buildShape, toZod, and related shape parameters) to use ZodTypeAny instead,
ensuring createZodType return types align and eliminate the TypeScript mismatch.
In `@packages/platform/platform-mcp/vitest.config.mts`:
- Around line 11-16: The coverage thresholds in the vitest config are all set to
0 (the thresholds object with keys statements, branches, functions, lines),
which disables enforcement; update the thresholds object in vitest.config.mts to
non-zero sensible defaults (e.g., incrementally raise
statements/branches/functions/lines to target percentages appropriate for the
package maturity) and add a TODO or issue reference to track future increases as
tests expand so the thresholds can be tightened over time.
In `@packages/specs/schema/src/decorators/operations/returns.ts`:
- Around line 621-630: The Returns overload allows a model-first call but the
implementation only checks isClass(status), so array or primitive constructors
(e.g., Returns([Model]) or Returns(String)) get treated as a status; update the
Returns implementation to detect model-first shapes by also checking
isArrayOrArrayClass(status) and isPrimitiveOrPrimitiveClass(status) (or adjust
overloads to remove Type<any>[] from the first signature). Specifically, in the
Returns function that constructs new ReturnDecoratorContext, branch so that if
status is a class OR an array-class OR a primitive-class you set model = status
and status = 200, otherwise treat the first arg as the numeric/string status;
reference Returns, ReturnDecoratorContext, isClass, isArrayOrArrayClass, and
isPrimitiveOrPrimitiveClass when making the change.
Comment on lines
+27
to
+38
| ```typescript [src/Server.ts] | ||
| import {Configuration} from "@tsed/di"; | ||
| import "@tsed/platform-express"; | ||
| import "@tsed/platform-mcp"; | ||
|
|
||
| @Configuration({ | ||
| mcp: { | ||
| path: "/mcp" // defaults to "/mcp" | ||
| } | ||
| }) | ||
| export class Server {} | ||
| ``` |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify the two setup patterns.
The documentation shows two different setup approaches:
- Lines 27-38: Side-effect import (
import "@tsed/platform-mcp") - Lines 132-140: Explicit module import (
imports: [PlatformMcpModule])
Consider adding a brief note explaining when to use each approach, or whether the side-effect import automatically registers the module (making explicit imports only necessary for advanced customization).
Also applies to: 132-140
🤖 Prompt for AI Agents
In `@docs/docs/mcp.md` around lines 27 - 38, Add a short clarifying note in the
docs explaining the two MCP setup patterns: the side-effect import "import
\"@tsed/platform-mcp\"" (used for automatic/default registration of the MCP
platform with no extra configuration) versus the explicit module import via
imports: [PlatformMcpModule] (used when you need to customize or control module
configuration); mention that the side-effect import will auto-register the
module in typical cases and the explicit import is required only for advanced
customization or when you need to provide custom providers/configuration.
Comment on lines
+66
to
+69
| Add the token to any `@Module({providers: [...]})` array (or export it from a feature module). All functional helpers | ||
| (`defineTool`, `defineResource`, `definePrompt`) wrap handler execution inside a Ts.ED `DIContext`, so you can inject | ||
| and | ||
| use other services exactly like a controller method. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor formatting nit: awkward line break.
The text wraps mid-phrase creating an orphan "and" which affects readability.
Suggested improvement
Add the token to any `@Module({providers: [...]})` array (or export it from a feature module). All functional helpers
-(`defineTool`, `defineResource`, `definePrompt`) wrap handler execution inside a Ts.ED `DIContext`, so you can inject
-and
-use other services exactly like a controller method.
+(`defineTool`, `defineResource`, `definePrompt`) wrap handler execution inside a Ts.ED `DIContext`, so you can inject and use other services exactly like a controller method.📝 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
| Add the token to any `@Module({providers: [...]})` array (or export it from a feature module). All functional helpers | |
| (`defineTool`, `defineResource`, `definePrompt`) wrap handler execution inside a Ts.ED `DIContext`, so you can inject | |
| and | |
| use other services exactly like a controller method. | |
| Add the token to any `@Module({providers: [...]})` array (or export it from a feature module). All functional helpers | |
| (`defineTool`, `defineResource`, `definePrompt`) wrap handler execution inside a Ts.ED `DIContext`, so you can inject and use other services exactly like a controller method. |
🤖 Prompt for AI Agents
In `@docs/docs/mcp.md` around lines 66 - 69, Fix the awkward mid-phrase line break
that leaves an orphan "and" in the sentence describing DI usage: reflow the
paragraph so the phrase reads naturally (e.g., combine the broken lines into a
single sentence) where the documentation references `@Module`({providers: [...]})
and the functional helpers defineTool, defineResource, definePrompt that wrap
handlers in a Ts.ED DIContext to allow injecting other services like a
controller method.
Comment on lines
+1
to
+3
| ## Context | ||
|
|
||
| - The existing MCP effort lives entirely inside `packages/third-parties/mcp/.cli-mcp`, which boots a standalone MCP server for the Ts.ED CLI. Its helpers (`defineTool`, `definePrompt`, `defineResource`, server factory) already assume Ts.ED's DI container but are not published as a reusable package. |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add a top-level heading to the design document.
The document should start with a level-1 heading per markdown best practices (MD041). This helps with document structure and accessibility.
♻️ Suggested fix
+# Design: Add MCP Package
+
## Context📝 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
| ## Context | |
| - The existing MCP effort lives entirely inside `packages/third-parties/mcp/.cli-mcp`, which boots a standalone MCP server for the Ts.ED CLI. Its helpers (`defineTool`, `definePrompt`, `defineResource`, server factory) already assume Ts.ED's DI container but are not published as a reusable package. | |
| # Design: Add MCP Package | |
| ## Context | |
| - The existing MCP effort lives entirely inside `packages/third-parties/mcp/.cli-mcp`, which boots a standalone MCP server for the Ts.ED CLI. Its helpers (`defineTool`, `definePrompt`, `defineResource`, server factory) already assume Ts.ED's DI container but are not published as a reusable package. |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In `@openspec/changes/add-mcp-package/design.md` around lines 1 - 3, Add a
top-level level-1 heading to the design document to satisfy MD041: insert a
leading line like "# MCP package design" (or another appropriate main title) at
the very start of the file before the "## Context" section so the doc begins
with a single H1 heading; ensure the new H1 is followed by a blank line and
leave the existing "## Context" and subsequent content unchanged.
Comment on lines
+25
to
+29
| ## Impact | ||
|
|
||
| - Affected specs: `mcp-endpoint` | ||
| - Affected code: `packages/platform/platform-mcp`, `.cli-mcp` (shared helpers), Ts.ED platform wiring via | ||
| `@tsed/platform-http` |
Contributor
There was a problem hiding this comment.
Align spec references and link the related PR.
The openspec guideline asks for spec references in specs/... form and links to related PRs. Consider updating the “Affected specs” line to the canonical spec path and adding the PR link.
✍️ Suggested update
## Impact
-- Affected specs: `mcp-endpoint`
+- Affected specs: `specs/mcp-endpoint/spec.md`
- Affected code: `packages/platform/platform-mcp`, `.cli-mcp` (shared helpers), Ts.ED platform wiring via
`@tsed/platform-http`
+- Related PRs: `https://github.com/tsedio/tsed/pull/3244`As per coding guidelines, reference specs using specs/... paths and link related PRs.
📝 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
| ## Impact | |
| - Affected specs: `mcp-endpoint` | |
| - Affected code: `packages/platform/platform-mcp`, `.cli-mcp` (shared helpers), Ts.ED platform wiring via | |
| `@tsed/platform-http` | |
| ## Impact | |
| - Affected specs: `specs/mcp-endpoint/spec.md` | |
| - Affected code: `packages/platform/platform-mcp`, `.cli-mcp` (shared helpers), Ts.ED platform wiring via | |
| `@tsed/platform-http` | |
| - Related PRs: `https://github.com/tsedio/tsed/pull/3244` |
🤖 Prompt for AI Agents
In `@openspec/changes/add-mcp-package/proposal.md` around lines 25 - 29, Update
the "Impact" section to use the canonical spec path and include the related PR
link: replace the `mcp-endpoint` token with `specs/mcp-endpoint` in the
"Affected specs" line, ensure the "Affected code" entries
(`packages/platform/platform-mcp`, `.cli-mcp`, Ts.ED platform wiring via
`@tsed/platform-http`) remain accurate, and add a "Related PR" bullet linking to
the PR that introduces this change; keep formatting consistent with other
proposals (use backticks for code paths and a single bullet for the PR link).
Comment on lines
+6
to
+12
| @Injectable() | ||
| export class TestPrompt { | ||
| @Prompt() | ||
| @Title("Title") | ||
| @Description("Description") | ||
| prompt() {} | ||
| } |
Contributor
There was a problem hiding this comment.
Remove exported test class to satisfy lint.
Biome flags noExportsInTest; exporting TestPrompt in a spec file will fail linting.
✅ Proposed fix
-@Injectable()
-export class TestPrompt {
+@Injectable()
+class TestPrompt {
`@Prompt`()
`@Title`("Title")
`@Description`("Description")
prompt() {}
}As per coding guidelines, test files must comply with the Vitest lint rules.
📝 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
| @Injectable() | |
| export class TestPrompt { | |
| @Prompt() | |
| @Title("Title") | |
| @Description("Description") | |
| prompt() {} | |
| } | |
| `@Injectable`() | |
| class TestPrompt { | |
| `@Prompt`() | |
| `@Title`("Title") | |
| `@Description`("Description") | |
| prompt() {} | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 5-12: Do not export from a test file.
(lint/suspicious/noExportsInTest)
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/decorators/prompt.spec.ts` around lines 6
- 12, The exported test class TestPrompt in prompt.spec.ts violates the
noExportsInTest lint rule; remove the export modifier from the class declaration
(leave "class TestPrompt" with its decorators `@Prompt`, `@Title`, `@Description` and
method prompt()) so the spec file no longer exports symbols and satisfies the
Vitest/Biome linting rule.
Comment on lines
32
to
37
| beforeEach(() => | ||
| PlatformTest.create({ | ||
| mcp: { | ||
| path: "/ai/mcp" | ||
| } | ||
| }) |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Make MCP enablement explicit in test config.
PlatformMcpModule only registers routes when mcp.enabled is truthy; the test currently relies on a default that isn’t visible here. Consider setting it explicitly to avoid brittle defaults.
♻️ Suggested change
beforeEach(() =>
PlatformTest.create({
mcp: {
+ enabled: true,
path: "/ai/mcp"
}
})
);📝 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
| beforeEach(() => | |
| PlatformTest.create({ | |
| mcp: { | |
| path: "/ai/mcp" | |
| } | |
| }) | |
| beforeEach(() => | |
| PlatformTest.create({ | |
| mcp: { | |
| enabled: true, | |
| path: "/ai/mcp" | |
| } | |
| }) | |
| ); |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/services/PlatformMcpModule.spec.ts` around
lines 32 - 37, The test setup for PlatformTest.create relies on an implicit
default for MCP enablement; update the test configuration passed to
PlatformTest.create to explicitly set mcp.enabled to true (e.g., include mcp: {
path: "/ai/mcp", enabled: true }) so PlatformMcpModule registers routes as
expected—modify the beforeEach that calls PlatformTest.create to include this
explicit flag to avoid brittle defaults.
Comment on lines
15
to
35
| $onRoutesInit() { | ||
| if (this.settings?.enabled) { | ||
| const path = this.settings?.path || "/mcp"; | ||
|
|
||
| this.app.post( | ||
| path, | ||
| useContextHandler(async ($ctx: PlatformContext) => this.dispatch($ctx)) | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| $logRoutes(routes: PlatformRouteDetails[]) { | ||
| return [ | ||
| ...routes, | ||
| this.settings?.enabled && { | ||
| method: "POST", | ||
| name: "PlatformMcpModule.dispatch()", | ||
| url: this.settings?.path || "/mcp" | ||
| } | ||
| ].filter(Boolean); | ||
| } |
Contributor
There was a problem hiding this comment.
Default enabled handling disables MCP by default.
enabled is documented as defaulting to true, but the current check treats undefined as false, so the /mcp route never registers unless explicitly enabled. Default to true in both $onRoutesInit and $logRoutes.
🛠️ Proposed fix
$onRoutesInit() {
- if (this.settings?.enabled) {
+ const enabled = this.settings?.enabled ?? true;
+ if (enabled) {
const path = this.settings?.path || "/mcp";
this.app.post(
path,
useContextHandler(async ($ctx: PlatformContext) => this.dispatch($ctx))
);
}
}
$logRoutes(routes: PlatformRouteDetails[]) {
- return [
+ const enabled = this.settings?.enabled ?? true;
+ return [
...routes,
- this.settings?.enabled && {
+ enabled && {
method: "POST",
name: "PlatformMcpModule.dispatch()",
url: this.settings?.path || "/mcp"
}
].filter(Boolean);
}📝 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
| $onRoutesInit() { | |
| if (this.settings?.enabled) { | |
| const path = this.settings?.path || "/mcp"; | |
| this.app.post( | |
| path, | |
| useContextHandler(async ($ctx: PlatformContext) => this.dispatch($ctx)) | |
| ); | |
| } | |
| } | |
| $logRoutes(routes: PlatformRouteDetails[]) { | |
| return [ | |
| ...routes, | |
| this.settings?.enabled && { | |
| method: "POST", | |
| name: "PlatformMcpModule.dispatch()", | |
| url: this.settings?.path || "/mcp" | |
| } | |
| ].filter(Boolean); | |
| } | |
| $onRoutesInit() { | |
| const enabled = this.settings?.enabled ?? true; | |
| if (enabled) { | |
| const path = this.settings?.path || "/mcp"; | |
| this.app.post( | |
| path, | |
| useContextHandler(async ($ctx: PlatformContext) => this.dispatch($ctx)) | |
| ); | |
| } | |
| } | |
| $logRoutes(routes: PlatformRouteDetails[]) { | |
| const enabled = this.settings?.enabled ?? true; | |
| return [ | |
| ...routes, | |
| enabled && { | |
| method: "POST", | |
| name: "PlatformMcpModule.dispatch()", | |
| url: this.settings?.path || "/mcp" | |
| } | |
| ].filter(Boolean); | |
| } |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/services/PlatformMcpModule.ts` around
lines 15 - 35, The checks for this.settings?.enabled currently treat undefined
as false so MCP is disabled by default; change both $onRoutesInit and $logRoutes
to treat enabled as true when undefined (e.g. use this.settings?.enabled ?? true
or explicit !== false) and keep using this.settings?.path || "/mcp" and the
existing dispatch handler; update the conditional registrations in $onRoutesInit
and the route entry in $logRoutes to use that defaulted enabled value so the
POST /mcp route and PlatformMcpModule.dispatch() appear unless enabled is
explicitly false.
Comment on lines
+11
to
+16
| thresholds: { | ||
| statements: 0, | ||
| branches: 0, | ||
| functions: 0, | ||
| lines: 0 | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider raising coverage thresholds as the package matures.
Setting all coverage thresholds to 0 is acceptable for initial development, but this effectively disables coverage enforcement. Track this as technical debt and incrementally raise thresholds as tests are added.
Do you want me to open an issue to track raising the coverage thresholds once the package stabilizes?
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/vitest.config.mts` around lines 11 - 16, The
coverage thresholds in the vitest config are all set to 0 (the thresholds object
with keys statements, branches, functions, lines), which disables enforcement;
update the thresholds object in vitest.config.mts to non-zero sensible defaults
(e.g., incrementally raise statements/branches/functions/lines to target
percentages appropriate for the package maturity) and add a TODO or issue
reference to track future increases as tests expand so the thresholds can be
tightened over time.
Comment on lines
+621
to
630
| export function Returns(model?: Type<any> | Type<any>[]): ReturnsChainedDecorators; | ||
| export function Returns(status?: string | number, model?: Type<any> | Type<any>[]): ReturnsChainedDecorators; | ||
| export function Returns(status?: string | number, model?: Type<any> | Type<any>[] | any): ReturnsChainedDecorators { | ||
| export function Returns( | ||
| status?: string | number | Type<any> | Type<any>[], | ||
| model?: Type<any> | Type<any>[] | any | ||
| ): ReturnsChainedDecorators { | ||
| const context = new ReturnDecoratorContext({ | ||
| status, | ||
| model | ||
| status: isClass(status) ? 200 : status, | ||
| model: isClass(status) ? status : model | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate isClass implementation
rg -nP -C3 "export function isClass|function isClass|const isClass" -g '*.ts' packagesRepository: tsedio/tsed
Length of output: 863
🏁 Script executed:
# Also check for imports of isClass in the returns.ts file
rg -nP "import.*isClass|from.*isClass" packages/specs/schema/src/decorators/operations/returns.tsRepository: tsedio/tsed
Length of output: 37
🏁 Script executed:
# Check the full Returns function signature and related code context
cat -n packages/specs/schema/src/decorators/operations/returns.ts | sed -n '615,640p'Repository: tsedio/tsed
Length of output: 768
🏁 Script executed:
# Look for existing tests of the Returns decorator to understand expected behavior
fd -e spec.ts -e test.ts packages/specs/schema/src/decorators/operations/ | head -5Repository: tsedio/tsed
Length of output: 552
🏁 Script executed:
# Read full isClass implementation
cat -n packages/core/src/utils/isClass.tsRepository: tsedio/tsed
Length of output: 1297
🏁 Script executed:
# Search for test cases of Returns decorator with arrays or primitives
rg -nP "Returns\(\[|Returns\(String|Returns\(Number|Returns\(Array|Returns\(Boolean" -g '*.ts' packagesRepository: tsedio/tsed
Length of output: 37
🏁 Script executed:
# Check what types can be passed to Returns in practice
rg -nA5 -B5 "export function Returns" packages/specs/schema/src/decorators/operations/returns.ts | head -40Repository: tsedio/tsed
Length of output: 594
🏁 Script executed:
# Search for Returns decorator tests to understand intended usage
fd -type f -name "*.spec.ts" packages/specs/schema/src/decorators/operations/Repository: tsedio/tsed
Length of output: 225
🏁 Script executed:
# Look for Returns usage examples in codebase
rg -nP "Returns\(" packages/specs/schema/src/decorators/operations/returns.ts | head -20Repository: tsedio/tsed
Length of output: 624
🏁 Script executed:
# Check if there are integration tests showing how Returns should be called
rg -nP "Returns\(\[|Returns\(MyModel\)|Returns\(200" -g '*.spec.ts' packages | head -10Repository: tsedio/tsed
Length of output: 979
The overload signature allows model-first usage (Returns([Model]) or Returns(String)) but the implementation does not support it correctly.
The first overload (line 621) declares that Returns accepts Type<any> | Type<any>[] as the first parameter. However, the implementation at line 628 uses only isClass(status) to disambiguate between model-first and status-first calls. Since isClass explicitly filters out arrays and primitive constructors (see isArrayOrArrayClass and isPrimitiveOrPrimitiveClass checks in the implementation), calling Returns([Model]) or Returns(String) will incorrectly set the array/constructor as the HTTP status code and leave the model undefined.
Current codebase usage patterns (e.g., @Returns(200, String), @Returns(200, Array)) always include an explicit status code, so this bug is not triggered in practice. However, the type signature permits the problematic patterns. Either remove Type<any>[] from the first overload, or add guards to handle arrays and primitives as the suggested fix indicates.
🛠️ Suggested fix
- const context = new ReturnDecoratorContext({
- status: isClass(status) ? 200 : status,
- model: isClass(status) ? status : model
- });
+ const isModelArg =
+ isClass(status) ||
+ isPrimitiveOrPrimitiveClass(status) ||
+ (isArray(status) && status.every((t) => isClass(t) || isPrimitiveOrPrimitiveClass(t)));
+
+ const context = new ReturnDecoratorContext({
+ status: isModelArg ? 200 : status,
+ model: isModelArg ? status : model
+ });📝 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
| export function Returns(model?: Type<any> | Type<any>[]): ReturnsChainedDecorators; | |
| export function Returns(status?: string | number, model?: Type<any> | Type<any>[]): ReturnsChainedDecorators; | |
| export function Returns(status?: string | number, model?: Type<any> | Type<any>[] | any): ReturnsChainedDecorators { | |
| export function Returns( | |
| status?: string | number | Type<any> | Type<any>[], | |
| model?: Type<any> | Type<any>[] | any | |
| ): ReturnsChainedDecorators { | |
| const context = new ReturnDecoratorContext({ | |
| status, | |
| model | |
| status: isClass(status) ? 200 : status, | |
| model: isClass(status) ? status : model | |
| }); | |
| export function Returns(model?: Type<any> | Type<any>[]): ReturnsChainedDecorators; | |
| export function Returns(status?: string | number, model?: Type<any> | Type<any>[]): ReturnsChainedDecorators; | |
| export function Returns( | |
| status?: string | number | Type<any> | Type<any>[], | |
| model?: Type<any> | Type<any>[] | any | |
| ): ReturnsChainedDecorators { | |
| const isModelArg = | |
| isClass(status) || | |
| isPrimitiveOrPrimitiveClass(status) || | |
| (isArray(status) && status.every((t) => isClass(t) || isPrimitiveOrPrimitiveClass(t))); | |
| const context = new ReturnDecoratorContext({ | |
| status: isModelArg ? 200 : status, | |
| model: isModelArg ? status : model | |
| }); |
🤖 Prompt for AI Agents
In `@packages/specs/schema/src/decorators/operations/returns.ts` around lines 621
- 630, The Returns overload allows a model-first call but the implementation
only checks isClass(status), so array or primitive constructors (e.g.,
Returns([Model]) or Returns(String)) get treated as a status; update the Returns
implementation to detect model-first shapes by also checking
isArrayOrArrayClass(status) and isPrimitiveOrPrimitiveClass(status) (or adjust
overloads to remove Type<any>[] from the first signature). Specifically, in the
Returns function that constructs new ReturnDecoratorContext, branch so that if
status is a class OR an array-class OR a primitive-class you set model = status
and status = 200, otherwise treat the first arg as the numeric/string status;
reference Returns, ReturnDecoratorContext, isClass, isArrayOrArrayClass, and
isPrimitiveOrPrimitiveClass when making the change.
Romakita
force-pushed
the
feat-platform-mcp
branch
from
6c82de8 to
3899174
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@docs/docs/mcp.md`:
- Around line 132-140: The snippet uses PlatformMcpModule in the `@Configuration`
imports but lacks its import; add an import for PlatformMcpModule (from the
package that provides it, e.g., `@tsed/platform-mcp`) at the top of the file so
the symbol PlatformMcpModule is defined before the `@Configuration` block and the
example compiles and is clear to readers.
In `@openspec/changes/add-mcp-package/specs/mcp-endpoint/spec.md`:
- Line 1: The file currently starts with a second-level heading "## ADDED
Requirements" which triggers MD041 (missing top-level heading); add a top-level
heading (e.g., "# MCP Endpoint Requirements" or another appropriate H1) at the
very top of the document so the existing "## ADDED Requirements" becomes a
subsection, ensuring the file has a single H1 as the first line.
- Around line 1-36: The spec delta currently only contains "## ADDED
Requirements" but must include all four top-level sections; add "## MODIFIED
Requirements", "## REMOVED Requirements", and "## RENAMED Requirements" to the
same file and, if there are no entries for those categories, include the headers
with a single-line placeholder (e.g., "None." or "No changes.") so openspec
validation passes; preserve the existing "## ADDED Requirements" content and
formatting for headers like "PlatformMcpModule",
"defineTool"/"definePrompt"/"defineResource", and decorator examples so the
delta remains complete and consistent.
In `@openspec/changes/add-mcp-package/tasks.md`:
- Line 1: Add a top-level heading to satisfy the MD041 rule by inserting a
single H1 at the very start of the document (e.g., "# Tasks" or another
descriptive title) in tasks.md so the file begins with a top-level heading
before the existing "## 1. Package scaffolding" section.
In `@packages/platform/platform-mcp/package.json`:
- Around line 31-40: The package.json devDependencies are missing `@tsed/vitest`
while vitest.config.mts imports "@tsed/vitest/presets"; update the
packages/platform/platform-mcp package.json devDependencies to include
"@tsed/vitest" (use the workspace:* version like other workspace `@tsed` packages
or the same version used in packages/hooks and packages/core) so the import in
vitest.config.mts resolves in isolated installs, then run an install to update
lockfiles.
In `@packages/platform/platform-mcp/src/decorators/tool.spec.ts`:
- Around line 1-4: The test file is relying on global Vitest functions instead
of explicit imports; update the top of the spec (tool.spec.ts) to import the
Vitest globals used (e.g., describe, it, expect, beforeEach, afterEach — only
those actually used) from "vitest" so it matches other tests like
defineTool.spec.ts and PlatformMcpModule.spec.ts and satisfies the Vitest ESLint
plugin; place the import alongside the existing imports so functions used in the
file (describe/it/expect/etc.) are explicitly imported.
Comment on lines
+132
to
+140
| ```typescript | ||
| @Configuration({ | ||
| imports: [PlatformMcpModule], | ||
| mcp: { | ||
| path: "/ai/mcp", | ||
| enabled: process.env.MCP_DISABLED !== "true" | ||
| } | ||
| }) | ||
| ``` |
Contributor
There was a problem hiding this comment.
Missing import statement for PlatformMcpModule.
The code snippet references PlatformMcpModule but doesn't show the import statement, which may confuse users.
📝 Suggested fix
```typescript
+import {PlatformMcpModule} from "@tsed/platform-mcp";
+
`@Configuration`({
imports: [PlatformMcpModule],
mcp: {📝 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
| ```typescript | |
| @Configuration({ | |
| imports: [PlatformMcpModule], | |
| mcp: { | |
| path: "/ai/mcp", | |
| enabled: process.env.MCP_DISABLED !== "true" | |
| } | |
| }) | |
| ``` |
🤖 Prompt for AI Agents
In `@docs/docs/mcp.md` around lines 132 - 140, The snippet uses PlatformMcpModule
in the `@Configuration` imports but lacks its import; add an import for
PlatformMcpModule (from the package that provides it, e.g., `@tsed/platform-mcp`)
at the top of the file so the symbol PlatformMcpModule is defined before the
`@Configuration` block and the example compiles and is clear to readers.
| @@ -0,0 +1,37 @@ | |||
| ## ADDED Requirements | |||
Contributor
There was a problem hiding this comment.
Add a top-level heading to satisfy MD041.
♻️ Suggested fix
+# MCP endpoint spec
+
## ADDED Requirements📝 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
| ## ADDED Requirements | |
| # MCP endpoint spec | |
| ## ADDED Requirements |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In `@openspec/changes/add-mcp-package/specs/mcp-endpoint/spec.md` at line 1, The
file currently starts with a second-level heading "## ADDED Requirements" which
triggers MD041 (missing top-level heading); add a top-level heading (e.g., "#
MCP Endpoint Requirements" or another appropriate H1) at the very top of the
document so the existing "## ADDED Requirements" becomes a subsection, ensuring
the file has a single H1 as the first line.
Comment on lines
+1
to
+36
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: @tsed/mcp package scaffold | ||
|
|
||
| Ts.ED SHALL publish a new workspace package `@tsed/mcp` under `packages/platform/platform-mcp` that bundles MCP helpers, depends on `@tsed/platform-http`, and exposes ESM/CJS/type entrypoints plus README/tests consistent with other platform packages. The package SHALL also surface the MCP helper implementations that currently live in `.cli-mcp` so both CLI and HTTP apps consume the same source of truth, and it MUST build its transports and definitions on top of `@modelcontextprotocol/sdk`. | ||
|
|
||
| #### Scenario: Package builds inside the workspace | ||
|
|
||
| - **WHEN** the monorepo build/test pipeline runs after adding `@tsed/mcp` to the workspace | ||
| - **THEN** the package compiles, emits types, and can be imported (e.g., `import {PlatformMcpModule} from "@tsed/mcp"`) without unresolved dependencies. | ||
|
|
||
| ### Requirement: Platform MCP endpoint | ||
|
|
||
| `@tsed/mcp` SHALL provide a `PlatformMcpModule` (or equivalent) that injects `@tsed/platform-http` to register a configurable POST endpoint (default `/mcp`). The module SHALL instantiate a singleton `McpServer`, wire `StreamableHTTPServerTransport` per request, and allow Ts.ED apps on Express, Fastify, or Koa to mount the endpoint by declaring `imports: [PlatformMcpModule]` and optional `mcp` configuration (path, mode). | ||
|
|
||
| #### Scenario: Custom path served on Express | ||
|
|
||
| - **WHEN** a Ts.ED Express application imports `PlatformMcpModule` and sets `configuration.mcp.path = "/ai/mcp"` | ||
| - **THEN** issuing a `POST /ai/mcp` request delivers the payload to the shared `McpServer` via `StreamableHTTPServerTransport`, and the request is logged alongside other platform routes. | ||
|
|
||
| ### Requirement: Functional MCP APIs | ||
|
|
||
| The package SHALL export functional helpers (`defineTool`, `definePrompt`, `defineResource`) that accept JsonSchema/Zod definitions, register DI tokens, and reuse the `.cli-mcp` DI-context execution model (per-call `DIContext`, structured error handling). Tools/resources/prompts registered through these helpers MUST automatically attach to the singleton `McpServer` without manual wiring. | ||
|
|
||
| #### Scenario: Register tool via helper | ||
|
|
||
| - **WHEN** a developer calls `defineTool({ name: "hello", handler })` and adds the returned provider token to a module's `providers` | ||
| - **THEN** the `McpServer` reports a `hello` tool in its metadata and the handler executes inside a Ts.ED DI context when the MCP client invokes it. | ||
|
|
||
| ### Requirement: Decorator-based MCP registration | ||
|
|
||
| `@tsed/mcp` SHALL expose decorators (e.g., `@Tool`, `@Prompt`, `@Resource`) that make it possible to annotate services/controllers instead of using the functional API. Decorator metadata MUST be collected during module initialization and translated into the same provider registrations used by the functional helpers so both approaches behave identically. | ||
|
|
||
| #### Scenario: Annotated service registers a prompt | ||
|
|
||
| - **WHEN** a service method is decorated with `@Prompt({name: "ask-tsed"})` and the containing provider is loaded by Ts.ED |
Contributor
There was a problem hiding this comment.
Add the required MODIFIED/REMOVED/RENAMED sections.
The spec delta must include all four requirement sections even if some are empty, otherwise openspec validation will fail.
🛠️ Proposed fix
### Requirement: Decorator-based MCP registration
@@
- **WHEN** a service method is decorated with `@Prompt({name: "ask-tsed"})` and the containing provider is loaded by Ts.ED
- **THEN** the MCP module registers the prompt with the singleton server, and MCP clients can call `ask-tsed` without additional manual wiring.
+
+## MODIFIED Requirements
+
+None.
+
+## REMOVED Requirements
+
+None.
+
+## RENAMED Requirements
+
+None.📝 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
| ## ADDED Requirements | |
| ### Requirement: @tsed/mcp package scaffold | |
| Ts.ED SHALL publish a new workspace package `@tsed/mcp` under `packages/platform/platform-mcp` that bundles MCP helpers, depends on `@tsed/platform-http`, and exposes ESM/CJS/type entrypoints plus README/tests consistent with other platform packages. The package SHALL also surface the MCP helper implementations that currently live in `.cli-mcp` so both CLI and HTTP apps consume the same source of truth, and it MUST build its transports and definitions on top of `@modelcontextprotocol/sdk`. | |
| #### Scenario: Package builds inside the workspace | |
| - **WHEN** the monorepo build/test pipeline runs after adding `@tsed/mcp` to the workspace | |
| - **THEN** the package compiles, emits types, and can be imported (e.g., `import {PlatformMcpModule} from "@tsed/mcp"`) without unresolved dependencies. | |
| ### Requirement: Platform MCP endpoint | |
| `@tsed/mcp` SHALL provide a `PlatformMcpModule` (or equivalent) that injects `@tsed/platform-http` to register a configurable POST endpoint (default `/mcp`). The module SHALL instantiate a singleton `McpServer`, wire `StreamableHTTPServerTransport` per request, and allow Ts.ED apps on Express, Fastify, or Koa to mount the endpoint by declaring `imports: [PlatformMcpModule]` and optional `mcp` configuration (path, mode). | |
| #### Scenario: Custom path served on Express | |
| - **WHEN** a Ts.ED Express application imports `PlatformMcpModule` and sets `configuration.mcp.path = "/ai/mcp"` | |
| - **THEN** issuing a `POST /ai/mcp` request delivers the payload to the shared `McpServer` via `StreamableHTTPServerTransport`, and the request is logged alongside other platform routes. | |
| ### Requirement: Functional MCP APIs | |
| The package SHALL export functional helpers (`defineTool`, `definePrompt`, `defineResource`) that accept JsonSchema/Zod definitions, register DI tokens, and reuse the `.cli-mcp` DI-context execution model (per-call `DIContext`, structured error handling). Tools/resources/prompts registered through these helpers MUST automatically attach to the singleton `McpServer` without manual wiring. | |
| #### Scenario: Register tool via helper | |
| - **WHEN** a developer calls `defineTool({ name: "hello", handler })` and adds the returned provider token to a module's `providers` | |
| - **THEN** the `McpServer` reports a `hello` tool in its metadata and the handler executes inside a Ts.ED DI context when the MCP client invokes it. | |
| ### Requirement: Decorator-based MCP registration | |
| `@tsed/mcp` SHALL expose decorators (e.g., `@Tool`, `@Prompt`, `@Resource`) that make it possible to annotate services/controllers instead of using the functional API. Decorator metadata MUST be collected during module initialization and translated into the same provider registrations used by the functional helpers so both approaches behave identically. | |
| #### Scenario: Annotated service registers a prompt | |
| - **WHEN** a service method is decorated with `@Prompt({name: "ask-tsed"})` and the containing provider is loaded by Ts.ED | |
| ## ADDED Requirements | |
| ### Requirement: `@tsed/mcp` package scaffold | |
| Ts.ED SHALL publish a new workspace package `@tsed/mcp` under `packages/platform/platform-mcp` that bundles MCP helpers, depends on `@tsed/platform-http`, and exposes ESM/CJS/type entrypoints plus README/tests consistent with other platform packages. The package SHALL also surface the MCP helper implementations that currently live in `.cli-mcp` so both CLI and HTTP apps consume the same source of truth, and it MUST build its transports and definitions on top of `@modelcontextprotocol/sdk`. | |
| #### Scenario: Package builds inside the workspace | |
| - **WHEN** the monorepo build/test pipeline runs after adding `@tsed/mcp` to the workspace | |
| - **THEN** the package compiles, emits types, and can be imported (e.g., `import {PlatformMcpModule} from "@tsed/mcp"`) without unresolved dependencies. | |
| ### Requirement: Platform MCP endpoint | |
| `@tsed/mcp` SHALL provide a `PlatformMcpModule` (or equivalent) that injects `@tsed/platform-http` to register a configurable POST endpoint (default `/mcp`). The module SHALL instantiate a singleton `McpServer`, wire `StreamableHTTPServerTransport` per request, and allow Ts.ED apps on Express, Fastify, or Koa to mount the endpoint by declaring `imports: [PlatformMcpModule]` and optional `mcp` configuration (path, mode). | |
| #### Scenario: Custom path served on Express | |
| - **WHEN** a Ts.ED Express application imports `PlatformMcpModule` and sets `configuration.mcp.path = "/ai/mcp"` | |
| - **THEN** issuing a `POST /ai/mcp` request delivers the payload to the shared `McpServer` via `StreamableHTTPServerTransport`, and the request is logged alongside other platform routes. | |
| ### Requirement: Functional MCP APIs | |
| The package SHALL export functional helpers (`defineTool`, `definePrompt`, `defineResource`) that accept JsonSchema/Zod definitions, register DI tokens, and reuse the `.cli-mcp` DI-context execution model (per-call `DIContext`, structured error handling). Tools/resources/prompts registered through these helpers MUST automatically attach to the singleton `McpServer` without manual wiring. | |
| #### Scenario: Register tool via helper | |
| - **WHEN** a developer calls `defineTool({ name: "hello", handler })` and adds the returned provider token to a module's `providers` | |
| - **THEN** the `McpServer` reports a `hello` tool in its metadata and the handler executes inside a Ts.ED DI context when the MCP client invokes it. | |
| ### Requirement: Decorator-based MCP registration | |
| `@tsed/mcp` SHALL expose decorators (e.g., `@Tool`, `@Prompt`, `@Resource`) that make it possible to annotate services/controllers instead of using the functional API. Decorator metadata MUST be collected during module initialization and translated into the same provider registrations used by the functional helpers so both approaches behave identically. | |
| #### Scenario: Annotated service registers a prompt | |
| - **WHEN** a service method is decorated with `@Prompt({name: "ask-tsed"})` and the containing provider is loaded by Ts.ED | |
| - **THEN** the MCP module registers the prompt with the singleton server, and MCP clients can call `ask-tsed` without additional manual wiring. | |
| ## MODIFIED Requirements | |
| None. | |
| ## REMOVED Requirements | |
| None. | |
| ## RENAMED Requirements | |
| None. |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In `@openspec/changes/add-mcp-package/specs/mcp-endpoint/spec.md` around lines 1 -
36, The spec delta currently only contains "## ADDED Requirements" but must
include all four top-level sections; add "## MODIFIED Requirements", "## REMOVED
Requirements", and "## RENAMED Requirements" to the same file and, if there are
no entries for those categories, include the headers with a single-line
placeholder (e.g., "None." or "No changes.") so openspec validation passes;
preserve the existing "## ADDED Requirements" content and formatting for headers
like "PlatformMcpModule", "defineTool"/"definePrompt"/"defineResource", and
decorator examples so the delta remains complete and consistent.
| @@ -0,0 +1,30 @@ | |||
| ## 1. Package scaffolding | |||
Contributor
There was a problem hiding this comment.
Add a top-level heading to satisfy MD041.
♻️ Suggested fix
+# MCP package tasks
+
## 1. Package scaffolding📝 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
| ## 1. Package scaffolding | |
| # MCP package tasks | |
| ## 1. Package scaffolding |
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
In `@openspec/changes/add-mcp-package/tasks.md` at line 1, Add a top-level heading
to satisfy the MD041 rule by inserting a single H1 at the very start of the
document (e.g., "# Tasks" or another descriptive title) in tasks.md so the file
begins with a top-level heading before the existing "## 1. Package scaffolding"
section.
Comment on lines
1
to
4
| import {DITest, inject, Injectable} from "@tsed/di"; | ||
| import {CollectionOf, Description, Property, Returns} from "@tsed/schema"; | ||
|
|
||
| import {Tool} from "../.."; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit vitest imports for consistency.
This test file relies on global test functions, but other test files in this package (defineTool.spec.ts, PlatformMcpModule.spec.ts) explicitly import from vitest. For consistency and to comply with Vitest ESLint plugin rules, add explicit imports.
♻️ Suggested fix
import {DITest, inject, Injectable} from "@tsed/di";
import {CollectionOf, Description, Property, Returns} from "@tsed/schema";
+import {afterEach, beforeEach, describe, expect, it} from "vitest";
import {Tool} from "../..";As per coding guidelines: **/*.spec.ts: All TypeScript files in test suites must follow Vitest ESLint plugin rules.
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/decorators/tool.spec.ts` around lines 1 -
4, The test file is relying on global Vitest functions instead of explicit
imports; update the top of the spec (tool.spec.ts) to import the Vitest globals
used (e.g., describe, it, expect, beforeEach, afterEach — only those actually
used) from "vitest" so it matches other tests like defineTool.spec.ts and
PlatformMcpModule.spec.ts and satisfies the Vitest ESLint plugin; place the
import alongside the existing imports so functions used in the file
(describe/it/expect/etc.) are explicitly imported.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@packages/platform/platform-mcp/package.json`:
- Line 29: Update the zod dependency in package.json from the exact-pinned
string "3.25.76" to a caret range "^3.25.76" to match the project's versioning
style (similar to "uuid") — locate the "zod": "3.25.76" entry in package.json,
change it to "zod": "^3.25.76", then reinstall/update your lockfile (npm install
/ yarn install) to ensure the lockfile reflects the change.
- Line 26: The package.json entry for the dependency "@modelcontextprotocol/sdk"
uses a permissive range ">=1.25.3"; change this to a caret range "^1.25.3" so
the dependency follows SemVer best practices and is limited to compatible 1.x
minor/patch updates—edit the dependency value for "@modelcontextprotocol/sdk" in
package.json to "^1.25.3".
In `@packages/platform/platform-mcp/src/decorators/resource.spec.ts`:
- Around line 20-22: The test in resource.spec.ts uses a misleading variable
name: rename the variable declared as tool (the result of
inject<any>(Symbol.for(`MCP:RESOURCE:resource`))) to resource and update its
usage in the expect(...) assertion so the variable name matches the resource
descriptor being tested; ensure any other references in this spec file are also
renamed from tool to resource.
In `@packages/platform/platform-mcp/src/decorators/tool.spec.ts`:
- Line 42: The test description strings use incorrect grammar ("should
returns"); update the two test titles passed to the it(...) calls (e.g., the one
around the test in tool.spec.ts that currently reads "should returns metadata
with name" and the similar occurrence at line 55) to "should return metadata
with name" so the test descriptions are grammatically correct.
In `@packages/platform/platform-mcp/src/utils/toZod.ts`:
- Around line 17-20: In the toZod conversion (function toZod) the "string"
branch currently uses z.string().refine(...) for string enums; change it to use
z.enum(schema.enum as [string, ...string[]]) when schema.enum is present and
non-empty so Zod produces proper enum types and better errors; ensure you
cast/validate schema.enum to the non-empty tuple type required by z.enum and
keep the original fallback (z.string()) for non-enum strings.
- Around line 51-57: The current toZod(schema) unsafely casts unknown to
ZodRawShape when schema is not a JsonSchema; add a minimal guard: if schema is
an object, verify it's a Zod shape by iterating its values and checking each
value looks like a Zod type (e.g., typeof value === "object" && (typeof
value.parse === "function" || "_def" in value)); only then return schema as
ZodRawShape, otherwise return undefined (or throw a clear error). Update the
function around toZod, referencing JsonSchema, buildShape and ZodRawShape so
callers get a safe early failure instead of a downstream runtime error.
| "test:ci": "vitest run --coverage.thresholds.autoUpdate=true" | ||
| }, | ||
| "dependencies": { | ||
| "@modelcontextprotocol/sdk": ">=1.25.3", |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🌐 Web query:
@modelcontextprotocol/sdk npm latest version
💡 Result:
The latest published npm version of @modelcontextprotocol/sdk is 1.26.0 (released Feb 4, 2026). [1] [2]
Use caret range for @modelcontextprotocol/sdk to follow SemVer best practices.
>=1.25.3 permits any future version, including hypothetical major versions with breaking changes. Although no 2.x version currently exists on npm, using ^1.25.3 constrains to compatible minor/patch updates within the 1.x line and future-proofs the dependency.
Proposed fix
- "@modelcontextprotocol/sdk": ">=1.25.3",
+ "@modelcontextprotocol/sdk": "^1.25.3",📝 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
| "@modelcontextprotocol/sdk": ">=1.25.3", | |
| "@modelcontextprotocol/sdk": "^1.25.3", |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/package.json` at line 26, The package.json
entry for the dependency "@modelcontextprotocol/sdk" uses a permissive range
">=1.25.3"; change this to a caret range "^1.25.3" so the dependency follows
SemVer best practices and is limited to compatible 1.x minor/patch updates—edit
the dependency value for "@modelcontextprotocol/sdk" in package.json to
"^1.25.3".
| "@modelcontextprotocol/sdk": ">=1.25.3", | ||
| "tslib": "2.7.0", | ||
| "uuid": "^11.0.2", | ||
| "zod": "3.25.76" |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🌐 Web query:
zod 3.25.76 npm registry
💡 Result:
zod@3.25.76 is published on the npm registry and available to install. [1]
- Install:
npm i zod@3.25.76[1] - Published at (UTC): 2025-07-08 09:10Z [2]
- Tarball (registry URL):
https://registry.npmjs.org/zod/-/zod-3.25.76.tgz[3]
Sources: [1] npm package page (v3.25.76), [2] independ.org npm metadata, [3] libup log showing resolved tarball URL.
🏁 Script executed:
cat packages/platform/platform-mcp/package.json | grep -A 50 '"dependencies"' | head -20Repository: tsedio/tsed
Length of output: 587
Consider using caret range for consistency across dependencies.
zod@3.25.76 exists on npm and installs without issue. However, zod and tslib are exact-pinned (2.7.0, 3.25.76) while uuid uses a caret range (^11.0.2). For consistency and flexibility, consider using ^3.25.76 for zod unless there's a specific reason to pin the exact version.
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/package.json` at line 29, Update the zod
dependency in package.json from the exact-pinned string "3.25.76" to a caret
range "^3.25.76" to match the project's versioning style (similar to "uuid") —
locate the "zod": "3.25.76" entry in package.json, change it to "zod":
"^3.25.76", then reinstall/update your lockfile (npm install / yarn install) to
ensure the lockfile reflects the change.
Comment on lines
+20
to
+22
| const tool = inject<any>(Symbol.for(`MCP:RESOURCE:resource`)); | ||
|
|
||
| expect(tool).toEqual({ |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Misleading variable name: tool should be resource.
This is a resource descriptor test, but the variable is named tool — likely a copy-paste artifact from tool.spec.ts.
Proposed fix
- const tool = inject<any>(Symbol.for(`MCP:RESOURCE:resource`));
+ const resource = inject<any>(Symbol.for(`MCP:RESOURCE:resource`));
- expect(tool).toEqual({
+ expect(resource).toEqual({📝 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
| const tool = inject<any>(Symbol.for(`MCP:RESOURCE:resource`)); | |
| expect(tool).toEqual({ | |
| const resource = inject<any>(Symbol.for(`MCP:RESOURCE:resource`)); | |
| expect(resource).toEqual({ |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/decorators/resource.spec.ts` around lines
20 - 22, The test in resource.spec.ts uses a misleading variable name: rename
the variable declared as tool (the result of
inject<any>(Symbol.for(`MCP:RESOURCE:resource`))) to resource and update its
usage in the expect(...) assertion so the variable name matches the resource
descriptor being tested; ensure any other references in this spec file are also
renamed from tool to resource.
| beforeEach(() => DITest.create()); | ||
| afterEach(() => DITest.reset()); | ||
|
|
||
| it("should returns metadata with name", () => { |
Contributor
There was a problem hiding this comment.
Minor grammar nit: "should returns" → "should return".
- it("should returns metadata with name", () => {
+ it("should return metadata with name", () => {- it("should returns metadata with propertyKey as tool name", () => {
+ it("should return metadata with propertyKey as tool name", () => {Also applies to: 55-55
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/decorators/tool.spec.ts` at line 42, The
test description strings use incorrect grammar ("should returns"); update the
two test titles passed to the it(...) calls (e.g., the one around the test in
tool.spec.ts that currently reads "should returns metadata with name" and the
similar occurrence at line 55) to "should return metadata with name" so the test
descriptions are grammatically correct.
Comment on lines
51
to
57
| export function toZod(schema: unknown): ZodRawShape | undefined { | ||
| if (schema instanceof JsonSchema) { | ||
| return buildShape(schema.toJSON() as JsonSchemaLike); | ||
| } | ||
|
|
||
| return schema as ZodRawShape | undefined; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unsafe cast of unknown to ZodRawShape — consider a minimal guard.
When schema is not a JsonSchema instance, it's blindly cast to ZodRawShape. If a caller accidentally passes a plain JSON object (e.g., a raw JSON Schema literal), it will silently be treated as a Zod shape and fail at runtime with a confusing error downstream.
A lightweight type guard or an explicit check (e.g., verifying that values are ZodType instances) would make misuse easier to diagnose.
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/utils/toZod.ts` around lines 51 - 57, The
current toZod(schema) unsafely casts unknown to ZodRawShape when schema is not a
JsonSchema; add a minimal guard: if schema is an object, verify it's a Zod shape
by iterating its values and checking each value looks like a Zod type (e.g.,
typeof value === "object" && (typeof value.parse === "function" || "_def" in
value)); only then return schema as ZodRawShape, otherwise return undefined (or
throw a clear error). Update the function around toZod, referencing JsonSchema,
buildShape and ZodRawShape so callers get a safe early failure instead of a
downstream runtime error.
Romakita
force-pushed
the
feat-platform-mcp
branch
from
629184b to
803d08f
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+5
to
+8
| return `z.any().refine((value) => !${parseSchema(schema.not, { | ||
| ...refs, | ||
| path: [...refs.path, "not"] | ||
| })}.safeParse(value).success, "Invalid input: Should NOT be valid against schema")`; |
Check warning
Code scanning / CodeQL
Improper code sanitization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, the problem is that we’re constructing code strings that interpolate untrusted values after only applying JSON.stringify, which does not escape all characters that are dangerous in a JavaScript or <script> context. To fix this, we should post-process JSON.stringify output through a small escaping function that replaces characters like <, >, /, backslash, control characters, and \u2028/\u2029 with their safe escape sequences. Then we must use this helper consistently for all places where such values are inserted into generated code.
The best fix here, without changing existing functionality, is:
- Define a reusable
escapeUnsafeChars(and acharMap) in one of the central parser files that already depends on all others, such asparseSchema.ts. This keeps the helpers in a single place and avoids needing new external dependencies. - Replace all direct uses of
JSON.stringifywhose results flow into code strings withescapeUnsafeChars(JSON.stringify(...)). Concretely:- In
parseConst.ts, wrapJSON.stringify(schema.const). - In
parseEnum.ts, wrap eachJSON.stringify(...)call used inside template literals producing code. - In
parseSchema.ts, wrapJSON.stringify(schema.description)andJSON.stringify(schema.default).
- In
- Ensure that the escaping function’s regex covers all characters in the provided background example:
<,>,/,\,\b,\f,\n,\r,\t,\0,\u2028, and\u2029.
No external library is strictly necessary; a concise local helper suffices. We keep all return types and overall string shapes the same; only the inner escaped values change, preserving semantics while avoiding injection.
Suggested changeset
3
packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseConst.ts
Outside changed files
| @@ -1,5 +1,5 @@ | ||
| import {JsonSchemaObject, Serializable} from "../Types.js"; | ||
|
|
||
| export const parseConst = (schema: JsonSchemaObject & {const: Serializable}) => { | ||
| return `z.literal(${JSON.stringify(schema.const)})`; | ||
| return `z.literal(${escapeUnsafeChars(JSON.stringify(schema.const))})`; | ||
| }; |
packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseSchema.ts
Outside changed files
| @@ -16,6 +16,24 @@ | ||
| import {parseOneOf} from "./parseOneOf.js"; | ||
| import {parseString} from "./parseString.js"; | ||
|
|
||
| const charMap: {[key: string]: string} = { | ||
| "<": "\\u003C", | ||
| ">": "\\u003E", | ||
| "/": "\\u002F", | ||
| "\\": "\\\\", | ||
| "\b": "\\b", | ||
| "\f": "\\f", | ||
| "\n": "\\n", | ||
| "\r": "\\r", | ||
| "\t": "\\t", | ||
| "\0": "\\0", | ||
| "\u2028": "\\u2028", | ||
| "\u2029": "\\u2029" | ||
| }; | ||
|
|
||
| const escapeUnsafeChars = (str: string): string => | ||
| str.replace(/[<>\/\\\b\f\n\r\t\0\u2028\u2029]/g, (x) => charMap[x] ?? x); | ||
|
|
||
| export const parseSchema = (schema: JsonSchema, refs: Refs = {seen: new Map(), path: []}, blockMeta?: boolean): string => { | ||
| if (typeof schema !== "object") return schema ? "z.any()" : "z.never()"; | ||
|
|
||
| @@ -64,7 +82,7 @@ | ||
|
|
||
| const addDescribes = (schema: JsonSchemaObject, parsed: string): string => { | ||
| if (schema.description) { | ||
| parsed += `.describe(${JSON.stringify(schema.description)})`; | ||
| parsed += `.describe(${escapeUnsafeChars(JSON.stringify(schema.description))})`; | ||
| } | ||
|
|
||
| return parsed; | ||
| @@ -72,7 +90,7 @@ | ||
|
|
||
| const addDefaults = (schema: JsonSchemaObject, parsed: string): string => { | ||
| if (schema.default !== undefined) { | ||
| parsed += `.default(${JSON.stringify(schema.default)})`; | ||
| parsed += `.default(${escapeUnsafeChars(JSON.stringify(schema.default))})`; | ||
| } | ||
|
|
||
| return parsed; |
packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseEnum.ts
Outside changed files
| @@ -5,10 +5,10 @@ | ||
| return "z.never()"; | ||
| } else if (schema.enum.length === 1) { | ||
| // union does not work when there is only one element | ||
| return `z.literal(${JSON.stringify(schema.enum[0])})`; | ||
| return `z.literal(${escapeUnsafeChars(JSON.stringify(schema.enum[0]))})`; | ||
| } else if (schema.enum.every((x) => typeof x === "string")) { | ||
| return `z.enum([${schema.enum.map((x) => JSON.stringify(x))}])`; | ||
| return `z.enum([${schema.enum.map((x) => escapeUnsafeChars(JSON.stringify(x)))}])`; | ||
| } else { | ||
| return `z.union([${schema.enum.map((x) => `z.literal(${JSON.stringify(x)})`).join(", ")}])`; | ||
| return `z.union([${schema.enum.map((x) => `z.literal(${escapeUnsafeChars(JSON.stringify(x))})`).join(", ")}])`; | ||
| } | ||
| }; |
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
| import {jsonSchemaToZod} from "./json-schema-to-zod/index.js"; | ||
|
|
||
| function transform(schema: JsonSchema): ZodObject { | ||
| return eval(`(z) => ${jsonSchemaToZod(schema.toJSON(), {zodVersion: 4})}`)(z); |
Check warning
Code scanning / CodeQL
Improper code sanitization
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
General approach: keep using code generation but ensure that any stringified user-controlled data that ends up in generated JavaScript is passed through an additional sanitizer that escapes characters problematic in script contexts (<, >, /, backslash, control whitespace, \u2028, \u2029, etc.). Then use this sanitizer consistently wherever we currently interpolate JSON.stringify(...) into code strings, and also sanitize the final code string before passing it to eval.
Best way in this codebase:
- Define a reusable
escapeUnsafeCharsfunction (and itscharMap) in a shared spot that is imported by the parsers andjsonSchemaToZod. Since we’re constrained to the provided files, we can define it once injsonSchemaToZod.tsand re-export it (or in a tiny local helper in each file). To minimize changes, we’ll:- Implement
escapeUnsafeChars(str: string): stringinjsonSchemaToZod.ts. - Export it from there.
- Import and use it in
parseConst.ts,parseEnum.ts, andparseSchema.ts.
- Implement
- Replace direct
JSON.stringify(...)usages that feed into code templates withescapeUnsafeChars(JSON.stringify(...)):JSON.stringify(name)injsonSchemaToZod.ts.JSON.stringify(schema.const)inparseConst.ts.- All
JSON.stringify(...)calls inparseEnum.ts. JSON.stringify(schema.description)andJSON.stringify(schema.default)inparseSchema.ts.
- In
toZod.ts, instead of feeding the raw result ofjsonSchemaToZod(...)intoeval, run the sameescapeUnsafeCharssanitizer on that code string before evaluation. This protects against any remaining risky characters that might slip through or come from other parts of the generator. - Keep functionality intact: the generated JavaScript is still valid, and Zod semantics remain unchanged because we are only changing how strings are encoded at the JavaScript source level, not their runtime values.
Concretely:
- packages/platform/platform-mcp/src/utils/json-schema-to-zod/jsonSchemaToZod.ts
- Add
charMapandescapeUnsafeCharshelper at top-level. - Export
escapeUnsafeChars. - Wrap
JSON.stringify(name)withescapeUnsafeChars(...).
- Add
- packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseConst.ts
- Import
escapeUnsafeCharsfrom../jsonSchemaToZod.js. - Wrap
JSON.stringify(schema.const)withescapeUnsafeChars(...).
- Import
- packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseEnum.ts
- Import
escapeUnsafeChars. - Wrap all
JSON.stringify(...)calls withescapeUnsafeChars(...).
- Import
- packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseSchema.ts
- Import
escapeUnsafeChars. - Wrap
JSON.stringify(schema.description)andJSON.stringify(schema.default)withescapeUnsafeChars(...).
- Import
- packages/platform/platform-mcp/src/utils/toZod.ts
-
Import
escapeUnsafeCharsfrom./json-schema-to-zod/index.js(assuming it re-exports fromjsonSchemaToZod.tsdirectly or via barrel; if not, from./json-schema-to-zod/jsonSchemaToZod.jsas appropriate to the existing index). -
Sanitize the generated string before
eval, e.g.:const code = escapeUnsafeChars(jsonSchemaToZod(schema.toJSON(), {zodVersion: 4})); return eval(`(z) => ${code}`)(z);
-
This keeps the existing public API and behavior, while hardening against code injection.
Suggested changeset
5
packages/platform/platform-mcp/src/utils/toZod.ts
| @@ -1,10 +1,12 @@ | ||
| import {JsonSchema} from "@tsed/schema"; | ||
| import {z, type ZodObject} from "zod"; | ||
|
|
||
| import {jsonSchemaToZod} from "./json-schema-to-zod/index.js"; | ||
| import {jsonSchemaToZod, escapeUnsafeChars} from "./json-schema-to-zod/index.js"; | ||
|
|
||
| function transform(schema: JsonSchema): ZodObject { | ||
| return eval(`(z) => ${jsonSchemaToZod(schema.toJSON(), {zodVersion: 4})}`)(z); | ||
| const generated = jsonSchemaToZod(schema.toJSON(), {zodVersion: 4}); | ||
| const safeGenerated = escapeUnsafeChars(generated); | ||
| return eval(`(z) => ${safeGenerated}`)(z); | ||
| } | ||
|
|
||
| export function toZod(schema: unknown): ZodObject | undefined { |
packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseConst.ts
Outside changed files
| @@ -1,5 +1,6 @@ | ||
| import {JsonSchemaObject, Serializable} from "../Types.js"; | ||
| import {escapeUnsafeChars} from "../jsonSchemaToZod.js"; | ||
|
|
||
| export const parseConst = (schema: JsonSchemaObject & {const: Serializable}) => { | ||
| return `z.literal(${JSON.stringify(schema.const)})`; | ||
| return `z.literal(${escapeUnsafeChars(JSON.stringify(schema.const))})`; | ||
| }; |
packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseSchema.ts
Outside changed files
| @@ -15,6 +15,7 @@ | ||
| import {parseObject} from "./parseObject.js"; | ||
| import {parseOneOf} from "./parseOneOf.js"; | ||
| import {parseString} from "./parseString.js"; | ||
| import {escapeUnsafeChars} from "../jsonSchemaToZod.js"; | ||
|
|
||
| export const parseSchema = (schema: JsonSchema, refs: Refs = {seen: new Map(), path: []}, blockMeta?: boolean): string => { | ||
| if (typeof schema !== "object") return schema ? "z.any()" : "z.never()"; | ||
| @@ -64,7 +65,7 @@ | ||
|
|
||
| const addDescribes = (schema: JsonSchemaObject, parsed: string): string => { | ||
| if (schema.description) { | ||
| parsed += `.describe(${JSON.stringify(schema.description)})`; | ||
| parsed += `.describe(${escapeUnsafeChars(JSON.stringify(schema.description))})`; | ||
| } | ||
|
|
||
| return parsed; | ||
| @@ -72,7 +73,7 @@ | ||
|
|
||
| const addDefaults = (schema: JsonSchemaObject, parsed: string): string => { | ||
| if (schema.default !== undefined) { | ||
| parsed += `.default(${JSON.stringify(schema.default)})`; | ||
| parsed += `.default(${escapeUnsafeChars(JSON.stringify(schema.default))})`; | ||
| } | ||
|
|
||
| return parsed; |
packages/platform/platform-mcp/src/utils/json-schema-to-zod/jsonSchemaToZod.ts
Outside changed files
| @@ -2,6 +2,25 @@ | ||
| import {JsonSchema, Options} from "./Types.js"; | ||
| import {expandJsdocs} from "./utils/jsdocs.js"; | ||
|
|
||
| const charMap: Record<string, string> = { | ||
| "<": "\\u003C", | ||
| ">": "\\u003E", | ||
| "/": "\\u002F", | ||
| "\\": "\\\\", | ||
| "\b": "\\b", | ||
| "\f": "\\f", | ||
| "\n": "\\n", | ||
| "\r": "\\r", | ||
| "\t": "\\t", | ||
| "\0": "\\0", | ||
| "\u2028": "\\u2028", | ||
| "\u2029": "\\u2029" | ||
| }; | ||
|
|
||
| export const escapeUnsafeChars = (str: string): string => { | ||
| return str.replace(/[<>\\/\b\f\n\r\t\0\u2028\u2029]/g, (ch) => charMap[ch] ?? ch); | ||
| }; | ||
|
|
||
| export const jsonSchemaToZod = (schema: JsonSchema, {module, name, type, noImport, zodVersion = 4, ...rest}: Options = {}): string => { | ||
| if (type && (!name || module !== "esm")) { | ||
| throw new Error("Option `type` requires `name` to be set and `module` to be `esm`"); | ||
| @@ -19,7 +38,7 @@ | ||
| const jsdocs = rest.withJsdocs && typeof schema !== "boolean" && schema.description ? expandJsdocs(schema.description) : ""; | ||
|
|
||
| if (module === "cjs") { | ||
| result = `${jsdocs}module.exports = ${name ? `{ ${JSON.stringify(name)}: ${result} }` : result} | ||
| result = `${jsdocs}module.exports = ${name ? `{ ${escapeUnsafeChars(JSON.stringify(name))}: ${result} }` : result} | ||
| `; | ||
|
|
||
| if (!noImport) { |
packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseEnum.ts
Outside changed files
| @@ -1,14 +1,15 @@ | ||
| import {JsonSchemaObject, Serializable} from "../Types.js"; | ||
| import {escapeUnsafeChars} from "../jsonSchemaToZod.js"; | ||
|
|
||
| export const parseEnum = (schema: JsonSchemaObject & {enum: Serializable[]}) => { | ||
| if (schema.enum.length === 0) { | ||
| return "z.never()"; | ||
| } else if (schema.enum.length === 1) { | ||
| // union does not work when there is only one element | ||
| return `z.literal(${JSON.stringify(schema.enum[0])})`; | ||
| return `z.literal(${escapeUnsafeChars(JSON.stringify(schema.enum[0]))})`; | ||
| } else if (schema.enum.every((x) => typeof x === "string")) { | ||
| return `z.enum([${schema.enum.map((x) => JSON.stringify(x))}])`; | ||
| return `z.enum([${schema.enum.map((x) => escapeUnsafeChars(JSON.stringify(x)))}])`; | ||
| } else { | ||
| return `z.union([${schema.enum.map((x) => `z.literal(${JSON.stringify(x)})`).join(", ")}])`; | ||
| return `z.union([${schema.enum.map((x) => `z.literal(${escapeUnsafeChars(JSON.stringify(x))})`).join(", ")}])`; | ||
| } | ||
| }; |
Copilot is powered by AI and may make mistakes. Always verify output.
Unable to commit as this autofix suggestion is now outdated
Contributor
There was a problem hiding this comment.
Actionable comments posted: 31
🤖 Fix all issues with AI agents
In `@packages/platform/platform-mcp/package.json`:
- Line 29: The dependency version for zod in package.json is too permissive
("zod": ">=4.3.6"); change it to a caret range to constrain to compatible 4.x
releases by replacing the version string with "^4.3.6" so package resolution
stays within Zod 4 (update the "zod" entry in package.json accordingly and run
your package manager install to refresh lockfiles).
In `@packages/platform/platform-mcp/src/decorators/prompt.spec.ts`:
- Line 18: Fix the test description string in the it() call currently written as
"should returns metadata without explicit given options" to correct grammar;
change it to "should return metadata without explicit given options" in the test
declaration (the it(...) for the prompt.spec.ts test).
- Line 25: The test assertion currently expects lowercase "title" but the
decorator `@Title`("Title") stores "Title" in the schema, so update the expected
value in the assertion within the prompt.spec.ts test to title: "Title" (the
value returned by methodStore.schema.get("title") via mapOptions()) so the
assertion matches the stored schema value from the `@Title` decorator.
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/cli.ts`:
- Line 79: The CLI currently calls void main() which swallows Promise
rejections; change the invocation to call main() and attach a rejection handler
that logs the error (e.g., console.error or processLogger) and sets a non-zero
exit code (process.exitCode = 1 or process.exit(1)); specifically update the
top-level call around the main function invocation so that main().catch(err => {
log the error with details; set non-zero exit/exitCode }) to ensure invalid
JSON, missing files, or unreadable pipes produce a clear message and proper exit
code.
- Around line 61-69: The CLI currently forwards raw parseArgs results (which are
false when flags are omitted) into jsonSchemaToZod (in the call building
zodSchema), causing type-unsafe false values for optional params like name,
depth, and type; update the call so each optional arg is coerced to undefined
when not provided (e.g., set name to undefined when args.name is falsey, depth
to undefined when args.depth === false, and type to undefined when args.type is
falsey) while keeping the existing default for module ("esm") and preserving
zodVersion and withJsdocs; locate the jsonSchemaToZod invocation to change the
name, depth, and type properties accordingly.
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/index.ts`:
- Around line 1-26: Move the import for jsonSchemaToZod so it appears before the
re-export statements: import { jsonSchemaToZod } from "./jsonSchemaToZod.js";
should be placed at the top of the module, then keep all export * lines (e.g.,
exports of "./jsonSchemaToZod.js", "./parsers/parseObject.js",
"./utils/withMessage.js", etc.) after it, and finally export default
jsonSchemaToZod; so the file follows the conventional import-then-export
ordering and the default export references the already-imported symbol.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/jsonSchemaToZod.ts`:
- Around line 21-29: The CJS branch duplicates the JSDoc because jsdocs is
prepended both to the export (`module === "cjs"` branch building `result` with
`module.exports`) and again when prepending the import (`const { z } =
require("zod")`) if `!noImport`; change the assembly so jsdocs is applied only
once (to the export line) and prepend the import without jsdocs when adding
`const { z } = require("zod")`; update the logic around the `result`
construction in the `module === "cjs"` block (references: jsdocs, result,
noImport, and the `const { z } = require("zod")`/`module.exports` pieces) so the
final output contains a single JSDoc block.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseAllOf.ts`:
- Around line 27-33: parseAllOf's single-item branch builds refs.path using
(item as any)[originalIndex] without calling ensureOriginalIndex, so the path
gets undefined; fix by computing the original index before calling parseSchema —
call ensureOriginalIndex(item, schema.allOf) (or otherwise derive the index,
e.g. const index = ensureOriginalIndex(item, schema.allOf) ?? 0) and then use
that index when extending refs.path and invoking parseSchema; update the branch
that returns parseSchema to use the ensured index instead of directly
referencing (item as any)[originalIndex].
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseAnyOf.ts`:
- Line 11: The inner .map callback in parseAnyOf.ts shadows the outer function
parameter named schema; rename the inner callback parameter (e.g., from schema
to subschema or item) inside the expression that builds the union so it no
longer shadows the outer parameter, and update any references inside the
callback (notably the parseSchema call and the path [...refs.path, "anyOf", i])
to use the new name.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseArray.ts`:
- Around line 5-8: The parseArray function builds a tuple string by implicitly
joining schema.items results without spaces, causing inconsistent formatting
versus other parsers; update parseArray (the parseArray function that calls
parseSchema with refs.path [..., "items", i]) to explicitly join the mapped item
strings with ", " (use .map(...).join(", ")) when creating the z.tuple([...])
output so generated Zod tuples match the formatting of parseAnyOf and sibling
parsers.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseConst.ts`:
- Around line 3-5: parseConst currently emits z.literal for any Serializable
const, but z.literal only supports primitives; update parseConst to detect
primitive values (string, number, boolean, bigint, symbol, null, undefined) and
return `z.literal(...)` only for those, otherwise return a fallback that
enforces deep-equality (for example `z.any().refine(v => deepEqual(v,
schema.const), { message: "Expected constant value" })`). Use or add a reliable
deep equality helper (e.g., isDeepStrictEqual/deepEqual utility) and reference
parseConst and schema.const when making the change.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseEnum.ts`:
- Around line 9-10: The returned zod enum string in parseEnum.ts uses implicit
array-to-string conversion which yields inconsistent formatting; update the
expression that builds the enum values (the branch checking
schema.enum.every(...)) to explicitly join the mapped JSON.stringify values with
", " (i.e., use schema.enum.map((x) => JSON.stringify(x)).join(", ")) so the
generated z.enum([...]) matches the formatting used elsewhere.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseIfThenElse.ts`:
- Around line 4-9: The parser currently requires both then and else but JSON
Schema allows only one; update parseIfThenElse to accept then?: JsonSchema and
else?: JsonSchema (make them optional in the parseIfThenElse parameter type) and
adjust logic inside parseIfThenElse to handle missing branches by only parsing
and applying the branch that exists (call parseSchema/parseSubSchema on then
when present, on else when present) and composing the resulting Zod schemas so
that: if only then exists, apply the constraint when the if-condition matches;
if only else exists, apply the constraint when the if-condition does not match;
if both exist keep current behavior. Reference parseIfThenElse and the internal
calls to parseSchema/its.a.conditional to locate where to change types and
branching logic.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseMultipleType.ts`:
- Around line 4-6: parseMultipleType currently builds z.union([...]) from
schema.type without guarding an empty array; if schema.type.length === 0 it
outputs z.union([]) which fails at runtime. Update parseMultipleType to check
schema.type length: if empty return the same fallback parseAnyOf uses (z.any()
via invoking parseSchema or directly returning "z.any()"), otherwise map types
as before, passing {...refs, withoutDefaults: true} into parseSchema; keep
function name parseMultipleType and the use of parseSchema and refs intact.
Ensure behavior mirrors parseAnyOf's empty-case handling so empty type arrays
produce z.any() not z.union([]).
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseNumber.ts`:
- Around line 17-27: The code uses a brittle string-prefix check
(r.startsWith("z.number().int(")) inside the withMessage callback to detect if
`.int()` was already emitted; replace this with an explicit boolean flag tracked
in the parseNumber scope (e.g., emittedInt or a small mutable state object) that
you set to true wherever the `.int()` fragment is appended, and then check that
flag in the multipleOf withMessage callback instead of inspecting the `r`
string; ensure the flag is in the closure visible to withMessage so the
deduplication logic reliably prevents adding a second `.int()` regardless of
future formatting changes.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseObject.ts`:
- Around line 74-85: The map call in parseObject.ts passes an unused thisArg
(the {} after the arrow) when creating parsedPatternProperties from
objectSchema.patternProperties; remove the extraneous second parameter to
Object.entries(...).map so the arrow function mapping ([key, value]) => [key,
parseSchema(value, { ...refs, path: [...refs.path, "patternProperties", key] })]
runs without the confusing thisArg. Locate the block handling
objectSchema.patternProperties in the parseObject function and remove the
trailing ", {}" from the .map(...) call.
- Line 95: The code uses Object.values(parsedPatternProperties) inside template
literals and emitRecord calls which relies on array-to-string coercion;
explicitly extract the single expected value (e.g., const [first] =
Object.values(parsedPatternProperties) or use
Object.values(parsedPatternProperties)[0]) and use that variable in
patternProperties and emitRecord so the output is correct and不会 silently join
multiple entries; update occurrences where patternProperties is built (the
`.catchall(...)` line) and the place that calls emitRecord to use the single
extracted value instead of the array.
- Around line 17-24: emitErrorPath currently returns just "path: [key]" for Zod
v4 which drops parent context; change it to emit the full path by joining
refs.path entries and appending key so superRefine errors include the parent
path (e.g., build "path: [<serialized refs.path entries>, key]"). Update the v4
branch in emitErrorPath to serialize refs.path (using JSON.stringify per entry)
and produce a string like path: [<joined serialized entries>, key] so nested
errors land at the correct nested location; keep the v3 branch as-is.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseOneOf.ts`:
- Around line 11-36: The generated code string shadows the outer parameter named
`schema` and `errors`; update the inner callback parameter names to avoid
shadowing: in the map over `schema.oneOf` rename the callback param from
`schema` to `subSchema` (and update its usage where `parseSchema(subSchema, {
... })` is called), and in the reduce change `(errors, schema)` to `(acc,
subSchema)` (and adjust the reducer to push into `acc` and return `acc`),
ensuring that `schemas` and `errors` variable names remain clear and the call to
`parseSchema` and `schema.oneOf` still reference the correct identifiers.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseSchema.ts`:
- Around line 89-123: selectParser currently routes typed schemas (e.g., type:
"object") to parseObject before checking for conditionals, so if/then/else is
ignored; fix by reordering the conditional check (its.a.conditional) to run
before the type-specific branches that short-circuit (move the parseIfThenElse
branch up so it executes prior to its.an.object, its.an.array, and primitive
checks) or alternatively implement conditional handling inside parseObject;
update selectParser accordingly (refer to selectParser, parseObject, and
parseIfThenElse).
- Around line 162-168: The conditional type guard currently requires all three
keys and should instead accept schemas with "if" plus optionally "then" and/or
"else"; update the predicate in the conditional guard to check for "if" in x and
that x.if is truthy and that at least one of x.then or x.else exists, and change
its type predicate to reflect JsonSchemaObject & { if: JsonSchema; then?:
JsonSchema; else?: JsonSchema }; then update the parseIfThenElse implementation
so it handles partial conditionals (only then, only else, or both) instead of
assuming both branches—ensure parseDefault is not chosen for those cases.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseString.ts`:
- Around line 28-29: The mapping in parseString (case "binary") currently
returns ".base64()" which can double-apply when contentEncoding === "base64";
update the parseString logic so format: "binary" does not unconditionally emit
".base64()": either skip emitting anything for "binary" (return []), or check
the schema's contentEncoding and only emit ".base64()" when contentEncoding is
not already "base64"; modify the code in parseString (the "case \"binary\""
branch) to perform that conditional logic using the
schema.contentEncoding/property rather than always returning [".base64(", ")"].
- Line 57: The condition using loose equality should be changed to a strict
check: replace the `contentMediaType != ""` test in the parseString logic with a
strict comparison (`contentMediaType !== ""`) or a truthy check (`if
(contentMediaType)`) to avoid coercion; update the conditional in the function
handling `contentMediaType` (look for the `contentMediaType` variable and the
surrounding code in parseString.ts) so it uses `!==` or a truthy check and
retains the existing behavior.
- Around line 59-63: The current use of withMessage around the contentSchema
branch is incorrect because it adds a second argument to .pipe(...) which only
accepts a single schema; locate the block in parseString.ts that calls
withMessage(schema, "contentSchema", ({value}) => ...) and replace it with a
plain conditional: if value is a non-null object then append a single .pipe call
using parseSchema(value) (i.e. add ".pipe(${parseSchema(value)})") — do not pass
any error message into .pipe; keep any existing surrounding punctuation/closing
parentheses consistent with how other .pipe usages are emitted.
- Around line 47-55: The transform created in the contentMediaType parser (the
withMessage call that handles schema.contentMediaType === "application/json")
currently calls ctx.addIssue on JSON.parse failure but returns undefined; update
the transform callback inside contentMediaType to explicitly return z.NEVER
immediately after calling ctx.addIssue so Zod knows the validation failed, and
ensure the transform has access to the Zod namespace (z) when making that return
from the transform in parseString.ts (the same transform that currently returns
JSON.parse on success).
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/cliTools.ts`:
- Around line 56-63: The branch that handles missing args currently always sets
result[name] = false for absent options, which yields booleans for non-boolean
params and conflicts with InferReturnType (which expects undefined for missing
non-boolean values); update the logic in cliTools.ts so that when index === -1
and the arg is optional (required is falsy) you set result[name] = (expected
type is boolean ? false : undefined) — determine expected type from the parsed
argument descriptor (e.g., the field used to infer types/value kind) and ensure
callers receive undefined for missing string/number params and false only for
missing boolean flags.
- Around line 126-149: The header in printParams misaligns because data rows
prepend "--" to names but the header calculation doesn't account for that;
update printParams to include the two-character prefix when building the
header/padding (either add 2 to the computed longest or include the "--" length
when constructing header) so the "Short Description" column lines up with the
console.log("--" + name ...) rows; adjust the header variable and any
.repeat(...) usages accordingly to keep column alignment consistent.
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/jsdocs.ts`:
- Around line 3-8: The expandJsdocs function produces compact single-line
comments like /**hello*/; update its single-line branch so it inserts a leading
and trailing space around the content (e.g., `/** hello */`) while preserving
the existing multi-line formatting and trailing newline; locate expandJsdocs and
change the single-line result generation to include those spaces around lines[0]
(keep the rest of the function behavior identical).
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/withMessage.ts`:
- Line 27: There is a stray no-op expression "r;" in withMessage.ts; remove this
dead statement so it doesn't leave a leftover/debug artifact—locate the bare
"r;" in the withMessage.ts implementation (inside the withMessage
helper/function) and delete that line, leaving the surrounding logic and return
values unchanged.
In `@packages/platform/platform-mcp/src/utils/toZod.spec.ts`:
- Around line 5-14: Extend the existing test for toZod to assert actual
validation and round-trip behavior: after calling toZod(schema) (schema created
with s.object and string()), call result.parse with a valid object like { prop1:
"hello" } and assert it succeeds, call result.parse with an invalid object like
{ prop1: 123 } and assert it throws, and compare result.toJSONSchema() (or its
JSON) to the original s.object schema's JSON to ensure the schema round-trips;
update the test around the toZod call and use the existing symbols (toZod,
s.object, string(), result.parse, result.toJSONSchema) to locate where to add
these assertions.
In `@packages/platform/platform-mcp/src/utils/toZod.ts`:
- Around line 6-8: The current transform function uses eval to execute the
string returned by jsonSchemaToZod, which is a code injection risk; replace the
eval call in transform with new Function to limit scope (e.g., construct a
function with new Function('z', `return ${jsonSchemaToZod(...)})(z)`), and
ideally refactor jsonSchemaToZod to return a programmatic Zod AST or builder
instead of raw code strings so transform can instantiate Zod objects directly;
update the transform function (and any callers) to invoke the new Function
result or the programmatic builder rather than eval to eliminate closure scope
leakage.
| "@modelcontextprotocol/sdk": ">=1.26.0", | ||
| "tslib": "2.7.0", | ||
| "uuid": "^11.0.2", | ||
| "zod": ">=4.3.6" |
Contributor
There was a problem hiding this comment.
>=4.3.6 for zod is dangerously permissive — use a caret range.
Zod 4 already introduced significant breaking changes from Zod 3 (new error model, changed .default() semantics, redesigned z.function(), z.record() requiring both key and value schemas, etc.). Using >=4.3.6 would allow a hypothetical Zod 5+ with further breaking changes to be resolved. Pin to ^4.3.6 to stay within compatible 4.x releases.
- "zod": ">=4.3.6"
+ "zod": "^4.3.6"📝 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
| "zod": ">=4.3.6" | |
| "zod": "^4.3.6" |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/package.json` at line 29, The dependency
version for zod in package.json is too permissive ("zod": ">=4.3.6"); change it
to a caret range to constrain to compatible 4.x releases by replacing the
version string with "^4.3.6" so package resolution stays within Zod 4 (update
the "zod" entry in package.json accordingly and run your package manager install
to refresh lockfiles).
| beforeEach(() => DITest.create()); | ||
| afterEach(() => DITest.reset()); | ||
|
|
||
| it("should returns metadata without explicit given options", () => { |
Contributor
There was a problem hiding this comment.
Grammar: "should returns" → "should return".
- it("should returns metadata without explicit given options", () => {
+ it("should return metadata without explicit given options", () => {📝 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
| it("should returns metadata without explicit given options", () => { | |
| it("should return metadata without explicit given options", () => { |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/decorators/prompt.spec.ts` at line 18, Fix
the test description string in the it() call currently written as "should
returns metadata without explicit given options" to correct grammar; change it
to "should return metadata without explicit given options" in the test
declaration (the it(...) for the prompt.spec.ts test).
Comment on lines
+61
to
+69
| const zodSchema = jsonSchemaToZod(jsonSchema as JsonSchema, { | ||
| name: args.name, | ||
| depth: args.depth, | ||
| module: args.module || "esm", | ||
| noImport: args.noImport, | ||
| type: args.type, | ||
| withJsdocs: args.withJsdocs, | ||
| zodVersion | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
false is passed instead of undefined for optional fields.
When CLI flags like --name, --depth, or --type are omitted, parseArgs returns false. These false values are forwarded directly to jsonSchemaToZod, which expects string | undefined or number | undefined. This works accidentally due to JS coercion/falsy checks but is type-unsafe and fragile.
♻️ Proposed fix: coerce falsy values to undefined
const zodSchema = jsonSchemaToZod(jsonSchema as JsonSchema, {
- name: args.name,
- depth: args.depth,
- module: args.module || "esm",
+ name: args.name || undefined,
+ depth: args.depth || undefined,
+ module: args.module || "esm",
noImport: args.noImport,
- type: args.type,
+ type: args.type || undefined,
withJsdocs: args.withJsdocs,
zodVersion
});📝 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
| const zodSchema = jsonSchemaToZod(jsonSchema as JsonSchema, { | |
| name: args.name, | |
| depth: args.depth, | |
| module: args.module || "esm", | |
| noImport: args.noImport, | |
| type: args.type, | |
| withJsdocs: args.withJsdocs, | |
| zodVersion | |
| }); | |
| const zodSchema = jsonSchemaToZod(jsonSchema as JsonSchema, { | |
| name: args.name || undefined, | |
| depth: args.depth || undefined, | |
| module: args.module || "esm", | |
| noImport: args.noImport, | |
| type: args.type || undefined, | |
| withJsdocs: args.withJsdocs, | |
| zodVersion | |
| }); |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/cli.ts` around
lines 61 - 69, The CLI currently forwards raw parseArgs results (which are false
when flags are omitted) into jsonSchemaToZod (in the call building zodSchema),
causing type-unsafe false values for optional params like name, depth, and type;
update the call so each optional arg is coerced to undefined when not provided
(e.g., set name to undefined when args.name is falsey, depth to undefined when
args.depth === false, and type to undefined when args.type is falsey) while
keeping the existing default for module ("esm") and preserving zodVersion and
withJsdocs; locate the jsonSchemaToZod invocation to change the name, depth, and
type properties accordingly.
| } | ||
| } | ||
|
|
||
| void main(); |
Contributor
There was a problem hiding this comment.
Unhandled promise rejection on CLI errors.
void main() swallows the rejection. If the user provides invalid JSON, a missing file, or an unreadable pipe, the process may emit an UnhandledPromiseRejection warning/crash without a clear message or proper exit code.
🛡️ Proposed fix: add a catch handler
-void main();
+main().catch((err) => {
+ console.error(err instanceof Error ? err.message : err);
+ process.exit(1);
+});📝 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
| void main(); | |
| main().catch((err) => { | |
| console.error(err instanceof Error ? err.message : err); | |
| process.exit(1); | |
| }); |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/cli.ts` at line
79, The CLI currently calls void main() which swallows Promise rejections;
change the invocation to call main() and attach a rejection handler that logs
the error (e.g., console.error or processLogger) and sets a non-zero exit code
(process.exitCode = 1 or process.exit(1)); specifically update the top-level
call around the main function invocation so that main().catch(err => { log the
error with details; set non-zero exit/exitCode }) to ensure invalid JSON,
missing files, or unreadable pipes produce a clear message and proper exit code.
packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/cliTools.ts
Show resolved
Hide resolved
Comment on lines
+3
to
+8
| export const expandJsdocs = (jsdocs: string): string => { | ||
| const lines = jsdocs.split("\n"); | ||
| const result = lines.length === 1 ? lines[0] : `\n${lines.map((x) => `* ${x}`).join("\n")}\n`; | ||
|
|
||
| return `/**${result}*/\n`; | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Single-line JSDoc is missing spaces around the content.
For a single-line description "hello", expandJsdocs produces /**hello*/ instead of the conventional /** hello */.
Proposed fix
- const result = lines.length === 1 ? lines[0] : `\n${lines.map((x) => `* ${x}`).join("\n")}\n`;
-
- return `/**${result}*/\n`;
+ const result = lines.length === 1 ? ` ${lines[0]} ` : `\n${lines.map((x) => ` * ${x}`).join("\n")}\n `;
+
+ return `/**${result}*/\n`;📝 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
| export const expandJsdocs = (jsdocs: string): string => { | |
| const lines = jsdocs.split("\n"); | |
| const result = lines.length === 1 ? lines[0] : `\n${lines.map((x) => `* ${x}`).join("\n")}\n`; | |
| return `/**${result}*/\n`; | |
| }; | |
| export const expandJsdocs = (jsdocs: string): string => { | |
| const lines = jsdocs.split("\n"); | |
| const result = lines.length === 1 ? ` ${lines[0]} ` : `\n${lines.map((x) => ` * ${x}`).join("\n")}\n `; | |
| return `/**${result}*/\n`; | |
| }; |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/jsdocs.ts`
around lines 3 - 8, The expandJsdocs function produces compact single-line
comments like /**hello*/; update its single-line branch so it inserts a leading
and trailing space around the content (e.g., `/** hello */`) while preserving
the existing multi-line formatting and trailing newline; locate expandJsdocs and
change the single-line result generation to include those spaces around lines[0]
(keep the rest of the function behavior identical).
| if (schema.errorMessage?.[key] !== undefined) { | ||
| r += prefix + JSON.stringify(schema.errorMessage[key]); | ||
| } | ||
| r; |
Contributor
There was a problem hiding this comment.
Dead expression statement — r; is a no-op.
Line 27 is a bare r; that has no effect. This looks like a leftover from debugging or an incomplete edit. Remove it.
Proposed fix
if (schema.errorMessage?.[key] !== undefined) {
r += prefix + JSON.stringify(schema.errorMessage[key]);
}
- r;
r += closer;📝 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
| r; | |
| if (schema.errorMessage?.[key] !== undefined) { | |
| r += prefix + JSON.stringify(schema.errorMessage[key]); | |
| } | |
| r += closer; |
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/withMessage.ts`
at line 27, There is a stray no-op expression "r;" in withMessage.ts; remove
this dead statement so it doesn't leave a leftover/debug artifact—locate the
bare "r;" in the withMessage.ts implementation (inside the withMessage
helper/function) and delete that line, leaving the surrounding logic and return
values unchanged.
Comment on lines
+5
to
+14
| describe("toZod", () => { | ||
| it("should transform JsonSchema to Zod instance", async () => { | ||
| const schema = s.object({ | ||
| prop1: string() | ||
| }); | ||
|
|
||
| const result = toZod(schema); | ||
|
|
||
| expect(result?.toJSONSchema).toBeDefined(); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Test coverage is very thin — consider asserting actual validation behavior.
The test only checks that toJSONSchema is defined on the result. It doesn't verify that the returned Zod schema actually validates/rejects values or that the schema structure round-trips correctly. For a foundational utility that underpins MCP tool/resource/prompt definitions, consider adding assertions such as:
result.parse({ prop1: "hello" })succeedsresult.parse({ prop1: 123 })throwsresult.toJSONSchema()produces a schema matching the input
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/utils/toZod.spec.ts` around lines 5 - 14,
Extend the existing test for toZod to assert actual validation and round-trip
behavior: after calling toZod(schema) (schema created with s.object and
string()), call result.parse with a valid object like { prop1: "hello" } and
assert it succeeds, call result.parse with an invalid object like { prop1: 123 }
and assert it throws, and compare result.toJSONSchema() (or its JSON) to the
original s.object schema's JSON to ensure the schema round-trips; update the
test around the toZod call and use the existing symbols (toZod, s.object,
string(), result.parse, result.toJSONSchema) to locate where to add these
assertions.
c3d1f95 to
37984a3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@packages/platform/platform-mcp/src/fn/defineTool.ts`:
- Around line 69-103: The DI token is created with
Symbol.for(`MCP:TOOL:${options.name}`) before the tool name is resolved, so
ClassToolProps without a name yields "MCP:TOOL:undefined"; call
mapOptions(options) first (or otherwise resolve the final name as mapOptions
does) and use the resolved opts.name when building the injectable token (i.e.,
use the resolved name for Symbol.for), or alternatively enforce name required in
the public API; update defineTool to resolve name via mapOptions before calling
injectable so the token uses the real tool name.
- Around line 80-98: In the handler function inside defineTool.ts (the async
handler(args, extra) wrapper), update the catch branch so the returned
CallToolResult sets isError: true and populates content with at least one
ContentBlock containing the human/LLM-readable error text (e.g., error message
and brief context), rather than an empty array; keep the structuredContent
object (code "E_MCP_TOOL_ERROR" and message) and include the existing logger
call, but ensure the returned object conforms to CallToolResult by including
content: [{ type: "text", text: er?.message ?? "Unknown tool error" }] (or the
project’s ContentBlock shape) and isError: true so MCP consumers can detect
errors correctly.
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/index.ts`:
- Around line 1-23: The barrel currently re-exports many internal helpers and
individual parser modules (e.g., ./parsers/*, ./utils/half.js, ./utils/omit.js,
./utils/withMessage.js) which should be internal; update the index.ts to only
export the intended public API (export jsonSchemaToZod from
"./jsonSchemaToZod.js", export parseSchema from "./parsers/parseSchema.js", and
export Types from "./Types.js") and remove exports for parser internals and
utils so consumers only see jsonSchemaToZod, parseSchema, and Types.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseAllOf.ts`:
- Around line 7-22: The function ensureOriginalIndex currently returns the
original arr as soon as it finds any item containing the symbol originalIndex,
which can look like a bug; add a concise comment above the early-return in
ensureOriginalIndex explaining the invariant that callers (the multi-element
branch) guarantee either all items are pre-annotated or none are, so an early
return is safe — mention the multi-element processing behavior that ensures
full-array annotation before split to make future maintainers aware.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseNot.ts`:
- Around line 4-9: The generated refine callback in parseNot currently inlines
parseSchema(schema.not, {...refs, path: [...refs.path, "not"]}) causing a new
Zod schema to be created on every validation; modify parseNot so it hoists the
parsed inner schema into a const (e.g., const inner = parseSchema(...)) outside
the refine callback and then use inner.safeParse(value).success inside the
refine; keep the refs/path usage exactly as in parseNot and ensure the exported
parseNot still returns the same refine expression but referencing the hoisted
const to avoid repeated instantiation.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseObject.ts`:
- Around line 177-211: The code is needlessly spreading the whole objectSchema
into parseAnyOf/parseOneOf/parseAllOf; change each call to pass only the
specific array property (e.g., call parseAnyOf({ anyOf:
objectSchema.anyOf.map(...) }, refs) instead of parseAnyOf({ ...objectSchema,
anyOf: ... }, refs)), and do the same for parseOneOf and parseAllOf while
preserving the existing mapping that injects type: "object" for anonymous object
subschemas and keeping refs passed through.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseSchema.ts`:
- Around line 125-175: The its.an.enum type guard currently narrows to {enum:
Serializable | Serializable[]} but JsonSchemaObject defines enum as
Serializable[]; update the predicate return type in the its.an.enum entry to
{enum: Serializable[]} so the guard matches the schema type exactly (locate the
its object and specifically the an.enum function to change its return type
annotation).
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseString.ts`:
- Around line 47-55: The contentMediaType handler in parseString.ts should not
use withMessage to append a second argument to .transform; instead emit only the
.transform(callback) form and ensure the transform's catch branch calls
ctx.addIssue(...) and then returns z.NEVER (referencing z.NEVER) so invalid JSON
produces the proper Zod failure; remove any use of withMessage for
contentMediaType (and avoid passing schema.errorMessage.contentMediaType as a
second param), keep the transform callback signature (str, ctx) => { try {
return JSON.parse(str); } catch (err) { ctx.addIssue({ code: "custom", message:
"Invalid JSON" }); return z.NEVER; } } referenced from the contentMediaType
variable and withMessage helper.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/cliTools.ts`:
- Around line 3-7: The Param type's union branch using {value: {[key: number]:
string}} is inconsistent with the runtime check (Array.isArray and .includes)
and should be changed to {value: string[]} to reflect enum-like arrays; update
the type declaration for Param (the union alternatives) to use string[] instead
of {[key: number]: string} and ensure any call sites that pass enum values
supply an actual array so the runtime validation in cliTools (the
Array.isArray(...)/includes(...) check) works as intended.
- Around line 98-106: The parseOrReadJSON function currently calls
statSync/readFileSync heuristically then JSON.parse, which yields an unhelpful
SyntaxError when a short non-JSON string (e.g. a mistyped path) is passed;
update parseOrReadJSON to catch JSON.parse errors and rethrow a descriptive
error that states whether the function attempted to read a file (use the
statSync(...)?.isFile() check and readFileSync result) or to parse the input
literal, include the original jsonOrPath value and the underlying parse error
message for debugging, and ensure any file I/O errors from statSync/readFileSync
are surfaced or wrapped similarly so the thrown error clearly identifies the
attempted operation and cause.
Comment on lines
+69
to
+103
| export function defineTool<Input, Output = undefined>(options: ToolProps<Input, Output>) { | ||
| const provider = injectable(Symbol.for(`MCP:TOOL:${options.name}`)) | ||
| .type(MCP_PROVIDER_TYPES.TOOL) | ||
| .factory(() => { | ||
| let {handler, ...opts} = mapOptions(options); | ||
|
|
||
| return { | ||
| ...opts, | ||
| name: opts.name, | ||
| inputSchema: toZod(isArrowFn(opts.inputSchema) ? opts.inputSchema() : opts.inputSchema), | ||
| outputSchema: toZod(opts.outputSchema), | ||
| async handler(args: Input, extra: RequestHandlerExtra<ServerRequest, ServerNotification>) { | ||
| try { | ||
| return await handler(args as Input, extra); | ||
| } catch (er: any) { | ||
| logger().error({ | ||
| event: "MCP_TOOL_ERROR", | ||
| tool: opts.name, | ||
| error_message: er?.message, | ||
| stack: er?.stack | ||
| }); | ||
|
|
||
| return { | ||
| content: [], | ||
| structuredContent: { | ||
| code: "E_MCP_TOOL_ERROR", | ||
| message: er?.message | ||
| } | ||
| } satisfies CallToolResult; | ||
| } | ||
| } | ||
| }; | ||
| }); | ||
|
|
||
| return provider.token(); |
Contributor
There was a problem hiding this comment.
Tool name can be undefined for class-based tools before mapOptions resolves it.
In ClassToolProps, name is optional (name?: string). The Symbol.for(\MCP:TOOL:${options.name}`)at Line 70 will produceSymbol.for("MCP:TOOL:undefined")ifnameis not provided. The name is only resolved frompropertyKey inside the decorator (tool.ts), but if defineToolis called directly with aClassToolPropswithoutname`, the DI token will be wrong.
Consider either making name required in the public API or resolving the name before creating the symbol:
export function defineTool<Input, Output = undefined>(options: ToolProps<Input, Output>) {
+ if (!options.name && "propertyKey" in options) {
+ options.name = String(options.propertyKey);
+ }
const provider = injectable(Symbol.for(`MCP:TOOL:${options.name}`))🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/fn/defineTool.ts` around lines 69 - 103,
The DI token is created with Symbol.for(`MCP:TOOL:${options.name}`) before the
tool name is resolved, so ClassToolProps without a name yields
"MCP:TOOL:undefined"; call mapOptions(options) first (or otherwise resolve the
final name as mapOptions does) and use the resolved opts.name when building the
injectable token (i.e., use the resolved name for Symbol.for), or alternatively
enforce name required in the public API; update defineTool to resolve name via
mapOptions before calling injectable so the token uses the real tool name.
Comment on lines
+80
to
+98
| async handler(args: Input, extra: RequestHandlerExtra<ServerRequest, ServerNotification>) { | ||
| try { | ||
| return await handler(args as Input, extra); | ||
| } catch (er: any) { | ||
| logger().error({ | ||
| event: "MCP_TOOL_ERROR", | ||
| tool: opts.name, | ||
| error_message: er?.message, | ||
| stack: er?.stack | ||
| }); | ||
|
|
||
| return { | ||
| content: [], | ||
| structuredContent: { | ||
| code: "E_MCP_TOOL_ERROR", | ||
| message: er?.message | ||
| } | ||
| } satisfies CallToolResult; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
MCP CallToolResult specification isError field
💡 Result:
In MCP, CallToolResult.isError is an optional boolean field on the result object returned from tools/call. The CallToolResult shape is:
content: ContentBlock[](required)structuredContent?: {…}(optional)isError?: boolean(optional)_meta?: {…}(optional) (modelcontextprotocol.io)
Meaning / when to use it
- Use
isError: trueto report tool execution errors (e.g., input validation errors, downstream/API failures, business-logic failures) inside the tool result, with human/LLM-readable details incontent. (modelcontextprotocol.io) - Use JSON-RPC protocol errors (the top-level
errorresponse) for protocol-level problems such as unknown tool name, malformed request that fails theCallToolRequestschema, or other MCP/server exceptional conditions. (modelcontextprotocol.io)
Example (tool execution error)
A server can still return a normal JSON-RPC result, but set isError: true:
{
"jsonrpc": "2.0",
"id": 4,
"result": {
"content": [{ "type": "text", "text": "Invalid departure date: must be in the future." }],
"isError": true
}
}
``` ([modelcontextprotocol.io](https://modelcontextprotocol.io/specification/draft/server/tools?utm_source=openai))
Citations:
- 1: https://modelcontextprotocol.io/specification/2025-06-18/schema?utm_source=openai
- 2: https://modelcontextprotocol.io/specification/draft/server/tools?utm_source=openai
- 3: https://modelcontextprotocol.io/specification/draft/server/tools?utm_source=openai
- 4: https://modelcontextprotocol.io/specification/draft/server/tools?utm_source=openai
---
</details>
**Error response violates MCP spec — both `content` array and `isError` field need fixes.**
The MCP specification requires `content: ContentBlock[]` in `CallToolResult`, but the current code returns an empty array. Additionally, the `isError: true` field should be included to properly signal tool execution errors. Per the MCP spec, error details must be in the `content` array as human/LLM-readable text, not only in `structuredContent`.
<details>
<summary>Proposed fix</summary>
```diff
return {
+ isError: true,
- content: [],
+ content: [{ type: "text", text: er?.message || "Tool execution failed" }],
structuredContent: {
code: "E_MCP_TOOL_ERROR",
message: er?.message
}
} satisfies CallToolResult;🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/fn/defineTool.ts` around lines 80 - 98, In
the handler function inside defineTool.ts (the async handler(args, extra)
wrapper), update the catch branch so the returned CallToolResult sets isError:
true and populates content with at least one ContentBlock containing the
human/LLM-readable error text (e.g., error message and brief context), rather
than an empty array; keep the structuredContent object (code "E_MCP_TOOL_ERROR"
and message) and include the existing logger call, but ensure the returned
object conforms to CallToolResult by including content: [{ type: "text", text:
er?.message ?? "Unknown tool error" }] (or the project’s ContentBlock shape) and
isError: true so MCP consumers can detect errors correctly.
Comment on lines
+1
to
+23
| export * from "./jsonSchemaToZod.js"; | ||
| export * from "./parsers/parseAllOf.js"; | ||
| export * from "./parsers/parseAnyOf.js"; | ||
| export * from "./parsers/parseArray.js"; | ||
| export * from "./parsers/parseBoolean.js"; | ||
| export * from "./parsers/parseConst.js"; | ||
| export * from "./parsers/parseDefault.js"; | ||
| export * from "./parsers/parseEnum.js"; | ||
| export * from "./parsers/parseIfThenElse.js"; | ||
| export * from "./parsers/parseMultipleType.js"; | ||
| export * from "./parsers/parseNot.js"; | ||
| export * from "./parsers/parseNull.js"; | ||
| export * from "./parsers/parseNullable.js"; | ||
| export * from "./parsers/parseNumber.js"; | ||
| export * from "./parsers/parseObject.js"; | ||
| export * from "./parsers/parseOneOf.js"; | ||
| export * from "./parsers/parseSchema.js"; | ||
| export * from "./parsers/parseString.js"; | ||
| export * from "./Types.js"; | ||
| export * from "./utils/half.js"; | ||
| export * from "./utils/jsdocs.js"; | ||
| export * from "./utils/omit.js"; | ||
| export * from "./utils/withMessage.js"; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider whether all internal utilities should be part of the public API.
The barrel re-exports internal helpers like half, omit, withMessage, and all individual parsers. If these are implementation details, narrowing the public surface to just jsonSchemaToZod, parseSchema, and the Types would reduce the maintenance burden and avoid accidental coupling by consumers.
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/index.ts` around
lines 1 - 23, The barrel currently re-exports many internal helpers and
individual parser modules (e.g., ./parsers/*, ./utils/half.js, ./utils/omit.js,
./utils/withMessage.js) which should be internal; update the index.ts to only
export the intended public API (export jsonSchemaToZod from
"./jsonSchemaToZod.js", export parseSchema from "./parsers/parseSchema.js", and
export Types from "./Types.js") and remove exports for parser internals and
utils so consumers only see jsonSchemaToZod, parseSchema, and Types.
Comment on lines
+7
to
+22
| const ensureOriginalIndex = (arr: JsonSchema[]) => { | ||
| let newArr = []; | ||
|
|
||
| for (let i = 0; i < arr.length; i++) { | ||
| const item = arr[i]; | ||
| if (typeof item === "boolean") { | ||
| newArr.push(item ? {[originalIndex]: i} : {[originalIndex]: i, not: {}}); | ||
| } else if (originalIndex in item) { | ||
| return arr; | ||
| } else { | ||
| newArr.push({...item, [originalIndex]: i}); | ||
| } | ||
| } | ||
|
|
||
| return newArr; | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
ensureOriginalIndex silently returns the original array if any item already carries the symbol, even if other items don't.
On Line 14-15, if the first item already has originalIndex set, the function returns the original arr immediately—but items later in the array may lack the annotation (e.g., if a partially-annotated array is passed). This is safe only because the multi-element branch always processes the full array in one call before splitting, so either all or none will be annotated. Worth a brief comment clarifying this invariant for future maintainers.
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseAllOf.ts`
around lines 7 - 22, The function ensureOriginalIndex currently returns the
original arr as soon as it finds any item containing the symbol originalIndex,
which can look like a bug; add a concise comment above the early-return in
ensureOriginalIndex explaining the invariant that callers (the multi-element
branch) guarantee either all items are pre-annotated or none are, so an early
return is safe — mention the multi-element processing behavior that ensures
full-array annotation before split to make future maintainers aware.
Comment on lines
+4
to
+9
| export const parseNot = (schema: JsonSchemaObject & {not: JsonSchema}, refs: Refs) => { | ||
| return `z.any().refine((value) => !${parseSchema(schema.not, { | ||
| ...refs, | ||
| path: [...refs.path, "not"] | ||
| })}.safeParse(value).success, "Invalid input: Should NOT be valid against schema")`; | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Generated not refinement re-instantiates the inner schema on every validation call.
The template inlines the full schema expression inside the refine callback, so each validation creates a new Zod schema instance. For schemas used in hot paths, this is wasteful. Hoisting the parsed schema into a const would avoid repeated instantiation:
Proposed fix
export const parseNot = (schema: JsonSchemaObject & {not: JsonSchema}, refs: Refs) => {
- return `z.any().refine((value) => !${parseSchema(schema.not, {
+ const notSchema = parseSchema(schema.not, {
...refs,
path: [...refs.path, "not"]
- })}.safeParse(value).success, "Invalid input: Should NOT be valid against schema")`;
+ });
+ return `(() => { const s = ${notSchema}; return z.any().refine((value) => !s.safeParse(value).success, "Invalid input: Should NOT be valid against schema"); })()`;
};📝 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
| export const parseNot = (schema: JsonSchemaObject & {not: JsonSchema}, refs: Refs) => { | |
| return `z.any().refine((value) => !${parseSchema(schema.not, { | |
| ...refs, | |
| path: [...refs.path, "not"] | |
| })}.safeParse(value).success, "Invalid input: Should NOT be valid against schema")`; | |
| }; | |
| export const parseNot = (schema: JsonSchemaObject & {not: JsonSchema}, refs: Refs) => { | |
| const notSchema = parseSchema(schema.not, { | |
| ...refs, | |
| path: [...refs.path, "not"] | |
| }); | |
| return `(() => { const s = ${notSchema}; return z.any().refine((value) => !s.safeParse(value).success, "Invalid input: Should NOT be valid against schema"); })()`; | |
| }; |
🧰 Tools
🪛 GitHub Check: CodeQL
[warning] 5-8: Improper code sanitization
Code construction depends on an improperly sanitized value.
Code construction depends on an improperly sanitized value.
Code construction depends on an improperly sanitized value.
Code construction depends on an improperly sanitized value.
Code construction depends on an improperly sanitized value.
Code construction depends on an improperly sanitized value.
Code construction depends on an improperly sanitized value.
Code construction depends on an improperly sanitized value.
Code construction depends on an improperly sanitized value.
Code construction depends on an improperly sanitized value.
Code construction depends on an improperly sanitized value.
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseNot.ts`
around lines 4 - 9, The generated refine callback in parseNot currently inlines
parseSchema(schema.not, {...refs, path: [...refs.path, "not"]}) causing a new
Zod schema to be created on every validation; modify parseNot so it hoists the
parsed inner schema into a const (e.g., const inner = parseSchema(...)) outside
the refine callback and then use inner.safeParse(value).success inside the
refine; keep the refs/path usage exactly as in parseNot and ensure the exported
parseNot still returns the same refine expression but referencing the hoisted
const to avoid repeated instantiation.
packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseObject.ts
Show resolved
Hide resolved
Comment on lines
+125
to
+175
| export const its = { | ||
| an: { | ||
| object: (x: JsonSchemaObject): x is JsonSchemaObject & {type: "object"} => x.type === "object", | ||
| array: (x: JsonSchemaObject): x is JsonSchemaObject & {type: "array"} => x.type === "array", | ||
| anyOf: ( | ||
| x: JsonSchemaObject | ||
| ): x is JsonSchemaObject & { | ||
| anyOf: JsonSchema[]; | ||
| } => x.anyOf !== undefined, | ||
| allOf: ( | ||
| x: JsonSchemaObject | ||
| ): x is JsonSchemaObject & { | ||
| allOf: JsonSchema[]; | ||
| } => x.allOf !== undefined, | ||
| enum: ( | ||
| x: JsonSchemaObject | ||
| ): x is JsonSchemaObject & { | ||
| enum: Serializable | Serializable[]; | ||
| } => x.enum !== undefined | ||
| }, | ||
| a: { | ||
| nullable: (x: JsonSchemaObject): x is JsonSchemaObject & {nullable: true} => (x as any).nullable === true, | ||
| multipleType: (x: JsonSchemaObject): x is JsonSchemaObject & {type: string[]} => Array.isArray(x.type), | ||
| not: ( | ||
| x: JsonSchemaObject | ||
| ): x is JsonSchemaObject & { | ||
| not: JsonSchema; | ||
| } => x.not !== undefined, | ||
| const: ( | ||
| x: JsonSchemaObject | ||
| ): x is JsonSchemaObject & { | ||
| const: Serializable; | ||
| } => x.const !== undefined, | ||
| primitive: <T extends "string" | "number" | "integer" | "boolean" | "null">( | ||
| x: JsonSchemaObject, | ||
| p: T | ||
| ): x is JsonSchemaObject & {type: T} => x.type === p, | ||
| conditional: ( | ||
| x: JsonSchemaObject | ||
| ): x is JsonSchemaObject & { | ||
| if: JsonSchema; | ||
| then: JsonSchema; | ||
| else: JsonSchema; | ||
| } => Boolean("if" in x && x.if && "then" in x && "else" in x && x.then && x.else), | ||
| oneOf: ( | ||
| x: JsonSchemaObject | ||
| ): x is JsonSchemaObject & { | ||
| oneOf: JsonSchema[]; | ||
| } => x.oneOf !== undefined | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Type guard object its is well-organized.
Clear predicate structure for routing. Minor note: its.an.enum types the predicate return as {enum: Serializable | Serializable[]} but the JsonSchemaObject type defines enum as Serializable[]. The wider type in the guard is harmless but slightly inconsistent.
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseSchema.ts`
around lines 125 - 175, The its.an.enum type guard currently narrows to {enum:
Serializable | Serializable[]} but JsonSchemaObject defines enum as
Serializable[]; update the predicate return type in the its.an.enum entry to
{enum: Serializable[]} so the guard matches the schema type exactly (locate the
its object and specifically the an.enum function to change its return type
annotation).
Comment on lines
+47
to
+55
| const contentMediaType = withMessage(schema, "contentMediaType", ({value}) => { | ||
| if (value === "application/json") { | ||
| return [ | ||
| '.transform((str, ctx) => { try { return JSON.parse(str); } catch (err) { ctx.addIssue({ code: "custom", message: "Invalid JSON" }); }}', | ||
| ", ", | ||
| ")" | ||
| ]; | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
.transform() does not accept a second message argument — withMessage pattern is incorrect here too.
Same issue as previously flagged for .pipe(): if schema.errorMessage.contentMediaType is defined, withMessage will produce .transform((str, ctx) => { ... }, "some message"). Zod's .transform() only accepts the callback — no message parameter. This will silently pass the extra argument (ignored at runtime) but is semantically wrong and misleading.
Additionally, the missing return z.NEVER after ctx.addIssue in the catch block (previously flagged) remains unaddressed.
Proposed fix: avoid withMessage for contentMediaType
- const contentMediaType = withMessage(schema, "contentMediaType", ({value}) => {
- if (value === "application/json") {
- return [
- '.transform((str, ctx) => { try { return JSON.parse(str); } catch (err) { ctx.addIssue({ code: "custom", message: "Invalid JSON" }); }}',
- ", ",
- ")"
- ];
- }
- });
+ const contentMediaType = schema.contentMediaType === "application/json"
+ ? '.transform((str, ctx) => { try { return JSON.parse(str); } catch (err) { ctx.addIssue({ code: "custom", message: "Invalid JSON" }); return z.NEVER; }})'
+ : "";🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseString.ts`
around lines 47 - 55, The contentMediaType handler in parseString.ts should not
use withMessage to append a second argument to .transform; instead emit only the
.transform(callback) form and ensure the transform's catch branch calls
ctx.addIssue(...) and then returns z.NEVER (referencing z.NEVER) so invalid JSON
produces the proper Zod failure; remove any use of withMessage for
contentMediaType (and avoid passing schema.errorMessage.contentMediaType as a
second param), keep the transform callback signature (str, ctx) => { try {
return JSON.parse(str); } catch (err) { ctx.addIssue({ code: "custom", message:
"Invalid JSON" }); return z.NEVER; } } referenced from the contentMediaType
variable and withMessage helper.
Comment on lines
+3
to
+7
| export type Param = { | ||
| shorthand?: string; | ||
| description?: string; | ||
| required?: boolean | string | undefined; | ||
| } & ({value?: "boolean"} | {value: "number"} | {value: "string"} | {value: {[key: number]: string}}); |
Contributor
There was a problem hiding this comment.
Enum-like param type {[key: number]: string} is fragile — use string[] instead.
The type {value: {[key: number]: string}} accepts plain objects like {0: "esm", 1: "cjs"}, but the runtime check at Line 85 uses Array.isArray(value) and .includes(), which only work on actual arrays. If a caller passes a plain object matching the type signature, enum validation silently skips. Use string[] to make the type and runtime behavior consistent.
Proposed fix
export type Param = {
shorthand?: string;
description?: string;
required?: boolean | string | undefined;
-} & ({value?: "boolean"} | {value: "number"} | {value: "string"} | {value: {[key: number]: string}});
+} & ({value?: "boolean"} | {value: "number"} | {value: "string"} | {value: string[]});Also applies to: 85-87
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/cliTools.ts`
around lines 3 - 7, The Param type's union branch using {value: {[key: number]:
string}} is inconsistent with the runtime check (Array.isArray and .includes)
and should be changed to {value: string[]} to reflect enum-like arrays; update
the type declaration for Param (the union alternatives) to use string[] instead
of {[key: number]: string} and ensure any call sites that pass enum values
supply an actual array so the runtime validation in cliTools (the
Array.isArray(...)/includes(...) check) works as intended.
Comment on lines
+98
to
+106
| export function parseOrReadJSON(jsonOrPath: string): unknown { | ||
| jsonOrPath = jsonOrPath.trim(); | ||
|
|
||
| if (jsonOrPath.length < 255 && statSync(jsonOrPath, {throwIfNoEntry: false})?.isFile()) { | ||
| jsonOrPath = readFileSync(jsonOrPath, "utf-8"); | ||
| } | ||
|
|
||
| return JSON.parse(jsonOrPath); | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
parseOrReadJSON throws an unhelpful error on invalid JSON after a failed file heuristic.
If the input is a short non-JSON string that doesn't correspond to a file (e.g., a typo), statSync returns undefined, then JSON.parse throws a generic syntax error with no indication that a file path was attempted. Consider wrapping with a more descriptive error.
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/cliTools.ts`
around lines 98 - 106, The parseOrReadJSON function currently calls
statSync/readFileSync heuristically then JSON.parse, which yields an unhelpful
SyntaxError when a short non-JSON string (e.g. a mistyped path) is passed;
update parseOrReadJSON to catch JSON.parse errors and rethrow a descriptive
error that states whether the function attempted to read a file (use the
statSync(...)?.isFile() check and readFileSync result) or to parse the input
literal, include the original jsonOrPath value and the underlying parse error
message for debugging, and ensure any file I/O errors from statSync/readFileSync
are surfaced or wrapped similarly so the thrown error clearly identifies the
attempted operation and cause.
37984a3 to
e240d57
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@packages/platform/platform-mcp/src/services/PlatformMcpModule.ts`:
- Around line 46-53: The response "close" listener and the finally block can
both call transport.close(), causing a double-close; remove the redundant
response.raw?.on("close", ...) listener (or replace it with a guard) so
transport.close() is only invoked from the finally block that follows
this.server.connect(...) and transport.handleRequest(...). Locate the
response.raw?.on("close", ...) registration and either delete it or change the
transport.close() call there to a no-op when already closed (e.g., check a
closed flag on the transport), ensuring this.server.connect and
transport.handleRequest remain unchanged and cleanup is performed exclusively in
the finally block.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseIfThenElse.ts`:
- Around line 21-28: The current generator emits z.union([${$then},
${$else}]).superRefine(...) which causes the value to be parsed multiple times;
change the base schema to z.any() so the only validations are the conditional
branch checks inside superRefine. Concretely, replace the prefix
z.union([${$then}, ${$else}]) with z.any(), and inside the superRefine use a
single conditional parse flow: call ${$if}.safeParse(value) to decide branch,
then call either ${$then}.safeParse(value) or ${$else}.safeParse(value) once and
add their errors to ctx via ctx.addIssue; avoid calling the branch parser twice
or relying on the union to validate first. Ensure you update the returned
template string that builds the schema (the code that references ${$if},
${$then}, ${$else}, and superRefine).
In `@packages/platform/platform-mcp/test/app/resources/TestResource.ts`:
- Around line 3-7: Add the dependency-injection decorator to the TestResource
class so the DI container can discover and instantiate it: annotate the class
TestResource with `@Injectable`() (above the class declaration and alongside the
existing `@Resource`("/test") usage) so the DI registry will register the resource
and the async test() handler will be available at runtime.
In `@packages/platform/platform-mcp/test/mcp.integration.spec.ts`:
- Around line 1-6: The test file is missing explicit Vitest imports; add a
top-level import for the Vitest globals used in this spec (e.g. import {
describe, it, beforeAll, afterAll, beforeEach, afterEach, expect, vi } from
"vitest") so linting rules for **/*.spec.ts are satisfied; place this import
above the existing imports (near the top of mcp.integration.spec.ts) and ensure
you include only the globals actually used by the test suite.
- Around line 20-29: There are duplicate beforeEach hooks; merge them into a
single async beforeEach that awaits the bootstrap function returned by
utils.bootstrap({ mcp: { path: "/mcp" } }) (which is PlatformTest.bootstrap())
and then initializes the SuperTest agent by assigning request =
SuperTest(PlatformTest.callback()); place this combined logic in one beforeEach
to remove the duplicate hooks flagged by lint/suspicious/noDuplicateTestHooks.
Comment on lines
+46
to
+53
| response.raw?.on("close", () => transport.close()); | ||
|
|
||
| try { | ||
| await this.server.connect(transport as any); | ||
| await transport.handleRequest(request.getReq(), response.getRes(), request.body); | ||
| } finally { | ||
| await transport.close(); | ||
| } |
Contributor
There was a problem hiding this comment.
transport.close() may be called twice — once from the response.raw "close" listener and once from the finally block.
If the response closes normally, both paths fire. Depending on the transport implementation, a double-close could throw or log warnings. Consider guarding against it or removing the redundant response.raw?.on("close", ...) listener since the finally block already ensures cleanup.
Proposed fix — remove the redundant listener
const {request, response} = $ctx;
- response.raw?.on("close", () => transport.close());
-
try {
await this.server.connect(transport as any);
await transport.handleRequest(request.getReq(), response.getRes(), request.body);
} finally {
await transport.close();
}📝 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
| response.raw?.on("close", () => transport.close()); | |
| try { | |
| await this.server.connect(transport as any); | |
| await transport.handleRequest(request.getReq(), response.getRes(), request.body); | |
| } finally { | |
| await transport.close(); | |
| } | |
| try { | |
| await this.server.connect(transport as any); | |
| await transport.handleRequest(request.getReq(), response.getRes(), request.body); | |
| } finally { | |
| await transport.close(); | |
| } |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/services/PlatformMcpModule.ts` around
lines 46 - 53, The response "close" listener and the finally block can both call
transport.close(), causing a double-close; remove the redundant
response.raw?.on("close", ...) listener (or replace it with a guard) so
transport.close() is only invoked from the finally block that follows
this.server.connect(...) and transport.handleRequest(...). Locate the
response.raw?.on("close", ...) registration and either delete it or change the
transport.close() call there to a no-op when already closed (e.g., check a
closed flag on the transport), ensuring this.server.connect and
transport.handleRequest remain unchanged and cleanup is performed exclusively in
the finally block.
Comment on lines
+21
to
+28
| return `z.union([${$then}, ${$else}]).superRefine((value,ctx) => { | ||
| const result = ${$if}.safeParse(value).success | ||
| ? ${$then}.safeParse(value) | ||
| : ${$else}.safeParse(value); | ||
| if (!result.success) { | ||
| result.error.errors.forEach((error) => ctx.addIssue(error)) | ||
| } | ||
| })`; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Generated code triple-parses the value at runtime.
The z.union([then, else]) validates the value once, then superRefine calls .safeParse(value) again on the chosen branch. Every successful validation runs the branch parser twice. This is functionally correct but wasteful. Consider using z.any() as the base instead of z.union(...) if the superRefine already gates correctness, or cache the union result.
♻️ Suggested: use z.any() as base to avoid redundant parsing
- return `z.union([${$then}, ${$else}]).superRefine((value,ctx) => {
+ return `z.any().superRefine((value,ctx) => {
const result = ${$if}.safeParse(value).success
? ${$then}.safeParse(value)
: ${$else}.safeParse(value);
if (!result.success) {
result.error.errors.forEach((error) => ctx.addIssue(error))
}
})`;📝 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
| return `z.union([${$then}, ${$else}]).superRefine((value,ctx) => { | |
| const result = ${$if}.safeParse(value).success | |
| ? ${$then}.safeParse(value) | |
| : ${$else}.safeParse(value); | |
| if (!result.success) { | |
| result.error.errors.forEach((error) => ctx.addIssue(error)) | |
| } | |
| })`; | |
| return `z.any().superRefine((value,ctx) => { | |
| const result = ${$if}.safeParse(value).success | |
| ? ${$then}.safeParse(value) | |
| : ${$else}.safeParse(value); | |
| if (!result.success) { | |
| result.error.errors.forEach((error) => ctx.addIssue(error)) | |
| } | |
| })`; |
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseIfThenElse.ts`
around lines 21 - 28, The current generator emits z.union([${$then},
${$else}]).superRefine(...) which causes the value to be parsed multiple times;
change the base schema to z.any() so the only validations are the conditional
branch checks inside superRefine. Concretely, replace the prefix
z.union([${$then}, ${$else}]) with z.any(), and inside the superRefine use a
single conditional parse flow: call ${$if}.safeParse(value) to decide branch,
then call either ${$then}.safeParse(value) or ${$else}.safeParse(value) once and
add their errors to ctx via ctx.addIssue; avoid calling the branch parser twice
or relying on the union to validate first. Ensure you update the returned
template string that builds the schema (the code that references ${$if},
${$then}, ${$else}, and superRefine).
Comment on lines
3
to
20
| export class TestResource { | ||
| @Resource("/test") | ||
| async test() { | ||
| return "test"; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether `@Injectable` is used on other test app fixtures
rg -n '@Injectable' packages/platform/platform-mcp/test/app/ --type ts -B 2 -A 2Repository: tsedio/tsed
Length of output: 830
🏁 Script executed:
#!/bin/bash
# Check TestResource.ts imports and structure
cat -n packages/platform/platform-mcp/test/app/resources/TestResource.ts
# Also check if there are other test fixtures in test/app
echo "=== Other test fixtures ==="
fd -t f '\.ts$' packages/platform/platform-mcp/test/app --exec basename {} \;Repository: tsedio/tsed
Length of output: 317
Add @Injectable() decorator to enable DI container registration.
TestPrompt and TestTool are both decorated with @Injectable(), but TestResource is not. Without this decorator, the DI container won't discover and instantiate the class, so the resource handler won't be available.
Proposed fix
+import {Injectable} from "@tsed/di";
import {Resource} from "../../..";
+@Injectable()
export class TestResource {
`@Resource`("/test")
async test() {
return "test";
}
}📝 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
| export class TestResource { | |
| @Resource("/test") | |
| async test() { | |
| return "test"; | |
| } | |
| import {Injectable} from "@tsed/di"; | |
| import {Resource} from "../../.."; | |
| `@Injectable`() | |
| export class TestResource { | |
| `@Resource`("/test") | |
| async test() { | |
| return "test"; | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/test/app/resources/TestResource.ts` around
lines 3 - 7, Add the dependency-injection decorator to the TestResource class so
the DI container can discover and instantiate it: annotate the class
TestResource with `@Injectable`() (above the class declaration and alongside the
existing `@Resource`("/test") usage) so the DI registry will register the resource
and the async test() handler will be available at runtime.
Comment on lines
+1
to
+6
| import {PlatformExpress} from "@tsed/platform-express"; | ||
| import {PlatformTest} from "@tsed/platform-http/testing"; | ||
| import {PlatformTestSdk} from "@tsed/platform-test-sdk"; | ||
| import SuperTest from "supertest"; | ||
|
|
||
| import {rootDir, Server} from "./app/Server.js"; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit vitest imports per coding guidelines.
import {PlatformExpress} from "@tsed/platform-express";
import {PlatformTest} from "@tsed/platform-http/testing";
import {PlatformTestSdk} from "@tsed/platform-test-sdk";
import SuperTest from "supertest";
+import {afterEach, beforeEach, describe, expect, it} from "vitest";As per coding guidelines, **/*.spec.ts: All TypeScript files in test suites must follow Vitest ESLint plugin rules.
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/test/mcp.integration.spec.ts` around lines 1 -
6, The test file is missing explicit Vitest imports; add a top-level import for
the Vitest globals used in this spec (e.g. import { describe, it, beforeAll,
afterAll, beforeEach, afterEach, expect, vi } from "vitest") so linting rules
for **/*.spec.ts are satisfied; place this import above the existing imports
(near the top of mcp.integration.spec.ts) and ensure you include only the
globals actually used by the test suite.
Comment on lines
+20
to
+37
| beforeEach( | ||
| utils.bootstrap({ | ||
| mcp: { | ||
| path: "/mcp" | ||
| } | ||
| }) | ||
| ); | ||
| beforeEach(() => { | ||
| request = SuperTest(PlatformTest.callback()); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/platform/platform-mcp && find . -type f -name "*.ts" | head -20Repository: tsedio/tsed
Length of output: 710
🏁 Script executed:
# Search for bootstrap function definition across the codebase
rg -t typescript "export.*bootstrap|function bootstrap" --max-count=20Repository: tsedio/tsed
Length of output: 89
🏁 Script executed:
# Look at the test file to understand the context better
cat -n packages/platform/platform-mcp/test/mcp.integration.spec.ts | head -40Repository: tsedio/tsed
Length of output: 1288
🏁 Script executed:
# Find PlatformTestSdk definition
rg -i "platformtestsdk|class.*TestSdk" --max-count=20Repository: tsedio/tsed
Length of output: 7409
🏁 Script executed:
# Look for bootstrap method definition
rg "bootstrap\s*\(" -A 5 --max-count=30Repository: tsedio/tsed
Length of output: 50370
🏁 Script executed:
# Check if there's a platform-test-sdk package in the repo
find . -type d -name "*test-sdk*" 2>/dev/nullRepository: tsedio/tsed
Length of output: 92
🏁 Script executed:
cat packages/platform/platform-test-sdk/src/components/PlatformTestSdk.ts | head -100Repository: tsedio/tsed
Length of output: 765
🏁 Script executed:
# Search for bootstrap method signature in the PlatformTestSdk file
rg "bootstrap" packages/platform/platform-test-sdk/src/components/PlatformTestSdk.ts -A 15 --max-count=5Repository: tsedio/tsed
Length of output: 266
🏁 Script executed:
# Find PlatformTest.bootstrap definition
rg "bootstrap" packages/platform/platform-http/testing -A 10 --max-count=10 | head -50Repository: tsedio/tsed
Length of output: 134
🏁 Script executed:
# Look for PlatformTest class/file
find packages/platform/platform-http -name "*test*" -o -name "*Test*" | grep -E "\.(ts|js)$"Repository: tsedio/tsed
Length of output: 400
🏁 Script executed:
# Check the PlatformTest.bootstrap method
rg "bootstrap" packages/platform/platform-http/src/testing/PlatformTest.ts -A 15 --max-count=5Repository: tsedio/tsed
Length of output: 1688
🏁 Script executed:
# Get the full PlatformTest.ts file to understand the structure
wc -l packages/platform/platform-http/src/testing/PlatformTest.tsRepository: tsedio/tsed
Length of output: 118
🏁 Script executed:
# Check the full test file context around lines 20-29
cat -n packages/platform/platform-mcp/test/mcp.integration.spec.ts | sed -n '17,35p'Repository: tsedio/tsed
Length of output: 570
🏁 Script executed:
# Also check if there are any linting rules or Biome configuration
find packages/platform/platform-mcp -name "biome.json" -o -name ".biomignore" -o -name "eslint.config.*"Repository: tsedio/tsed
Length of output: 37
Consolidate duplicate beforeEach hooks.
The test suite has duplicate beforeEach hooks (lines 20-26 and 27-29), which Biome flags with lint/suspicious/noDuplicateTestHooks. Since utils.bootstrap() returns PlatformTest.bootstrap(), which is a function returning Promise<void>, merge them into a single async hook that awaits the bootstrap function before creating the SuperTest agent:
Proposed fix
- beforeEach(
- utils.bootstrap({
- mcp: {
- path: "/mcp"
- }
- })
- );
- beforeEach(() => {
- request = SuperTest(PlatformTest.callback());
- });
+ beforeEach(async () => {
+ await utils.bootstrap({
+ mcp: {
+ path: "/mcp"
+ }
+ })();
+ request = SuperTest(PlatformTest.callback());
+ });📝 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
| beforeEach( | |
| utils.bootstrap({ | |
| mcp: { | |
| path: "/mcp" | |
| } | |
| }) | |
| ); | |
| beforeEach(() => { | |
| request = SuperTest(PlatformTest.callback()); | |
| }); | |
| beforeEach(async () => { | |
| await utils.bootstrap({ | |
| mcp: { | |
| path: "/mcp" | |
| } | |
| })(); | |
| request = SuperTest(PlatformTest.callback()); | |
| }); |
🧰 Tools
🪛 Biome (2.3.13)
[error] 27-29: Duplicate beforeEach hook found.
Remove this duplicate hook or consolidate the logic into a single hook.
(lint/suspicious/noDuplicateTestHooks)
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/test/mcp.integration.spec.ts` around lines 20
- 29, There are duplicate beforeEach hooks; merge them into a single async
beforeEach that awaits the bootstrap function returned by utils.bootstrap({ mcp:
{ path: "/mcp" } }) (which is PlatformTest.bootstrap()) and then initializes the
SuperTest agent by assigning request = SuperTest(PlatformTest.callback()); place
this combined logic in one beforeEach to remove the duplicate hooks flagged by
lint/suspicious/noDuplicateTestHooks.
e240d57 to
c561f76
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@packages/platform/platform-mcp/src/services/PlatformMcpModule.ts`:
- Around line 26-35: In PlatformMcpModule.$logRoutes, the route inclusion check
uses this.settings?.enabled (which treats undefined as falsy) so the MCP route
can be omitted from logs; change the condition to call this.isEnabled() instead
(i.e., include the extra route only when this.isEnabled() returns truthy) and
keep using this.settings?.path || "/mcp" for the URL so the logged route matches
the registered path.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseDefault.ts`:
- Around line 3-5: The function parseDefault is misleadingly named because it
suggests handling the JSON Schema "default" keyword while it actually returns a
catch-all fallback; rename the function (and its exported identifier) to a
clearer name like parseFallback or parseUnknown (e.g., replace parseDefault with
parseFallback) and update all references/imports/exports across the module so
callers (and any index/export barrel files) use the new name; ensure the
implementation still returns "z.any()" and run tests/TypeScript build to catch
any remaining references.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseEnum.ts`:
- Around line 6-8: In parseEnum, when schema.enum.length === 1, guard against
non-primitive lone values (objects/arrays) before returning z.literal; detect
primitives by checking typeof value === 'string'|'number'|'boolean' or value ===
null (and exclude Array.isArray/objects), and only emit z.literal(...) for those
cases; for non-primitive single values delegate to the same handling used by
parseConst (call the parseConst helper or the existing constant-value
serializer) so the generated Zod for that object/array is valid rather than
producing an invalid z.literal.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseOneOf.ts`:
- Around line 28-34: The superRefine emitted in parseOneOf.ts currently passes
path: ctx.path to ctx.addIssue (via the errors branch in parseOneOf), but Zod v4
removed ctx.path; update parseOneOf to either use the same emitErrorPath(refs)
helper used in parseObject.ts or omit the path property so Zod uses the current
refinement path: thread the existing refs parameter into parseOneOf (if not
already) and replace the literal ctx.path usage with a call to
emitErrorPath(refs) or remove the path field from the ctx.addIssue call in the
errors branch (keep code that constructs unionErrors/errors and message
unchanged).
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/cliTools.ts`:
- Around line 25-46: The parseArgs function currently calls process.exit(0) when
help is requested, which prevents isolated testing; change parseArgs (and its
callers) to stop calling process.exit directly: instead, when help is detected
in parseArgs (the block that calls printParams), either throw a specific
sentinel error (e.g., HelpRequestedError) or return a discriminated result
(e.g., { helpShown: true } union) so the caller can decide to exit; ensure the
printed help still uses printParams and update any call sites of parseArgs to
catch the sentinel error or check the discriminant and call process.exit there
if desired.
- Around line 66-89: The parser in cliTools.ts uses args[index + 1] as the value
and will accept a next flag like "--other" as a value; update the logic that
reads val (args[index + 1]) inside the block handling a present value (symbols:
args, index, name, value, result) to treat any val that starts with "-" (e.g.,
/^-/) as a missing value and throw the same "Expected a value for argument
${name}" error; at minimum enforce this guard for numeric branches (where value
=== "number") and enum branches (Array.isArray(value)), and apply to general
string params if you want to avoid silently consuming flags. Ensure errors are
thrown before numeric parsing or enum membership checks so flags are never
accepted as values.
In `@packages/platform/platform-mcp/test/mcp.integration.spec.ts`:
- Around line 105-121: The test fails because internal fields `propertyKey` (and
`token`) are leaking into the MCP resources response; update the destructuring
in the resource registration path (the code that builds resource metadata passed
to server.registerResource / the definition destructuring in McpServerFactory)
to explicitly remove those internals by including `propertyKey` and `token` in
the left-hand side (e.g., change `{ name, handler, uri, template, ...opts }` to
destructure out `propertyKey` and `token`), so they are not included in `opts`
and therefore do not appear in the response.
Comment on lines
+26
to
+35
| $logRoutes(routes: PlatformRouteDetails[]) { | ||
| return [ | ||
| ...routes, | ||
| this.settings?.enabled && { | ||
| method: "POST", | ||
| name: "PlatformMcpModule.dispatch()", | ||
| url: this.settings?.path || "/mcp" | ||
| } | ||
| ].filter(Boolean); | ||
| } |
Contributor
There was a problem hiding this comment.
$logRoutes still uses this.settings?.enabled instead of this.isEnabled() — route is not logged when enabled is undefined.
$onRoutesInit was fixed to use isEnabled() (which correctly defaults undefined to true), but $logRoutes on line 29 still checks this.settings?.enabled directly. When enabled is not explicitly set, this evaluates to undefined (falsy), so the MCP route won't appear in logs even though it's registered.
Proposed fix
$logRoutes(routes: PlatformRouteDetails[]) {
return [
...routes,
- this.settings?.enabled && {
+ this.isEnabled() && {
method: "POST",
name: "PlatformMcpModule.dispatch()",
url: this.settings?.path || "/mcp"
}
].filter(Boolean);
}📝 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
| $logRoutes(routes: PlatformRouteDetails[]) { | |
| return [ | |
| ...routes, | |
| this.settings?.enabled && { | |
| method: "POST", | |
| name: "PlatformMcpModule.dispatch()", | |
| url: this.settings?.path || "/mcp" | |
| } | |
| ].filter(Boolean); | |
| } | |
| $logRoutes(routes: PlatformRouteDetails[]) { | |
| return [ | |
| ...routes, | |
| this.isEnabled() && { | |
| method: "POST", | |
| name: "PlatformMcpModule.dispatch()", | |
| url: this.settings?.path || "/mcp" | |
| } | |
| ].filter(Boolean); | |
| } |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/services/PlatformMcpModule.ts` around
lines 26 - 35, In PlatformMcpModule.$logRoutes, the route inclusion check uses
this.settings?.enabled (which treats undefined as falsy) so the MCP route can be
omitted from logs; change the condition to call this.isEnabled() instead (i.e.,
include the extra route only when this.isEnabled() returns truthy) and keep
using this.settings?.path || "/mcp" for the URL so the logged route matches the
registered path.
Comment on lines
+3
to
+5
| export const parseDefault = (_schema: JsonSchemaObject) => { | ||
| return "z.any()"; | ||
| }; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider renaming to avoid confusion with JSON Schema's default keyword.
parseDefault reads as if it handles the JSON Schema default field (which sets default values for missing properties). In reality, this is a catch-all fallback for unrecognized schema types. A name like parseFallback or parseUnknown would be clearer and avoid ambiguity with withoutDefaults in Options.
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseDefault.ts`
around lines 3 - 5, The function parseDefault is misleadingly named because it
suggests handling the JSON Schema "default" keyword while it actually returns a
catch-all fallback; rename the function (and its exported identifier) to a
clearer name like parseFallback or parseUnknown (e.g., replace parseDefault with
parseFallback) and update all references/imports/exports across the module so
callers (and any index/export barrel files) use the new name; ensure the
implementation still returns "z.any()" and run tests/TypeScript build to catch
any remaining references.
Comment on lines
+6
to
+8
| } else if (schema.enum.length === 1) { | ||
| // union does not work when there is only one element | ||
| return `z.literal(${JSON.stringify(schema.enum[0])})`; |
Contributor
There was a problem hiding this comment.
z.literal() on single enum value shares the same non-primitive risk as parseConst.
If the lone enum value is an object or array (allowed by the Serializable type), z.literal(...) will produce invalid Zod. Same root cause as parseConst — consider guarding against non-primitive values here too.
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseEnum.ts`
around lines 6 - 8, In parseEnum, when schema.enum.length === 1, guard against
non-primitive lone values (objects/arrays) before returning z.literal; detect
primitives by checking typeof value === 'string'|'number'|'boolean' or value ===
null (and exclude Array.isArray/objects), and only emit z.literal(...) for those
cases; for non-primitive single values delegate to the same handling used by
parseConst (call the parseConst helper or the existing constant-value
serializer) so the generated Zod for that object/array is valid rather than
producing an invalid z.literal.
Comment on lines
+28
to
+34
| if (schemas.length - errors.length !== 1) { | ||
| ctx.addIssue({ | ||
| path: ctx.path, | ||
| code: "invalid_union", | ||
| unionErrors: errors, | ||
| message: "Invalid input: Should pass single schema", | ||
| }); |
Contributor
There was a problem hiding this comment.
ctx.path is unavailable in Zod v4 — generated superRefine code will fail at runtime.
The generated code emits path: ctx.path (line 30), but in Zod v4 ctx.path has been removed. The sibling parseObject.ts already has an emitErrorPath(refs) helper that switches between v3 and v4 syntax, but parseOneOf doesn't use it — and currently doesn't even accept refs in a way that threads through to the generated string.
You should use a similar approach to emitErrorPath or omit the path property entirely (Zod will use the current refinement path by default when path is omitted from addIssue).
Proposed fix
- ctx.addIssue({
- path: ctx.path,
- code: "invalid_union",
- unionErrors: errors,
- message: "Invalid input: Should pass single schema",
- });
+ ctx.addIssue({
+ code: "invalid_union",
+ unionErrors: errors,
+ message: "Invalid input: Should pass single schema",
+ });📝 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
| if (schemas.length - errors.length !== 1) { | |
| ctx.addIssue({ | |
| path: ctx.path, | |
| code: "invalid_union", | |
| unionErrors: errors, | |
| message: "Invalid input: Should pass single schema", | |
| }); | |
| if (schemas.length - errors.length !== 1) { | |
| ctx.addIssue({ | |
| code: "invalid_union", | |
| unionErrors: errors, | |
| message: "Invalid input: Should pass single schema", | |
| }); |
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseOneOf.ts`
around lines 28 - 34, The superRefine emitted in parseOneOf.ts currently passes
path: ctx.path to ctx.addIssue (via the errors branch in parseOneOf), but Zod v4
removed ctx.path; update parseOneOf to either use the same emitErrorPath(refs)
helper used in parseObject.ts or omit the path property so Zod uses the current
refinement path: thread the existing refs parameter into parseOneOf (if not
already) and replace the literal ctx.path usage with a call to
emitErrorPath(refs) or remove the path field from the ctx.addIssue call in the
errors branch (keep code that constructs unionErrors/errors and message
unchanged).
Comment on lines
+25
to
+46
| export function parseArgs<T extends Params>(params: T, args: string[], help?: boolean | string): InferReturnType<T> { | ||
| const result: Record<string, string | number | boolean> = {}; | ||
|
|
||
| if (help) { | ||
| let index = args.indexOf("--help"); | ||
|
|
||
| if (index === -1) { | ||
| index = args.indexOf("-h"); | ||
| } | ||
|
|
||
| if (index !== -1) { | ||
| printParams({ | ||
| ...params, | ||
| help: { | ||
| shorthand: "h", | ||
| description: typeof help === "string" ? help : "Display this message :)" | ||
| } | ||
| }); | ||
|
|
||
| process.exit(0); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
process.exit(0) makes parseArgs untestable in isolation.
Calling process.exit directly in a utility function makes it difficult to unit-test the help path without mocking globals. Consider throwing a sentinel error or returning a discriminated result instead, letting the caller decide whether to exit.
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/cliTools.ts`
around lines 25 - 46, The parseArgs function currently calls process.exit(0)
when help is requested, which prevents isolated testing; change parseArgs (and
its callers) to stop calling process.exit directly: instead, when help is
detected in parseArgs (the block that calls printParams), either throw a
specific sentinel error (e.g., HelpRequestedError) or return a discriminated
result (e.g., { helpShown: true } union) so the caller can decide to exit;
ensure the printed help still uses printParams and update any call sites of
parseArgs to catch the sentinel error or check the discriminant and call
process.exit there if desired.
Comment on lines
+66
to
+89
| if (value) { | ||
| const val = args[index + 1]; | ||
|
|
||
| if (val === undefined) { | ||
| throw new Error(`Expected a value for argument ${name}`); | ||
| } | ||
|
|
||
| if (value === "number") { | ||
| const asNumber = Number(val); | ||
|
|
||
| if (isNaN(asNumber)) { | ||
| throw new Error(`Value of argument ${name} must be a valid number`); | ||
| } | ||
|
|
||
| result[name] = asNumber; | ||
|
|
||
| continue; | ||
| } | ||
|
|
||
| if (Array.isArray(value) && !value.includes(val)) { | ||
| throw new Error(`Value of argument ${name} must be one of ${value}`); | ||
| } | ||
|
|
||
| result[name] = val; |
Contributor
There was a problem hiding this comment.
args[index + 1] can accidentally consume the next flag as a value.
When a user writes --name --other, args[index + 1] yields "--other", which is silently accepted as the value of --name. This is a common pitfall in hand-rolled arg parsers. Consider rejecting values that start with -- (or - followed by a known shorthand) for non-string-typed params, or at minimum for numeric and enum params where such values are clearly invalid.
Proposed guard
if (value) {
const val = args[index + 1];
- if (val === undefined) {
+ if (val === undefined || (val.startsWith("-") && value !== "string")) {
throw new Error(`Expected a value for argument ${name}`);
}🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/cliTools.ts`
around lines 66 - 89, The parser in cliTools.ts uses args[index + 1] as the
value and will accept a next flag like "--other" as a value; update the logic
that reads val (args[index + 1]) inside the block handling a present value
(symbols: args, index, name, value, result) to treat any val that starts with
"-" (e.g., /^-/) as a missing value and throw the same "Expected a value for
argument ${name}" error; at minimum enforce this guard for numeric branches
(where value === "number") and enum branches (Array.isArray(value)), and apply
to general string params if you want to avoid silently consuming flags. Ensure
errors are thrown before numeric parsing or enum membership checks so flags are
never accepted as values.
Comment on lines
+105
to
+121
| expect(response.body).toMatchInlineSnapshot(` | ||
| { | ||
| "id": 1, | ||
| "jsonrpc": "2.0", | ||
| "result": { | ||
| "resources": [ | ||
| { | ||
| "description": "Returns a static payload for integration tests", | ||
| "name": "test", | ||
| "propertyKey": "test", | ||
| "title": "Test resource", | ||
| "uri": "tsed://resources/test", | ||
| }, | ||
| ], | ||
| }, | ||
| } | ||
| `); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how resources are serialized and if propertyKey is intentionally included
rg -n "propertyKey" packages/platform/platform-mcp/src/ --type ts -C 3Repository: tsedio/tsed
Length of output: 13609
🏁 Script executed:
#!/bin/bash
# Also check the test file itself to understand the context
cat -n packages/platform/platform-mcp/test/mcp.integration.spec.ts | sed -n '95,130p'Repository: tsedio/tsed
Length of output: 1188
🏁 Script executed:
#!/bin/bash
# Look for resource definition and serialization logic
rg -n "resources\|serialize" packages/platform/platform-mcp/src/ --type ts -A 2 | head -50Repository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Search for resource response building logic
rg -n "resources.*list\|serialize.*resource" packages/platform/platform-mcp/src/ --type ts -A 5 | head -60Repository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Look for the handler that responds to resources/list MCP calls
rg -n "resources/list" packages/platform/platform-mcp/src/ --type ts -B 3 -A 10Repository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Check what fields are returned in resource responses
rg -n "toJSON\|toResponse\|format" packages/platform/platform-mcp/src/ --type ts | grep -i resourceRepository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Look at more of the integration test to understand setup
cat -n packages/platform/platform-mcp/test/mcp.integration.spec.ts | head -110Repository: tsedio/tsed
Length of output: 3529
🏁 Script executed:
#!/bin/bash
# Find where test resources are defined
fd -t f "mcp.integration.spec.ts" packages/platform/platform-mcp/test/ -exec cat {} \;Repository: tsedio/tsed
Length of output: 201
🏁 Script executed:
#!/bin/bash
# Search for where resources registry is built and returned
rg -n "registry\|getResources" packages/platform/platform-mcp/src/ --type ts -A 3Repository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Find the Server configuration
cat -n packages/platform/platform-mcp/test/app/Server.tsRepository: tsedio/tsed
Length of output: 936
🏁 Script executed:
#!/bin/bash
# Look for resource handler implementations
rg -n "resourcesList\|listResources\|resources.*list" packages/platform/platform-mcp/src/ --type ts -B 2 -A 8Repository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Search for MCP handler/middleware that builds responses
find packages/platform/platform-mcp/src -name "*.ts" -type f | head -20 | xargs -I {} basename {}Repository: tsedio/tsed
Length of output: 340
🏁 Script executed:
#!/bin/bash
# Find TestResource implementation
find packages/platform/platform-mcp/test -name "TestResource.ts" -exec cat -n {} \;Repository: tsedio/tsed
Length of output: 752
🏁 Script executed:
#!/bin/bash
# Compare prompt and resource implementations
echo "=== PROMPT.TS ===" && cat -n packages/platform/platform-mcp/src/decorators/prompt.ts && echo -e "\n=== RESOURCE.TS ===" && cat -n packages/platform/platform-mcp/src/decorators/resource.tsRepository: tsedio/tsed
Length of output: 1689
🏁 Script executed:
#!/bin/bash
# Check for MCP server handler/middleware that builds list responses
rg -n "prompts.*list|resources.*list" packages/platform/platform-mcp/src/ --type ts -B 5 -A 10Repository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Look for McpServerFactory and server handlers
cat -n packages/platform/platform-mcp/src/McpServerFactory.tsRepository: tsedio/tsed
Length of output: 141
🏁 Script executed:
#!/bin/bash
# Search for resource list building
rg -n "resources\s*:" packages/platform/platform-mcp/src/ --type ts -B 3 -A 5 | head -80Repository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Find where metadata is converted to response
rg -n "toJSON\|getMetadata\|getRegistry" packages/platform/platform-mcp/src/ --type ts -A 3Repository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# List all files in the src directory structure
find packages/platform/platform-mcp/src -type f -name "*.ts" | sortRepository: tsedio/tsed
Length of output: 3109
🏁 Script executed:
#!/bin/bash
# Look for MCP server or handler implementations
rg -l "resources\|prompts" packages/platform/platform-mcp/src --type tsRepository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Search in index files to understand exports
cat -n packages/platform/platform-mcp/src/index.ts | head -50Repository: tsedio/tsed
Length of output: 550
🏁 Script executed:
#!/bin/bash
# Look at McpServerFactory.ts to understand response building
cat -n packages/platform/platform-mcp/src/services/McpServerFactory.tsRepository: tsedio/tsed
Length of output: 2710
🏁 Script executed:
#!/bin/bash
# Also check PlatformMcpModule.ts which likely integrates the server
cat -n packages/platform/platform-mcp/src/services/PlatformMcpModule.tsRepository: tsedio/tsed
Length of output: 2346
🏁 Script executed:
#!/bin/bash
# Check what properties are passed in opts for resources vs prompts
# Look at ResourceProps definition
cat -n packages/platform/platform-mcp/src/fn/defineResource.ts | head -50Repository: tsedio/tsed
Length of output: 1958
🏁 Script executed:
#!/bin/bash
# Compare with PromptsSettings definition
cat -n packages/platform/platform-mcp/src/fn/definePrompt.ts | head -50Repository: tsedio/tsed
Length of output: 2244
🏁 Script executed:
#!/bin/bash
# Search for what MCP SDK expects in resource vs prompt list responses
rg -n "registerResource\|registerPrompt" packages/ --type ts -B 2 -A 2Repository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Let's check the full defineResource function to see what gets stored
cat -n packages/platform/platform-mcp/src/fn/defineResource.tsRepository: tsedio/tsed
Length of output: 2220
🏁 Script executed:
#!/bin/bash
# Check what fields ResourceMetadata includes from MCP SDK
rg -n "ResourceMetadata" packages/platform/platform-mcp/src/ --type ts -A 5Repository: tsedio/tsed
Length of output: 1203
🏁 Script executed:
#!/bin/bash
# Look at the actual test data to confirm if propertyKey is in prompts or just resources
grep -n "propertyKey" packages/platform/platform-mcp/test/mcp.integration.spec.tsRepository: tsedio/tsed
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Let's verify what fields appear in prompts vs resources in the test snapshots
cat -n packages/platform/platform-mcp/test/mcp.integration.spec.ts | sed -n '51,68p;102,122p'Repository: tsedio/tsed
Length of output: 1411
🏁 Script executed:
#!/bin/bash
# Check MCP SDK types to understand what ResourceMetadata should contain
# Search for how registerResource and registerPrompt handle metadata
rg -n "MCP_SERVER" packages/platform/platform-mcp/src/services/McpServerFactory.ts -B 5 -A 15Repository: tsedio/tsed
Length of output: 970
🏁 Script executed:
#!/bin/bash
# Verify by checking if there are any filtering operations on resourceMetadata
rg -n "propertyKey" packages/platform/platform-mcp/src/services/ --type tsRepository: tsedio/tsed
Length of output: 37
🏁 Script executed:
#!/bin/bash
# Let's trace exactly what gets passed to registerResource vs registerPrompt
# The key is line 42 in McpServerFactory - what does "...opts" contain?
cat -n packages/platform/platform-mcp/src/services/McpServerFactory.ts | sed -n '39,51p'Repository: tsedio/tsed
Length of output: 834
🏁 Script executed:
#!/bin/bash
# Check if propertyKey should be filtered in the destructuring
# Compare the destructuring patterns for resources and prompts
cat -n packages/platform/platform-mcp/src/services/McpServerFactory.ts | sed -n '32,51p'Repository: tsedio/tsed
Length of output: 1199
🏁 Script executed:
#!/bin/bash
# Search for any documentation or comments about propertyKey visibility
rg -n "propertyKey.*internal\|internal.*propertyKey\|leak" packages/platform/platform-mcp/ --type tsRepository: tsedio/tsed
Length of output: 37
Remove propertyKey from the resources/list response — it's an internal implementation detail.
The propertyKey field appears in the resource metadata passed to server.registerResource() (line 42 of McpServerFactory.ts) because the destructuring {name, handler, uri, template, ...opts} does not explicitly exclude it. This causes the internal property key to leak into the MCP protocol response, inconsistent with how prompts are handled.
The fix is to explicitly exclude propertyKey and token from the opts object:
const {name, handler, uri, template, propertyKey, token, ...opts} = definition;
These are internal implementation details used for method binding and dependency injection, not part of the MCP specification.
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/test/mcp.integration.spec.ts` around lines 105
- 121, The test fails because internal fields `propertyKey` (and `token`) are
leaking into the MCP resources response; update the destructuring in the
resource registration path (the code that builds resource metadata passed to
server.registerResource / the definition destructuring in McpServerFactory) to
explicitly remove those internals by including `propertyKey` and `token` in the
left-hand side (e.g., change `{ name, handler, uri, template, ...opts }` to
destructure out `propertyKey` and `token`), so they are not included in `opts`
and therefore do not appear in the response.
c561f76 to
5fb3166
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@packages/platform/platform-mcp/src/fn/defineTool.ts`:
- Around line 35-38: The function getOutputSchema currently declares return type
JsonSchema<Output> but can return undefined; change its signature to return
JsonSchema<Output> | undefined and remove the unconditional cast so the return
is the actual possibly-undefined value from
methodStore.operation.getResponseOf(200)?.getMedia("application/json")?.get("schema")?.itemSchema();
update any callers of getOutputSchema to handle the undefined case (or keep
their existing || fallbacks) so the type system accurately reflects optionality;
references: getOutputSchema, methodStore.operation.getResponseOf, getMedia, and
itemSchema.
In `@packages/platform/platform-mcp/src/services/PlatformMcpModule.ts`:
- Around line 37-42: The constructor call in PlatformMcpModule.dispatch uses
this.settings.transportOptions but this.settings can be undefined; update the
call that creates the StreamableHTTPServerTransport (in dispatch) to use
optional chaining or a safe default (e.g., this.settings?.transportOptions or an
empty object) so it never accesses transportOptions on undefined; ensure the
change is applied where StreamableHTTPServerTransport is instantiated in the
dispatch method.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseArray.ts`:
- Around line 5-8: The tuple branch in parseArray returns early for
Array.isArray(schema.items), skipping application of minItems/maxItems (and
handling of additionalItems) that the non-tuple path applies; update parseArray
so the tuple branch builds the base z.tuple([...]) expression but then continues
to apply the same size constraints and additionalItems handling as the
regular-array path (use schema.minItems, schema.maxItems and
schema.additionalItems) before returning; locate parseArray, the schema.items
tuple branch, and reuse the same constraint-appending logic used later for
non-tuple arrays so tuple schemas do not silently ignore
minItems/maxItems/additionalItems.
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/Types.ts`:
- Around line 4-52: JsonSchemaObject is missing common optional properties (e.g.
description, default, nullable, not, $ref) which forces casts like in addJsdocs
where schema.description is cast to string; update the JsonSchemaObject type to
include explicit optional fields such as description?: string, default?: any,
nullable?: boolean, not?: JsonSchema, $ref?: string (and any other frequently
used JSON Schema keywords your codebase expects) so consumers (and functions
like addJsdocs) can use them without unsafe casts while keeping the permissive
index signature.
In `@packages/platform/platform-mcp/src/utils/toZod.spec.ts`:
- Around line 1-3: Tests in toZod.spec.ts use Vitest globals (describe, it,
expect) without importing them, violating the Vitest ESLint plugin; add explicit
imports from 'vitest' (e.g., import { describe, it, expect } from "vitest") at
the top of the file so the test file complies with ESLint rules and references
the existing test block that calls describe/it/expect.
Comment on lines
+35
to
+38
| function getOutputSchema<Output>(methodStore: JsonMethodStore): JsonSchema<Output> { | ||
| const schema: JsonSchema = methodStore.operation.getResponseOf(200)?.getMedia("application/json")?.get("schema"); | ||
|
|
||
| return schema?.itemSchema() as JsonSchema<Output>; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Return type of getOutputSchema is misleading — it can return undefined.
schema?.itemSchema() returns undefined when there's no 200 response or no matching media type, but the return type annotation claims JsonSchema<Output>. This won't cause a runtime error (callers use || fallback), but it silently hides the optionality.
Proposed fix
-function getOutputSchema<Output>(methodStore: JsonMethodStore): JsonSchema<Output> {
+function getOutputSchema<Output>(methodStore: JsonMethodStore): JsonSchema<Output> | undefined {🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/fn/defineTool.ts` around lines 35 - 38,
The function getOutputSchema currently declares return type JsonSchema<Output>
but can return undefined; change its signature to return JsonSchema<Output> |
undefined and remove the unconditional cast so the return is the actual
possibly-undefined value from
methodStore.operation.getResponseOf(200)?.getMedia("application/json")?.get("schema")?.itemSchema();
update any callers of getOutputSchema to handle the undefined case (or keep
their existing || fallbacks) so the type system accurately reflects optionality;
references: getOutputSchema, methodStore.operation.getResponseOf, getMedia, and
itemSchema.
Comment on lines
+37
to
+42
| protected async dispatch($ctx: PlatformContext) { | ||
| const transport = new StreamableHTTPServerTransport({ | ||
| sessionIdGenerator: undefined, | ||
| enableJsonResponse: true, | ||
| ...this.settings.transportOptions | ||
| }); |
Contributor
There was a problem hiding this comment.
Missing optional chaining on this.settings at Line 41.
this.settings is typed as PlatformMcpSettings | undefined (from constant()), and every other access uses this.settings?., but Line 41 uses this.settings.transportOptions without ?.. This will throw a TypeError if settings is somehow undefined.
Proposed fix
const transport = new StreamableHTTPServerTransport({
sessionIdGenerator: undefined,
enableJsonResponse: true,
- ...this.settings.transportOptions
+ ...this.settings?.transportOptions
});🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/services/PlatformMcpModule.ts` around
lines 37 - 42, The constructor call in PlatformMcpModule.dispatch uses
this.settings.transportOptions but this.settings can be undefined; update the
call that creates the StreamableHTTPServerTransport (in dispatch) to use
optional chaining or a safe default (e.g., this.settings?.transportOptions or an
empty object) so it never accesses transportOptions on undefined; ensure the
change is applied where StreamableHTTPServerTransport is instantiated in the
dispatch method.
Comment on lines
+5
to
+8
| export const parseArray = (schema: JsonSchemaObject & {type: "array"}, refs: Refs) => { | ||
| if (Array.isArray(schema.items)) { | ||
| return `z.tuple([${schema.items.map((v, i) => parseSchema(v, {...refs, path: [...refs.path, "items", i]}))}])`; | ||
| } |
Contributor
There was a problem hiding this comment.
Tuple branch returns early, skipping minItems/maxItems constraints.
When schema.items is an array (tuple form), the function returns on Line 7 without appending the minItems/maxItems constraints that are applied to the regular array path (Lines 17–19). While tuple length is implicitly fixed by the number of elements, a JSON Schema can still define minItems/maxItems alongside tuple items (especially with additionalItems), and these would be silently ignored.
🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseArray.ts`
around lines 5 - 8, The tuple branch in parseArray returns early for
Array.isArray(schema.items), skipping application of minItems/maxItems (and
handling of additionalItems) that the non-tuple path applies; update parseArray
so the tuple branch builds the base z.tuple([...]) expression but then continues
to apply the same size constraints and additionalItems handling as the
regular-array path (use schema.minItems, schema.maxItems and
schema.additionalItems) before returning; locate parseArray, the schema.items
tuple branch, and reuse the same constraint-appending logic used later for
non-tuple arrays so tuple schemas do not silently ignore
minItems/maxItems/additionalItems.
Comment on lines
+4
to
+52
| export type JsonSchemaObject = { | ||
| // left permissive by design | ||
| type?: string | string[]; | ||
|
|
||
| // object | ||
| properties?: {[key: string]: JsonSchema}; | ||
| additionalProperties?: JsonSchema; | ||
| unevaluatedProperties?: JsonSchema; | ||
| patternProperties?: {[key: string]: JsonSchema}; | ||
| minProperties?: number; | ||
| maxProperties?: number; | ||
| required?: string[] | boolean; | ||
| propertyNames?: JsonSchema; | ||
|
|
||
| // array | ||
| items?: JsonSchema | JsonSchema[]; | ||
| additionalItems?: JsonSchema; | ||
| minItems?: number; | ||
| maxItems?: number; | ||
| uniqueItems?: boolean; | ||
|
|
||
| // string | ||
| minLength?: number; | ||
| maxLength?: number; | ||
| pattern?: string; | ||
| format?: string; | ||
|
|
||
| // number | ||
| minimum?: number; | ||
| maximum?: number; | ||
| exclusiveMinimum?: number | boolean; | ||
| exclusiveMaximum?: number | boolean; | ||
| multipleOf?: number; | ||
|
|
||
| // unions | ||
| anyOf?: JsonSchema[]; | ||
| allOf?: JsonSchema[]; | ||
| oneOf?: JsonSchema[]; | ||
|
|
||
| if?: JsonSchema; | ||
| then?: JsonSchema; | ||
| else?: JsonSchema; | ||
|
|
||
| // shared | ||
| const?: Serializable; | ||
| enum?: Serializable[]; | ||
|
|
||
| errorMessage?: {[key: string]: string | undefined}; | ||
| } & {[key: string]: any}; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
description and other commonly used properties are missing from JsonSchemaObject.
addJsdocs casts schema.description as string, and parsers likely access schema.default, schema.nullable, schema.not, and schema.$ref. These work at runtime via the permissive index signature, but adding explicit optional fields would improve type safety, IDE discoverability, and reduce the need for casts.
Suggested additions
export type JsonSchemaObject = {
// left permissive by design
type?: string | string[];
+
+ // metadata
+ $ref?: string;
+ $id?: string;
+ $schema?: string;
+ title?: string;
+ description?: string;
+ default?: Serializable;
+ nullable?: boolean;
+ not?: JsonSchema;
// object
properties?: {[key: string]: JsonSchema};🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/Types.ts` around
lines 4 - 52, JsonSchemaObject is missing common optional properties (e.g.
description, default, nullable, not, $ref) which forces casts like in addJsdocs
where schema.description is cast to string; update the JsonSchemaObject type to
include explicit optional fields such as description?: string, default?: any,
nullable?: boolean, not?: JsonSchema, $ref?: string (and any other frequently
used JSON Schema keywords your codebase expects) so consumers (and functions
like addJsdocs) can use them without unsafe casts while keeping the permissive
index signature.
Comment on lines
+1
to
+3
| import {s, string} from "@tsed/schema"; | ||
|
|
||
| import {toZod} from "./toZod.js"; |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add explicit vitest imports for Vitest ESLint plugin compliance.
describe, it, and expect are used without being imported from vitest.
Proposed fix
import {s, string} from "@tsed/schema";
+import {describe, expect, it} from "vitest";
import {toZod} from "./toZod.js";As per coding guidelines, **/*.spec.ts: All TypeScript files in test suites must follow Vitest ESLint plugin rules.
📝 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
| import {s, string} from "@tsed/schema"; | |
| import {toZod} from "./toZod.js"; | |
| import {s, string} from "@tsed/schema"; | |
| import {describe, expect, it} from "vitest"; | |
| import {toZod} from "./toZod.js"; |
🤖 Prompt for AI Agents
In `@packages/platform/platform-mcp/src/utils/toZod.spec.ts` around lines 1 - 3,
Tests in toZod.spec.ts use Vitest globals (describe, it, expect) without
importing them, violating the Vitest ESLint plugin; add explicit imports from
'vitest' (e.g., import { describe, it, expect } from "vitest") at the top of the
file so the test file complies with ESLint rules and references the existing
test block that calls describe/it/expect.
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
1 participant
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 by CodeRabbit
New Features
Documentation
Tests
Chores