feat: add base resource tools #1641
Conversation
WalkthroughAdds MCP resource management and discovery infrastructure, new base tools (discover/read/search/sequentialThinking), extensive design-doc resources with per-section templates, an EditPageSchema tool suite for editing page schema sections, and integrates resources/tools into MCP state and DesignCanvas exports. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as App
participant MCP as MCP Core
participant Tools as initRegisterTools
participant Res as initRegisterResources
participant Server as McpServer
App->>MCP: createServer(state)
MCP->>MCP: collectTools() + collectResources()
MCP->>Server: instantiate server
MCP->>Tools: initRegisterTools(state, server)
Tools->>Server: registerTool(name, config, cb)
MCP->>Res: initRegisterResources(state, server)
Res->>Server: registerResource / registerResourceTemplate
Res->>Server: sendResourceListChanged()
MCP-->>App: server + APIs ready (tools + resource APIs)
sequenceDiagram
autonumber
participant Client as Client Tool Call
participant ReadTool as read_resources
participant Utils as readResourceContent
participant Remote as mcpClient
Client->>ReadTool: { uri | uriTemplate+variables }
alt uriTemplate mode
ReadTool->>ReadTool: resolve final URI
end
ReadTool->>Utils: readResourceContent(uri, limits)
Utils->>Remote: readResource(uri)
alt success (textual)
Utils->>Utils: validate + truncate (if needed)
Utils-->>ReadTool: { ok, contents, truncated? }
ReadTool-->>Client: JSON payload (status, data)
else failure
Utils-->>ReadTool: { ok: false, error }
ReadTool-->>Client: JSON error payload
end
sequenceDiagram
autonumber
participant Client as Client Tool Call
participant Search as search_resources
participant Lists as tryFetchRemoteLists
participant Read as readResourceWithFallback
participant Remote as mcpClient
Client->>Search: { query, scope, filters }
Search->>Lists: fetch resources/templates
alt scope includes content
loop for candidate resources
Search->>Read: readResourceWithFallback(uri)
Read->>Remote: readResource(uri)
Read-->>Search: contents or error
Search->>Search: score by content/snippet
end
else metadata-only
Search->>Search: score by metadata only
end
Search-->>Client: topK results (score, snippet?, metadata)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
enhancement
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (28)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts (3)
149-154: Potential string concatenation issue with newline handling.The CSS append logic checks if the base ends with
\n, but this might miss other scenarios like carriage returns or empty strings with whitespace.Consider normalizing whitespace handling:
const computeAppendedCss = (oldCss: string, incoming: string) => { const base = typeof oldCss === 'string' ? oldCss : '' const add = typeof incoming === 'string' ? incoming : '' if (!add) return base - const sep = base && !base.endsWith('\n') ? '\n' : '' + const sep = base && !/[\r\n]$/.test(base) ? '\n' : '' return `${base}${sep}${add}` }
249-272: Consider extracting common patterns for ignored operations warnings.The logic for handling ignored add/update operations is duplicated across map sections and state handling. This could be extracted into a shared helper.
Consider creating a helper function:
const processIgnoredOperations = ( ignored: { add: string[], update: string[] }, warnings: string[] ) => { if (ignored.add.length) warnings.push(`ignored add (already exists): ${ignored.add.join(', ')}`) if (ignored.update.length) warnings.push(`ignored update (not exists): ${ignored.update.join(', ')}`) }
351-361: Consider making allowed schema keys configurable.The hardcoded
ALLOWED_SCHEMA_KEYSset might need updates as the schema evolves. Consider loading this from a configuration or schema definition.Would you like me to help create a configuration-based approach for managing allowed schema keys that can be maintained separately from the tool implementation?
packages/common/composable/mcp/toolUtils.ts (2)
105-142: Return registered instances and avoid JSON-only cloning; improve logs.Use structuredClone when available (keeps non-JSON data), continue loop instead of return inside forEach, and return the list for callers. Also include tool name in error logs.
-export const initRegisterTools = (state: IState, server: McpServer) => { - state.toolList.forEach((tool) => { +export const initRegisterTools = (state: IState, server: McpServer) => { + const registered: RegisteredTool[] = [] + for (const tool of state.toolList) { const { name, callback, inputSchema, outputSchema, ...restConfig } = tool - try { if (state.toolInstanceMap.has(name)) { logger.error(`tool ${name} already registered`) - return + continue } if (!name || typeof name !== 'string') { logger.error('tool name is required and must be a string') - return + continue } if (!callback || typeof callback !== 'function') { logger.error('tool callback is required and must be a function') - return + continue } - const toolInstance = server.registerTool( + const plainRestConfig = + typeof (globalThis as any).structuredClone === 'function' + ? (structuredClone as any)(restConfig) + : JSON.parse(JSON.stringify(restConfig)) + const toolInstance = server.registerTool( name, - // 需要序列化一次，否则 list tool 会超时，因为有 proxy 之后，内部会报错 - { - ...JSON.parse(JSON.stringify(restConfig)), + { + ...plainRestConfig, inputSchema, outputSchema }, callback as ToolCallback<ZodRawShape> ) state.toolInstanceMap.set(name, toolInstance) + registered.push(toolInstance) } catch (error) { - logger.error('error when register tool', error) + logger.error(`error when register tool "${name}"`, error) } - }) + } + return registered }
47-70: Parity with initRegisterTools: consider cloning here too.
registerToolspassesrestConfigdirectly;initRegisterToolsdeep-clones to avoid proxy issues. Align both to prevent inconsistent behavior when different code paths are used.packages/common/composable/mcp/baseTools/readResources.ts (3)
5-41: Enforce XOR of uri vs uriTemplate and validate required variables at schema level.Today both/none fall through to a generic error. Add Zod superRefine to catch invalid combinations and missing placeholders early with actionable messages.
-const inputSchema = z.object({ +const inputSchema = z + .object({ uri: z .string() .min(1) .optional() .describe( '目标资源的完整URI（与 uriTemplate 二选一）。用于读取已确定的具体资源全部内容。适合从 discover_resources 或 search_resources 获得URI后的完整读取。' ), uriTemplate: z .string() .min(1) .optional() .describe( '资源模板URI（与 uri 二选一）。配合 variables 参数使用，可实现参数化内容读取。适合读取文档特定章节、API特定接口等分段内容，有效控制内容体积。' ), variables: z .record(z.string(), z.string()) .optional() .describe( '模板变量键值对（仅当使用 uriTemplate 时必需）。用于替换模板中的占位符生成最终URI。例如：{"section": "api", "version": "v1"}。变量值会进行URL编码确保安全性。' ), maxBytes: z .number() .int() .min(10_000) .max(1_000_000) .optional() .describe( '内容读取字节上限（10k-1000k，默认200k）。用于防止读取超大文件导致性能问题。建议根据用途设置：快速预览用50k-100k，详细分析用200k-500k，完整处理用1000k。' ), truncate: z .boolean() .optional() .describe( '超出大小限制时是否允许截断（默认true）。true时返回截断内容并标记truncated=true；false时超限将报错。建议大多数场景保持true，避免因文件过大导致读取失败。' ) -}) + }) + .superRefine((val, ctx) => { + const hasUri = !!val.uri + const hasTpl = !!val.uriTemplate + if (hasUri === hasTpl) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['uri'], + message: 'Exactly one of "uri" or "uriTemplate" is required.' + }) + } + if (hasTpl) { + const placeholders = (val.uriTemplate!.match(/\{(.+?)\}/g) || []).map((m) => m.slice(1, -1)) + const missing = placeholders.filter((k) => !val.variables || !val.variables[k]) + if (missing.length) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['variables'], + message: `Missing template variables: ${missing.join(', ')}` + }) + } + } + })
106-153: Differentiate invalid-argument errors and short‑circuit early.When both/none of uri and uriTemplate are provided, return a clear
invalid_argumentserror instead of a generic failure.- try { - if (params.uri && !params.uriTemplate) { + try { + const both = !!params.uri && !!params.uriTemplate + const none = !params.uri && !params.uriTemplate + if (both || none) { + return errorContent('invalid_arguments') + } + if (params.uri && !params.uriTemplate) { const result = await readByUri(params.uri) @@ - return errorContent('read_resources_failed') + return errorContent('invalid_arguments') } catch { return errorContent('read_resources_failed') }
45-90: Consider returning JSON content type instead of JSON-in-text.If your MCP stack supports a
jsoncontent type, prefer structured responses to avoid double-encoding and downstream parsing.packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts (1)
64-76: Throw when section is not found to avoid returning empty markdown.
pickSectionByHeadingreturns''on miss, which currently yields an empty document instead of a clear error.- const text = pickSectionByHeading(editExamplesMd, heading) + const text = pickSectionByHeading(editExamplesMd, heading) + if (!text.trim()) { + throw new Error('resource_not_found') + }packages/common/composable/mcp/type.ts (1)
73-103: Tighten typings for annotations and mimeType (optional).Consider constraining
annotations.priorityto [0,1] andmimeTypeto known textual types to prevent accidental binary resources flowing into text-only tools.packages/common/composable/mcp/baseTools/discoverResources.ts (4)
166-170: Make MIME-type matching case-insensitive (and keep prefix support).Upstreams sometimes send mixed-case MIME types. Lowercase both sides to avoid misses.
Apply:
- if (!item?.mimeType) return false - return item.mimeType === mimeType || item.mimeType.startsWith(`${mimeType}`) + if (!item?.mimeType) return false + const itemMt = item.mimeType.toLowerCase() + const reqMt = mimeType.toLowerCase() + return itemMt === reqMt || itemMt.startsWith(reqMt)
128-157: Include explicit type on returned items to disambiguate resources vs templates.Downstream consumers won’t need to infer by presence of uri/uriTemplate.
Apply:
- type Entry = Omit<ResourceItem, 'readCallback'> | Omit<ResourceTemplateItem, 'readTemplateCallback'> + type Entry = + | (Omit<ResourceItem, 'readCallback'> & { type: 'resource' }) + | (Omit<ResourceTemplateItem, 'readTemplateCallback'> & { type: 'resource_template' }) @@ - entries.push({ + entries.push({ + type: 'resource', uri: resourceItem.uri, name: resourceItem.name || '', @@ - entries.push({ + entries.push({ + type: 'resource_template', uriTemplate: templateItem.uriTemplate, name: templateItem.name,
98-101: Expose an outputSchema for better contract discovery.Helps clients programmatically understand the payload shape.
Apply:
inputSchema: inputSchema.shape, + outputSchema: { + content: z.any().optional() + },
118-126: Minor readability nit: collapse ok/!ok branches.No behavior change; simpler flow.
Apply:
- if (ok) { - resources = rr - templates = rt - } - if (!ok) { - resources = state?.resources || [] - templates = state?.resourceTemplates || [] - } + resources = ok ? rr : state?.resources || [] + templates = ok ? rt : state?.resourceTemplates || []packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts (1)
62-80: Error when a mapped section isn’t found instead of returning empty content.Empty text is ambiguous; fail fast with a clear error.
Apply:
- const text = pickSectionByHeading(pageSchemaMd, heading) + const text = pickSectionByHeading(pageSchemaMd, heading) + if (!text) { + throw new Error(`Section not found: ${heading}`) + }packages/common/composable/mcp/baseTools/searchResources.ts (5)
196-206: Case-insensitive MIME-type comparison.Match behavior in discover tool and avoid misses.
Apply:
- return mimeTypeItem === mimeType || mimeTypeItem.startsWith(`${mimeType}`) + const itemMt = mimeTypeItem.toLowerCase() + const reqMt = mimeType.toLowerCase() + return itemMt === reqMt || itemMt.startsWith(reqMt)
299-301: Simplify textual check and make it case-insensitive.The equality check is redundant; handle mixed-case.
Apply:
- const isTextual = (mimeTypeItem?: string) => - mimeTypeItem ? mimeTypeItem.startsWith('text/') || mimeTypeItem === 'text/markdown' : true + const isTextual = (mimeTypeItem?: string) => { + if (!mimeTypeItem) return true + const mt = mimeTypeItem.toLowerCase() + return mt.startsWith('text/') + }
320-321: Optional: allow template content search when feasible.Some templates embed explanatory text; skipping them may hide relevant hits. Guard by size and a flag.
327-346: Concurrency loop is fine in JS, but make intent explicit.Single-threaded idx++ is safe; adding a brief comment avoids future “race” second-guessing.
352-354: Clamp or document priority range.If annotations.priority > 1, weighting can dominate. Consider clamping to [0,1] or documenting expected range.
- const priority = typeof candidateItem?.annotations?.priority === 'number' ? candidateItem.annotations.priority : 0 - candidateItem.score = candidateItem.score * (1 + priority * 0.5) + const raw = typeof candidateItem?.annotations?.priority === 'number' ? candidateItem.annotations.priority : 0 + const priority = Math.max(0, Math.min(1, raw)) + candidateItem.score = candidateItem.score * (1 + priority * 0.5)packages/common/composable/mcp/index.ts (1)
214-233: Deduplicate resources/templates during collection.Prevents duplicate registrations if multiple metas declare the same URI/uriTemplate.
Apply:
-const collectResources = (state: IState) => { +const collectResources = (state: IState) => { const allMetaData = getAllMergeMeta() - const resources: ResourceItem[] = [] - const resourceTemplates: ResourceTemplateItem[] = [] + const resources: ResourceItem[] = [] + const resourceTemplates: ResourceTemplateItem[] = [] + const seenUris = new Set<string>() + const seenTemplates = new Set<string>() @@ - if (meta && typeof meta === 'object' && Array.isArray(meta.mcp?.resources)) { - resources.push(...meta.mcp.resources) - } - if (meta && typeof meta === 'object' && Array.isArray(meta.mcp?.resourceTemplates)) { - resourceTemplates.push(...meta.mcp.resourceTemplates) - } + if (meta && typeof meta === 'object' && Array.isArray(meta.mcp?.resources)) { + for (const r of meta.mcp.resources) { + if (r?.uri && !seenUris.has(r.uri)) { + seenUris.add(r.uri) + resources.push(r) + } + } + } + if (meta && typeof meta === 'object' && Array.isArray(meta.mcp?.resourceTemplates)) { + for (const t of meta.mcp.resourceTemplates) { + if (t?.uriTemplate && !seenTemplates.has(t.uriTemplate)) { + seenTemplates.add(t.uriTemplate) + resourceTemplates.push(t) + } + } + }packages/common/composable/mcp/resources.ts (4)
19-25: Explicit return type and simpler init for ensureResourceMap.Add a concrete return type and use nullish assignment for brevity.
Apply this diff:
-const ensureResourceMap = (state: IState) => { - if (!state.resourceInstanceMap) { - state.resourceInstanceMap = new Map<string, RegisteredResource>() - } - return state.resourceInstanceMap -} +const ensureResourceMap = (state: IState): Map<string, RegisteredResource> => { + state.resourceInstanceMap ??= new Map<string, RegisteredResource>() + return state.resourceInstanceMap +}
28-36: Avoid JSON stringify clone for metadata.JSON cloning strips non‑JSON values and is slower. Prefer structuredClone with a JSON fallback.
Apply this diff:
- return JSON.parse(JSON.stringify(meta)) + return typeof structuredClone === 'function' ? structuredClone(meta) : JSON.parse(JSON.stringify(meta))
39-45: Make return type consistent; don’t return void on early exits.Currently the function returns either an array or undefined. Declare the return type and always return an array.
Apply this diff:
-export const registerResources = (state: IState, items: ResourceItem[]) => { - if (!Array.isArray(items) || !items.length) return +export const registerResources = (state: IState, items: ResourceItem[]): Array<RegisteredResource | undefined> => { + if (!Array.isArray(items) || !items.length) return [] if (!state.server) { logger.error('mcp server is not created') - return + return [] } @@ - return instances + return instancesAlso applies to: 77-78
191-205: ResourceTemplate construction and metadata typing.Passing { list: undefined } is odd and may rely on internal details. Also avoid any casts for variables fields.
Apply this diff and confirm with your SDK version:
- const template = resourceItem.template || new ResourceTemplate(resourceItem.uriTemplate, { list: undefined }) + const template = resourceItem.template || new ResourceTemplate(resourceItem.uriTemplate) @@ - { - title: resourceItem.title, - description: resourceItem.description, - mimeType: resourceItem.mimeType, - annotations: toRaw(resourceItem.annotations), - // 将 variables 与 variablesSchemaUri 作为元数据透传，便于远端 listResourceTemplates 返回 - variables: toRaw((resourceItem as any).variables), - variablesSchemaUri: toRaw((resourceItem as any).variablesSchemaUri) - }, + { + title: resourceItem.title, + description: resourceItem.description, + mimeType: resourceItem.mimeType, + annotations: toRaw(resourceItem.annotations), + variables: toRaw(resourceItem.variables), + variablesSchemaUri: resourceItem.variablesSchemaUri + },If the constructor signature differs in your SDK, adjust accordingly.
packages/common/composable/mcp/baseTools/utils.ts (3)
21-38: ok flag semantics are too optimistic.You set ok = true even if both list calls failed (due to per‑call catch). Set ok only if at least one succeeded.
Apply this diff:
- result.ok = true + result.ok = Boolean(resourcesList || resourceTemplatesList)
62-66: Avoid cutting multibyte UTF‑8 sequences mid‑codepoint.Current slice can produce replacement chars. Trim to a valid boundary.
Apply this diff:
export const truncateTextToBytes = (text: string, limit: number): string => { - const enc = new TextEncoder().encode(text) - const sliced = enc.slice(0, limit) - return new TextDecoder('utf-8').decode(sliced) + const enc = new TextEncoder().encode(text) + let end = Math.min(limit, enc.length) + // back up over UTF‑8 continuation bytes: 10xxxxxx + while (end > 0 && (enc[end - 1] & 0b1100_0000) === 0b1000_0000) { + end-- + } + return new TextDecoder('utf-8').decode(enc.slice(0, end)) }
129-151: Consider enforcing a total byte budget across all content items.Current logic applies maxBytes per item. If the API expects a total cap, track a remaining budget and truncate/stop accordingly.
I can provide a drop‑in version that enforces a cumulative budget if you confirm the intended behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
packages/canvas/DesignCanvas/src/mcp/index.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/index.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/pageSchema.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/utils.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts(1 hunks)packages/common/composable/mcp/baseTools/discoverResources.ts(1 hunks)packages/common/composable/mcp/baseTools/index.ts(1 hunks)packages/common/composable/mcp/baseTools/readResources.ts(1 hunks)packages/common/composable/mcp/baseTools/searchResources.ts(1 hunks)packages/common/composable/mcp/baseTools/utils.ts(1 hunks)packages/common/composable/mcp/index.ts(7 hunks)packages/common/composable/mcp/resources.ts(1 hunks)packages/common/composable/mcp/toolUtils.ts(2 hunks)packages/common/composable/mcp/type.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-01-14T06:55:14.457Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/canvas-function/design-mode.ts:6-13
Timestamp: 2025-01-14T06:55:14.457Z
Learning: The code in `packages/canvas/render/src/canvas-function/design-mode.ts` is migrated code that should be preserved in its current form during the migration process. Refactoring suggestions for type safety and state management improvements should be considered in future PRs.
Applied to files:
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.mdpackages/canvas/DesignCanvas/src/mcp/index.ts
📚 Learning: 2025-01-13T03:46:13.817Z
Learnt from: yy-wow
PR: opentiny/tiny-engine#940
File: packages/canvas/DesignCanvas/src/importMap.js:51-51
Timestamp: 2025-01-13T03:46:13.817Z
Learning: The `getImportMapData` function in `packages/canvas/DesignCanvas/src/importMap.js` has default parameter handling that initializes `canvasDeps` with empty arrays for `scripts` and `styles`, making additional null checks unnecessary.
Applied to files:
packages/canvas/DesignCanvas/src/mcp/index.ts
📚 Learning: 2025-01-14T08:45:57.032Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/application-function/global-state.ts:12-25
Timestamp: 2025-01-14T08:45:57.032Z
Learning: The code in `packages/canvas/render/src/application-function/global-state.ts` is migrated from an existing codebase and should be handled with care when making modifications.
Applied to files:
packages/canvas/DesignCanvas/src/mcp/index.ts
📚 Learning: 2025-01-14T06:59:02.999Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/material-function/support-collection.ts:3-15
Timestamp: 2025-01-14T06:59:02.999Z
Learning: The code in `packages/canvas/render/src/material-function/support-collection.ts` is migrated code that should not be modified at this time to maintain stability during the migration process.
Applied to files:
packages/canvas/DesignCanvas/src/mcp/index.ts
🧬 Code graph analysis (12)
packages/common/composable/mcp/baseTools/index.ts (4)
packages/common/composable/mcp/type.ts (2)
IState(33-46)ToolItem(21-29)packages/common/composable/mcp/baseTools/discoverResources.ts (1)
createDiscoverResourcesTool(70-205)packages/common/composable/mcp/baseTools/readResources.ts (1)
createReadResourcesTool(45-154)packages/common/composable/mcp/baseTools/searchResources.ts (1)
createSearchResourcesTool(91-372)
packages/common/composable/mcp/toolUtils.ts (1)
packages/common/composable/mcp/type.ts (1)
IState(33-46)
packages/canvas/DesignCanvas/src/mcp/resources/index.ts (2)
packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts (2)
pageSchemaResources(20-40)pageSchemaResourceTemplates(43-82)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts (2)
editExamplesResources(16-36)editExamplesResourceTemplates(39-78)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts (1)
packages/canvas/container/src/container.ts (1)
getSchema(97-97)
packages/common/composable/mcp/resources.ts (1)
packages/common/composable/mcp/type.ts (3)
IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)
packages/common/composable/mcp/baseTools/readResources.ts (2)
packages/common/composable/mcp/type.ts (1)
IState(33-46)packages/common/composable/mcp/baseTools/utils.ts (1)
readResourceContent(162-194)
packages/common/composable/mcp/baseTools/searchResources.ts (2)
packages/common/composable/mcp/type.ts (3)
IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)packages/common/composable/mcp/baseTools/utils.ts (4)
tryFetchRemoteLists(12-43)readResourceWithFallback(96-118)calculateByteLength(52-54)truncateTextToBytes(62-66)
packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts (1)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (1)
pickSectionByHeading(5-20)
packages/common/composable/mcp/index.ts (6)
packages/common/composable/mcp/toolUtils.ts (3)
initRegisterTools(106-142)UpdateToolConfig(10-18)updateTool(97-103)packages/common/composable/mcp/resources.ts (6)
initRegisterResources(166-215)registerResources(39-78)getResourceList(81-99)getResourceByUri(102-117)removeResource(120-134)updateResource(137-163)packages/common/composable/mcp/baseTools/index.ts (1)
getBaseTools(6-10)packages/common/composable/mcp/type.ts (4)
ToolItem(21-29)IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)packages/register/src/common.ts (1)
getAllMergeMeta(69-71)packages/canvas/DesignCanvas/src/mcp/resources/index.ts (2)
resources(4-4)resourceTemplates(6-6)
packages/common/composable/mcp/baseTools/utils.ts (1)
packages/common/composable/mcp/type.ts (3)
ResourceItem(73-85)ResourceTemplateItem(87-103)IState(33-46)
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts (1)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (1)
pickSectionByHeading(5-20)
packages/common/composable/mcp/baseTools/discoverResources.ts (2)
packages/common/composable/mcp/type.ts (3)
IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)packages/common/composable/mcp/baseTools/utils.ts (1)
tryFetchRemoteLists(12-43)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (17)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts (3)
1-11: LGTM! Well-structured schema definitions using Zod.The function schema validation approach with Zod is solid, ensuring type safety for lifecycle and method units.
422-494: Well-structured tool implementation with clear separation of concerns.The overall architecture with separate handlers for each section, consistent error handling patterns, and structured response formats demonstrates excellent code organization. The callback properly uses the canvas API and provides comprehensive input validation.
329-340: State merge logic doesn't validate existence for update operations.Unlike the map sections (lifeCycles/methods), the state merge doesn't check if keys exist before updating, which could lead to unintended additions when the user meant to update existing keys only.
Apply validation consistent with other sections:
+ const ignoredUpdate: string[] = [] Object.entries(payload?.update || {}).forEach(([k, v]) => { - if (k in nextState) { + if (!(k in nextState)) { + ignoredUpdate.push(k) + return + } nextState[k] = v affected.updated.push(k) - } }) + if (ignoredUpdate.length) warnings.push(`ignored update (not exists): ${ignoredUpdate.join(', ')}`)Likely an incorrect or invalid review comment.
packages/common/composable/mcp/baseTools/index.ts (1)
1-13: Clean aggregation of base tools.The module provides a clear and simple interface for retrieving MCP base tools. The implementation is straightforward and follows good practices.
packages/canvas/DesignCanvas/src/mcp/index.ts (1)
10-16: Successful integration of resource infrastructure.The addition of resources and resource templates to the MCP export extends the API surface cleanly without disrupting existing tool exports.
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md (1)
107-110: Excellent documentation with clear Do's and Don'ts.The guidance section provides valuable best practices that will help users avoid common mistakes. The examples are comprehensive and well-structured.
packages/canvas/DesignCanvas/src/mcp/resources/index.ts (1)
1-9: Clean resource aggregation pattern.The module effectively combines resources from multiple sources into unified exports. The spread operator usage ensures all resources are properly merged.
packages/common/composable/mcp/type.ts (1)
42-46: State extensions for resources look good.Confirm that all call sites initialize
resources,resourceTemplates, andresourceInstanceMapto avoid undefined access in registration paths.packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts (1)
5-13: Verify heading text parity — confirmed (curly apostrophe used)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md contains H2 "Do & Don’t" (curly U+2019), which matches EDIT_EXAMPLE_SECTION_TITLES — no change required.
packages/common/composable/mcp/baseTools/discoverResources.ts (1)
5-65: Input validation is solid.Schema covers practical bounds and docs read well.
packages/canvas/DesignCanvas/src/mcp/resources/pageSchema.md (1)
1-184: Comprehensive, actionable reference.Headings and examples align with the tool behavior and sectioning needs.
packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts (1)
6-17: Keep headings in sync with the markdown source.Any title drift will break extraction. Consider unit tests that assert every mapped heading exists.
packages/common/composable/mcp/baseTools/searchResources.ts (1)
5-79: Input schema and guidance look good.Parameters and bounds are pragmatic.
packages/common/composable/mcp/index.ts (3)
58-62: State extensions for resources look correct.Shapes align with types and downstream usage.
180-183: Good registration order.Register tools/resources before server.connect to ensure lists are ready.
269-279: API surface LGTM.Resources CRUD methods mirror tools API patterns and match helpers.
packages/common/composable/mcp/baseTools/utils.ts (1)
162-194: LGTM – coherent read/validate/truncate pipeline.The orchestration is clear and errors are surfaced with concise codes.
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (12)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (3)
14-20: Harden normalization and H2 detection (NFKC, tabs after ##, strip trailing #).This improves robustness for Unicode variants, tabs after hashes, and trailing ATX markers.
Apply:
- const normalize = (s: string) => - s - .trim() - .replace(/\u2019/g, "'") - .replace(/\s+/g, ' ') - .toLowerCase() + const normalize = (s: string) => + s + .normalize('NFKC') + .trim() + .replace(/[\u2018\u2019]/g, "'") + .replace(/\s+/g, ' ') + .toLowerCase() - const isH2 = (line: string) => line.trim().startsWith('## ') - const headingText = (line: string) => line.trim().replace(/^##\s+/, '') + const isH2 = (line: string) => /^\s*##\s+/.test(line) + const headingText = (line: string) => + line.trim() + .replace(/^\s*##\s+/, '') + .replace(/[ \t]*#+[ \t]*$/, '')Also applies to: 22-23
21-43: Optional: ignore fenced code blocks to avoid false-positive headings.If docs contain “## ” inside code fences, they’ll match. You can skip headings while inside triple‑backtick fences.
Apply:
const lines = markdown.split(/\r?\n/) - const isH2 = (line: string) => /^\s*##\s+/.test(line) + const isH2 = (line: string) => /^\s*##\s+/.test(line) const headingText = (line: string) => line.trim() .replace(/^\s*##\s+/, '') .replace(/[ \t]*#+[ \t]*$/, '') + let inFence = false let startIdx = -1 for (let i = 0; i < lines.length; i += 1) { - if (isH2(lines[i]) && normalize(headingText(lines[i])) === target) { + const line = lines[i] + if (/^\s*```/.test(line)) inFence = !inFence + if (!inFence && isH2(line) && normalize(headingText(line)) === target) { startIdx = i break } } @@ for (let i = startIdx + 1; i < lines.length; i += 1) { - if (isH2(lines[i])) { + const line = lines[i] + if (/^\s*```/.test(line)) inFence = !inFence + if (!inFence && isH2(line)) { endIdx = i break } }
5-46: Add targeted tests for edge cases.Cover: indented H2s, tabs after
##, multiple spaces, curly quotes (both U+2018/2019), trailing#, empty title, missing section, code‑fence headings.I can add a small test suite in packages/canvas/DesignCanvas/src/mcp/resources/tests/utils.spec.ts if you want.
packages/common/composable/mcp/resources.ts (9)
71-75: Make sendResourceListChanged robust for both sync/async implementations.try/catch won’t catch async rejections; wrap with Promise.resolve().catch().
- try { - state.server.sendResourceListChanged() - } catch (e) { - logger.error('error when sendResourceListChanged after registerResources', e) - } + Promise + .resolve(state.server?.sendResourceListChanged()) + .catch((e) => logger.error('error when sendResourceListChanged after registerResources', e))
85-96: Prefer definition fallback when instance fields are empty strings.Nullish coalescing (??) preserves empty strings; use || for name/title so blanks don’t override definitions.
- name: inst?.name ?? def.name, - title: inst?.title ?? def.title, + name: (inst?.name || def.name), + title: (inst?.title || def.title),
109-116: Same fallback tweak for getResourceByUri.- name: inst?.name ?? def.name, - title: inst?.title ?? def.title, + name: (inst?.name || def.name), + title: (inst?.title || def.title),
146-149: Clarify error path: distinguish missing instance vs invalid updates.- if (!inst || !updates || typeof updates !== 'object') { - logger.error('resource instance not found for uri:', uri) - return - } + if (!inst) { + logger.error('resource instance not found for uri:', uri) + return + } + if (!updates || typeof updates !== 'object') { + logger.error('invalid updates payload for uri:', uri, updates) + return + }
158-168: Guard against key migration collisions.If another instance already occupies newUri, skip migration to avoid overriding.
- if (Object.prototype.hasOwnProperty.call(updates, 'uri')) { - const newUri = updates.uri as string | null | undefined - if (typeof newUri === 'string' && newUri && newUri !== uri) { - try { - map.delete(uri) - map.set(newUri, inst) - } catch (e) { - logger.error('error when migrate resourceInstanceMap key', uri, '->', newUri, e) - } - } - } + if (Object.prototype.hasOwnProperty.call(updates, 'uri')) { + const newUri = updates.uri as string | null | undefined + if (typeof newUri === 'string' && newUri && newUri !== uri) { + try { + if (map.has(newUri) && map.get(newUri) !== inst) { + logger.warn('skip migrating map key due to existing instance at newUri', newUri) + } else { + map.delete(uri) + map.set(newUri, inst) + } + } catch (e) { + logger.error('error when migrate resourceInstanceMap key', uri, '->', newUri, e) + } + } + }
179-184: Sync state.server with the provided server for consistency.Keeps downstream sendResourceListChanged calls functional even if state.server wasn’t set yet.
try { + // keep state.server in sync for downstream operations that only read from state + if (!state.server) state.server = server const resources: ResourceItem[] = state.resources || [] const resourceTemplates: ResourceTemplateItem[] = state.resourceTemplates || []
187-193: Avoid empty resource names — default to URI when name is missing.Empty names can be problematic for some MCP consumers.
- const instance = server.registerResource( - resourceItem.name || '', + const instance = server.registerResource( + resourceItem.name || resourceItem.uri, resourceItem.uri, buildMetadata(resourceItem), resourceItem.readCallback )
203-204: Remove no-op{ list: undefined }from ResourceTemplate ctor.It’s unnecessary noise.
- const template = resourceItem.template || new ResourceTemplate(resourceItem.uriTemplate, { list: undefined }) + const template = resourceItem.template || new ResourceTemplate(resourceItem.uriTemplate)
223-223: Harden final list-changed notification (sync/async-safe).- server.sendResourceListChanged() + Promise + .resolve(server.sendResourceListChanged()) + .catch((e) => logger.error('error when sendResourceListChanged after initRegisterResources', e))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts(1 hunks)packages/common/composable/mcp/resources.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/common/composable/mcp/resources.ts (1)
packages/common/composable/mcp/type.ts (3)
IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (6)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (2)
5-46: Solid utility; clear guard clauses and section slicing.Overall approach is straightforward and readable; start/end detection between H2s is clear.
10-12: Empty title should not return the entire document.Returning the full markdown when
titleis empty is surprising for a “pick section” helper and risks large unintended payloads. Prefer returning an empty string.Apply:
- if (typeof title !== 'string' || !title.trim()) { - return markdown - } + if (typeof title !== 'string' || !title.trim()) { + return '' + }Run to locate call sites and confirm no caller relies on the old behavior:
#!/bin/bash # Find and review all call sites rg -nP --type=ts -C3 '\bpickSectionByHeading\s*\('packages/common/composable/mcp/resources.ts (4)
2-5: Good: using public mcp.js entry and type-only imports.Avoids the .d.ts path pitfall and keeps bundlers happy.
119-139: Good: emits list-changed after removal.The notification after delete is in place with error handling.
142-175: Good: safe URI migration and list-changed after update.Handles null/undefined correctly and emits the update notification.
53-65: Idempotency gap when resource URI changes — also match by stable name and migrate key.Without a name fallback, a URI change registers a duplicate and orphans the old instance.
- try { - const exist = map.get(uri) - if (exist) { - exist.update({ name, uri, metadata, callback: item.readCallback }) - return exist - } - - const instance = state.server!.registerResource(name, uri, metadata, item.readCallback) - if (instance) { - map.set(uri, instance) - } - return instance + try { + let exist = map.get(uri) + if (!exist && name) { + // fallback: keep idempotency across URI changes by matching on stable name + exist = [...map.values()].find(r => r.name === name) + if (exist) { + try { + const oldKey = [...map.entries()].find(([, v]) => v === exist)?.[0] + exist.update({ name, uri, metadata, callback: item.readCallback }) + if (oldKey && oldKey !== uri) { + map.delete(oldKey) + map.set(uri, exist) + } + return exist + } catch (e) { + logger.error('error when reconcile resource by name during register', name, e) + } + } + } + if (exist) { + exist.update({ name, uri, metadata, callback: item.readCallback }) + return exist + } + const instance = state.server!.registerResource(name, uri, metadata, item.readCallback) + if (instance) { + map.set(uri, instance) + } + return instance
chilingling
force-pushed
the
feat/addBaseResourceTools
branch
from
44a7818 to
37e96ba
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts (3)
439-457: Validate args with Zod before processing.You define a Zod schema but do not parse the incoming args. Use
safeParseto guard early and return a structured error.Apply this diff:
- try { - const { section, strategy, payload } = normalizeArgs(args) + try { + const parsed = inputSchema.safeParse(args) + if (!parsed.success) { + return err({ + errorCode: ERROR_CODES.INVALID_ARGUMENT, + reason: parsed.error.message, + userMessage: 'Invalid arguments for edit_page_schema', + next_action: nextActionGetSchema() + }) + } + const { section, strategy, payload } = normalizeArgs(parsed.data)
351-362: De-duplicate top-level schema keys; derive from a shared protocol.Hardcoding
ALLOWED_SCHEMA_KEYSrisks drift. Prefer importing a single source of truth (e.g., from pageSchemaProtocol) and building the Set from it.If
packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.tsexports top-level keys, consider:// import { PAGE_SCHEMA_TOP_LEVEL_KEYS } from '../../mcp/resources/pageSchemaProtocol' const ALLOWED_SCHEMA_KEYS = new Set(PAGE_SCHEMA_TOP_LEVEL_KEYS)
408-420: Tighten instruction typing to a discriminated union.This clarifies allowed shapes and removes optional fields.
Apply this diff:
-const applyWriteInstruction = ( - instruction: { type?: 'update' | 'import'; partial?: Record<string, any>; schema?: Record<string, any> } | undefined, - apis: { updateSchema: (p: any) => void; importSchema: (s: any) => void } -) => { - if (!instruction || !instruction.type) return - if (instruction.type === 'update') { - apis.updateSchema(instruction.partial || {}) - return - } - if (instruction.type === 'import') { - apis.importSchema(instruction.schema || {}) - } -} +type WriteInstruction = + | { type: 'update'; partial: Record<string, any> } + | { type: 'import'; schema: Record<string, any> } + +const applyWriteInstruction = ( + instruction: WriteInstruction | undefined, + apis: { updateSchema: (p: any) => void; importSchema: (s: any) => void } +) => { + if (!instruction) return + if (instruction.type === 'update') { + apis.updateSchema(instruction.partial) + } else { + apis.importSchema(instruction.schema) + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
packages/canvas/DesignCanvas/src/mcp/index.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/index.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/pageSchema.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/utils.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts(1 hunks)packages/common/composable/mcp/baseTools/discoverResources.ts(1 hunks)packages/common/composable/mcp/baseTools/index.ts(1 hunks)packages/common/composable/mcp/baseTools/readResources.ts(1 hunks)packages/common/composable/mcp/baseTools/searchResources.ts(1 hunks)packages/common/composable/mcp/baseTools/utils.ts(1 hunks)packages/common/composable/mcp/index.ts(7 hunks)packages/common/composable/mcp/resources.ts(1 hunks)packages/common/composable/mcp/toolUtils.ts(2 hunks)packages/common/composable/mcp/type.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/canvas/DesignCanvas/src/mcp/resources/utils.ts
- packages/canvas/DesignCanvas/src/mcp/resources/index.ts
- packages/common/composable/mcp/resources.ts
- packages/canvas/DesignCanvas/src/mcp/resources/pageSchema.md
- packages/common/composable/mcp/baseTools/readResources.ts
- packages/common/composable/mcp/baseTools/index.ts
- packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts
- packages/common/composable/mcp/baseTools/utils.ts
- packages/common/composable/mcp/toolUtils.ts
- packages/canvas/DesignCanvas/src/mcp/index.ts
- packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md
- packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-14T06:59:23.602Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/page-block-function/methods.ts:9-21
Timestamp: 2025-01-14T06:59:23.602Z
Learning: The code in packages/canvas/render/src/page-block-function/methods.ts is migrated code that should not be modified during the migration phase. Error handling improvements can be addressed in future PRs.
Applied to files:
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts
🧬 Code graph analysis (4)
packages/common/composable/mcp/baseTools/discoverResources.ts (2)
packages/common/composable/mcp/type.ts (3)
IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)packages/common/composable/mcp/baseTools/utils.ts (1)
tryFetchRemoteLists(12-43)
packages/common/composable/mcp/index.ts (5)
packages/common/composable/mcp/toolUtils.ts (1)
initRegisterTools(106-142)packages/common/composable/mcp/resources.ts (6)
initRegisterResources(178-227)registerResources(39-78)getResourceList(81-99)getResourceByUri(102-117)removeResource(120-140)updateResource(143-175)packages/common/composable/mcp/baseTools/index.ts (1)
getBaseTools(6-10)packages/common/composable/mcp/type.ts (4)
ToolItem(21-29)IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)packages/register/src/common.ts (1)
getAllMergeMeta(69-71)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts (1)
packages/canvas/container/src/container.ts (1)
getSchema(97-97)
packages/common/composable/mcp/baseTools/searchResources.ts (2)
packages/common/composable/mcp/type.ts (3)
IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)packages/common/composable/mcp/baseTools/utils.ts (4)
tryFetchRemoteLists(12-43)readResourceWithFallback(96-118)calculateByteLength(52-54)truncateTextToBytes(62-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (34)
packages/common/composable/mcp/type.ts (6)
3-9: LGTM! Proper MCP SDK type imports.The imports align with the MCP specification which provides standardized interfaces for resources and resource templates. The imported types (
ResourceTemplate,ReadResourceCallback,ReadResourceTemplateCallback,RegisteredResource) are essential for implementing MCP resource support properly.
42-46: Well-designed state extension for MCP resources.The addition of
resources,resourceTemplates, andresourceInstanceMapto theIStateinterface properly extends the MCP state to support MCP resources which "allow servers to expose data and content that can be read by clients and used as context for LLM interactions" and resource templates for dynamic content.The
resourceInstanceMapusing URI as key aligns with MCP's requirement that "each resource is uniquely identified by a URI".
48-55: Clear and comprehensive ResourceContent interface.The interface properly captures the essential fields for MCP resource content with appropriate optional fields. The structure aligns with MCP specification which supports both "text resources contain UTF-8 encoded text data" and binary resources.
57-71: Comprehensive VariableSpec interface for resource templates.This interface provides robust constraint specifications for template parameters, supporting various data types and validation rules. This is essential for resource templates which support "parameters" and dynamic content generation as per the MCP specification.
73-85: Well-structured ResourceItem interface aligns with MCP standards.The interface correctly implements the MCP resource structure with proper URI identification, optional metadata fields, and annotations supporting audience targeting and priority. The structure matches MCP's resource specification which includes "uri", "name", "description", and "mimeType" fields.
87-103: Comprehensive ResourceTemplateItem interface for dynamic resources.The interface properly supports MCP resource templates with "uriTemplate" for dynamic URI construction and support for variables. The inclusion of
variables,variablesSchemaUri, and optionaltemplatefield provides flexibility for both simple and complex templating scenarios.packages/common/composable/mcp/baseTools/discoverResources.ts (6)
5-65: Comprehensive input schema with excellent documentation.The Zod schema is well-designed with detailed Chinese descriptions that clearly explain each parameter's purpose, constraints, and usage scenarios. The parameter validation ranges are reasonable and the descriptions help users understand when and how to use each filter option.
70-97: Excellent tool documentation following MCP best practices.The tool description is comprehensive and user-friendly, explaining:
- Purpose and best use cases
- Core functionality and features
- Workflow recommendations
- Performance characteristics
- Integration with other MCP tools
This aligns with MCP's goal of providing "pre-defined templates to use tools or resources in the most optimal way" and enabling proper "context provision".
114-127: Proper remote-first data fetching with local fallback.The implementation correctly prioritizes remote MCP client data fetching and falls back to local state when remote access fails. This follows MCP's distributed architecture patterns and ensures resilience.
128-157: Clean entry aggregation logic handling both resource types.The code properly combines resources and resource templates into a unified structure based on the
typefilter, with appropriate handling of optional annotations. The conditional inclusion logic is correct.
159-189: Solid filtering and sorting implementation.The filtering functions are well-implemented with proper null-safety checks:
filterByAudience: Correctly handles missing audience arraysfilterByMime: Supports both exact and prefix matchingfilterByQuery: Case-insensitive search across name, title, and descriptionThe sorting logic properly handles missing priority values by treating them as -1 (low priority) and falls back to alphabetical sorting.
191-204: Proper pagination and response formatting.The pagination implementation is correct with proper bounds checking, and the response format follows the documented structure with success status wrapping.
packages/common/composable/mcp/baseTools/searchResources.ts (7)
5-79: Well-designed search schema with comprehensive options.The input schema provides excellent flexibility for search operations with proper constraints and detailed descriptions. The nested
snippetobject is well-structured, and thecontentMaxBytesPerDoclimit helps prevent performance issues with large documents.
81-87: Efficient snippet extraction logic.The
sliceSnippetfunction efficiently finds the query term and extracts surrounding context with proper bounds checking. The centering approach (Math.floor(max / 2)) provides good context around the match.
91-125: Comprehensive tool description with clear usage guidance.The description excellently explains the tool's purpose, use cases, search strategies, and performance considerations. The workflow recommendations and integration guidance with other MCP tools are particularly valuable.
141-155: Consistent data fetching pattern with remote priority.The implementation mirrors the pattern from
discoverResources.ts, prioritizing remote data and falling back to local state. This consistency is good for maintainability.
223-295: Smart initial scoring and filtering strategy.The metadata scoring system assigns appropriate weights (name: 2, title: 2, description: 1) and handles different search scopes correctly:
metadata/all: Scores based on metadata matchescontent: Placeholder scoring for content-based searchThe filtering by audience and MIME type is consistent with the discover tool.
297-346: Robust content search implementation with proper resource handling.The content search logic is well-designed:
- Properly identifies textual resources for content search
- Uses the shared
readResourceWithFallbackutility for consistent resource reading- Respects the
contentMaxBytesPerDoclimit to prevent performance issues- Implements controlled concurrency (4 workers) to avoid overwhelming the system
- Skips resource templates for content search, which is appropriate since they're parameterized
350-371: Appropriate priority weighting and result processing.The priority weighting formula (
score * (1 + priority * 0.5)) provides reasonable boost for high-priority resources without being overpowering. Filtering out zero-score results and applying topK limits are correct.packages/common/composable/mcp/index.ts (8)
5-5: Resource types properly imported for MCP integration.The addition of
ResourceItemandResourceTemplateItemimports correctly extends the MCP service to support resource management alongside existing tool management.
16-24: Clean import organization for resource management.The resource management imports are well-organized and provide comprehensive CRUD operations for MCP resources. The inclusion of
initRegisterResourcesensures proper initialization flow.
58-62: Proper state initialization for resource support.The initial state correctly includes the new resource-related fields with appropriate empty arrays and Map initialization. The
resourceInstanceMapusing a Map is efficient for URI-based lookups.
180-182: Correct initialization order for MCP server setup.The placement of
initRegisterToolsandinitRegisterResourcesbeforeserver.connect()is correct, ensuring all tools and resources are registered before the server becomes active.
197-204: Robust base tools injection with error handling.The try-catch block around base tools injection is good defensive programming. The console.error provides visibility into any issues with base tool registration while preventing complete failure of the MCP service initialization.
214-232: Well-structured resource collection from metadata.The
collectResourcesfunction properly mirrors thecollectToolspattern, collecting resources and resource templates from metadata and storing them in state for later registration. The metadata traversal logic is consistent and safe.
250-253: Proper integration of resource collection in initialization.The call to
collectResources(state)is correctly placed before MCP server creation, ensuring resources are available during the registration phase.
272-279: Complete resource management API surface.The addition of resource management APIs (
registerResources,getResourceList,getResourceByUri,removeResource,updateResource) provides a comprehensive interface for resource CRUD operations, matching the existing pattern for tool management.packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts (7)
156-185: CSS handling looks solid.Append vs replace behavior is clear; newline handling avoids accidental CSS concatenation issues.
187-281: Map-merge logic for lifeCycles/methods is well thought out.Good separation of add/update/remove with clear warnings for ignored entries and invalid function units.
283-349: State merge semantics are consistent and safe.Top-level keyed merge behavior and replace handling are correct; clear reporting of affected keys.
483-485: Wrap write instruction in try/catch to avoid unhandled failures.
updateSchema/importSchemamay throw; handle errors locally and return a structured failure instead of falling through to the outer generic catch.Apply this diff:
- applyWriteInstruction(out?.instruction, { updateSchema, importSchema }) + try { + applyWriteInstruction(out?.instruction, { updateSchema, importSchema }) + } catch (writeError) { + return err({ + errorCode: ERROR_CODES.UNEXPECTED_ERROR, + reason: writeError instanceof Error ? writeError.message : 'Failed to apply schema changes', + userMessage: 'Failed to apply schema changes to the canvas', + next_action: [{ type: 'tool_call', name: 'get_page_schema', args: {} }] + }) + }
98-101: get_page_schema is registered. Found export in packages/canvas/DesignCanvas/src/mcp/tools/getPageSchema.ts — export const getPageSchema = { name: 'get_page_schema', … }.
437-460: No API mismatch — useCanvas().getSchema exists; no change required.getSchema is implemented at packages/canvas/DesignCanvas/src/api/useCanvas.ts:623 and getPageSchema at :285; callers already use getSchema (e.g. packages/toolbars/redoundo/src/composable/useHistory.ts:97). The defensive fallback is unnecessary.
Likely an incorrect or invalid review comment.
435-435: No change required — repo expects a Zod shape (inputSchema.shape).
Common types declare inputSchema?: ZodRawShape and registerTools passes that shape into the MCP registration; many existing tools use inputSchema.shape (e.g. packages/common/composable/mcp/type.ts and packages/common/composable/mcp/toolUtils.ts).Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/canvas/DesignCanvas/src/mcp/index.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/index.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/index.ts(1 hunks)packages/common/composable/mcp/baseTools/index.ts(1 hunks)packages/common/composable/mcp/baseTools/sequentialThinking.ts(1 hunks)packages/plugins/robot/src/system-prompt.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/canvas/DesignCanvas/src/mcp/index.ts
- packages/common/composable/mcp/baseTools/index.ts
- packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-14T08:42:18.574Z
Learnt from: gene9831
PR: opentiny/tiny-engine#1038
File: packages/plugins/block/index.js:24-24
Timestamp: 2025-01-14T08:42:18.574Z
Learning: In the tiny-engine project, breaking changes are documented in the changelog rather than in JSDoc comments or separate migration guides.
Applied to files:
packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.md
🧬 Code graph analysis (3)
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts (1)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (1)
pickSectionByHeading(5-46)
packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.ts (1)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (1)
pickSectionByHeading(5-46)
packages/canvas/DesignCanvas/src/mcp/resources/index.ts (3)
packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts (2)
pageSchemaResources(20-40)pageSchemaResourceTemplates(43-82)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts (2)
editExamplesResources(17-37)editExamplesResourceTemplates(40-79)packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.ts (2)
aiInstructResources(16-36)aiInstructResourceTemplates(39-78)
🪛 markdownlint-cli2 (0.18.1)
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md
153-153: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
234-234: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
300-300: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
308-308: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.md
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
235-235: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
239-239: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
243-243: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
251-251: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
255-255: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
265-265: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
288-288: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
314-314: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (2)
packages/canvas/DesignCanvas/src/mcp/tools/index.ts (1)
8-8: Nice addition — tool now exposed.Re-exporting
EditPageSchemahere keeps the public MCP tool surface complete with the new editor helper. 👍packages/canvas/DesignCanvas/src/mcp/resources/index.ts (1)
1-13: Resources aggregation looks solid.The combined arrays give consumers a single entry point without mutating the source lists. All good.
| ``` | ||
| 步骤1：通过 edit_page_schema 添加CSS类 | ||
| 步骤2：通过 change_node_props 设置className | ||
| ``` |
There was a problem hiding this comment.
Add explicit languages to fenced code blocks.
markdownlint-cli2 is failing (MD040) because this fence omits a language. Please annotate this block—and the other unlabeled fences in the file—with text (or another appropriate language) so the lint step passes.
-```
+```text
步骤1：通过 edit_page_schema 添加CSS类
步骤2：通过 change_node_props 设置className
As per coding guidelines
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
153-153: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md lines
153-156: fenced code blocks in this region (and other unlabeled fences in the
file) lack an explicit language which causes markdownlint MD040 failures; update
each triple-backtick fence to include an appropriate language tag such as text
(e.g., ```text) so the blocks are labeled and the linter passes.
</details>
<!-- fingerprinting:phantom:poseidon:sabertoothed -->
<!-- This is an auto-generated comment by CodeRabbit -->
| **场景**：为卡片组件创建完整的样式系统（通过 Tailwind utility 类） | ||
|
|
||
| **步骤：应用到组件结构** | ||
| ```javascript | ||
| // 父容器 |
There was a problem hiding this comment.
Replace bold pseudo-headings with proper headings.
Markdownlint (MD036) flags these bolded lines as “emphasis used instead of headings.” Convert them—and the other bold-only pseudo-headings in this document—to real headings or plain text to satisfy the lint rule.
-**场景**：为卡片组件创建完整的样式系统（通过 Tailwind utility 类）
-
-**步骤：应用到组件结构**
+场景：为卡片组件创建完整的样式系统（通过 Tailwind utility 类）
+
+#### 步骤：应用到组件结构As per coding guidelines
📝 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.
| **场景**：为卡片组件创建完整的样式系统（通过 Tailwind utility 类） | |
| **步骤：应用到组件结构** | |
| ```javascript | |
| // 父容器 | |
| 场景：为卡片组件创建完整的样式系统（通过 Tailwind utility 类） | |
| #### 步骤：应用到组件结构 |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
234-234: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md
around lines 232 to 236, the document uses bold text as pseudo-headings (e.g.,
**场景** and other **…** lines) which triggers MD036; replace those bold-only
lines with proper Markdown headings (e.g., prefixed with #/##/### as appropriate
for the document hierarchy) or convert them to normal paragraph text if they are
not headings, ensure a blank line before and after each heading, and apply the
same change to all other occurrences of bold-only pseudo-headings in the file to
satisfy the lint rule.
| **用户需求**："创建一个完整的用户管理页面" | ||
|
|
||
| **使用 Sequential Thinking 规划**： | ||
| ``` | ||
| Thought 1/5: 分析需求，用户管理页面需要：列表展示、搜索、新增、编辑、删除 | ||
| Thought 2/6: 设计页面结构：顶部搜索栏、中间表格、操作按钮 | ||
| Thought 3/6: 规划状态管理：用户列表、搜索条件、编辑状态 | ||
| Thought 4/6: 设计交互流程：搜索→列表更新、点击编辑→弹窗 | ||
| Thought 5/7: 发现需要添加分页功能（调整总数） | ||
| Thought 6/7: 确定实施顺序：1.基础布局 2.状态 3.方法 4.样式 | ||
| Thought 7/7: 生成执行计划并验证完整性 | ||
| ``` | ||
|
|
||
| **执行步骤**： | ||
| 1. 使用资源学习页面结构设计 |
There was a problem hiding this comment.
Change bold pseudo-headings to real headings or plain text.
markdownlint MD036 complains about the standalone bold lines such as “用户需求” and “使用 Sequential Thinking 规划”. Convert them—and similar patterns throughout the doc—to actual headings or regular text.
-**用户需求**："创建一个完整的用户管理页面"
-
-**使用 Sequential Thinking 规划**：
+用户需求："创建一个完整的用户管理页面"
+
+#### 使用 Sequential Thinking 规划As per coding guidelines
📝 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.
| **用户需求**："创建一个完整的用户管理页面" | |
| **使用 Sequential Thinking 规划**： | |
| ``` | |
| Thought 1/5: 分析需求，用户管理页面需要：列表展示、搜索、新增、编辑、删除 | |
| Thought 2/6: 设计页面结构：顶部搜索栏、中间表格、操作按钮 | |
| Thought 3/6: 规划状态管理：用户列表、搜索条件、编辑状态 | |
| Thought 4/6: 设计交互流程：搜索→列表更新、点击编辑→弹窗 | |
| Thought 5/7: 发现需要添加分页功能（调整总数） | |
| Thought 6/7: 确定实施顺序：1.基础布局 2.状态 3.方法 4.样式 | |
| Thought 7/7: 生成执行计划并验证完整性 | |
| ``` | |
| **执行步骤**： | |
| 1. 使用资源学习页面结构设计 | |
| 用户需求："创建一个完整的用户管理页面" | |
| #### 使用 Sequential Thinking 规划 |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.md around
lines 10–24, the document uses standalone bold lines (e.g., "**用户需求**", "**使用
Sequential Thinking 规划**") which triggers markdownlint MD036; replace those bold
pseudo-headings with proper Markdown headings or plain text (for example convert
to "## 用户需求" and "## 使用 Sequential Thinking 规划" or remove bold and make them
inline sentences), apply the same conversion to all similar bold-only lines in
the file, ensure heading levels are consistent, and re-run markdownlint to
confirm MD036 is resolved.
| ``` | ||
| Thought 1/5: 分析需求，用户管理页面需要：列表展示、搜索、新增、编辑、删除 | ||
| Thought 2/6: 设计页面结构：顶部搜索栏、中间表格、操作按钮 | ||
| Thought 3/6: 规划状态管理：用户列表、搜索条件、编辑状态 | ||
| Thought 4/6: 设计交互流程：搜索→列表更新、点击编辑→弹窗 | ||
| Thought 5/7: 发现需要添加分页功能（调整总数） | ||
| Thought 6/7: 确定实施顺序：1.基础布局 2.状态 3.方法 4.样式 | ||
| Thought 7/7: 生成执行计划并验证完整性 | ||
| ``` |
There was a problem hiding this comment.
Label fenced blocks with a language.
The sequential-thinking example (and other unlabeled fences in this doc) triggers markdownlint MD040. Please tag them with text (or a suitable language) so lint passes.
-```
+```text
Thought 1/5: 分析需求，用户管理页面需要：列表展示、搜索、新增、编辑、删除
Thought 2/6: 设计页面结构：顶部搜索栏、中间表格、操作按钮
Thought 3/6: 规划状态管理：用户列表、搜索条件、编辑状态
Thought 4/6: 设计交互流程：搜索→列表更新、点击编辑→弹窗
Thought 5/7: 发现需要添加分页功能（调整总数）
Thought 6/7: 确定实施顺序：1.基础布局 2.状态 3.方法 4.样式
Thought 7/7: 生成执行计划并验证完整性
As per coding guidelines
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.md lines
13-21: the fenced code blocks in this section are unlabeled and trigger
markdownlint MD040; update each triple-backtick fence to include a language tag
(e.g., ```text) so the blocks are explicitly labeled, and review the rest of the
file for any other unlabeled fenced blocks and tag them similarly to satisfy the
lint rule.
</details>
<!-- fingerprinting:phantom:poseidon:sabertoothed -->
<!-- This is an auto-generated comment by CodeRabbit -->
| readTemplateCallback: async (_uri: URL, variables: Record<string, string>) => { | ||
| const section = (variables?.section || '').toString() | ||
| const heading = AI_INSTRUCT_SECTION_TITLES[section] | ||
| if (!heading) { | ||
| throw new Error('Invalid template parameter: section') | ||
| } | ||
| const text = pickSectionByHeading(aiInstructMd, heading) | ||
| return { | ||
| contents: [ | ||
| { | ||
| uri: `tinyengine://docs/ai-instruct/${section}`, | ||
| name: `ai-instruct-${section}.md`, | ||
| title: `TinyEngine 操作指南 - ${heading}`, | ||
| mimeType: 'text/markdown', | ||
| text | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
Guard against empty section slices.
pickSectionByHeading returns an empty string when the heading isn’t found (e.g., document drift, heading typo). In that case we silently return a resource with no content, which is a confusing failure mode for MCP clients. Please fail fast.
const heading = AI_INSTRUCT_SECTION_TITLES[section]
if (!heading) {
throw new Error('Invalid template parameter: section')
}
const text = pickSectionByHeading(aiInstructMd, heading)
+ if (!text.trim()) {
+ throw new Error(`Section "${section}" could not be found in TinyEngine 操作指南`)
+ }
return {
contents: [📝 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.
| readTemplateCallback: async (_uri: URL, variables: Record<string, string>) => { | |
| const section = (variables?.section || '').toString() | |
| const heading = AI_INSTRUCT_SECTION_TITLES[section] | |
| if (!heading) { | |
| throw new Error('Invalid template parameter: section') | |
| } | |
| const text = pickSectionByHeading(aiInstructMd, heading) | |
| return { | |
| contents: [ | |
| { | |
| uri: `tinyengine://docs/ai-instruct/${section}`, | |
| name: `ai-instruct-${section}.md`, | |
| title: `TinyEngine 操作指南 - ${heading}`, | |
| mimeType: 'text/markdown', | |
| text | |
| } | |
| ] | |
| } | |
| readTemplateCallback: async (_uri: URL, variables: Record<string, string>) => { | |
| const section = (variables?.section || '').toString() | |
| const heading = AI_INSTRUCT_SECTION_TITLES[section] | |
| if (!heading) { | |
| throw new Error('Invalid template parameter: section') | |
| } | |
| const text = pickSectionByHeading(aiInstructMd, heading) | |
| if (!text.trim()) { | |
| throw new Error(`Section "${section}" could not be found in TinyEngine 操作指南`) | |
| } | |
| return { | |
| contents: [ | |
| { | |
| uri: `tinyengine://docs/ai-instruct/${section}`, | |
| name: `ai-instruct-${section}.md`, | |
| title: `TinyEngine 操作指南 - ${heading}`, | |
| mimeType: 'text/markdown', | |
| text | |
| } | |
| ] | |
| } |
🤖 Prompt for AI Agents
In packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.ts around
lines 58 to 75, pickSectionByHeading can return an empty string if the heading
isn't actually found; detect when text is empty or only whitespace and throw a
clear Error (e.g., "ai-instruct section not found: <section>") instead of
returning an empty resource so MCP clients fail fast; implement the check
immediately after calling pickSectionByHeading and ensure the thrown error
includes the section identifier and heading for easier debugging.
| 9. Repeat the process until satisfied with the solution | ||
| 10. Provide a single, ideally correct answer as the final output | ||
| 11. Only set next_thought_needed to false when truly done and a satisfactory answer is reached`, | ||
| inputSchema: inputSchema.shape, |
There was a problem hiding this comment.
Export the full Zod schema instead of shape.
Using inputSchema.shape strips away the ZodObject instance, so downstream registration loses parsing/validation and any parse/safeParse or JSON-schema conversion will throw. Please export the actual schema object.
- inputSchema: inputSchema.shape,
+ inputSchema,📝 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.
| inputSchema: inputSchema.shape, | |
| inputSchema, |
🤖 Prompt for AI Agents
In packages/common/composable/mcp/baseTools/sequentialThinking.ts around line
148, the code exports inputSchema.shape which strips the ZodObject and breaks
downstream parse/validation and JSON-schema conversion; replace the exported
value with the actual Zod schema object (e.g., inputSchema) so consumers receive
the full ZodObject instance, not its .shape, and update any type/usage
references accordingly to use the schema's parse/safeParse or conversion
methods.
08c58ab to
cad96bb
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
packages/canvas/DesignCanvas/src/mcp/index.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/index.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/pageSchema.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/utils.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/index.ts(1 hunks)packages/common/composable/mcp/baseTools/discoverResources.ts(1 hunks)packages/common/composable/mcp/baseTools/index.ts(1 hunks)packages/common/composable/mcp/baseTools/readResources.ts(1 hunks)packages/common/composable/mcp/baseTools/searchResources.ts(1 hunks)packages/common/composable/mcp/baseTools/sequentialThinking.ts(1 hunks)packages/common/composable/mcp/baseTools/utils.ts(1 hunks)packages/common/composable/mcp/index.ts(7 hunks)packages/common/composable/mcp/resources.ts(1 hunks)packages/common/composable/mcp/toolUtils.ts(2 hunks)packages/common/composable/mcp/type.ts(2 hunks)packages/plugins/robot/src/system-prompt.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/canvas/DesignCanvas/src/mcp/index.ts
- packages/common/composable/mcp/baseTools/index.ts
- packages/canvas/DesignCanvas/src/mcp/tools/index.ts
- packages/canvas/DesignCanvas/src/mcp/resources/index.ts
- packages/common/composable/mcp/baseTools/searchResources.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-01-14T06:55:14.457Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/canvas-function/design-mode.ts:6-13
Timestamp: 2025-01-14T06:55:14.457Z
Learning: The code in `packages/canvas/render/src/canvas-function/design-mode.ts` is migrated code that should be preserved in its current form during the migration process. Refactoring suggestions for type safety and state management improvements should be considered in future PRs.
Applied to files:
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts
📚 Learning: 2025-01-14T06:59:23.602Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/page-block-function/methods.ts:9-21
Timestamp: 2025-01-14T06:59:23.602Z
Learning: The code in packages/canvas/render/src/page-block-function/methods.ts is migrated code that should not be modified during the migration phase. Error handling improvements can be addressed in future PRs.
Applied to files:
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts
🧬 Code graph analysis (10)
packages/common/composable/mcp/baseTools/discoverResources.ts (2)
packages/common/composable/mcp/type.ts (3)
IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)packages/common/composable/mcp/baseTools/utils.ts (1)
tryFetchRemoteLists(12-43)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema.ts (2)
packages/common/js/completion.js (1)
currentSchema(186-186)packages/canvas/container/src/container.ts (1)
getSchema(97-97)
packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.ts (1)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (1)
pickSectionByHeading(5-46)
packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts (1)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (1)
pickSectionByHeading(5-46)
packages/common/composable/mcp/index.ts (5)
packages/common/composable/mcp/toolUtils.ts (3)
initRegisterTools(106-142)UpdateToolConfig(10-18)updateTool(97-103)packages/common/composable/mcp/resources.ts (6)
initRegisterResources(178-227)registerResources(39-78)getResourceList(81-99)getResourceByUri(102-117)removeResource(120-140)updateResource(143-175)packages/common/composable/mcp/baseTools/index.ts (1)
getBaseTools(7-12)packages/common/composable/mcp/type.ts (4)
ToolItem(21-29)IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)packages/register/src/common.ts (1)
getAllMergeMeta(69-71)
packages/common/composable/mcp/toolUtils.ts (1)
packages/common/composable/mcp/type.ts (1)
IState(33-46)
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts (1)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (1)
pickSectionByHeading(5-46)
packages/common/composable/mcp/baseTools/utils.ts (1)
packages/common/composable/mcp/type.ts (3)
ResourceItem(73-85)ResourceTemplateItem(87-103)IState(33-46)
packages/common/composable/mcp/baseTools/readResources.ts (2)
packages/common/composable/mcp/type.ts (1)
IState(33-46)packages/common/composable/mcp/baseTools/utils.ts (1)
readResourceContent(162-194)
packages/common/composable/mcp/resources.ts (1)
packages/common/composable/mcp/type.ts (3)
IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)
🪛 markdownlint-cli2 (0.18.1)
packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.md
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
235-235: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
239-239: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
243-243: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
251-251: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
255-255: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
265-265: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
288-288: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
314-314: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (11)
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md (2)
17-44: Convert bold pseudo-headings into proper headingsMarkdownlint MD036 is still firing because lines such as Line 17 (
**场景**) and Line 40 (**说明**) are using bold text instead of real headings. Please promote these to actual headings (e.g.,#### 场景：管理页面的状态变量,#### 说明) or drop the emphasis so the linter passes. Apply the same fix across the file wherever bold-only headings remain. As per coding guidelines
152-156: Tag fenced blocks with a languageThe fenced block in this section (and the other unlabeled fences in the doc) still lacks a language tag, so markdownlint MD040 continues to fail. Annotate the fences with an appropriate language such as
text(e.g.,```text) to satisfy the linter. Please update every unlabeled fence in this file. As per coding guidelinespackages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.md (2)
10-36: Stop using bold text as headingsLines like Line 10 (
**用户需求**) and Line 12 (**使用 Sequential Thinking 规划**) are still bold-only pseudo-headings, triggering markdownlint MD036. Convert them into real headings (e.g.,#### 用户需求/#### 使用 Sequential Thinking 规划) or regular paragraphs, and repeat the fix for every similar instance in this document. As per coding guidelines
13-21: Add language identifiers to fenced blocksThe Sequential Thinking example fence (Line 13) — along with the other unlabeled fences — still lacks a language tag, so MD040 remains unresolved. Label these fences with
text(or another suitable language) to make markdownlint pass, covering all remaining unlabeled fences in the file. As per coding guidelinespackages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts (1)
11-13: Align section keys with actual Markdown headingsThe enum entries
case,do-dont, andfaqstill have no matching##headings ineditPageSchemaExample.md, sopickSectionByHeadingreturns an empty string and the template serves blank content. Please either add the corresponding sections to the Markdown or drop these keys (and their enum values) so every advertised section returns real data. [See previous review for the same issue.]packages/common/composable/mcp/baseTools/utils.ts (1)
96-117: Implement the promised local fallback after remote read failure.The docstring still says “远端优先…失败时回退到本地”, yet we immediately return
{ ok: false }when the remote read fails. Please reuse the locally registered resource (e.g.,state.resourceInstanceMaporstate.resources’readCallback) before giving up, so callers actually get the fallback they rely on.Apply this diff:
- // 不做本地回退 - if (!res) { - return { ok: false, error: 'read_resources_failed' } - } + if (!res) { + try { + const local = + state.resourceInstanceMap?.get(uri) ?? + state.resources?.find((item) => item.uri === uri) + if (local && typeof (local as any).read === 'function') { + res = await (local as any).read() + } else if (local && typeof (local as any).readCallback === 'function') { + res = await (local as any).readCallback() + } + } catch { + // ignore local fallback errors + } + } + + if (!res) { + return { ok: false, error: 'read_resources_failed' } + }packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (1)
10-12: Return an empty result for empty titles.When
titleis blank we now return the entire document, which reintroduces the surprising behaviour that was called out earlier. Make the helper return''so callers don’t accidentally dump whole docs when no section is provided.Apply this diff:
- if (typeof title !== 'string' || !title.trim()) { - return markdown - } + if (typeof title !== 'string' || !title.trim()) { + return '' + }packages/plugins/robot/src/system-prompt.md (1)
12-62: Align examples with the “no descriptive tool text” rule.The examples still show lines like “assistant: 调用
discover_resources工具…”, but later (Lines 136‑139) we explicitly forbid emitting descriptive tool-call text. Either adjust the examples to demonstrate the allowed invocation format or relax the prohibition; keeping both as-is leaves the prompt internally contradictory.packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.ts (1)
64-74: Fail fast when the section content is missing.Line 64 still returns a resource even when
pickSectionByHeadingcan’t find the heading, so MCP callers quietly receive an empty document—same issue highlighted earlier. Throw whentextis empty/whitespace so the client gets a clear failure instead of silent loss.const text = pickSectionByHeading(aiInstructMd, heading) + if (!text.trim()) { + throw new Error(`Section "${section}" (heading "${heading}") was not found in TinyEngine 操作指南`) + }packages/common/composable/mcp/baseTools/sequentialThinking.ts (2)
88-164: Scope history/branches per session.Lines 88-164 keep
thoughtHistoryandbranchesat module scope, so a new run inherits prior runs’ thoughts/branch IDs, producing bogusthoughtHistoryLengthand leaking private context—this was previously flagged. Derive a session identifier from_extra(or clear when it changes) and store histories per session (e.g.,Map<sessionId, ThoughtData[]>) so each invocation is isolated.
148-148: Export the full Zod schema.Line 148 uses
inputSchema.shape, which downgrades the schema to a raw shape and removes runtime validation—exactly the earlier bug. Provide theZodObjectitself so MCP tooling can parse/safeParse and emit JSON Schema.- inputSchema: inputSchema.shape, + inputSchema,
| '', | ||
| '返回格式：{items: Array<ResourceInfo>, page: number, pageSize: number, total: number}' | ||
| ].join('\n'), | ||
| inputSchema: inputSchema.shape, |
There was a problem hiding this comment.
Return the Zod object instead of .shape.
Line 98 assigns inputSchema.shape, which collapses the schema into a plain shape object and strips all parsing/validation behavior. Downstream registration expects a full ZodObject, so this will break runtime validation and any schema-to-JSON conversion. Pass the schema itself.
- inputSchema: inputSchema.shape,
+ inputSchema,📝 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.
| inputSchema: inputSchema.shape, | |
| inputSchema, |
🤖 Prompt for AI Agents
In packages/common/composable/mcp/baseTools/discoverResources.ts around line 98,
the code assigns inputSchema.shape which extracts the plain shape and removes
Zod parsing/validation functionality; replace the assignment to return the full
ZodObject (pass inputSchema itself) so downstream code receives a Zod schema
with parsing, validation and metadata intact.
| try { | ||
| const [resourcesList, resourceTemplatesList] = await Promise.all([ | ||
| client.listResources().catch(() => null), | ||
| client.listResourceTemplates().catch(() => null) | ||
| ]) | ||
|
|
||
| const remoteResources = resourcesList?.resources || [] | ||
| const remoteTemplates = resourceTemplatesList?.resourceTemplates || [] | ||
|
|
||
| if (Array.isArray(remoteResources)) { | ||
| result.resources = remoteResources | ||
| } | ||
|
|
||
| if (Array.isArray(remoteTemplates)) { | ||
| result.resourceTemplates = remoteTemplates | ||
| } | ||
|
|
||
| result.ok = true | ||
| } catch { | ||
| result.ok = false | ||
| } |
There was a problem hiding this comment.
Fix ok flag when remote fetch fails silently.
Because each remote call is wrapped in .catch(() => null), Promise.all resolves even if both requests fail, and the code always sets result.ok = true. Callers therefore skip the documented local fallback path and end up with empty lists. Set ok only when at least one remote call succeeds (e.g., check resourcesList/resourceTemplatesList before flipping the flag).
Apply this diff:
- const remoteResources = resourcesList?.resources || []
- const remoteTemplates = resourceTemplatesList?.resourceTemplates || []
+ const remoteResources = resourcesList?.resources || []
+ const remoteTemplates = resourceTemplatesList?.resourceTemplates || []
@@
- result.ok = true
+ result.ok = Boolean(resourcesList) || Boolean(resourceTemplatesList)📝 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.
| try { | |
| const [resourcesList, resourceTemplatesList] = await Promise.all([ | |
| client.listResources().catch(() => null), | |
| client.listResourceTemplates().catch(() => null) | |
| ]) | |
| const remoteResources = resourcesList?.resources || [] | |
| const remoteTemplates = resourceTemplatesList?.resourceTemplates || [] | |
| if (Array.isArray(remoteResources)) { | |
| result.resources = remoteResources | |
| } | |
| if (Array.isArray(remoteTemplates)) { | |
| result.resourceTemplates = remoteTemplates | |
| } | |
| result.ok = true | |
| } catch { | |
| result.ok = false | |
| } | |
| try { | |
| const [resourcesList, resourceTemplatesList] = await Promise.all([ | |
| client.listResources().catch(() => null), | |
| client.listResourceTemplates().catch(() => null) | |
| ]) | |
| const remoteResources = resourcesList?.resources || [] | |
| const remoteTemplates = resourceTemplatesList?.resourceTemplates || [] | |
| if (Array.isArray(remoteResources)) { | |
| result.resources = remoteResources | |
| } | |
| if (Array.isArray(remoteTemplates)) { | |
| result.resourceTemplates = remoteTemplates | |
| } | |
| // Only mark ok=true if at least one remote call actually returned a non-null response | |
| result.ok = Boolean(resourcesList) || Boolean(resourceTemplatesList) | |
| } catch { | |
| result.ok = false | |
| } |
| // 批量注册资源（幂等：同 uri 已有实例则 update 对齐） | ||
| export const registerResources = (state: IState, items: ResourceItem[]) => { | ||
| if (!Array.isArray(items) || !items.length) return | ||
| if (!state.server) { | ||
| logger.error('mcp server is not created') | ||
| return | ||
| } | ||
|
|
||
| const map = ensureResourceMap(state) | ||
|
|
||
| const instances: (RegisteredResource | undefined)[] = items.map((item) => { | ||
| const uri = item.uri | ||
| const name = item.name || '' | ||
| const metadata = buildMetadata(item) | ||
|
|
||
| try { | ||
| const exist = map.get(uri) | ||
| if (exist) { | ||
| exist.update({ name, uri, metadata, callback: item.readCallback }) | ||
| return exist | ||
| } | ||
|
|
||
| const instance = state.server!.registerResource(name, uri, metadata, item.readCallback) | ||
| if (instance) { | ||
| map.set(uri, instance) | ||
| } | ||
| return instance | ||
| } catch (e) { | ||
| logger.error('error when register resource', uri, e) | ||
| return undefined | ||
| } | ||
| }) | ||
|
|
||
| try { | ||
| state.server.sendResourceListChanged() | ||
| } catch (e) { | ||
| logger.error('error when sendResourceListChanged after registerResources', e) | ||
| } | ||
|
|
||
| return instances | ||
| } |
There was a problem hiding this comment.
Persist registered resource definitions in state
getResourceList/getResourceByUri derive their data entirely from state.resources, but registerResources never mutates that array. Any resources registered through this API will be invisible to the read/list endpoints (and stay undiscoverable once removed) because the state never records the new definitions. Please mirror what the tool APIs do and update state.resources alongside the SDK registration.
Apply this diff to keep the state in sync:
const map = ensureResourceMap(state)
- const instances: (RegisteredResource | undefined)[] = items.map((item) => {
+ const nextDefs = (state.resources ? [...state.resources] : []) as ResourceItem[]
+
+ const instances: (RegisteredResource | undefined)[] = items.map((item) => {
const uri = item.uri
const name = item.name || ''
const metadata = buildMetadata(item)
+ const defIndex = nextDefs.findIndex((def) => def.uri === uri)
+ if (defIndex >= 0) {
+ nextDefs[defIndex] = { ...nextDefs[defIndex], ...item }
+ } else {
+ nextDefs.push(item)
+ }
try {
const exist = map.get(uri)
@@
try {
state.server.sendResourceListChanged()
} catch (e) {
logger.error('error when sendResourceListChanged after registerResources', e)
}
- return instances
+ state.resources = nextDefs
+
+ return instances
}📝 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.
| // 批量注册资源（幂等：同 uri 已有实例则 update 对齐） | |
| export const registerResources = (state: IState, items: ResourceItem[]) => { | |
| if (!Array.isArray(items) || !items.length) return | |
| if (!state.server) { | |
| logger.error('mcp server is not created') | |
| return | |
| } | |
| const map = ensureResourceMap(state) | |
| const instances: (RegisteredResource | undefined)[] = items.map((item) => { | |
| const uri = item.uri | |
| const name = item.name || '' | |
| const metadata = buildMetadata(item) | |
| try { | |
| const exist = map.get(uri) | |
| if (exist) { | |
| exist.update({ name, uri, metadata, callback: item.readCallback }) | |
| return exist | |
| } | |
| const instance = state.server!.registerResource(name, uri, metadata, item.readCallback) | |
| if (instance) { | |
| map.set(uri, instance) | |
| } | |
| return instance | |
| } catch (e) { | |
| logger.error('error when register resource', uri, e) | |
| return undefined | |
| } | |
| }) | |
| try { | |
| state.server.sendResourceListChanged() | |
| } catch (e) { | |
| logger.error('error when sendResourceListChanged after registerResources', e) | |
| } | |
| return instances | |
| } | |
| export const registerResources = (state: IState, items: ResourceItem[]) => { | |
| if (!Array.isArray(items) || !items.length) return | |
| if (!state.server) { | |
| logger.error('mcp server is not created') | |
| return | |
| } | |
| const map = ensureResourceMap(state) | |
| const nextDefs = (state.resources ? [...state.resources] : []) as ResourceItem[] | |
| const instances: (RegisteredResource | undefined)[] = items.map((item) => { | |
| const uri = item.uri | |
| const name = item.name || '' | |
| const metadata = buildMetadata(item) | |
| // keep state.resources in sync | |
| const defIndex = nextDefs.findIndex((def) => def.uri === uri) | |
| if (defIndex >= 0) { | |
| nextDefs[defIndex] = { ...nextDefs[defIndex], ...item } | |
| } else { | |
| nextDefs.push(item) | |
| } | |
| try { | |
| const exist = map.get(uri) | |
| if (exist) { | |
| exist.update({ name, uri, metadata, callback: item.readCallback }) | |
| return exist | |
| } | |
| const instance = state.server!.registerResource(name, uri, metadata, item.readCallback) | |
| if (instance) { | |
| map.set(uri, instance) | |
| } | |
| return instance | |
| } catch (e) { | |
| logger.error('error when register resource', uri, e) | |
| return undefined | |
| } | |
| }) | |
| try { | |
| state.server.sendResourceListChanged() | |
| } catch (e) { | |
| logger.error('error when sendResourceListChanged after registerResources', e) | |
| } | |
| state.resources = nextDefs | |
| return instances | |
| } |
chilingling
force-pushed
the
feat/addBaseResourceTools
branch
from
cad96bb to
bf0a0c3
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (12)
packages/plugins/robot/src/system-prompt.md (1)
13-59: Eliminate the example vs. rule contradiction.Examples still show literal“调用
xxx工具”文字说明，而第136-139行又明令禁止输出此类描述；按现状无法同时满足两端要求。请二选一：要么把示例改成系统允许的真实工具调用格式，要么放宽/改写规则，允许示例里的文本。Also applies to: 136-139
packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.md (4)
10-24: Resolve markdownlint MD036 on pseudo-headings.The bold-only lines (e.g., Line 10 "用户需求", Line 12 "使用 Sequential Thinking 规划", Line 23 "执行步骤") still trip MD036. Convert them to actual headings (
### 用户需求) or fold them into normal text.
13-21: Add a language tag to the Sequential Thinking fence.The fenced block at Lines 13-21 is unlabeled, so MD040 persists. Annotate it with an appropriate language such as
text.-``` +```text Thought 1/5: 分析需求，用户管理页面需要：列表展示、搜索、新增、编辑、删除 Thought 2/6: 设计页面结构：顶部搜索栏、中间表格、操作按钮 Thought 3/6: 规划状态管理：用户列表、搜索条件、编辑状态 Thought 4/6: 设计交互流程：搜索→列表更新、点击编辑→弹窗 Thought 5/7: 发现需要添加分页功能（调整总数） Thought 6/7: 确定实施顺序：1.基础布局 2.状态 3.方法 4.样式 Thought 7/7: 生成执行计划并验证完整性--- `235-258`: **Convert bold labels to real headings to clear MD036.** Lines 235, 239, 243, 251, and 255 still use bold emphasis as headings (e.g., "**state不是对象**"). Promote them to proper `###`/`####` headings or keep them inline with normal body text so markdownlint passes. --- `265-329`: **Label the decision-tree code fences.** The flowchart-style fences at Lines 265-284, 288-310, and 314-329 lack language annotations, violating MD040. Add a language tag like `text` (or another suitable identifier) to each. ```diff -``` +```text 收到用户请求 ├─ 评估任务复杂度 │ ├─ 简单任务（1-2步）？ │ │ └─ 直接执行操作 ...Repeat for the subsequent decision-tree fences.
packages/common/composable/mcp/baseTools/discoverResources.ts (1)
98-100: Register the Zod schema object, not.shape.Passing
inputSchema.shapedrops theZodObjectinstance, so downstream registration loses parse/validation behavior and JSON schema generation. Supply the schema itself.- inputSchema: inputSchema.shape, + inputSchema,packages/common/composable/mcp/baseTools/sequentialThinking.ts (2)
88-165: Scope history per session to avoid cross-run leakage.
thoughtHistory/brancheslive at module scope, so every invocation inherits prior runs’ data, producing wrongthoughtHistoryLength, branch lists, and leaking user context. Key them by a session/run id (from_extra) or reset per invocation before appending.-const thoughtHistory: ThoughtData[] = [] -const branches: Record<string, ThoughtData[]> = {} +const thoughtHistories = new Map<string, ThoughtData[]>() +const branchHistories = new Map<string, ThoughtData[]>()Then derive a session key (
const sessionKey = _extra?.sessionId ?? _extra?.conversationId ?? 'default') and use the keyed arrays (const history = thoughtHistories.get(sessionKey) ?? [], etc.), updating the maps after mutation.
148-150: Expose the full Zod schema.As in the other tools, exporting
inputSchema.shapestrips the Zod instance and breaks runtime validation/metadata. Return the schema object itself.- inputSchema: inputSchema.shape, + inputSchema,packages/common/composable/mcp/resources.ts (2)
39-78: Persist registered resource definitions in state.
getResourceList/getResourceByUriderive their data entirely fromstate.resources, butregisterResourcesnever mutates that array. Resources registered through this API will be invisible to the read/list endpoints (and stay undiscovered once removed) because the state never records the new definitions. Please mirror what the tool APIs do and updatestate.resourcesalongside the SDK registration.Apply this diff to keep the state in sync:
const map = ensureResourceMap(state) + const nextDefs = (state.resources ? [...state.resources] : []) as ResourceItem[] + const instances: (RegisteredResource | undefined)[] = items.map((item) => { const uri = item.uri const name = item.name || '' const metadata = buildMetadata(item) + const defIndex = nextDefs.findIndex((def) => def.uri === uri) + if (defIndex >= 0) { + nextDefs[defIndex] = { ...nextDefs[defIndex], ...item } + } else { + nextDefs.push(item) + } try { const exist = map.get(uri) @@ try { state.server.sendResourceListChanged() } catch (e) { logger.error('error when sendResourceListChanged after registerResources', e) } + state.resources = nextDefs + return instances }
54-65: Idempotency gap when a resource's URI changes.Looking up the instance by
item.urionly misses previously registered instances whose URI was modified (e.g., definitions changed). This will register a duplicate resource and orphan the old one.Apply this diff to also match by stable name when URI miss occurs:
- const exist = map.get(uri) + let exist = map.get(uri) + if (!exist && name) { + // fallback: find by name to keep idempotency across URI changes + exist = [...map.values()].find(r => r.name === name) + }Optionally, when exist is found by name and uri differs, call
exist.update({ uri })and migrate the map key (same logic as updateResource).packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md (2)
85-92: Correct getter/setter function semantics.The getter/setter functions have the same issues as flagged in
pageSchema.md:
- Line 87: Getter should return a value, not set
this.state.fullName- Line 91: Setter should properly handle the incoming value parameter
Apply the same corrections as suggested for
pageSchema.md:"getter": { "type": "JSFunction", - "value": "function getter(){ this.state.fullName = `${this.state.firstName} ${this.state.lastName}` }" + "value": "function getter() { return `${this.state.firstName} ${this.state.lastName}` }" }, "setter": { "type": "JSFunction", - "value": "function setter(val){ const [firstName, lastName] = val.split(' '); this.emit('update:firstName', firstName); this.emit('update:lastName', lastName) }" + "value": "function setter(val) { const [firstName, lastName] = (val || '').split(' '); this.state.firstName = firstName || ''; this.state.lastName = lastName || '' }" }
234-249: Convert bold emphasis to proper headings.Lines 234-249 use bold text (
**推荐做法**,**需要注意**) as section dividers, which triggers markdownlint MD036 (no-emphasis-as-heading). These should be proper headings.As per coding guidelines
Apply this diff:
-**推荐做法**： +### 推荐做法 + 1. **使用Tailwind优先**：利用Tailwind内置的实用类快速开发 2. **语义化命名**：使用描述性的类名如`.user-card`而非`.card1` 3. **模块化组织**：相关样式放在一起,添加清晰注释 4. **渐进式修改**：使用merge策略逐步添加样式 -**需要注意**： +### 需要注意 + 1. **谨慎覆盖基础样式**：修改component-base-style影响全局 2. **记得绑定className**：添加新类后必须绑定到组件 3. **合理选择样式方式**：
🧹 Nitpick comments (3)
packages/common/composable/mcp/resources.ts (1)
213-214: Consider avoiding type assertions for better type safety.The cast to
anybypasses type checking. SincevariablesandvariablesSchemaUriare defined in theResourceTemplateIteminterface (per the type definitions), you can access them directly without the cast.Apply this diff if the type checker allows direct access:
- variables: toRaw((resourceItem as any).variables), - variablesSchemaUri: toRaw((resourceItem as any).variablesSchemaUri) + variables: toRaw(resourceItem.variables), + variablesSchemaUri: toRaw(resourceItem.variablesSchemaUri)If TypeScript still complains, verify that the
ResourceTemplateIteminterface properly exports these fields or adjust the type narrowing logic.packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts (1)
57-63: Simplify section variable extraction.Line 58 uses
.toString()on a variable that's already typed asstringin thevariablesparameter. This is redundant.Apply this diff to simplify:
- const section = (variables?.section || '').toString() + const section = variables?.section || ''packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editLifeCycleOrMethod.ts (1)
86-115: Warn when merge payload contains invalid function units.During merge, invalid lifecycle/method payloads are silently discarded. Consumers just see a “merged” response with empty
affectedKeys, making the failure opaque. Please emit warnings (similar to the replace branch) so callers know which keys were ignored.One approach:
- const ignoredAdd: string[] = [] + const ignoredAdd: string[] = [] + const invalidAdd: string[] = [] Object.entries(payload?.add || {}).forEach(([k, v]) => { if (k in nextMap) { ignoredAdd.push(k) return } if (isValidJSFuncUnit(v)) { nextMap[k] = v affected.added.push(k) + } else { + invalidAdd.push(k) } }) - const ignoredUpdate: string[] = [] + const ignoredUpdate: string[] = [] + const invalidUpdate: string[] = [] Object.entries(payload?.update || {}).forEach(([k, v]) => { if (!(k in nextMap)) { ignoredUpdate.push(k) return } if (isValidJSFuncUnit(v)) { nextMap[k] = v affected.updated.push(k) + } else { + invalidUpdate.push(k) } }) @@ if (ignoredAdd.length) { warnings.push(`ignored add (already exists): ${ignoredAdd.join(', ')}`) } + if (invalidAdd.length) { + warnings.push(`ignored add (invalid function units): ${invalidAdd.join(', ')}`) + } if (ignoredUpdate.length) { warnings.push(`ignored update (not exists): ${ignoredUpdate.join(', ')}`) } + if (invalidUpdate.length) { + warnings.push(`ignored update (invalid function units): ${invalidUpdate.join(', ')}`) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
packages/canvas/DesignCanvas/src/mcp/index.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/index.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/pageSchema.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.md(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/resources/utils.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editCSS.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editLifeCycleOrMethod.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editSchema.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editState.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/index.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/utils.ts(1 hunks)packages/canvas/DesignCanvas/src/mcp/tools/index.ts(1 hunks)packages/common/composable/mcp/baseTools/discoverResources.ts(1 hunks)packages/common/composable/mcp/baseTools/index.ts(1 hunks)packages/common/composable/mcp/baseTools/readResources.ts(1 hunks)packages/common/composable/mcp/baseTools/searchResources.ts(1 hunks)packages/common/composable/mcp/baseTools/sequentialThinking.ts(1 hunks)packages/common/composable/mcp/baseTools/utils.ts(1 hunks)packages/common/composable/mcp/index.ts(7 hunks)packages/common/composable/mcp/resources.ts(1 hunks)packages/common/composable/mcp/toolUtils.ts(2 hunks)packages/common/composable/mcp/type.ts(2 hunks)packages/plugins/robot/src/system-prompt.md(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/common/composable/mcp/baseTools/index.ts
- packages/canvas/DesignCanvas/src/mcp/tools/index.ts
- packages/common/composable/mcp/baseTools/readResources.ts
- packages/canvas/DesignCanvas/src/mcp/resources/utils.ts
- packages/canvas/DesignCanvas/src/mcp/resources/index.ts
- packages/common/composable/mcp/baseTools/utils.ts
- packages/common/composable/mcp/baseTools/searchResources.ts
- packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.ts
🧰 Additional context used
🧬 Code graph analysis (11)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editSchema.ts (1)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/utils.ts (2)
ERROR_CODES(1-5)nextActionGetSchema(7-9)
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts (1)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (1)
pickSectionByHeading(5-46)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editCSS.ts (2)
packages/canvas/container/src/container.ts (1)
getSchema(97-97)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/utils.ts (2)
ERROR_CODES(1-5)nextActionGetSchema(7-9)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editState.ts (3)
packages/common/js/completion.js (1)
currentSchema(186-186)packages/canvas/container/src/container.ts (1)
getSchema(97-97)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/utils.ts (1)
isNoChange(13-18)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/index.ts (5)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/utils.ts (2)
ERROR_CODES(1-5)nextActionGetSchema(7-9)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editLifeCycleOrMethod.ts (1)
editLifeCycleOrMethod(4-132)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editCSS.ts (1)
editCSS(16-54)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editSchema.ts (1)
editSchema(16-71)packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editState.ts (1)
editState(4-81)
packages/common/composable/mcp/toolUtils.ts (1)
packages/common/composable/mcp/type.ts (1)
IState(33-46)
packages/common/composable/mcp/resources.ts (1)
packages/common/composable/mcp/type.ts (3)
IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editLifeCycleOrMethod.ts (1)
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/utils.ts (2)
isValidJSFuncUnit(11-11)isNoChange(13-18)
packages/common/composable/mcp/baseTools/discoverResources.ts (2)
packages/common/composable/mcp/type.ts (3)
IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)packages/common/composable/mcp/baseTools/utils.ts (1)
tryFetchRemoteLists(12-43)
packages/common/composable/mcp/index.ts (6)
packages/common/composable/mcp/toolUtils.ts (3)
initRegisterTools(106-142)UpdateToolConfig(10-18)updateTool(97-103)packages/common/composable/mcp/resources.ts (6)
initRegisterResources(178-227)registerResources(39-78)getResourceList(81-99)getResourceByUri(102-117)removeResource(120-140)updateResource(143-175)packages/common/composable/mcp/baseTools/index.ts (1)
getBaseTools(7-12)packages/common/composable/mcp/type.ts (4)
ToolItem(21-29)IState(33-46)ResourceItem(73-85)ResourceTemplateItem(87-103)packages/register/src/common.ts (1)
getAllMergeMeta(69-71)packages/canvas/DesignCanvas/src/mcp/resources/index.ts (2)
resources(5-5)resourceTemplates(7-11)
packages/canvas/DesignCanvas/src/mcp/resources/pageSchemaProtocol.ts (1)
packages/canvas/DesignCanvas/src/mcp/resources/utils.ts (1)
pickSectionByHeading(5-46)
🪛 markdownlint-cli2 (0.18.1)
packages/canvas/DesignCanvas/src/mcp/resources/tinyEngineAIInstruct.md
13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
235-235: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
239-239: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
243-243: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
251-251: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
255-255: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
265-265: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
288-288: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
314-314: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (19)
packages/common/composable/mcp/resources.ts (2)
120-140: LGTM!The resource removal logic correctly cleans up the instance and notifies subscribers of the list change. The intentional preservation of the definition (as noted in the comment) maintains the expected behavior.
143-175: LGTM!The resource update logic correctly handles URI changes with proper guards and notifies subscribers of the list change. The map key migration is safe and only executes when the URI truly changes.
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.ts (3)
4-12: LGTM! Section mapping aligns with document structure.All section keys correctly map to existing headings in
editPageSchemaExample.md. The previous concern about missing sections appears to have been addressed.
14-35: LGTM! Resource structure follows MCP conventions.The resource definition is well-structured with appropriate metadata and annotations.
63-63: All expected section headings are present in editPageSchemaExample.md; pickSectionByHeading will return content for each.packages/canvas/DesignCanvas/src/mcp/resources/pageSchema.md (4)
5-38: LGTM! Clear structure definitions.The interface definitions and field descriptions are accurate and well-explained.
147-213: LGTM! Clear documentation for CSS, LifeCycles, and Methods.The examples correctly demonstrate the function unit format and merge strategies.
214-267: LGTM! Node structure and examples are accurate.The children section correctly describes the node tree structure with clear examples.
268-325: LGTM! Condition and loop examples are correct.The examples accurately demonstrate conditional and loop rendering patterns.
packages/canvas/DesignCanvas/src/mcp/resources/editPageSchemaExample.md (1)
1-11: LGTM! Clear introduction and principles.The document overview and key principles provide good context for users of the
edit_page_schematool.packages/canvas/DesignCanvas/src/mcp/index.ts (1)
1-26: LGTM! Clean integration of new tools and resources.The additions properly extend the MCP API surface with EditPageSchema tooling and resource templates. The import/export structure follows existing patterns.
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/utils.ts (1)
1-18: LGTM! Well-designed utility functions.All helper functions are correctly implemented:
- ERROR_CODES uses const assertion for type safety
- nextActionGetSchema provides consistent error recovery
- isValidJSFuncUnit properly validates function units
- isNoChange correctly detects no-op scenarios
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editCSS.ts (2)
4-14: LGTM! Safe CSS concatenation logic.The
computeAppendedCsshelper correctly handles edge cases including empty strings and missing trailing newlines.
16-54: LGTM! Robust CSS editing implementation.The function properly handles both replace and merge strategies with appropriate validation and change detection. Error handling includes helpful recovery actions.
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editSchema.ts (3)
4-14: LGTM! Appropriate use of whitelist for security.Using a Set for allowed keys provides efficient lookup and prevents unauthorized schema modifications.
33-40: Clarify replace strategy semantics.The
replacestrategy usesimportSchema(Object.assign({}, currentSchema, provided)), which merges the provided keys into the current schema rather than replacing it entirely. This behavior seems similar to a merge operation.Consider whether:
- The naming accurately reflects the behavior (merge in both strategies, just with different validation)
- A true "replace" should use
importSchema(provided)without merging with current schemaCan you verify this is the intended behavior? If yes, consider adding a comment explaining why both strategies merge with current schema.
42-70: LGTM! Merge strategy correctly tracks changes.The merge implementation properly filters allowed keys, detects changes, and returns detailed tracking information.
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editState.ts (2)
14-42: LGTM! Replace strategy handles both full and partial replacement.The two-path implementation (with
alloradd+update) is well-designed and includes helpful warnings whenremoveis ignored.
44-81: LGTM! Merge strategy correctly implements top-level state merging.The merge implementation properly handles add/update/remove operations with correct conditional logic and change detection.
| 2. getter/setter 示例： | ||
|
|
||
| ```json | ||
| { | ||
| "state": { | ||
| "firstName": "", | ||
| "lastName": "", | ||
| "fullName": { | ||
| "defaultValue": "", | ||
| "accessor": { | ||
| "getter": { | ||
| "type": "JSFunction", | ||
| "value": "function getter() { this.state.fullName = `${this.props.firstName} ${this.props.lastName}` }" | ||
| }, | ||
| "setter": { | ||
| "type": "JSFunction", | ||
| "value": "function setter(val) { this.state.fullName = `${this.props.firstName} ${this.props.lastName}` }" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| vue 等效代码： | ||
|
|
||
| ```javascript | ||
| const state = vue.reactive({ | ||
| firstName: "", | ||
| lastName: "", | ||
| fullName: "" | ||
| }) | ||
|
|
||
| vue.watchEffect( | ||
| wrap(function getter() { | ||
| this.state.fullName = `${this.props.firstName} ${this.props.lastName}` | ||
| }) | ||
| ) | ||
|
|
||
| vue.watchEffect( | ||
| wrap(function setter() { | ||
| this.state.fullName = `${this.props.firstName} ${this.props.lastName}` | ||
| }) | ||
| ) | ||
| ``` |
There was a problem hiding this comment.
Correct getter/setter example semantics.
The getter/setter example has logic issues:
- Line 113: The getter function sets
this.state.fullNameinstead of returning a value. Getters should return the computed value. - Line 117: The setter function also sets
this.state.fullNamebut doesn't properly destructure the input parameterval. - Lines 134-144: The Vue equivalent using
watchEffectdoesn't accurately represent getter/setter behavior. Getters typically use computed properties.
Consider this corrected example:
"getter": {
"type": "JSFunction",
- "value": "function getter() { this.state.fullName = `${this.props.firstName} ${this.props.lastName}` }"
+ "value": "function getter() { return `${this.state.firstName} ${this.state.lastName}` }"
},
"setter": {
"type": "JSFunction",
- "value": "function setter(val) { this.state.fullName = `${this.props.firstName} ${this.props.lastName}` }"
+ "value": "function setter(val) { const [firstName, lastName] = (val || '').split(' '); this.state.firstName = firstName || ''; this.state.lastName = lastName || '' }"
}And the Vue equivalent should use computed:
const state = vue.reactive({
firstName: "",
lastName: "",
- fullName: ""
})
-vue.watchEffect(
- wrap(function getter() {
- this.state.fullName = `${this.props.firstName} ${this.props.lastName}`
- })
-)
-
-vue.watchEffect(
- wrap(function setter() {
- this.state.fullName = `${this.props.firstName} ${this.props.lastName}`
- })
-)
+const fullName = vue.computed({
+ get: () => `${state.firstName} ${state.lastName}`,
+ set: (val) => {
+ const [firstName, lastName] = (val || '').split(' ')
+ state.firstName = firstName || ''
+ state.lastName = lastName || ''
+ }
+})🤖 Prompt for AI Agents
packages/canvas/DesignCanvas/src/mcp/resources/pageSchema.md around lines
101-145: the getter currently writes to this.state.fullName instead of returning
the computed value, the setter ignores/destructures its input and also writes
incorrectly, and the Vue example uses watchEffect instead of a proper computed
getter/setter; change the JSON getter to return the computed string (not assign
state), change the JSON setter to accept a single parameter (val) and update the
underlying state appropriately (e.g., assign fullName or split val to update
firstName/lastName as intended), and replace the Vue watchEffect examples with a
single vue.computed that provides a proper get that returns the concatenated
name and a set that updates state based on the incoming val.
| Object.entries(addMap).forEach(([k, v]) => { | ||
| if (isValidJSFuncUnit(v)) { | ||
| newMap[k] = v | ||
| } else { | ||
| invalidReplaceKeys.push(k) | ||
| } | ||
| }) | ||
|
|
||
| Object.entries(updateMap).forEach(([k, v]) => { | ||
| if (isValidJSFuncUnit(v)) { | ||
| newMap[k] = v | ||
| } else { | ||
| invalidReplaceKeys.push(k) | ||
| } | ||
| }) | ||
|
|
||
| if (payload?.remove?.length) { | ||
| warnings.push(`remove ignored in replace without all: ${payload.remove.join(', ')}`) | ||
| } | ||
| if (invalidReplaceKeys.length) { | ||
| warnings.push(`ignored invalid ${sectionKey} function units: ${invalidReplaceKeys.join(', ')}`) | ||
| } | ||
|
|
||
| updateSchema({ [sectionKey]: newMap }) | ||
|
|
||
| return { | ||
| message: `${sectionKey} rebuilt by add+update`, | ||
| affectedKeys: { added: Object.keys(addMap), updated: Object.keys(updateMap), removed: [] }, | ||
| warnings |
There was a problem hiding this comment.
Fix affectedKeys to reflect only applied functions.
Invalid JSFunction units are filtered out, yet affectedKeys still lists them because we spread Object.keys(addMap) / Object.keys(updateMap). That misleads clients into believing failed entries were applied. Please track only the keys whose payloads passed validation.
Apply this diff:
const newMap: Record<string, any> = {}
const addMap = payload?.add || {}
const updateMap = payload?.update || {}
const invalidReplaceKeys: string[] = []
+ const addedKeys: string[] = []
+ const updatedKeys: string[] = []
Object.entries(addMap).forEach(([k, v]) => {
if (isValidJSFuncUnit(v)) {
newMap[k] = v
+ addedKeys.push(k)
} else {
invalidReplaceKeys.push(k)
}
})
Object.entries(updateMap).forEach(([k, v]) => {
if (isValidJSFuncUnit(v)) {
newMap[k] = v
+ updatedKeys.push(k)
} else {
invalidReplaceKeys.push(k)
}
})
@@
return {
message: `${sectionKey} rebuilt by add+update`,
- affectedKeys: { added: Object.keys(addMap), updated: Object.keys(updateMap), removed: [] },
+ affectedKeys: { added: addedKeys, updated: updatedKeys, removed: [] },
warnings
}📝 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.
| Object.entries(addMap).forEach(([k, v]) => { | |
| if (isValidJSFuncUnit(v)) { | |
| newMap[k] = v | |
| } else { | |
| invalidReplaceKeys.push(k) | |
| } | |
| }) | |
| Object.entries(updateMap).forEach(([k, v]) => { | |
| if (isValidJSFuncUnit(v)) { | |
| newMap[k] = v | |
| } else { | |
| invalidReplaceKeys.push(k) | |
| } | |
| }) | |
| if (payload?.remove?.length) { | |
| warnings.push(`remove ignored in replace without all: ${payload.remove.join(', ')}`) | |
| } | |
| if (invalidReplaceKeys.length) { | |
| warnings.push(`ignored invalid ${sectionKey} function units: ${invalidReplaceKeys.join(', ')}`) | |
| } | |
| updateSchema({ [sectionKey]: newMap }) | |
| return { | |
| message: `${sectionKey} rebuilt by add+update`, | |
| affectedKeys: { added: Object.keys(addMap), updated: Object.keys(updateMap), removed: [] }, | |
| warnings | |
| const newMap: Record<string, any> = {} | |
| const addMap = payload?.add || {} | |
| const updateMap = payload?.update || {} | |
| const invalidReplaceKeys: string[] = [] | |
| const addedKeys: string[] = [] | |
| const updatedKeys: string[] = [] | |
| Object.entries(addMap).forEach(([k, v]) => { | |
| if (isValidJSFuncUnit(v)) { | |
| newMap[k] = v | |
| addedKeys.push(k) | |
| } else { | |
| invalidReplaceKeys.push(k) | |
| } | |
| }) | |
| Object.entries(updateMap).forEach(([k, v]) => { | |
| if (isValidJSFuncUnit(v)) { | |
| newMap[k] = v | |
| updatedKeys.push(k) | |
| } else { | |
| invalidReplaceKeys.push(k) | |
| } | |
| }) | |
| if (payload?.remove?.length) { | |
| warnings.push(`remove ignored in replace without all: ${payload.remove.join(', ')}`) | |
| } | |
| if (invalidReplaceKeys.length) { | |
| warnings.push(`ignored invalid ${sectionKey} function units: ${invalidReplaceKeys.join(', ')}`) | |
| } | |
| updateSchema({ [sectionKey]: newMap }) | |
| return { | |
| message: `${sectionKey} rebuilt by add+update`, | |
| affectedKeys: { added: addedKeys, updated: updatedKeys, removed: [] }, | |
| warnings | |
| } |
🤖 Prompt for AI Agents
packages/canvas/DesignCanvas/src/mcp/tools/editPageSchema/editLifeCycleOrMethod.ts
around lines 40 to 68: affectedKeys currently uses Object.keys(addMap) and
Object.keys(updateMap) which includes invalid entries filtered out later; change
the code to track and populate arrays of successfully applied add keys and
successfully applied update keys as you validate (push the key into appliedAdded
or appliedUpdated inside the isValidJSFuncUnit true branch), then use those
arrays in affectedKeys (removed remains []); ensure warnings still record
invalidReplaceKeys as before.
1 participant
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
新增 MCP 资源支持
新增 MCP 资源
新增编辑页面 schema 工具类
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Documentation
Chores