Feat platform mcp - #3244
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HTTPServer as HTTP Server<br/>(POST /mcp)
participant DIContainer as DI Container
participant MCPModule as PlatformMcpModule
participant MCPServer as McpServer<br/>(Singleton)
participant Transport as StreamableHTTP<br/>Transport
participant Handler as Tool/Prompt/<br/>Resource Handler
Client->>HTTPServer: JSON-RPC Request
HTTPServer->>DIContainer: Resolve PlatformMcpModule
DIContainer->>MCPModule: Initialize with settings
MCPModule->>DIContainer: Resolve MCP_SERVER
DIContainer->>MCPServer: Build & register tools/prompts/resources
MCPModule->>Transport: Create per-request transport
Transport->>MCPServer: Forward handleRequest
MCPServer->>DIContainer: Resolve handler (tool/prompt/resource)
DIContainer->>Handler: Instantiate & call method
Handler->>Handler: Execute with input schema validation
Handler-->>MCPServer: Return result
MCPServer-->>Transport: Response
Transport-->>HTTPServer: JSON-RPC Result
HTTPServer-->>Client: HTTP 200 with response
MCPModule->>Transport: Close transport
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![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
+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.
Romakita
force-pushed
the
feat-platform-mcp
branch
from
February 4, 2026 16:42
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.
| @@ -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.
Romakita
force-pushed
the
feat-platform-mcp
branch
from
February 7, 2026 07:29
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 Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
In general, the fix is to ensure that any untrusted value that will be embedded inside generated JavaScript/TypeScript source is passed through an escaping function that neutralizes characters that could break out of the intended literal context or close a surrounding <script> tag. We can follow the pattern from the provided example: define a charMap and an escapeUnsafeChars function that replaces <, >, /, backslash, control characters, and the line/paragraph separators with safe escape sequences. Then, wrap every JSON.stringify(...) whose output is interpolated into code with escapeUnsafeChars(...).
Concretely, in this codebase we should:
- Define a shared
escapeUnsafeCharshelper inparseSchema.ts(since most parsers import from there, and we already importSerializablewhich may be used by the helper). - Export this helper from
parseSchema.ts. - In
parseEnum.tsandparseConst.ts, importescapeUnsafeCharsfrom./parseSchema.jsand wrap eachJSON.stringify(...)call with it. - In
parseSchema.ts, wrapJSON.stringify(schema.description)andJSON.stringify(schema.default)withescapeUnsafeChars. - The sink (
parseNot) is already usingparseSchema, so by sanitizing at theJSON.stringifycall sites and keeping all code generation using escaped strings, we address all variants at once without changing runtime semantics (values are still valid JS literals, just encoded more safely).
No behavior change is intended beyond making the generated code safer to embed; JSON.stringify output is being post‑processed to keep it semantically equivalent JavaScript while avoiding dangerous characters.
Suggested changeset
3
packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseConst.ts
Outside changed files
| @@ -1,4 +1,5 @@ | ||
| import {JsonSchemaObject, Serializable} from "../Types.js"; | ||
| import {escapeUnsafeChars} from "./parseSchema.js"; | ||
|
|
||
| /** | ||
| * Parses constant schemas into `z.literal` expressions. | ||
| @@ -7,5 +8,5 @@ | ||
| * @since 8.17.0 | ||
| */ | ||
| 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,25 @@ | ||
| import {parseOneOf} from "./parseOneOf.js"; | ||
| import {parseString} from "./parseString.js"; | ||
|
|
||
| const unsafeCharMap: 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) => unsafeCharMap[ch] ?? ch); | ||
| }; | ||
|
|
||
| /** | ||
| * Recursively parses a JSON Schema node into a Zod expression string. | ||
| * | ||
| @@ -73,7 +92,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; | ||
| @@ -81,7 +100,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
| @@ -1,4 +1,5 @@ | ||
| import {JsonSchemaObject, Serializable} from "../Types.js"; | ||
| import {escapeUnsafeChars} from "./parseSchema.js"; | ||
|
|
||
| /** | ||
| * Parses enum schemas into either `z.enum` or unions of literals depending on value types. | ||
| @@ -11,10 +12,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.
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.
Comment on lines
+56
to
+63
| if (index === -1) { | ||
| if (required || required === "") { | ||
| throw new Error(typeof required === "string" && required !== "" ? required : `Missing required argument ${name}`); | ||
| } | ||
|
|
||
| result[name] = false; | ||
|
|
||
| continue; |
Contributor
There was a problem hiding this comment.
Missing optional args default to false instead of undefined — type mismatch for non-boolean params.
When an optional param with value: "string" or value: "number" is not provided, this assigns false (a boolean). The declared return type (InferReturnType) says the fallback is undefined, and callers checking result.someStringParam === undefined will miss the absent case. Consider defaulting to undefined for non-boolean params.
Proposed fix
if (index === -1) {
if (required || required === "") {
throw new Error(typeof required === "string" && required !== "" ? required : `Missing required argument ${name}`);
}
- result[name] = false;
+ result[name] = value ? undefined! : false;
continue;
}Or more explicitly:
- result[name] = false;
+ result[name] = !value || value === "boolean" ? false : (undefined as any);🤖 Prompt for AI Agents
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/cliTools.ts`
around lines 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.
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.
Romakita
force-pushed
the
feat-platform-mcp
branch
2 times, most recently
from
February 7, 2026 08:29
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.
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.
Romakita
force-pushed
the
feat-platform-mcp
branch
from
February 7, 2026 09:49
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
+7
| 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
+29
| 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.
Romakita
force-pushed
the
feat-platform-mcp
branch
from
February 7, 2026 09:55
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
+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
+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.
Romakita
force-pushed
the
feat-platform-mcp
branch
from
February 8, 2026 07:38
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.
Romakita
force-pushed
the
feat-platform-mcp
branch
from
February 24, 2026 08:34
5fb3166 to
23caca5
Compare
| 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 Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 5 months ago
In general, the fix is to ensure that any schema-derived string that is inserted into generated JavaScript source is additionally sanitized to remove or escape characters that can break out of the intended context, beyond what JSON.stringify does. The standard hardening is to post-process the JSON.stringify output with an escape function that replaces unsafe characters (<, >, /, backslash, control characters, \u2028, \u2029, etc.) with safe escape sequences, and to use this function consistently wherever user-controlled strings are turned into code.
Concretely for this codebase, we should:
- Add a small helper (in a file that already participates in code generation) that escapes unsafe characters in a JSON-stringified value. To minimize changes, we’ll define it in
jsonSchemaToZod.ts, which is the central generator, and reuse it inparseEnum.tsandparseSchema.ts. - Replace direct
JSON.stringify(...)calls used in template literals that form code with a wrapped version, e.g.escapeUnsafeJson(JSON.stringify(schema.description)),escapeUnsafeJson(JSON.stringify(x)), etc. - For
jsonSchemaToZod.ts, wrapJSON.stringify(name)(which is used to create an object literal key name inside generated code) with the same escape helper so it can’t embed problematic characters in the resulting source string. - Keep the external behavior identical (they still generate syntactically valid JS/TS code and feed it to
eval), only adding extra escaping at the string-literal level. No API signatures or imports need to change.
We only touch the snippets shown in the four allowed files, and we do not introduce extra dependencies; the escape helper is defined inline.
Suggested changeset
3
packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseSchema.ts
Outside changed files
| @@ -16,6 +16,25 @@ | ||
| import {parseOneOf} from "./parseOneOf.js"; | ||
| import {parseString} from "./parseString.js"; | ||
|
|
||
| const schemaCharMap: Record<string, string> = { | ||
| "<": "\\u003C", | ||
| ">": "\\u003E", | ||
| "/": "\\u002F", | ||
| "\\": "\\\\", | ||
| "\b": "\\b", | ||
| "\f": "\\f", | ||
| "\n": "\\n", | ||
| "\r": "\\r", | ||
| "\t": "\\t", | ||
| "\0": "\\0", | ||
| "\u2028": "\\u2028", | ||
| "\u2029": "\\u2029" | ||
| }; | ||
|
|
||
| const escapeSchemaJson = (str: string): string => { | ||
| return str.replace(/[<>\/\\\b\f\n\r\t\0\u2028\u2029]/g, (x) => schemaCharMap[x] ?? x); | ||
| }; | ||
|
|
||
| /** | ||
| * Recursively parses a JSON Schema node into a Zod expression string. | ||
| * | ||
| @@ -73,7 +92,7 @@ | ||
|
|
||
| const addDescribes = (schema: JsonSchemaObject, parsed: string): string => { | ||
| if (schema.description) { | ||
| parsed += `.describe(${JSON.stringify(schema.description)})`; | ||
| parsed += `.describe(${escapeSchemaJson(JSON.stringify(schema.description))})`; | ||
| } | ||
|
|
||
| return parsed; | ||
| @@ -81,7 +100,7 @@ | ||
|
|
||
| const addDefaults = (schema: JsonSchemaObject, parsed: string): string => { | ||
| if (schema.default !== undefined) { | ||
| parsed += `.default(${JSON.stringify(schema.default)})`; | ||
| parsed += `.default(${escapeSchemaJson(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" | ||
| }; | ||
|
|
||
| const escapeUnsafeJson = (str: string): string => { | ||
| return str.replace(/[<>\/\\\b\f\n\r\t\0\u2028\u2029]/g, (x) => charMap[x] ?? x); | ||
| }; | ||
|
|
||
| /** | ||
| * Generates Zod code from a JSON Schema, optionally targeting specific module systems and versions. | ||
| * | ||
| @@ -28,7 +47,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 ? `{ ${escapeUnsafeJson(JSON.stringify(name))}: ${result} }` : result} | ||
| `; | ||
|
|
||
| if (!noImport) { |
packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseEnum.ts
Outside changed files
| @@ -1,5 +1,24 @@ | ||
| import {JsonSchemaObject, Serializable} from "../Types.js"; | ||
|
|
||
| const enumCharMap: Record<string, string> = { | ||
| "<": "\\u003C", | ||
| ">": "\\u003E", | ||
| "/": "\\u002F", | ||
| "\\": "\\\\", | ||
| "\b": "\\b", | ||
| "\f": "\\f", | ||
| "\n": "\\n", | ||
| "\r": "\\r", | ||
| "\t": "\\t", | ||
| "\0": "\\0", | ||
| "\u2028": "\\u2028", | ||
| "\u2029": "\\u2029" | ||
| }; | ||
|
|
||
| const escapeEnumJson = (str: string): string => { | ||
| return str.replace(/[<>\/\\\b\f\n\r\t\0\u2028\u2029]/g, (x) => enumCharMap[x] ?? x); | ||
| }; | ||
|
|
||
| /** | ||
| * Parses enum schemas into either `z.enum` or unions of literals depending on value types. | ||
| * | ||
| @@ -11,10 +30,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(${escapeEnumJson(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) => escapeEnumJson(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(${escapeEnumJson(JSON.stringify(x))})`).join(", ")}])`; | ||
| } | ||
| }; |
Copilot is powered by AI and may make mistakes. Always verify output.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 52
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/opsx/apply.md:
- Around line 99-109: Add a language specifier (e.g., text or markdown) to each
fenced code block that currently starts with "## Implementing: <change-name>
(schema: <schema-name>)" and the two subsequent blocks containing "Working on
task X/7: <task description>" so they satisfy markdownlint MD040 and render
correctly; update the opening fences from ``` to ```text (or ```markdown) for
the three blocks in .claude/commands/opsx/apply.md that show the example output
sequences so all three fenced code blocks include the language identifier.
In @.claude/commands/opsx/archive.md:
- Around line 1-156: The markdown lacks a top-level heading and fenced-code
languages causing MD041/MD040; add a single H1 title line immediately after the
YAML front matter (e.g., "# OPSX: Archive") and annotate all fenced code blocks
with appropriate languages (e.g., ```bash for shell commands like the mkdir/mv
examples and ```text for the output examples like "## Archive Complete"); update
every fenced block in .claude/commands/opsx/archive.md accordingly so lint
warnings are resolved.
In @.claude/commands/opsx/bulk-archive.md:
- Around line 55-59: The fenced code blocks in the document are missing language
identifiers (e.g., the block starting with ``` and containing "auth ->
[change-a, change-b] <- CONFLICT..." and all other fenced blocks referenced),
which violates markdownlint MD040; update every fenced code block (including the
ones around lines shown in the review: the sample block plus the blocks covering
89-96, 100-103, 107-110, 151-165, 169-172, 178-189, 194-207, 211-221, 225-236,
240-244) by adding an appropriate language tag after the opening backticks (for
example use `text` for plain output, `bash` for shell snippets, or `markdown`
for MD examples) so each opening ``` becomes ```text (or other suitable
language) consistently.
- Around line 8-12: Add a top-level H1 heading immediately after the frontmatter
to satisfy markdownlint MD041; locate the markdown whose first non-frontmatter
line starts with "Archive multiple completed changes in a single operation." and
insert a concise H1 (e.g., "# Bulk Archive Changes") above that paragraph so the
first non-frontmatter line is a heading and improves scan-ability.
In @.claude/commands/opsx/continue.md:
- Around line 56-61: The fenced code blocks inside the list items "Get its
instructions:" and the later similar block must be surrounded by blank lines to
satisfy MD031; insert a blank line immediately before the opening ```bash and a
blank line immediately after the closing ``` for the block that contains
openspec instructions <artifact-id> --change "<name>" --json (and apply the same
change to the second block referenced around lines 84-86) so each fenced code
block is separated from surrounding list text.
In @.claude/commands/opsx/ff.md:
- Around line 85-90: The summary text currently claims "All artifacts created!
Ready for implementation." and prompts "Run `/opsx:apply` to start
implementing." but the flow only guarantees applyRequired artifacts
(applyRequires); change the copy to accurately reflect that only apply-required
artifacts were produced (e.g., "All apply-required artifacts created. Ready for
apply.") and update the final prompt to something like "Run `/opsx:apply` to
begin applying the generated artifacts." Ensure you reference and mention
applyRequires in the message so it’s clear the guarantee scope is limited to
apply-required artifacts.
In @.claude/commands/opsx/new.md:
- Around line 8-79: The markdown is missing a top-level H1 after the
frontmatter; open .claude/commands/opsx/new.md, locate the YAML frontmatter
block (the leading --- ... ---) and insert a single H1 line immediately after it
(for example "# Start a new change using the experimental artifact-driven
approach") so the document has a top-level heading satisfying markdownlint; keep
the existing content and examples unchanged and ensure the H1 matches the
document's intent.
In @.claude/commands/opsx/onboard.md:
- Around line 310-316: Update the onboarding doc so the "Design" section is
presented as optional rather than mandatory: change the "## Design" heading
paragraph to state that creating design.md is required only for complex or
cross-cutting changes and can be skipped for small/simple tasks, and add the
suggested acceptance criteria (cross-service changes, new architectural pattern,
new external dependency, significant data-model changes, security/perf/migration
complexity, or ambiguous technical decisions) as the conditions under which
design.md should be created; ensure references use the existing "## Design"
title and the filename design.md so reviewers can find and apply the conditional
wording.
In @.claude/commands/opsx/sync.md:
- Line 125: The Markdown contains an unlabelled fenced code block (the triple
backticks block shown as ``` ... ```) which triggers MD040; fix it by adding a
language identifier to the opening fence (for example change ``` to ```text or
another appropriate language) so the fenced code block is labelled.
- Line 8: Add a top-level Markdown heading immediately after the YAML front
matter to satisfy MD041; for example insert a descriptive H1 like "# Sync delta
specs" above the existing sentence "Sync delta specs from a change to main
specs." in .claude/commands/opsx/sync.md so the file has a single top-level
heading right after the front matter.
In @.claude/commands/opsx/verify.md:
- Around line 1-8: The markdown file missing a top-level H1 after the YAML front
matter causes MD041; add a single H1 heading line immediately after the closing
front-matter delimiter (---) — e.g., add "## OPSX: Verify" or preferably "#
OPSX: Verify" as the top-level heading — so the content following the front
matter begins with an H1 and satisfies the lint rule for the file that contains
the front matter and the existing description lines from the diff.
- Around line 120-179: Update the top fenced code block and the wording that
refers to it: change the opening ``` to specify the language as ```markdown for
the block that begins with "## Verification Report: <change-name>" and change
the phrase "Use clear markdown with:" to "Use clear Markdown with:". Locate
these in the verify.md content around the "## Verification Report" section and
the "Output Format" paragraph and apply the two textual edits.
In @.claude/skills/openspec-apply-change/SKILL.md:
- Line 12: Add a top-level heading immediately after the YAML front matter in
SKILL.md to satisfy MD041; open SKILL.md, locate the YAML front matter block at
the top, and insert a single H1 line (e.g., "# Implement tasks from an OpenSpec
change") directly after it so the document has a top-level heading following the
front matter.
- Line 103: Update the three unlabelled fenced code blocks (the bare ```
occurrences) to include a language identifier to satisfy MD040: replace each ```
with ```text for generic blocks or ```markdown for output templates; target the
three unlabelled code fences currently opening without a language identifier so
they become e.g. ```text (or ```markdown where the block is an output template).
In @.claude/skills/openspec-archive-change/SKILL.md:
- Line 12: The file openspec-archive-change/SKILL.md is missing a top-level H1
heading as the first content line after frontmatter (MD041); add a plain "#
Archive a completed change in the experimental workflow" (or equivalent H1)
immediately after the YAML frontmatter so the plain-prose opener becomes an H1,
matching the same fix applied in openspec-sync-specs/SKILL.md and satisfying the
MD041 rule.
- Line 71: The line uses inconsistent invocation syntax `/opsx:sync logic`;
replace this with the same reference style used elsewhere by invoking the
openspec-sync-specs skill (e.g., "execute the openspec-sync-specs skill to
perform sync") so callers and agents know to call the openspec-sync-specs skill
rather than a slash-command; update any adjacent text to match this phrasing and
remove the `/opsx:sync` slash-command form.
- Around line 104-113: The fenced code block around the "## Archive Complete"
section in SKILL.md lacks a language tag (MD040); update the opening
triple-backtick to include a language such as "text" or "markdown" (e.g., change
``` to ```text) so the block is properly annotated, leaving the rest of the
block (the "## Archive Complete" header and subsequent lines) unchanged.
In @.claude/skills/openspec-bulk-archive-change/SKILL.md:
- Around line 45-49: Add a hard gate in the archive flow that reads
openspec/changes/<name>/tasks.md and counts checklist items (`- [ ]` vs `-
[x]`), and if any `- [ ]` items exist (or the file is missing instead of "No
tasks"), prevent confirmation of archive and surface a clear message requiring
all tasks be marked `- [x]` or offer a forced-update action; ensure this check
is invoked at the archive confirmation step so archiving cannot proceed when
incomplete tasks remain.
- Around line 128-143: Replace the manual mkdir/mv archive steps in the "Perform
the archive" section with calls to the OpenSpec CLI: call openspec archive
"<change-id>" --yes (or openspec archive "<change-id>" --skip-specs --yes for
tooling-only changes) for each confirmed change, ensure you pass the explicit
change ID, and after processing the batch run openspec validate --strict
--no-interactive to verify validation; also document the post-deploy guidance to
move changes to changes/archive/YYYY-MM-DD-[name] and update specs/ when
capabilities change as part of the workflow notes.
In @.claude/skills/openspec-ff-change/SKILL.md:
- Around line 89-94: Update the summary wording in SKILL.md so it doesn't
overpromise when only applyRequires are satisfied: replace the final bullet that
currently states "All artifacts created! Ready for implementation." with a
conditional, more accurate message such as "Required artifacts for apply have
been created; additional artifacts may still be pending." Also adjust the
follow-up prompt around "Run `/opsx:apply` or ask me to implement..." to clarify
that running /opsx:apply will attempt to apply the prepared changes (not
necessarily that every artifact already exists). Target the summary block that
lists "Change name and location", "List of artifacts created...", and references
applyRequires to make these text changes.
In @.claude/skills/openspec-sync-specs/SKILL.md:
- Line 12: The file's first non-frontmatter line is plain prose and violates
MD041; insert a top-level heading line (a single leading "#" heading)
immediately after the YAML frontmatter so the heading is the first content line,
then keep the existing opener text below it unchanged; locate the frontmatter
block and add the new top-level heading as the next line.
- Around line 129-143: The fenced output block that begins with "## Specs
Synced: <change-name>" lacks a language hint and triggers markdownlint MD040;
update the opening fence from ``` to include a language (e.g., ```text or ```md)
for the block in .claude/skills/openspec-sync-specs/SKILL.md so the example
block is fenced as ```text (or ```md) to satisfy MD040 while leaving the block
contents unchanged.
In @.claude/skills/openspec-verify-change/SKILL.md:
- Line 177: Update the wording that currently reads "Use clear markdown with:"
to capitalize Markdown as a proper noun; locate the exact string "Use clear
markdown with:" in SKILL.md and change it to "Use clear Markdown with:" so the
term Markdown is capitalized consistently.
- Around line 1-10: Update the compatibility field in the SKILL frontmatter to
pin a minimum openspec CLI version (e.g., change "Requires openspec CLI." to
"Requires openspec CLI >= X.Y.Z") so the skill will not run against older,
incompatible CLI releases; edit the top-matter in
.claude/skills/openspec-verify-change/SKILL.md and replace the generic
compatibility string with a semver constraint that reflects the minimum CLI that
supports the flags/features this skill uses (referencing the compatibility field
in the YAML frontmatter).
- Around line 168-173: Update the "Graceful Degradation" section to include CLI
failure handling: add a clause that if any openspec command (e.g., the `openspec
status` or `openspec instructions apply` invocations) exits with a non-zero
code, returns empty output, or yields malformed/non-JSON output, the agent must
halt and report a clear error such as "openspec CLI error: `<command>` failed.
Ensure the openspec CLI is installed and the change name is valid." Also
instruct the agent to include the raw CLI stderr/stdout when available for
debugging and to explicitly note that checks were skipped due to the CLI
failure.
- Around line 68-77: Update Step 5 to stop re-deriving a hardcoded path and
instead read the spec artifacts from the previously loaded contextFiles, and
change all occurrences of the path fragment "openspec/changes/<name>/specs/" and
the placeholder "<name>" to the canonical "openspec/changes/<id>/specs/" (or
"<id>") so naming is consistent with the rest of the skill; in practice,
reference the already-populated contextFiles collection rather than constructing
a path string in the Step 5 logic and ensure any checks or issue messages (e.g.,
"Requirement not found: <requirement name>") use <id> when referring to the
change directory key.
- Around line 41-43: Update the example usage for the command string `openspec
instructions apply --change "<name>" --json` to make it explicit that it is
read-only and does not perform any write/mutation; either add an inline comment
after the code line such as `# read-only: returns context files only` or add a
one-sentence clarification immediately before or after the code block stating
the command returns only the implementation-phase prompt bundle (context, rules,
template, instruction metadata) and does not mutate state.
In @.codex/skills/openspec-archive-change/SKILL.md:
- Around line 92-123: Add a required post-archive validation step: after moving
changes/[name]/ to changes/archive/YYYY-MM-DD-[name]/ (and after any
openspec-sync-specs run or when delta specs exist), ensure specs/ are updated
for capability changes and run "openspec validate --strict --no-interactive" to
enforce compliance; update the SKILL.md guidance near the archive completion
summary and guardrails to state this order (use openspec status --json for
artifact completion checking, preserve .openspec.yaml, run sync assessment when
delta specs exist, then run the strict validate command and surface any
validation failures/warnings in the summary).
- Around line 42-55: Update the "Check task completion status" flow in SKILL.md
so that tasks.md must be fully checked off before archiving: replace the current
warning-and-confirm behavior with a hard block when any `- [ ]` items are found
(count incomplete tasks and, if count > 0, abort the archive flow and return a
clear error directing the user to mark all tasks `- [x]`), remove or repurpose
the AskUserQuestion tool prompt (do not allow proceeding on user confirmation),
and document that absence of tasks.md still allows proceeding but a present
tasks.md must have every task marked `- [x]` before archive.
In @.codex/skills/openspec-bulk-archive-change/SKILL.md:
- Around line 128-143: Replace the manual filesystem move in the "Execute
archive for each confirmed change" step with the official CLI to preserve
OpenSpec validations: instead of creating openspec/changes/archive and running
mv for <name>, call the tool with openspec archive <change-id> --yes and, when
the change is tool-only, include --skip-specs; ensure the instructions
explicitly require passing the change ID (not a filename) and document using
--yes for automation and --skip-specs only when appropriate so the archive step
uses the openspec archive command rather than direct filesystem operations.
- Around line 59-62: Multiple fenced code blocks (e.g., the block containing
"auth -> [change-a, change-b] <- CONFLICT (2+ changes)" and the other ranges
listed) are missing language tags; update each triple-backtick fence in SKILL.md
to include an appropriate language identifier such as text, bash, or markdown
(for example change ``` to ```text for plain output blocks or ```bash for shell
examples) across the noted blocks (lines referenced in the comment) so all
fenced code blocks include a language tag to satisfy MD040.
- Line 12: Add a top-level H1 immediately after the YAML frontmatter in
.codex/skills/openspec-bulk-archive-change/SKILL.md to satisfy MD041: insert a
single-line heading (e.g., "# Archive multiple completed changes") right after
the closing --- of the frontmatter so the existing first-line content becomes a
paragraph under that H1.
In @.codex/skills/openspec-explore/SKILL.md:
- Around line 58-73: Add explicit language identifiers to all fenced code blocks
in SKILL.md: change the ASCII diagram fences to use ```text and the CLI/command
examples (e.g., the snippet containing "openspec list --json") to use ```bash
(or another appropriate language tag). Update every similar fence mentioned in
the review (the ASCII diagram at the shown example and the ranges noted: the
other code fences around the CLI snippets and examples) so they include the
language token immediately after the opening backticks.
In @.codex/skills/openspec-onboard/SKILL.md:
- Around line 16-29: Update the Preflight section to include explicit OpenSpec
context checks: after the existing "openspec status --json" step, add steps to
read openspec/project.md, run "openspec list" and "openspec list --specs" to
surface active changes and existing capabilities, and instruct the user to
inspect specs under specs/[capability]/spec.md and pending files in changes/ for
conflicts before onboarding; reference these checks (openspec status,
openspec/project.md, openspec list, openspec list --specs,
specs/[capability]/spec.md, and changes/) so reviewers can locate and verify the
additions.
- Around line 1-12: Add a top-level H1 immediately after the YAML frontmatter in
SKILL.md to satisfy markdownlint rule MD041; open the file SKILL.md, locate the
closing frontmatter marker (---) and insert a single H1 line such as "# OpenSpec
Onboarding" on the next line (optionally leaving one blank line after the
frontmatter), ensuring the first non-frontmatter content is the new heading.
- Around line 36-536: The markdown file contains many fenced code blocks without
a language tag (MD040); open .codex/skills/openspec-onboard/SKILL.md and for
each triple-backtick block (for example the block under "## Welcome to
OpenSpec!", the "## Task Suggestions" examples, and the bash snippets showing
git/openspec commands) add an explicit language token such as ```text, ```md, or
```bash to match the content; update every unlabeled fence in the file so
linting passes and readability improves.
In @.codex/skills/openspec-sync-specs/SKILL.md:
- Around line 12-143: Add a top-level H1 heading after the frontmatter and label
the output fenced code block with a language (e.g., "text") in
.codex/skills/openspec-sync-specs/SKILL.md to satisfy markdownlint: insert a
single "#" heading line before the "## Specs Synced: <change-name>" block (or
make that block the H1) and change the opening triple-backtick for the example
output from ``` to ```text so the "Specs Synced" output fence is
language-labeled.
In @.codex/skills/openspec-verify-change/SKILL.md:
- Line 177: Replace the lowercase word "markdown" in the heading line 'Use clear
markdown with:' with the proper noun "Markdown" so the line reads 'Use clear
Markdown with:'; locate the exact string "Use clear markdown with:" in SKILL.md
and update its capitalization accordingly.
- Around line 1-10: The MD041 false-positive for the openspec-verify-change
skill occurs because the file .codex/skills/openspec-verify-change/SKILL.md
intentionally begins with YAML front-matter; add or update your markdownlint
config (e.g., create or modify .markdownlint.json at project root) to set the
MD041 rule to allow empty front_matter_title so front-matter is exempt (i.e.,
set MD041.front_matter_title to an empty string) and commit that config.
In `@openspec/changes/archive/2026-02-07-add-mcp-package/tasks.md`:
- Around line 1-30: The archived change checklist still contains open items (all
lines like "- [ ] 1.1", "2.1", "3.1", etc. in tasks.md) but must reflect
completion; update every checklist entry in this file (all "- [ ]" occurrences
under sections "1. Package scaffolding", "2. Shared MCP primitives", "3.
Platform module & configuration", "4. Decorators & DI registration", "5. Tests &
documentation", and "6. Validation & release prep") to "- [x]" so the archived
change accurately shows all tasks completed before archiving.
In `@openspec/changes/improve-jsdoc-platform-mcp/design.md`:
- Line 1: The document currently starts with a second-level heading "##
Context", violating MD041; update the top-level heading by replacing the leading
"## Context" with a single H1 "# Context" so the file's first heading becomes
the document title and satisfies markdownlint; ensure no other H1 appears before
this line.
In `@openspec/changes/improve-jsdoc-platform-mcp/proposal.md`:
- Around line 1-26: Update the Impact section of proposal.md to replace the glob
with explicit file references and spec links: list each affected source file
under packages/platform/platform-mcp/src using the file.ts:42 format for exact
locations (e.g., packages/platform/platform-mcp/src/someFile.ts:42) and
reference affected specs using spec-style paths (e.g., specs/auth/spec.md);
ensure the Impact section includes these concrete file and spec entries and
keeps the existing sections ('Why', 'What Changes', 'Impact') intact without
changing behavior claims.
In
`@openspec/changes/improve-jsdoc-platform-mcp/specs/platform-mcp-jsdoc-coverage/spec.md`:
- Around line 1-27: The spec delta only contains "## ADDED Requirements"; update
the document (the top-level headings in the file) to include the required
complementary sections by adding "## MODIFIED Requirements", "## REMOVED
Requirements", and "## RENAMED Requirements" (use the literal string "_None_"
under each section if there are no entries) so the file conforms to the mandated
spec delta format used by specs/platform-mcp-jsdoc-coverage/spec.md.
In `@openspec/changes/improve-jsdoc-platform-mcp/tasks.md`:
- Around line 1-2: The file currently begins with a second-level heading "## 1.
Preparation" which triggers MD041; add a top-level heading above it (for example
"# Tasks" or "# 1. Preparation") so the document's first line is an H1, ensuring
the existing "## 1. Preparation" remains unchanged and the lint rule is
satisfied.
In `@openspec/specs/mcp-endpoint/spec.md`:
- Around line 3-6: Replace the "TBD" placeholder under the "Purpose" heading in
spec.md with a concise, final description of the MCP endpoint's intent and
scope: locate the "## Purpose" section in openspec/specs/mcp-endpoint/spec.md
and update the text to state what the MCP endpoint provides, its primary use
cases, intended consumers, and any boundaries or constraints (one or two short
paragraphs) so the spec is complete for readers.
In `@packages/platform/platform-mcp/src/decorators/prompt.ts`:
- Line 11: PromptDecoratorOptions is currently defined as Omit<PromptProps,
"handler"> which distributes over the union PromptProps = FnPromptProps |
ClassPromptProps and allows callers to pass a FnPromptProps-shaped object;
change the type to scope it to class-shaped prompts by using
Omit<ClassPromptProps, "handler" | "token" | "propertyKey"> so the decorator
contract reflects that it always injects token and propertyKey and requires the
class-specific fields; update the type alias PromptDecoratorOptions accordingly
and ensure any usages expecting the narrower class form are adjusted to the new
type.
In `@packages/platform/platform-mcp/src/fn/definePrompt.ts`:
- Around line 14-15: The generic constraint "Args extends undefined = any" is
misleading; remove the "extends undefined" and use a plain default like "Args =
any" (or "Args = unknown" if you want stricter typing) for the FnPromptProps,
the other similar generic declarations around the file (including the types
using BasePromptProps and PromptCallback), and any other occurrences at the
noted locations; if this pattern intentionally mirrors the MCP SDK, add a
one-line comment above the type declarations (e.g., above FnPromptProps)
explaining that the form mirrors MCP SDK conventions to avoid future confusion.
- Around line 85-93: The current definePrompt uses
Symbol.for(`MCP:PROMPT:${options.name}`) which creates a global symbol and
causes same-name prompts to silently override; update definePrompt (and
analogous defineTool/defineResource) to either generate a unique symbol via
Symbol() instead of Symbol.for to avoid cross-module collisions, or add a
duplicate-name check before creating the provider (e.g., maintain a local
registry keyed by options.name and throw or warn if already registered) so
duplicate registrations are detected; ensure you reference the provider creation
flow (the injectable(...).type(...).factory(...) chain and provider.token())
when updating the symbol creation or adding the registry validation.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseIfThenElse.ts`:
- Line 32: The generated superRefine handler in parseIfThenElse.ts uses
result.error.errors which was removed in Zod v4; update the code that iterates
validation failures (inside the superRefine created by parseIfThenElse) to use
result.error.issues instead and call ctx.addIssue for each issue (i.e., replace
any result.error.errors.forEach((error) => ctx.addIssue(error)) with
result.error.issues.forEach((issue) => ctx.addIssue(issue))). Ensure this change
is applied to the block where result and ctx are referenced so it works for both
Zod v3 and v4.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseObject.ts`:
- Around line 169-181: The nested ternary that builds the output string in
parseObject (variable output in parseObject.ts) is correct but dense; add a
short inline comment above or beside the expression explaining the decision
matrix: when properties exist prefer properties, when patternProperties exist
prefer patternProperties, and how additionalProperties is handled (z.never() →
.strict(), otherwise .catchall(...) or emitRecord fallback), so future readers
quickly understand why each branch is chosen and what each combination produces;
keep the comment to one or two sentences and reference the symbols properties,
patternProperties, additionalProperties, emitRecord, and z.never() for clarity.
In `@packages/platform/platform-mcp/test/app/prompts/TestPrompt.ts`:
- Around line 12-25: The test fixture returns PromptMessage.content as an array
but per MCP spec it must be a single ContentBlock; update the object returned in
TestPrompt.ts so messages contains { role: "assistant", content: { type: "text",
text: "Use `@tsed/platform-mcp` to interact with tools and resources." } } instead
of content: [ ... ], and mirror the same change where definePrompt.ts constructs
PromptMessage objects (the code around the function that builds messages) to
ensure PromptMessage.content is a single object rather than an array; keep
messages as an array of PromptMessage but enforce content as a single
discriminated ContentBlock.
In `@reports/jsdoc/platform-mcp.md`:
- Around line 1-85: The tracker file reports/jsdoc/platform-mcp.md is not an
allowed JSDoc tracker filename; move its contents into one of the approved
reports under reports/jsdoc/ (e.g., append as a new section in core.md, di.md,
hooks.md, json-mapper.md, or schema.md) and remove
reports/jsdoc/platform-mcp.md; ensure the section retains the "Platform MCP —
JSDoc Coverage Tracker" heading and the export checklist (symbols like
MCP_PROVIDER_TYPES, PromptDecoratorOptions, toZod, parseSchema,
PlatformMcpSettings, PlatformMcpModule, etc.) so reviewers can find entries, and
update any README/CI references or cross-links that pointed to
reports/jsdoc/platform-mcp.md (or alternatively update the repo guideline to
permit this filename if you intend to keep it).
---
Duplicate comments:
In `@docs/docs/mcp.md`:
- Around line 132-140: The example configuration references PlatformMcpModule
but doesn't import it; add an import statement for PlatformMcpModule (from
"@tsed/platform-mcp") at the top of the snippet so the symbol used in the
Configuration({ imports: [PlatformMcpModule], ... }) is defined and the example
compiles.
- Around line 27-38: Add a short clarifying note near the example explaining the
difference between the side‑effect import "import \"@tsed/platform-mcp\"" and an
explicit module import: state that the side‑effect import is a convenience that
auto‑registers the MCP platform module for typical setups (so the shown Server
class and `@Configuration`({ mcp: { path: "/mcp" } }) work), but if your
bundler/tree‑shaker strips side effects or you need explicit control you should
import the module directly (e.g. import the MCP module and add it to the
Configuration imports). Add the same concise note at the other occurrence (lines
132–140) so users know when to use the side‑effect vs explicit import.
In `@openspec/changes/archive/2026-02-07-add-mcp-package/design.md`:
- Around line 1-3: The document is missing a top-level H1 heading; add a
descriptive H1 above the existing "## Context" heading to serve as the file's
title (for example, a line starting with "# Add MCP package design" or similar)
so the file begins with a single H1 followed by the existing "## Context"
section; update the header text to clearly describe the design and keep "##
Context" unchanged.
In `@openspec/changes/archive/2026-02-07-add-mcp-package/proposal.md`:
- Around line 27-29: The spec reference and impact details in proposal.md are
not using the canonical paths or required link formats; update the "Affected
specs" entry to the canonical specs path (e.g., specs/mcp-endpoint) and change
the "Affected code" list to use file.ts:42-style locations for each package/file
(e.g., packages/platform/platform-mcp:line, .cli-mcp:line) and include the
related PR link(s) under Impact; ensure references to Ts.ED wiring and the
`@tsed/platform-http` package remain but are expressed using the canonical spec
path and file.ts:line format.
In
`@openspec/changes/archive/2026-02-07-add-mcp-package/specs/mcp-endpoint/spec.md`:
- Around line 1-3: Add a top-level H1 heading to the spec above the existing "##
ADDED Requirements" section so the document begins with a single top-level
heading (for example "# `@tsed/mcp` package scaffold" or another brief title),
ensuring the subsequent "## ADDED Requirements" and "Requirement: `@tsed/mcp`
package scaffold" headings remain unchanged.
- Around line 30-37: The spec delta is missing the required sections; add
top-level headings "## MODIFIED Requirements", "## REMOVED Requirements", and
"## RENAMED Requirements" (in addition to the existing "## ADDED Requirements"
content) to the MCP endpoint spec so the delta contains all four buckets; ensure
the new sections are present even if empty and keep the existing decorator-based
requirement (the paragraph referencing `@tsed/mcp` and decorators like `@Tool`,
`@Prompt`, `@Resource` and the example scenario) under the appropriate "## ADDED
Requirements" area so metadata collection and provider registration behavior
remains unchanged.
In `@openspec/changes/archive/2026-02-07-add-mcp-package/tasks.md`:
- Line 1: Add a top-level H1 heading to satisfy MD041: insert a single top-level
title line above the existing "## 1. Package scaffolding" heading (for example
change or add a "# 1. Package scaffolding" line or add a separate "# Tasks"
title above it) so the document begins with an H1 before subsequent H2/H3
headings.
In `@packages/platform/platform-mcp/package.json`:
- Around line 26-29: The package.json currently uses open-ended ranges
">=1.26.0" and ">=4.3.6" for `@modelcontextprotocol/sdk` and zod which can pull
future breaking majors; change those to caret ranges "^1.26.0" and "^4.3.6" in
packages/platform/platform-mcp/package.json to enforce SemVer-compatible
updates, and while editing verify the actual latest published npm versions for
`@modelcontextprotocol/sdk` and zod and confirm their maintainers’ guidance
(typically caret ranges are recommended for 1.x/4.x to allow nonbreaking
updates).
In `@packages/platform/platform-mcp/src/decorators/prompt.spec.ts`:
- Around line 6-12: The spec exports a test-only class which violates the
noExportsInTest rule — remove the export keyword from the TestPrompt class
declaration so it becomes `class TestPrompt` (keep the decorators `@Injectable`(),
`@Prompt`(), `@Title`(), `@Description`() and the prompt() method intact) and update
any in-spec references to use the now-local TestPrompt; do not add any other
exports in this .spec.ts file.
In `@packages/platform/platform-mcp/src/decorators/resource.spec.ts`:
- Around line 6-13: The test class TestResource is exported causing the Biome
lint error noExportsInTest; remove the export modifier from the class
declaration (change "export class TestResource" to "class TestResource") so the
class is local to the test file and keep the decorators (`@Injectable`, `@Resource`,
`@Title`, `@Description`, `@ContentType`) and the resource() method unchanged.
- Around line 20-22: Rename the test variable `tool` to `resource` in
resource.spec.ts where you call inject<any>(Symbol.for(`MCP:RESOURCE:resource`))
and in its subsequent expect assertion; update both the declaration and every
use in the test (e.g., the expect(...) block) so the variable name matches the
entity under test and removes the copy-paste artifact from tool.spec.ts.
In `@packages/platform/platform-mcp/src/decorators/resource.ts`:
- Around line 33-42: Resource currently constructs an object with both uri and
template (one undefined) and casts to any, which defeats the ResourceProps
discriminated union checks; change Resource to build a properly typed
ResourceProps instance without a cast by branching on isString(uriOrTemplate):
create a base props object containing name, token (use classOf(target)), and
propertyKey, then if isString(uriOrTemplate) return/assign { ...base, uri:
uriOrTemplate } typed as the URI variant, else return/assign { ...base,
template: uriOrTemplate } typed as the Template variant, and pass that typed
object into defineResource instead of using "as any".
In `@packages/platform/platform-mcp/src/decorators/tool.spec.ts`:
- Line 42: Fix the grammar in the test titles by replacing "should returns" with
"should return" in the affected it blocks; specifically update the test whose
title is "should returns metadata with name" (and the duplicate at the other it
block) so both read "should return metadata with name" to keep test descriptions
correct and consistent.
- Around line 1-4: Tests in tool.spec.ts lack explicit Vitest imports causing
ESLint failures; add the necessary named imports from 'vitest' (e.g., describe,
it, expect, beforeEach/afterEach as used) at the top of the file alongside the
existing imports so the spec file explicitly imports Vitest globals and
satisfies the Vitest ESLint plugin; update the import list to include the
specific Vitest symbols actually used in this test file.
In `@packages/platform/platform-mcp/src/fn/defineResource.ts`:
- Around line 33-43: When options.propertyKey is provided you must require
options.token before using it; update the branch that handles "propertyKey" in
defineResource.ts to first assert or throw if options.token is missing, or bail
out early, so inject(options.token) and JsonEntityStore.fromMethod(token,
propertyKey) never receive undefined. Specifically, guard the block that builds
handler and calls inject(...) and JsonEntityStore.fromMethod(...) by checking
options.token (the local token variable) and surface a clear error (or skip
registering) when it's absent so handler, inject, and JsonEntityStore.fromMethod
are only invoked with a valid token.
In `@packages/platform/platform-mcp/src/fn/defineTool.ts`:
- Around line 106-111: The DI token is built using options.name before the
tool's name is resolved, causing tokens like MCP:TOOL:undefined for class-based
tools; call mapOptions(options) first to get the resolved name (e.g., const {
name, handler, ... } = mapOptions(options)) and then create the injectable token
using Symbol.for(`MCP:TOOL:${name}`) inside defineTool so the token uses the
actual resolved tool name rather than the original options.name.
- Around line 54-58: getOutputSchema currently claims to return
JsonSchema<Output> but calls schema?.itemSchema() which can be undefined; change
getOutputSchema's return type to JsonSchema<Output> | undefined and update any
callers to handle the undefined case (or narrow before use). Locate the function
getOutputSchema and the use of
methodStore.operation.getResponseOf(200)?.getMedia("application/json")?.get("schema")
and modify the signature and downstream call sites (e.g., any code assuming a
non-null result) to perform null checks or provide fallbacks.
- Around line 117-134: The error return in the handler inside defineTool.ts
currently returns structuredContent only; update the async handler (the inner
try/catch in handler(args, extra)) to return an MCP-compliant CallToolResult
that sets isError: true, includes a human-readable ContentBlock in the content
array (e.g., a single ContentBlock with a plain text message derived from
er?.message), and preserves structuredContent with code "E_MCP_TOOL_ERROR" and
message er?.message; keep the existing logger().error call unchanged but ensure
the returned shape satisfies CallToolResult (isError + content +
structuredContent).
In `@packages/platform/platform-mcp/src/services/PlatformMcpModule.ts`:
- Around line 43-48: In dispatch(), the StreamableHTTPServerTransport
constructor uses this.settings.transportOptions without optional chaining which
can throw if this.settings is undefined; update the call to pass
this.settings?.transportOptions (or default to {} if needed) when constructing
StreamableHTTPServerTransport in the dispatch method so it matches other safe
accesses to this.settings.
- Around line 32-41: In $logRoutes replace the direct settings check with the
module helper so the MCP route shows when enabled is undefined: change the
conditional that uses this.settings?.enabled to call this.isEnabled() (keep the
same method/name/url structure and the fallback this.settings?.path || "/mcp"),
so $logRoutes uses isEnabled() like $onRoutesInit does.
- Around line 50-59: The response "close" listener duplicates
cleanup—transport.close() may be invoked twice from response.raw?.on("close",
...) and the finally block; remove the redundant listener to avoid double-close
errors. In PlatformMcpModule (the block that calls this.server.connect and
transport.handleRequest), delete the response.raw?.on("close", () =>
transport.close()) registration and rely on the finally { await
transport.close(); } to always perform cleanup after server.connect and
transport.handleRequest complete.
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/cli.ts`:
- Line 87: The current call "void main();" swallows promise rejections; change
the invocation so any rejection from the async function main is caught and
reported and the process exits with a non-zero code (e.g., call main() and
attach a .catch handler that logs the error (including stack/message) and calls
process.exit(1)). Ensure you update the invocation site where "void main()" is
used so all errors from main (parsing JSON, file IO, unreadable pipes) are
handled and produce a clean error exit.
- Around line 69-74: parseArgs returns false for absent flag values and those
false values are being forwarded to jsonSchemaToZod via the options object
(name, depth, type), which is type-unsafe; coerce those flags to undefined
before passing them in the call that constructs the options (the object using
args.name, args.depth, args.type) so jsonSchemaToZod receives undefined instead
of false — e.g., replace the direct forwards with a conditional/coercion (using
args.name || undefined, args.depth == false ? undefined : args.depth, or
equivalent) when building the options passed to jsonSchemaToZod in cli.ts.
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/index.ts`:
- Around line 24-26: Move the import of jsonSchemaToZod to the top of the module
so it appears before the existing `export *` re-exports; specifically, ensure
the `import { jsonSchemaToZod } from "./jsonSchemaToZod.js";` statement is
placed above any `export * from ...` lines and then keep `export default
jsonSchemaToZod;` as the default export at the bottom. This preserves
import-first ordering for readability and linting while leaving the
`jsonSchemaToZod` symbol and the re-export declarations unchanged.
- Around line 1-23: The barrel file currently re-exports many internal helpers
and individual parsers (e.g., exports of "./utils/half.js", "./utils/omit.js",
"./utils/withMessage.js" and parser modules like "parseArray.js",
"parseBoolean.js", etc.), widening the public API; restrict the public surface
by removing re-exports of internal utilities and all individual parser modules
and only export the public API symbols such as "jsonSchemaToZod" (from
"./jsonSchemaToZod.js"), "parseSchema" (from "./parsers/parseSchema.js"), and
"Types" (from "./Types.js"); update the index.ts barrel to export only those
public modules so consumers cannot import implementation details like half,
omit, withMessage, or individual parse* functions.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/jsonSchemaToZod.ts`:
- Around line 30-38: The CJS branch is prepending jsdocs twice (once before the
require line and again before module.exports); update the logic in the module
=== "cjs" branch so jsdocs is only prefixed once to the final output: build the
inner body (including require when !noImport) without jsdocs, then prepend
jsdocs a single time before the assembled result (the module.exports assignment
using the name/ result wrapper) so only one JSDoc block appears; refer to the
variables/js branch using module === "cjs", jsdocs, result and the noImport
conditional when making the change.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseAllOf.ts`:
- Around line 33-39: The single-item allOf branch can push an undefined index
into refs.path because originalIndex may be unset; update the path construction
in parseAllOf.ts so it uses a safe fallback (e.g., originalIndex ?? 0 or typeof
originalIndex === "number" ? originalIndex : 0) instead of (item as
any)[originalIndex], e.g. set path: [...refs.path, "allOf", originalIndex ?? 0],
and ensure parseSchema is called with that non-undefined index to avoid broken
path metadata in parseSchema.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseAnyOf.ts`:
- Around line 10-17: The inner arrow function in parseAnyOf shadows the outer
parameter named schema; rename the inner parameter (e.g., to anyOfSchema or
subSchema) wherever it's used in the mapping to avoid confusion and improve
readability — update the map callback in parseAnyOf (currently using schema, i)
to use the new name and keep the existing refs.path update using that index
(parseSchema(anyOfSchema, {...refs, path: [...refs.path, "anyOf", i]})) so
behavior is unchanged.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseArray.ts`:
- Around line 12-14: The tuple branch in parseArray.ts currently returns early
for Array.isArray(schema.items), which omits applying minItems/maxItems (the
withMessage/.min/.max logic) and ignores schema.additionalItems; update the
Array.isArray(schema.items) handling in the parseArray function so that after
building the base z.tuple([...]) using parseSchema for each item (preserving
refs and path), you still apply the same .min/.max wrapping used for the
non-tuple branch and, if schema.additionalItems is present, translate it to
Zod's .rest(...) (parsing additionalItems with parseSchema and preserving
refs.path), rather than returning immediately. Ensure references to
schema.items, schema.minItems, schema.maxItems, schema.additionalItems,
parseSchema, refs/path, and the withMessage/.min/.max/.rest transformations are
used to guide the fix.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseConst.ts`:
- Around line 9-10: parseConst currently emits z.literal for any Serializable
value but z.literal only supports primitives; update parseConst to first check
whether schema.const is a primitive (string, number, boolean, or null) and only
return `z.literal(...)` for those cases; for non‑primitive values
(objects/arrays) return a safe fallback such as `z.unknown().refine(v =>
deepEqual(v, schema.const), { message: 'expected exact constant' })` or, if you
prefer not to add deep equality, use `z.unknown()` plus a comment and helper to
compare via JSON.stringify; reference the parseConst function and add or reuse a
deepEqual (or JSON stringify) helper to perform the equality check for
non‑primitives.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseEnum.ts`:
- Line 16: The return statement in parseEnum.ts builds a z.enum using
schema.enum.map(...) but relies on Array.toString(), producing inconsistent
spacing; change the generation to explicitly join the mapped values with ", "
(e.g., use schema.enum.map(...).join(", ")) so the returned string for
z.enum([...]) matches the z.union branch formatting and yields consistent
comma+space separation.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseIfThenElse.ts`:
- Around line 13-14: The TypeScript type for the IF/THEN/ELSE schema in
parseIfThenElse.ts incorrectly requires both branches; update the schema type
(the properties named then and else used in the parser, and any interface/type
exported from parseIfThenElse.ts) to make then?: JsonSchema and else?:
JsonSchema so that schemas with only if+then or if+else are correctly recognized
by parseSchema and handled by parseIfThenElse; ensure any code paths in
parseIfThenElse (e.g., checks inside parseIfThenElse function) account for the
branches being optionally undefined.
- Around line 27-34: The current generated schema uses z.union([${$then},
${$else}]) which causes double validation (base parse plus the superRefine
branch parse); change the returned template to use z.any() as the base so
superRefine is the only validation gate, i.e. return a schema starting with
z.any().superRefine(...) and keep the existing logic that calls
${$if}.safeParse(value) to pick ${$then} or ${$else} and forwards
result.error.errors via ctx.addIssue when !result.success; update the code in
parseIfThenElse.ts where the template string is built to replace
z.union([${$then}, ${$else}]) with z.any().
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseMultipleType.ts`:
- Around line 10-11: parseMultipleType currently builds z.union([]) when
schema.type is an empty array causing runtime errors; update parseMultipleType
to first guard schema.type length: if zero, return a safe fallback like
"z.any()" (or another project-appropriate fallback), if one, short-circuit and
return parseSchema({...schema, type: schema.type[0]}, {...refs, withoutDefaults:
true}), otherwise build the z.union from schema.type.map as before; keep
references to parseMultipleType, parseSchema and the refs.withoutDefaults
behavior so the change integrates with existing parsing logic.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseNot.ts`:
- Around line 10-14: The parseNot function currently inlines
parseSchema(schema.not, ...) inside the z.any().refine callback causing the
parsed Zod schema to be re-created on every validation; hoist the parsed result
by calling parseSchema once into a const (e.g., const notSchema =
parseSchema(schema.not, { ...refs, path: [...refs.path, "not"] })) and then use
notSchema.safeParse(value).success inside the refine predicate so the Zod tree
is stable and you avoid repeated instantiation.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseNumber.ts`:
- Around line 23-30: The current multipleOf handler in parseNumber.ts uses a
brittle r.startsWith("z.number().int(") check to detect if .int was already
emitted; replace this with an explicit boolean flag (e.g., emittedInt) that you
set when you append the ".int(" marker to r so the handler can check emittedInt
instead of string-prefix; update the code paths that append ".int(" (inside the
parseNumber logic where r is constructed) to set emittedInt = true and then in
the withMessage(schema, "multipleOf", ...) callback check emittedInt before
returning, ensuring the flag is in the same scope as both the construction and
the handler so formatting changes won’t break the logic.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseObject.ts`:
- Line 101: The template literal that builds patternProperties uses
Object.values(parsedPatternProperties) which implicitly joins multiple entries
into a comma-separated string; update the logic in parseObject.ts (around the
patternProperties construction) to explicitly use
Object.values(parsedPatternProperties)[0] (or otherwise assert/guard that
parsedPatternProperties has exactly one entry) when producing `.catchall(...)`
so the output is valid for the single-element expectation; adjust both
occurrences that currently use Object.values(parsedPatternProperties) (the lines
building `.catchall(${...})`) to reference the first element instead of the full
array.
- Around line 80-91: The map call in parseObject.ts passes an unnecessary
thisArg ({}), which is ignored by arrow functions; remove the third argument
from Object.entries(...).map so the mapping that builds parsedPatternProperties
(using parseSchema with refs.path [..., "patternProperties", key]) is called
with only the mapping function, keeping parsedPatternProperties and parseSchema
logic unchanged.
- Around line 17-24: The emitErrorPath helper currently returns `path: [key]`
for Zod v4 which drops parent context; change emitErrorPath(Refs) to reconstruct
the full path using refs.path for v4 (e.g. return something like `path:
[...refs.path, key]`) while keeping the v3 branch unchanged (still using
ctx.path for v3), so that addIssue receives the complete nested path; update any
callers that expect the string to remain the same if necessary.
- Around line 183-217: The code spreads the entire objectSchema into
parseAnyOf/parseOneOf/parseAllOf calls even though those parsers only need their
specific arrays; change the calls to pass only the mapped arrays (e.g., replace
{...objectSchema, anyOf: objectSchema.anyOf.map(...)} with { anyOf:
objectSchema.anyOf.map(...) }) and do the same for oneOf and allOf so
parseAnyOf/parseOneOf/parseAllOf receive only the required properties while
keeping the same mapping that injects type: "object" when needed.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseOneOf.ts`:
- Line 36: The generated superRefine in parseOneOf.ts is calling addIssue with a
non-existent ctx.path (Zod v4 removed this); remove the path: ctx.path property
from the addIssue call in the superRefine block inside parseOneOf so it relies
on Zod's default refinement path, and, if you need explicit path behavior, reuse
the emitErrorPath(refs) helper used in parseObject.ts instead of ctx.path;
update the code around the superRefine / addIssue call in parseOneOf.ts
accordingly.
- Around line 19-27: The inner callback parameter names shadow outer variables:
in the map that calls parseSchema and in the reduce that builds errors rename
the inner parameters (e.g., change .map((schema, i) => ...) to .map((subSchema,
i) => ...) and change .reduce((errors, schema) => ...) to .reduce((acc,
subSchema) => ...) so they no longer shadow the outer `schema` and `errors`;
adjust any uses inside those callbacks to the new names (references: parseSchema
call, the `schemas` array and the reduce callback in parseOneOf.ts).
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseSchema.ts`:
- Around line 177-183: The current type guard named conditional in
parseSchema.ts incorrectly requires all three branches (if, then, else); relax
it to treat `then` and `else` as optional when `if` exists (i.e., predicate
should assert x is JsonSchemaObject & { if: JsonSchema; then?: JsonSchema;
else?: JsonSchema }), and update the conditional-handling logic in the parser
(the branch that consumes conditional) to check for presence of x.then and
x.else before using them and to build Zod schemas accordingly (apply only the
present branch, or fallback to identity/no-op when a branch is missing). Ensure
all uses of the conditional guard (in parseSchema.ts) are updated to handle
optional then/else safely.
- Around line 98-129: The selectParser function currently checks its.an.object
and its.an.array before its.a.conditional, so conditional schemas on typed
objects/arrays are skipped; adjust the branch order in selectParser so that the
its.a.conditional check calls parseIfThenElse(schema, refs) before the object
and array checks (i.e., move the its.a.conditional block above the its.an.object
and its.an.array checks) to ensure conditional schemas are handled for typed
schemas.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/parsers/parseString.ts`:
- Around line 63-64: The condition in parseString.ts uses a non-strict
comparison for contentMediaType (if (contentMediaType != "")) which can cause
coercion edge-cases; update the check in the parseString function to use a
strict comparison (if (contentMediaType !== "")) or a truthy check (if
(contentMediaType)) before appending to r so contentMediaType is validated
safely.
- Around line 53-69: The generated Zod code for contentMediaType and
contentSchema currently wraps .transform and .pipe with withMessage (via
contentMediaType and the contentSchema branch), which produces an extra message
argument those Zod APIs don't accept; change the generation so you emit a plain
.transform(...) when contentMediaType === "application/json" and a plain
.pipe(${parseSchema(value)}) for contentSchema instead of using withMessage, and
inside the transform handler call ctx.addIssue(...) then return z.NEVER (not
undefined) to ensure deterministic validation failure; update the logic in
parseString.ts where contentMediaType, contentSchema, withMessage, parseSchema
and r are used to produce those snippets accordingly.
- Around line 34-36: The switch branch for format "binary" in parseString.ts
currently returns ".base64(" which can double-encode when schema.contentEncoding
=== "base64" and misrepresents binary as an encoded string; update the case
"binary" handling in the parseString function so it does not unconditionally
return .base64 — instead check schema.contentEncoding and: if contentEncoding
=== "base64" emit no extra .base64 wrapper (let the contentEncoding path handle
decoding), otherwise treat "binary" as raw bytes (do not map to .base64) or map
to an explicit bytes/Uint8Array representation used elsewhere; modify only the
case "binary" logic to consult schema.contentEncoding and return the appropriate
tokens instead of always ".base64(" / ")".
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/Types.ts`:
- Around line 22-70: The JsonSchemaObject type is missing common JSON Schema
metadata fields which forces unsafe casts; update the JsonSchemaObject
definition to include optional metadata properties like $id?: string, $schema?:
string, $ref?: string, title?: string, description?: string, default?:
Serializable, examples?: Serializable[], deprecated?: boolean, readOnly?:
boolean, writeOnly?: boolean, nullable?: boolean, $comment?: string, $defs?:
{[key: string]: JsonSchema}, definitions?: {[key: string]: JsonSchema},
contentMediaType?: string, contentEncoding?: string (and any other standard
metadata your project needs) so parsers can rely on typed access to these fields
(modify the JsonSchemaObject type in Types.ts where JsonSchemaObject is declared
and update call sites to remove unnecessary casts).
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/cliTools.ts`:
- Around line 46-67: The parseArgs function currently calls process.exit(0)
inside the help branch and blindly consumes args[index + 1] as a flag value;
change this so parseArgs no longer exits the process but instead returns a
sentinel/help result or throws a specific HelpRequested exception (so tests can
assert behavior) and update the flag-value parsing to validate that the next
token exists and does not start with '-' before consuming it (otherwise treat
the option as boolean/absent or raise a clear parse error). Update references in
the same function where printParams is used to produce help output, and ensure
any callers of parseArgs handle the new return/exception behavior.
- Around line 77-84: The branch handling missing args sets result[name] = false
for every optional param; change it so non-boolean optionals get undefined while
only boolean optionals get false. In the index === -1 block (referencing
variables index, required, result[name] and the argument descriptor's value
property), keep the existing required-throw logic, then set result[name] =
(argDescriptor.value === "boolean" ? false : undefined) (or equivalent
TypeScript-safe check) instead of always assigning false so optional
string/number params are undefined.
- Around line 9-13: The Param type's union allows an object shape `{[key:
number]: string}` but the runtime uses Array.isArray(value) and
value.includes(...), so update the type to use string[] instead of `{[key:
number]: string}` to match the runtime; specifically change the union branch in
the exported type Param (the branch currently `{value: {[key: number]:
string}}`) to `{value: string[]}` so the type of value aligns with the
Array.isArray(value) and value.includes(...) checks.
In `@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/jsdocs.ts`:
- Around line 9-13: The single-line JSDoc produced by expandJsdocs currently
emits `/**hello*/`; update expandJsdocs so when jsdocs.split("\n") yields a
single line it wraps the content with leading and trailing spaces inside the
comment (emit `/** hello */`) while preserving the existing multi-line
formatting; specifically adjust the single-line branch in expandJsdocs to insert
a space before and after lines[0] when building the returned string.
In
`@packages/platform/platform-mcp/src/utils/json-schema-to-zod/utils/withMessage.ts`:
- Around line 33-37: Remove the dead no-op statement "r;" inside the withMessage
logic: locate the withMessage function (file withMessage.ts) where variables r,
schema, key and prefix/closer are used and delete the standalone "r;" line so
the concatenation flow (adding prefix + JSON.stringify(schema.errorMessage[key])
and then r += closer) is uninterrupted; ensure no other side-effects rely on
that no-op.
In `@packages/platform/platform-mcp/src/utils/toZod.spec.ts`:
- Around line 1-5: The test file toZod.spec.ts is missing explicit Vitest
imports required by the linter; update the top of the file to import Vitest
globals explicitly (e.g., import { describe, it, expect, beforeEach } from
"vitest" as needed) so that the existing describe(...) and any other test
helpers are explicitly imported, leaving the existing imports of s, string and
toZod unchanged.
In `@packages/platform/platform-mcp/src/utils/toZod.ts`:
- Line 7: Replace the unsafe eval call in toZod.ts with a sandboxed Function
constructor so generated code cannot capture surrounding closure; instead of
eval(`(z) => ${jsonSchemaToZod(schema.toJSON(), {zodVersion: 4})}`)(z) create a
new Function that returns the zod-converter and invoke it with the z parameter
(use jsonSchemaToZod(...) output as the function body), ensuring you do not
interpolate surrounding variables into the generated code so the replacement
prevents access to process/imports/local scope while preserving the existing
behavior of the toZod helper.
In `@packages/platform/platform-mcp/test/app/resources/TestResource.ts`:
- Around line 6-20: The TestResource class is not registered with the DI
container; add the `@Injectable`() decorator to the TestResource class so the DI
system can discover it during tests; locate the class declaration "export class
TestResource" and annotate it with `@Injectable`() (keeping existing decorators
like `@Resource`, `@Title`, `@Description` on the test method) so the DI container can
instantiate TestResource for integration tests.
In `@packages/platform/platform-mcp/test/mcp.integration.spec.ts`:
- Around line 28-37: Merge the two duplicate beforeEach hooks into a single
async beforeEach that first awaits the test bootstrap (call utils.bootstrap({
mcp: { path: "/mcp" } })) and then initializes the SuperTest request (assign
request = SuperTest(PlatformTest.callback())); ensure you preserve existing
setup order and use async/await so the platform is bootstrapped before calling
PlatformTest.callback().
- Around line 1-5: The test file uses Vitest globals but doesn't import them; at
the top of packages/platform/platform-mcp/test/mcp.integration.spec.ts add
explicit imports from "vitest" (e.g., describe, it/test, expect, beforeAll,
afterAll, vi as needed) so the Vitest ESLint plugin rules are satisfied; ensure
the imported symbols match those used in the file (replace any implicit globals
with the imported identifiers) and keep the existing
PlatformExpress/PlatformTest/PlatformTestSdk/SuperTest imports intact.
- Around line 102-118: The inline snapshot in the "should return all resources"
test asserts an internal field propertyKey that should not be part of the public
MCP response; update the test (the one calling sendMcpRequest and asserting
response.body) to remove/ignore propertyKey before snapshotting — e.g. iterate
response.body.result.resources and delete or omit propertyKey (or map to a shape
without propertyKey) so the toMatchInlineSnapshot assertion only contains public
fields like name, title, description, uri; keep the test name and use the same
response object (response.body) but normalize resources to exclude propertyKey
prior to matching.
In `@packages/specs/schema/src/decorators/operations/returns.ts`:
- Around line 621-630: The overloads accept a model as either a class or an
array-of-classes (Type<any> | Type<any>[]), but the implementation only tests
isClass(status) so arrays and primitive constructors (e.g. String, Number) get
misinterpreted as the HTTP status; update the Returns(...) implementation to
detect "model-like" values for the first argument (e.g. isClass(status) ||
Array.isArray(status) || a primitive constructor/function check) and then set
status: isModelLike(status) ? 200 : status and model: isModelLike(status) ?
status : model (or create a small helper like isModelLike to encapsulate the
check), referencing the Returns function and ReturnDecoratorContext so arrays
and primitive constructors declared by the overloads are correctly treated as
the model.
Comment on lines
+8
to
+12
| Archive multiple completed changes in a single operation. | ||
|
|
||
| This skill allows you to batch-archive changes, handling spec conflicts intelligently by checking the codebase to determine what's actually implemented. | ||
|
|
||
| **Input**: None required (prompts for selection) |
Contributor
There was a problem hiding this comment.
Add a top-level heading after the frontmatter.
markdownlint MD041 flags the first non-frontmatter line as not being a heading. Adding an H1 improves scan-ability and clears linting.
💡 Suggested update
---
name: "OPSX: Bulk Archive"
description: Archive multiple completed changes at once
category: Workflow
tags: [workflow, archive, experimental, bulk]
---
+# OPSX: Bulk Archive
+
Archive multiple completed changes in a single operation.📝 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
| Archive multiple completed changes in a single operation. | |
| This skill allows you to batch-archive changes, handling spec conflicts intelligently by checking the codebase to determine what's actually implemented. | |
| **Input**: None required (prompts for selection) | |
| --- | |
| name: "OPSX: Bulk Archive" | |
| description: Archive multiple completed changes at once | |
| category: Workflow | |
| tags: [workflow, archive, experimental, bulk] | |
| --- | |
| # OPSX: Bulk Archive | |
| Archive multiple completed changes in a single operation. | |
| This skill allows you to batch-archive changes, handling spec conflicts intelligently by checking the codebase to determine what's actually implemented. | |
| **Input**: None required (prompts for selection) |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 8-8: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/opsx/bulk-archive.md around lines 8 - 12, Add a top-level
H1 heading immediately after the frontmatter to satisfy markdownlint MD041;
locate the markdown whose first non-frontmatter line starts with "Archive
multiple completed changes in a single operation." and insert a concise H1
(e.g., "# Bulk Archive Changes") above that paragraph so the first
non-frontmatter line is a heading and improves scan-ability.
Comment on lines
+85
to
+90
| After completing all artifacts, summarize: | ||
|
|
||
| - Change name and location | ||
| - List of artifacts created with brief descriptions | ||
| - What's ready: "All artifacts created! Ready for implementation." | ||
| - Prompt: "Run `/opsx:apply` to start implementing." |
Contributor
There was a problem hiding this comment.
Avoid claiming “all artifacts created” when stopping at applyRequires.
The flow only guarantees apply‑required artifacts. Reword to avoid misleading output.
♻️ Proposed edit
-After completing all artifacts, summarize:
+After completing all required artifacts (apply-ready), summarize:
@@
-- What's ready: "All artifacts created! Ready for implementation."
+- What's ready: "Required artifacts created (apply-ready). Ready for implementation."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/opsx/ff.md around lines 85 - 90, The summary text currently
claims "All artifacts created! Ready for implementation." and prompts "Run
`/opsx:apply` to start implementing." but the flow only guarantees applyRequired
artifacts (applyRequires); change the copy to accurately reflect that only
apply-required artifacts were produced (e.g., "All apply-required artifacts
created. Ready for apply.") and update the final prompt to something like "Run
`/opsx:apply` to begin applying the generated artifacts." Ensure you reference
and mention applyRequires in the message so it’s clear the guarantee scope is
limited to apply-required artifacts.
Comment on lines
+8
to
+79
| Start a new change using the experimental artifact-driven approach. | ||
|
|
||
| **Input**: The argument after `/opsx:new` is the change name (kebab-case), OR a description of what the user wants to build. | ||
|
|
||
| **Steps** | ||
|
|
||
| 1. **If no input provided, ask what they want to build** | ||
|
|
||
| Use the **AskUserQuestion tool** (open-ended, no preset options) to ask: | ||
|
|
||
| > "What change do you want to work on? Describe what you want to build or fix." | ||
|
|
||
| From their description, derive a kebab-case name (e.g., "add user authentication" → `add-user-auth`). | ||
|
|
||
| **IMPORTANT**: Do NOT proceed without understanding what the user wants to build. | ||
|
|
||
| 2. **Determine the workflow schema** | ||
|
|
||
| Use the default schema (omit `--schema`) unless the user explicitly requests a different workflow. | ||
|
|
||
| **Use a different schema only if the user mentions:** | ||
|
|
||
| - A specific schema name → use `--schema <name>` | ||
| - "show workflows" or "what workflows" → run `openspec schemas --json` and let them choose | ||
|
|
||
| **Otherwise**: Omit `--schema` to use the default. | ||
|
|
||
| 3. **Create the change directory** | ||
|
|
||
| ```bash | ||
| openspec new change "<name>" | ||
| ``` | ||
|
|
||
| Add `--schema <name>` only if the user requested a specific workflow. | ||
| This creates a scaffolded change at `openspec/changes/<name>/` with the selected schema. | ||
|
|
||
| 4. **Show the artifact status** | ||
|
|
||
| ```bash | ||
| openspec status --change "<name>" | ||
| ``` | ||
|
|
||
| This shows which artifacts need to be created and which are ready (dependencies satisfied). | ||
|
|
||
| 5. **Get instructions for the first artifact** | ||
| The first artifact depends on the schema. Check the status output to find the first artifact with status "ready". | ||
|
|
||
| ```bash | ||
| openspec instructions <first-artifact-id> --change "<name>" | ||
| ``` | ||
|
|
||
| This outputs the template and context for creating the first artifact. | ||
|
|
||
| 6. **STOP and wait for user direction** | ||
|
|
||
| **Output** | ||
|
|
||
| After completing the steps, summarize: | ||
|
|
||
| - Change name and location | ||
| - Schema/workflow being used and its artifact sequence | ||
| - Current status (0/N artifacts complete) | ||
| - The template for the first artifact | ||
| - Prompt: "Ready to create the first artifact? Run `/opsx:continue` or just describe what this change is about and I'll draft it." | ||
|
|
||
| **Guardrails** | ||
|
|
||
| - Do NOT create any artifacts yet - just show the instructions | ||
| - Do NOT advance beyond showing the first artifact template | ||
| - If the name is invalid (not kebab-case), ask for a valid name | ||
| - If a change with that name already exists, suggest using `/opsx:continue` instead | ||
| - Pass --schema if using a non-default workflow |
Contributor
There was a problem hiding this comment.
Add a top-level heading after frontmatter.
Markdownlint flags the missing H1. Add a # heading after the frontmatter to satisfy the style rule.
✍️ Suggested edit
---
name: "OPSX: New"
@@
---
-Start a new change using the experimental artifact-driven approach.
+# OPSX: New
+
+Start a new change using the experimental artifact-driven approach.🧰 Tools
🪛 LanguageTool
[style] ~14-~14: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: .... If no input provided, ask what they want to build Use the **AskUserQuestion t...
(REP_WANT_TO_VB)
[style] ~18-~18: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ... you want to work on? Describe what you want to build or fix." From their descripti...
(REP_WANT_TO_VB)
[style] ~22-~22: You have already used this phrasing in nearby sentences. Consider replacing it to add variety to your writing.
Context: ...eed without understanding what the user wants to build. 2. **Determine the workflow sch...
(REP_WANT_TO_VB)
🪛 markdownlint-cli2 (0.21.0)
[warning] 12-12: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
[warning] 59-59: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/opsx/new.md around lines 8 - 79, The markdown is missing a
top-level H1 after the frontmatter; open .claude/commands/opsx/new.md, locate
the YAML frontmatter block (the leading --- ... ---) and insert a single H1 line
immediately after it (for example "# Start a new change using the experimental
artifact-driven approach") so the document has a top-level heading satisfying
markdownlint; keep the existing content and examples unchanged and ensure the H1
matches the document's intent.
Comment on lines
+1
to
+26
| ## Why | ||
|
|
||
| Platform MCP currently mixes sparse, outdated, or missing JSDoc across its classes and helper functions. This makes it harder for developers (and LLM tooling) to understand behavior, and it blocks our TSDoc-powered doc generation pipeline from producing accurate API reference pages for the docs site. | ||
|
|
||
| ## What Changes | ||
|
|
||
| - Add or refresh TSDoc-compliant JSDoc blocks for every publicly exported class, function, and factory under `packages/platform/platform-mcp/src`. | ||
| - Describe params, generics, return values, and important side-effects/usage notes so tsdoc -> markdown conversion can surface the right metadata. | ||
| - Align tags/formatting with Ts.ED documentation conventions (e.g., `@param`, `@returns`, `@example`, `@deprecated` when relevant) to maintain consistency across packages. | ||
| - Ensure no behavioral changes—code stays the same, only comments improve clarity. | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### New Capabilities | ||
|
|
||
| - `platform-mcp-jsdoc-coverage`: Guarantees that exported symbols in Platform MCP expose complete, standardized TSDoc so automated reference docs stay accurate and rich enough for IDE/LLM consumption. | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| - _None_ | ||
|
|
||
| ## Impact | ||
|
|
||
| - Code: `packages/platform/platform-mcp/src/**/*` (comments only; no functional logic touched). | ||
| - Tooling: TSDoc parser + docs build gain higher-fidelity metadata for Platform MCP APIs. | ||
| - Documentation: Generated markdown under VitePress `/docs` reflects the clarified descriptions and parameter details. |
Contributor
There was a problem hiding this comment.
Include affected specs and file.ts:42 code locations.
The Impact section doesn’t reference affected specs and uses a glob instead of file.ts:42 locations. Please list the relevant specs and replace globs with concrete file references.
As per coding guidelines, “Write proposal.md with sections for 'Why', 'What Changes', and 'Impact' including affected specs and code” and “Use file.ts:42 format for code locations; reference specs as specs/auth/spec.md.”
📄 Suggested Impact structure
## Impact
-- Code: `packages/platform/platform-mcp/src/**/*` (comments only; no functional logic touched).
+- Specs: `specs/platform-mcp-jsdoc-coverage/spec.md`
+- Code: `packages/platform/platform-mcp/src/index.ts:1` (comments only; no functional logic touched).
- Tooling: TSDoc parser + docs build gain higher-fidelity metadata for Platform MCP APIs.
- Documentation: Generated markdown under VitePress `/docs` reflects the clarified descriptions and parameter details.📝 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
| ## Why | |
| Platform MCP currently mixes sparse, outdated, or missing JSDoc across its classes and helper functions. This makes it harder for developers (and LLM tooling) to understand behavior, and it blocks our TSDoc-powered doc generation pipeline from producing accurate API reference pages for the docs site. | |
| ## What Changes | |
| - Add or refresh TSDoc-compliant JSDoc blocks for every publicly exported class, function, and factory under `packages/platform/platform-mcp/src`. | |
| - Describe params, generics, return values, and important side-effects/usage notes so tsdoc -> markdown conversion can surface the right metadata. | |
| - Align tags/formatting with Ts.ED documentation conventions (e.g., `@param`, `@returns`, `@example`, `@deprecated` when relevant) to maintain consistency across packages. | |
| - Ensure no behavioral changes—code stays the same, only comments improve clarity. | |
| ## Capabilities | |
| ### New Capabilities | |
| - `platform-mcp-jsdoc-coverage`: Guarantees that exported symbols in Platform MCP expose complete, standardized TSDoc so automated reference docs stay accurate and rich enough for IDE/LLM consumption. | |
| ### Modified Capabilities | |
| - _None_ | |
| ## Impact | |
| - Code: `packages/platform/platform-mcp/src/**/*` (comments only; no functional logic touched). | |
| - Tooling: TSDoc parser + docs build gain higher-fidelity metadata for Platform MCP APIs. | |
| - Documentation: Generated markdown under VitePress `/docs` reflects the clarified descriptions and parameter details. | |
| ## Why | |
| Platform MCP currently mixes sparse, outdated, or missing JSDoc across its classes and helper functions. This makes it harder for developers (and LLM tooling) to understand behavior, and it blocks our TSDoc-powered doc generation pipeline from producing accurate API reference pages for the docs site. | |
| ## What Changes | |
| - Add or refresh TSDoc-compliant JSDoc blocks for every publicly exported class, function, and factory under `packages/platform/platform-mcp/src`. | |
| - Describe params, generics, return values, and important side-effects/usage notes so tsdoc -> markdown conversion can surface the right metadata. | |
| - Align tags/formatting with Ts.ED documentation conventions (e.g., `@param`, `@returns`, `@example`, `@deprecated` when relevant) to maintain consistency across packages. | |
| - Ensure no behavioral changes—code stays the same, only comments improve clarity. | |
| ## Capabilities | |
| ### New Capabilities | |
| - `platform-mcp-jsdoc-coverage`: Guarantees that exported symbols in Platform MCP expose complete, standardized TSDoc so automated reference docs stay accurate and rich enough for IDE/LLM consumption. | |
| ### Modified Capabilities | |
| - _None_ | |
| ## Impact | |
| - Specs: `specs/platform-mcp-jsdoc-coverage/spec.md` | |
| - Code: `packages/platform/platform-mcp/src/index.ts:1` (comments only; no functional logic touched). | |
| - Tooling: TSDoc parser + docs build gain higher-fidelity metadata for Platform MCP APIs. | |
| - Documentation: Generated markdown under VitePress `/docs` reflects the clarified descriptions and parameter details. |
🧰 Tools
🪛 markdownlint-cli2 (0.21.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
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/improve-jsdoc-platform-mcp/proposal.md` around lines 1 - 26,
Update the Impact section of proposal.md to replace the glob with explicit file
references and spec links: list each affected source file under
packages/platform/platform-mcp/src using the file.ts:42 format for exact
locations (e.g., packages/platform/platform-mcp/src/someFile.ts:42) and
reference affected specs using spec-style paths (e.g., specs/auth/spec.md);
ensure the Impact section includes these concrete file and spec entries and
keeps the existing sections ('Why', 'What Changes', 'Impact') intact without
changing behavior claims.
Comment on lines
+1
to
+27
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: Exported Platform MCP APIs include canonical TSDoc | ||
|
|
||
| All publicly exported classes, decorators, factories, and helpers under `packages/platform/platform-mcp/src` SHALL declare a TSDoc-compliant block that summarizes behavior and lists every parameter, type parameter, and return value. | ||
|
|
||
| #### Scenario: Export lacks documentation | ||
|
|
||
| - **WHEN** a developer inspects any symbol exported from the Platform MCP barrel files | ||
| - **THEN** they SHALL see a TSDoc block that includes a summary line, `@module platform/mcp`, and structured tags for all inputs/outputs | ||
|
|
||
| ### Requirement: Metadata supports automated docs and LLM context | ||
|
|
||
| JSDoc SHALL include additional tags (e.g., `@since`, `@example`, `@deprecated`) whenever applicable so that the TSDoc → Markdown pipeline can surface versioning/usage guidance without manual edits. | ||
|
|
||
| #### Scenario: Feature introduces usage nuance | ||
|
|
||
| - **WHEN** a decorator or helper has prerequisites, side effects, or version constraints | ||
| - **THEN** its TSDoc SHALL document those details using the appropriate tags so downstream consumers and LLM tools receive the context | ||
|
|
||
| ### Requirement: Documentation build validates coverage | ||
|
|
||
| `yarn api:build` (or an equivalent scoped run) MUST succeed without TSDoc warnings for Platform MCP before the change is merged. | ||
|
|
||
| #### Scenario: Docs build runs after comment updates | ||
|
|
||
| - **WHEN** the documentation build executes following the JSDoc refresh |
Contributor
There was a problem hiding this comment.
Add missing delta sections to comply with spec format.
This delta spec only defines ## ADDED Requirements. Please add the required ## MODIFIED Requirements, ## REMOVED Requirements, and ## RENAMED Requirements sections (can be _None_ when empty) to match the mandated delta format.
As per coding guidelines, “Create spec deltas as specs/[capability]/spec.md files with ## ADDED Requirements, ## MODIFIED Requirements, ## REMOVED Requirements, and ## RENAMED Requirements sections.”
📄 Suggested structure
## ADDED Requirements
@@
- **THEN** it SHALL complete without emitting TSDoc errors or dropping Platform MCP symbols from the generated markdown
+
+## MODIFIED Requirements
+
+_None._
+
+## REMOVED Requirements
+
+_None._
+
+## RENAMED Requirements
+
+_None._🧰 Tools
🪛 markdownlint-cli2 (0.21.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
Verify each finding against the current code and only fix it if needed.
In
`@openspec/changes/improve-jsdoc-platform-mcp/specs/platform-mcp-jsdoc-coverage/spec.md`
around lines 1 - 27, The spec delta only contains "## ADDED Requirements";
update the document (the top-level headings in the file) to include the required
complementary sections by adding "## MODIFIED Requirements", "## REMOVED
Requirements", and "## RENAMED Requirements" (use the literal string "_None_"
under each section if there are no entries) so the file conforms to the mandated
spec delta format used by specs/platform-mcp-jsdoc-coverage/spec.md.
Comment on lines
+1
to
+2
| ## 1. Preparation | ||
|
|
Contributor
There was a problem hiding this comment.
Add a top-level heading to satisfy MD041.
Line 1 starts at H2, which triggers the “first line should be a top-level heading” lint rule.
✏️ Suggested fix
-## 1. Preparation
+# Platform MCP JSDoc tasks
+
+## 1. Preparation📝 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. Preparation | |
| # Platform MCP JSDoc tasks | |
| ## 1. Preparation | |
🧰 Tools
🪛 markdownlint-cli2 (0.21.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
Verify each finding against the current code and only fix it if needed.
In `@openspec/changes/improve-jsdoc-platform-mcp/tasks.md` around lines 1 - 2, The
file currently begins with a second-level heading "## 1. Preparation" which
triggers MD041; add a top-level heading above it (for example "# Tasks" or "# 1.
Preparation") so the document's first line is an H1, ensuring the existing "##
1. Preparation" remains unchanged and the lint rule is satisfied.
| * @module platform/mcp | ||
| * @since 8.17.0 | ||
| */ | ||
| export type PromptDecoratorOptions = Omit<PromptProps, "handler">; |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Omit<PromptProps, "handler"> on a union type is wider than intended.
PromptProps = FnPromptProps | ClassPromptProps. TypeScript distributes Omit across each union member, so PromptDecoratorOptions becomes Omit<FnPromptProps, "handler"> | Omit<ClassPromptProps, "handler">. This means a caller can satisfy the type with just {name: "foo"} (the first union member), without any class-based fields, even though this decorator always injects token/propertyKey. Consider scoping the type to Omit<ClassPromptProps, "handler" | "token" | "propertyKey"> for a tighter, clearer contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/platform/platform-mcp/src/decorators/prompt.ts` at line 11,
PromptDecoratorOptions is currently defined as Omit<PromptProps, "handler">
which distributes over the union PromptProps = FnPromptProps | ClassPromptProps
and allows callers to pass a FnPromptProps-shaped object; change the type to
scope it to class-shaped prompts by using Omit<ClassPromptProps, "handler" |
"token" | "propertyKey"> so the decorator contract reflects that it always
injects token and propertyKey and requires the class-specific fields; update the
type alias PromptDecoratorOptions accordingly and ensure any usages expecting
the narrower class form are adjusted to the new type.
Comment on lines
+1
to
+85
| ### Platform MCP — JSDoc Coverage Tracker (packages/platform/platform-mcp/src) | ||
|
|
||
| Generated: 2026-02-08 | ||
|
|
||
| Purpose | ||
|
|
||
| - Guarantee every exported symbol in `packages/platform/platform-mcp/src` ships with consistent, TSDoc-compliant documentation. | ||
| - Provide a single place to track coverage, validation state, and any remaining follow-up work. | ||
| - Mirror the "symbols only" rule used elsewhere: document exported symbols (types, functions, classes, constants) rather than their private members. | ||
|
|
||
| Rules | ||
|
|
||
| - Document in English; keep the first sentence under ~120 chars for IDE summaries. | ||
| - Allowed tags: `@module platform/mcp`, `@since`, `@deprecated`, `@public`, `@typeParam`, `@param`, `@returns`, `@see`. | ||
| - Prefer markdown headings (### Usage, ### Example) inside the description over the legacy `@example` tag to keep the docs parser happy. | ||
| - Do not describe interface properties inline—document the interface itself and rely on the generated reference for member details. | ||
| - Keep edits comment-only unless a signature genuinely needs to change. | ||
|
|
||
| ## Canonical TSDoc Block | ||
|
|
||
| ````ts | ||
| /** | ||
| * One-sentence summary that explains the symbol. | ||
| * | ||
| * Optional longer description with additional context or usage notes. | ||
| * | ||
| * ### Usage | ||
| * ```ts | ||
| * const client = await platformMcp.connect(); | ||
| * ``` | ||
| * | ||
| * @module platform/mcp | ||
| * @since 8.x | ||
| * @typeParam T Provide type param summaries when generics are exposed | ||
| * @param options Describe each parameter | ||
| * @returns Explain the resolved value | ||
| */ | ||
| ```` | ||
|
|
||
| ## Export Checklist | ||
|
|
||
| ### constants | ||
|
|
||
| - [x] `MCP_PROVIDER_TYPES` (constants/constants.ts) | ||
|
|
||
| ### decorators | ||
|
|
||
| - [x] `PromptDecoratorOptions`, `Prompt` (decorators/prompt.ts) | ||
| - [x] `ResourceDecoratorOptions`, `Resource` (decorators/resource.ts) | ||
| - [x] `Tool` (decorators/tool.ts) | ||
|
|
||
| ### fn | ||
|
|
||
| - [x] `PromptProps`, `PromptsSettings`, `definePrompt` (fn/definePrompt.ts) | ||
| - [x] `ResourceProps`, `defineResource` overloads (fn/defineResource.ts) | ||
| - [x] `ToolCallback`, `ClassToolProps`, `ToolProps`, `defineTool` (fn/defineTool.ts) | ||
|
|
||
| ### interfaces | ||
|
|
||
| - [x] `PlatformMcpSettings` (interfaces/PlatformMcpSettings.ts) | ||
|
|
||
| ### services | ||
|
|
||
| - [x] `MCP_SERVER`, `MCP_SERVER` type alias (services/McpServerFactory.ts) | ||
| - [x] `PlatformMcpModule` (services/PlatformMcpModule.ts) | ||
|
|
||
| ### utils | ||
|
|
||
| - [x] `toZod` (utils/toZod.ts) | ||
| - [x] `jsonSchemaToZod` + `default` export (utils/json-schema-to-zod/jsonSchemaToZod.ts & index.ts) | ||
| - [x] `parseSchema`, `its` (parsers/parseSchema.ts) | ||
| - [x] `parseAllOf`, `parseAnyOf`, `parseArray`, `parseBoolean`, `parseConst`, `parseDefault`, `parseEnum`, `parseIfThenElse`, `parseMultipleType`, `parseNot`, `parseNull`, `parseNullable`, `parseNumber`, `parseObject`, `parseOneOf`, `parseString` (parsers/\*) | ||
| - [x] `Serializable`, `JsonSchema`, `JsonSchemaObject`, `ParserSelector`, `ParserOverride`, `Options`, `Refs`, `ZodVersion` (Types.ts) | ||
| - [x] `expandJsdocs`, `addJsdocs` (utils/jsdocs.ts) | ||
| - [x] `withMessage` (utils/withMessage.ts) | ||
| - [x] `half` (utils/half.ts) | ||
| - [x] `omit` (utils/omit.ts) | ||
| - [x] `parseArgs`, `parseOrReadJSON`, `readPipe`, `printParams`, `Param`, `Params` (utils/cliTools.ts) | ||
| - [x] `cli` entrypoint (utils/json-schema-to-zod/cli.ts) | ||
| - [x] `toZod` dependency surface validation (utils/toZod.ts) | ||
|
|
||
| ### misc | ||
|
|
||
| - [x] Ensure `index.ts` exports remain documented indirectly through their source files. | ||
| - [x] Update `reports/jsdoc/platform-mcp.md` after each documentation batch and log validation commands. |
Contributor
There was a problem hiding this comment.
Move this tracker into an approved JSDoc report file.
reports/jsdoc/platform-mcp.md isn’t an allowed tracker filename per repo rules, so this should be merged into one of the permitted reports or the guideline updated.
As per coding guidelines, "Package-specific JSDoc coverage progress reports must live under reports/jsdoc/ (core.md, di.md, hooks.md, json-mapper.md, schema.md)".
🧰 Tools
🪛 LanguageTool
[uncategorized] ~15-~15: Did you mean the formatting language “Markdown” (= proper noun)?
Context: ... @param, @returns, @see. - Prefer markdown headings (### Usage, ### Example) insid...
(MARKDOWN_NNP)
🪛 markdownlint-cli2 (0.21.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
Verify each finding against the current code and only fix it if needed.
In `@reports/jsdoc/platform-mcp.md` around lines 1 - 85, The tracker file
reports/jsdoc/platform-mcp.md is not an allowed JSDoc tracker filename; move its
contents into one of the approved reports under reports/jsdoc/ (e.g., append as
a new section in core.md, di.md, hooks.md, json-mapper.md, or schema.md) and
remove reports/jsdoc/platform-mcp.md; ensure the section retains the "Platform
MCP — JSDoc Coverage Tracker" heading and the export checklist (symbols like
MCP_PROVIDER_TYPES, PromptDecoratorOptions, toZod, parseSchema,
PlatformMcpSettings, PlatformMcpModule, etc.) so reviewers can find entries, and
update any README/CI references or cross-links that pointed to
reports/jsdoc/platform-mcp.md (or alternatively update the repo guideline to
permit this filename if you intend to keep it).
Romakita
force-pushed
the
feat-platform-mcp
branch
2 times, most recently
from
April 1, 2026 06:04
8ec2311 to
11742df
Compare
Romakita
force-pushed
the
feat-platform-mcp
branch
from
April 1, 2026 06:11
31c659c to
4f6913c
Compare
Romakita
force-pushed
the
feat-platform-mcp
branch
from
April 2, 2026 01:22
4f6913c to
6cd9aa5
Compare
Collaborator
Author
|
🎉 This PR is included in version 8.26.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
This was referenced Jul 21, 2026
Merged
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
@tsed/platform-mcppackage to expose Model Context Protocol (MCP) endpoints in Ts.ED applications.@Tool,@Prompt,@Resource) for defining MCP assets within services./mcpHTTP endpoint supporting MCP protocol requests.Documentation