feat: add globalState service and mcp tool #1635
Conversation
WalkthroughRemoved Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant UI as State UI (Main.vue)
participant GSS as GlobalStateService (MetaService)
participant GS as appSchemaState.globalState
participant HTTP as updateGlobalStateHttp
participant Meta as META_SERVICE.GlobalService
User->>UI: Confirm add/update
UI->>GSS: getGlobalStateById(id)
alt exists
UI->>GSS: updateGlobalState({ id, state, getters, actions })
else not exists
UI->>GSS: addGlobalState({ id, state, getters, actions })
end
GSS->>GS: mutate local list
GSS->>Meta: getBaseInfo() -> appId
GSS->>HTTP: updateGlobalStateHttp(appId, { global_state })
HTTP-->>GSS: response (maybe global_state)
alt response includes global_state
GSS->>GS: replace with server list
end
GSS-->>UI: Promise resolved/rejected
UI-->>User: Notify success/error
sequenceDiagram
autonumber
actor MCP as MCP Client
participant Tool as add_or_modify_global_state
participant API as GlobalStateService API
MCP->>Tool: callback(args: { id, state, getters?, actions? })
Tool->>API: getGlobalStateById(id)
alt found
Tool->>API: updateGlobalState(payload)
else not found
Tool->>API: addGlobalState(payload)
end
API-->>Tool: result
Tool-->>MCP: content: [{ type: "text", text: JSON.stringify({...}) }]
sequenceDiagram
autonumber
actor MCP as MCP Client
participant Get as get_global_state
participant Del as delete_global_state
participant API as GlobalStateService API
MCP->>Get: callback({ id? })
alt id omitted
Get->>API: getGlobalState()
API-->>Get: list
Get-->>MCP: success with list
else id provided
Get->>API: getGlobalStateById(id)
alt not found
Get-->>MCP: error with next_action: get_global_state({})
else found
Get-->>MCP: success with item
end
end
MCP->>Del: callback({ id })
Del->>API: getGlobalStateById(id)
alt found
Del->>API: deleteGlobalState(id)
API-->>Del: ok
Del-->>MCP: success with deleted item
else not found
Del-->>MCP: error with next_action: get_global_state({})
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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)
🔇 Additional comments (12)
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. 🧪 Early access (Sonnet 4.5): enabledWe are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience. Note:
Comment |
![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 (8)
packages/plugins/state/src/js/constants.ts (1)
18-23: Good use of const assertion; consider exporting the union type.Exporting the inferred union avoids repeating the typeof expression at call sites.
export const OPTION_TYPE = { ADD: 'add', UPDATE: 'update', COPY: 'copy' } as const + +export type OptionType = (typeof OPTION_TYPE)[keyof typeof OPTION_TYPE]packages/plugins/state/src/mcp/tools/getGlobalState.ts (1)
18-21: Treat empty string id as provided; use undefined check.- if (!id) { - const list = getGlobalState() + if (id === undefined) { + const list = listGlobalState()packages/plugins/state/src/js/globalStateService.ts (3)
5-10: Align IGlobalState field types with actual usage (editor yields strings).
state/getters/actionsflow from Monaco editors and are strings in UI; widen types to prevent unsafeanyand mismatches.Apply this diff:
interface IGlobalState { id: string - state: Record<string, any> - getters: Record<string, any> - actions: Record<string, any> + state: string | Record<string, any> + getters: string | Record<string, any> + actions: string | Record<string, any> }
77-80: Type the return of getGlobalStateById.Improves DX and catches mistakes at call sites.
-const getGlobalStateById = (id: string) => { +const getGlobalStateById = (id: string): IGlobalState | undefined => { const storeList = useResource().appSchemaState.globalState || [] return storeList.find((store) => store.id === id) }
17-34: Concurrent updates can clobber each other; consider simple serialization.Multiple rapid add/update/delete calls read-modify-write the same array; last write wins. Queue ops or refetch after write to merge safely.
I can sketch a minimal in-file promise queue if you want.
Also applies to: 37-55, 58-75
packages/plugins/state/src/Main.vue (3)
160-163: Guard against undefined and avoid O(n²) object spreads when mapping by id.Use a fallback and a simple loop for performance.
Apply this diff:
- const list = getGlobalState() - return list.reduce((acc: Record<string, any>, store: Record<string, any>) => ({ ...acc, [store.id]: store }), {}) + const list = getGlobalState() || [] + const map: Record<string, any> = {} + for (const store of list) { + map[store.id] = store + } + return map
354-385: Type the remove key.Keeps signatures consistent with other handlers.
- const remove = (key) => { + const remove = (key: string) => {
474-479: Drop unusedopenfrom returned bindings.Not referenced in template; reduces noise.
- open,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
packages/plugins/page/src/mcp/tools/addPage.ts(0 hunks)packages/plugins/page/src/mcp/tools/changePageBasicInfo.ts(0 hunks)packages/plugins/page/src/mcp/tools/delPage.ts(0 hunks)packages/plugins/page/src/mcp/tools/getPageDetail.ts(0 hunks)packages/plugins/page/src/mcp/tools/getPageList.ts(0 hunks)packages/plugins/state/index.ts(1 hunks)packages/plugins/state/package.json(1 hunks)packages/plugins/state/src/Main.vue(10 hunks)packages/plugins/state/src/js/constants.ts(1 hunks)packages/plugins/state/src/js/globalStateService.ts(1 hunks)packages/plugins/state/src/mcp/index.ts(1 hunks)packages/plugins/state/src/mcp/tools/addOrModifyGlobalState.ts(1 hunks)packages/plugins/state/src/mcp/tools/deleteGlobalState.ts(1 hunks)packages/plugins/state/src/mcp/tools/getGlobalState.ts(1 hunks)packages/register/src/constants.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/plugins/page/src/mcp/tools/delPage.ts
- packages/plugins/page/src/mcp/tools/changePageBasicInfo.ts
- packages/plugins/page/src/mcp/tools/getPageList.ts
- packages/plugins/page/src/mcp/tools/getPageDetail.ts
- packages/plugins/page/src/mcp/tools/addPage.ts
🧰 Additional context used
🧠 Learnings (1)
📚 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/plugins/state/src/js/globalStateService.ts
🧬 Code graph analysis (5)
packages/plugins/state/src/mcp/index.ts (3)
packages/plugins/state/src/mcp/tools/getGlobalState.ts (1)
getGlobalState(8-86)packages/plugins/state/src/mcp/tools/addOrModifyGlobalState.ts (1)
addOrModifyGlobalState(68-133)packages/plugins/state/src/mcp/tools/deleteGlobalState.ts (1)
deleteGlobalState(8-71)
packages/plugins/state/src/mcp/tools/deleteGlobalState.ts (2)
packages/register/src/common.ts (1)
getMetaApi(20-30)packages/register/src/constants.ts (1)
META_SERVICE(1-25)
packages/plugins/state/src/mcp/tools/addOrModifyGlobalState.ts (2)
packages/register/src/common.ts (1)
getMetaApi(20-30)packages/register/src/constants.ts (1)
META_SERVICE(1-25)
packages/plugins/state/src/js/globalStateService.ts (6)
packages/plugins/state/src/mcp/tools/getGlobalState.ts (1)
getGlobalState(8-86)packages/plugins/state/src/mcp/tools/deleteGlobalState.ts (1)
deleteGlobalState(8-71)packages/register/src/common.ts (1)
getMetaApi(20-30)packages/register/src/constants.ts (1)
META_SERVICE(1-25)packages/plugins/state/src/js/http.ts (1)
updateGlobalState(17-18)packages/register/src/service.ts (1)
defineService(33-77)
packages/plugins/state/src/mcp/tools/getGlobalState.ts (2)
packages/register/src/common.ts (1)
getMetaApi(20-30)packages/register/src/constants.ts (1)
META_SERVICE(1-25)
⏰ 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 (8)
packages/plugins/state/package.json (1)
31-33: Zod runtime dep addition looks good.Adding zod under dependencies (not devDependencies) matches its runtime use in MCP tools.
packages/plugins/state/src/mcp/tools/deleteGlobalState.ts (1)
12-13: Confirm inputSchema contract (shape vs schema).If the MCP host expects a ZodObject, pass inputSchema instead of inputSchema.shape.
packages/plugins/state/src/mcp/tools/addOrModifyGlobalState.ts (1)
72-73: Confirm inputSchema contract (shape vs schema).Double-check whether the host expects inputSchema or inputSchema.shape.
packages/plugins/state/src/mcp/tools/getGlobalState.ts (1)
12-13: Confirm inputSchema contract (shape vs schema).Validate whether the MCP runtime expects the Zod object directly.
packages/register/src/constants.ts (1)
23-25: New META_SERVICE mapping added—verify the service is registered with the same id.If the meta id in globalStateService differs, getMetaApi will return undefined and MCP tools will throw.
packages/plugins/state/index.ts (1)
15-26: Confirm meta id alignment for globalStateServiceglobalStateService sets id to META_SERVICE.GlobalStateService (packages/plugins/state/src/js/globalStateService.ts); confirm META_SERVICE.GlobalStateService === 'engine.service.globalStateService' (from @opentiny/tiny-engine-meta-register). If not, update the meta id to that string so getMetaApi(META_SERVICE.GlobalService) resolves.
packages/plugins/state/src/Main.vue (1)
227-229: Validate name vs. error message semantics.
validNameassigns the provided name toerrorMessage; confirm this is intentional or wire through the actual validation error string from the child.packages/plugins/state/src/js/globalStateService.ts (1)
27-31: Verify HTTP response shape (res vs res.data)updateGlobalState (packages/plugins/state/src/js/http.ts) calls getMetaApi(META_SERVICE.Http).post(...), while globalStateService expects res.global_state — confirm whether the HTTP client returns the raw body or a wrapper (e.g. { data: ... }) and if wrapped change the checks to use res.data.global_state or unwrap before checking. Applies to packages/plugins/state/src/js/globalStateService.ts lines 27–31, 48–52, 68–72.
chilingling
force-pushed
the
feat/addGlobalStateMcpTool
branch
from
29a96ea to
6274822
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/plugins/state/src/Main.vue (1)
474-474: Removeopenfromsetupreturn; it’s not defined.Leaving an undefined export is confusing and may mask mistakes.
- open,
🧹 Nitpick comments (13)
packages/plugins/state/src/mcp/tools/addOrModifyGlobalState.ts (3)
5-7: Harden id validation (empty/invalid identifiers).Make id non-empty and restrict to valid JS identifier pattern to avoid accidental whitespace or illegal names.
- id: z - .string() - .describe(['全局变量唯一标识（store 名称）。若同名已存在则执行更新，否则新增。', '示例："testState"'].join('\n')), + id: z + .string() + .min(1, 'id 不能为空') + .regex(/^[A-Za-z_$][A-Za-z0-9_$]*$/, 'id 必须是合法的 JS 标识符') + .describe(['全局变量唯一标识（store 名称）。若同名已存在则执行更新，否则新增。', '示例："testState"'].join('\n')),
31-35: Ensure function code strings are not empty.Guard against empty getters/actions value fields.
- value: z.string().describe('函数代码，this 指向 state。例如："function getAge(){ return this.age }"') + value: z.string().min(1, '函数代码不能为空').describe('函数代码，this 指向 state。例如："function getAge(){ return this.age }"') @@ - value: z.string().describe('函数代码，可修改 state。例如："function setAge(age){ this.age = age }"') + value: z.string().min(1, '函数代码不能为空').describe('函数代码，可修改 state。例如："function setAge(age){ this.age = age }"')Also applies to: 51-54
95-123: Avoid double reads if the service already returns the updated/created entity.If updateGlobalState/addGlobalState return the entity, prefer that value to reduce an extra read; otherwise keep the current fallback.
packages/plugins/state/src/mcp/tools/deleteGlobalState.ts (2)
5-5: Enforce non-empty id.- id: z.string().describe('要删除的全局变量 id') + id: z.string().min(1, 'id 不能为空').describe('要删除的全局变量 id')
14-16: Trim and validate id at runtime to avoid whitespace-only inputs.- const { id } = args + let { id } = args + id = id.trim() + if (!id) { + return { + content: [ + { + isError: true, + type: 'text', + text: JSON.stringify({ + status: 'error', + error: 'INVALID_ID', + message: 'id 不能为空或空白' + }) + } + ] + } + }packages/plugins/state/src/mcp/tools/getGlobalState.ts (2)
5-5: If id is provided, require it to be non-empty.- id: z.string().optional().describe('可选。若提供则返回指定 id 的全局状态；不提供则返回全量') + id: z.string().min(1, 'id 不能为空').optional().describe('可选。若提供则返回指定 id 的全局状态；不提供则返回全量')
14-15: Trim id at runtime.- const { id } = args || {} + const id = args?.id?.trim()packages/plugins/state/src/js/globalStateService.ts (3)
17-24: Align API behavior: throw on unknown id in delete for consistency with update.
updateGlobalStatethrows if the id is missing;deleteGlobalStatesilently returns. Make them consistent to avoid silent no‑ops.- if (index === -1) { - return - } + if (index === -1) { + throw new Error(`globalState ${id} not found 全局应用状态 ${id} 不存在`) + }
27-35: Tolerate variant HTTP response shapes; don’t assumeres.global_state.Many clients wrap payloads under
data. Normalize the response and fall back safely.- const res = await updateGlobalStateHttp(appId, { global_state: newStoreList }) - - let updatedStoreList = newStoreList - if (Array.isArray(res.global_state)) { - updatedStoreList = res.global_state - } - - useResource().appSchemaState.globalState = updatedStoreList + const res = await updateGlobalStateHttp(appId, { global_state: newStoreList }) + const remote = (res as any)?.data?.global_state ?? (res as any)?.global_state + const updated = Array.isArray(remote) ? remote : newStoreList + resource.appSchemaState.globalState = updatedApply the same pattern in update/add blocks above.
Also applies to: 51-59, 74-83
5-10: Type the model to match actual usage (editors emit strings).
state/getters/actionsare stored as code strings, not objects. Reflect that in the interface.interface IGlobalState { id: string - state: Record<string, any> - getters: Record<string, any> - actions: Record<string, any> + state: string + getters: string + actions: string }packages/plugins/state/src/Main.vue (3)
298-301: Surface the actual error to the user when save fails.Show
error.messagefor faster diagnosis.- } catch (error) { - useNotify({ message: '保存失败!', type: 'error' }) - } + } catch (error) { + useNotify({ message: error instanceof Error ? error.message : '保存失败!', type: 'error' }) + }
338-340: Same here: include error details in CURRENT_STATE save failures.- } catch (error) { - useNotify({ message: '保存失败!', type: 'error' }) - } + } catch (error) { + useNotify({ message: error instanceof Error ? error.message : '保存失败!', type: 'error' }) + }
354-362: Harden removal against missingschema.stateand add typing forkey.Avoid destructuring
undefinedand annotate the arg type.- const remove = (key) => { + const remove = (key: string) => { @@ - const { [key]: deletedKey, ...restState } = schema.state + const { [key]: _deleted, ...restState } = schema.state || {}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
packages/plugins/page/src/mcp/tools/addPage.ts(0 hunks)packages/plugins/page/src/mcp/tools/changePageBasicInfo.ts(0 hunks)packages/plugins/page/src/mcp/tools/delPage.ts(0 hunks)packages/plugins/page/src/mcp/tools/getPageDetail.ts(0 hunks)packages/plugins/page/src/mcp/tools/getPageList.ts(0 hunks)packages/plugins/state/index.ts(1 hunks)packages/plugins/state/package.json(1 hunks)packages/plugins/state/src/Main.vue(10 hunks)packages/plugins/state/src/js/constants.ts(1 hunks)packages/plugins/state/src/js/globalStateService.ts(1 hunks)packages/plugins/state/src/mcp/index.ts(1 hunks)packages/plugins/state/src/mcp/tools/addOrModifyGlobalState.ts(1 hunks)packages/plugins/state/src/mcp/tools/deleteGlobalState.ts(1 hunks)packages/plugins/state/src/mcp/tools/getGlobalState.ts(1 hunks)packages/register/src/constants.ts(1 hunks)
💤 Files with no reviewable changes (5)
- packages/plugins/page/src/mcp/tools/addPage.ts
- packages/plugins/page/src/mcp/tools/changePageBasicInfo.ts
- packages/plugins/page/src/mcp/tools/getPageList.ts
- packages/plugins/page/src/mcp/tools/getPageDetail.ts
- packages/plugins/page/src/mcp/tools/delPage.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/register/src/constants.ts
- packages/plugins/state/package.json
- packages/plugins/state/src/js/constants.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-10-30T02:19:37.775Z
Learnt from: yy-wow
PR: opentiny/tiny-engine#886
File: packages/plugins/state/src/js/http.js:19-19
Timestamp: 2024-10-30T02:19:37.775Z
Learning: In the `packages/plugins/state/src/js/http.js` file, errors for the `requestGlobalState` function are handled by the user, so additional error handling is unnecessary.
Applied to files:
packages/plugins/state/src/mcp/tools/addOrModifyGlobalState.tspackages/plugins/state/src/js/globalStateService.tspackages/plugins/state/src/Main.vue
📚 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/plugins/state/src/mcp/tools/addOrModifyGlobalState.tspackages/plugins/state/src/js/globalStateService.ts
🧬 Code graph analysis (5)
packages/plugins/state/src/mcp/index.ts (3)
packages/plugins/state/src/mcp/tools/getGlobalState.ts (1)
getGlobalState(8-104)packages/plugins/state/src/mcp/tools/addOrModifyGlobalState.ts (1)
addOrModifyGlobalState(68-151)packages/plugins/state/src/mcp/tools/deleteGlobalState.ts (1)
deleteGlobalState(8-89)
packages/plugins/state/src/mcp/tools/getGlobalState.ts (2)
packages/register/src/common.ts (1)
getMetaApi(20-30)packages/register/src/constants.ts (1)
META_SERVICE(1-25)
packages/plugins/state/src/mcp/tools/deleteGlobalState.ts (2)
packages/register/src/common.ts (1)
getMetaApi(20-30)packages/register/src/constants.ts (1)
META_SERVICE(1-25)
packages/plugins/state/src/mcp/tools/addOrModifyGlobalState.ts (3)
packages/register/src/common.ts (1)
getMetaApi(20-30)packages/register/src/constants.ts (1)
META_SERVICE(1-25)packages/plugins/state/src/js/http.ts (1)
updateGlobalState(17-18)
packages/plugins/state/src/js/globalStateService.ts (6)
packages/plugins/state/src/mcp/tools/getGlobalState.ts (1)
getGlobalState(8-104)packages/plugins/state/src/mcp/tools/deleteGlobalState.ts (1)
deleteGlobalState(8-89)packages/register/src/common.ts (1)
getMetaApi(20-30)packages/register/src/constants.ts (1)
META_SERVICE(1-25)packages/plugins/state/src/js/http.ts (1)
updateGlobalState(17-18)packages/register/src/service.ts (1)
defineService(33-77)
⏰ 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 (9)
packages/plugins/state/index.ts (1)
26-26: Re-export looks good.Named re-export of globalStateService improves ergonomics.
packages/plugins/state/src/mcp/index.ts (1)
5-7: Tools wiring LGTM; re-check uniqueness of tool names repo-wide.Avoid collisions during registration.
#!/bin/bash # Ensure tool names are unique across repo rg -nP -C1 "name:\s*'(get_global_state|add_or_modify_global_state|delete_global_state)'" --type tspackages/plugins/state/src/mcp/tools/deleteGlobalState.ts (1)
33-72: Good fixes: service guard and aliasing avoid shadowing.The apis guard and deleteGlobalState → removeGlobalState alias prevent runtime errors and name conflicts.
packages/plugins/state/src/mcp/tools/getGlobalState.ts (1)
33-49: Avoid shadowing the exported getGlobalState tool; alias the service method.Prevents confusion and improves readability.
- const { getGlobalState, getGlobalStateById } = apis + const { getGlobalState: listGlobalState, getGlobalStateById } = apis @@ - const list = getGlobalState() + const list = listGlobalState()packages/plugins/state/src/mcp/tools/addOrModifyGlobalState.ts (1)
68-72: No change needed: inputSchema.shape is the expected inputSchema format per MCP contract. Used consistently across all tools and matches the MCP validation workflow.packages/plugins/state/src/js/globalStateService.ts (2)
12-15: Don’t expose the internal reactive array; return a typed, defensive copy.Returning the underlying array invites accidental external mutation. Return a stable copy and add typing; cache
useResource()once.-const globalState = computed(() => useResource().appSchemaState.globalState || []) - -const getGlobalState = () => globalState.value +const resource = useResource() +const globalState = computed<IGlobalState[]>(() => resource.appSchemaState.globalState || []) + +const getGlobalState = (): readonly IGlobalState[] => globalState.value.slice()Based on learnings
25-26: Nice: immutable next-list computation avoids in-place optimistic updates.Switching to
filter/map/spreadprevents local/remote desync from failed calls. LGTM.Also applies to: 49-50, 72-74
packages/plugins/state/src/Main.vue (2)
287-298: Handle renames: delete the old id after a successful update.Currently, updating with a changed
nameduplicates instead of renaming.- const isUpdate = Boolean(globalState.value[name]) + const isUpdate = Boolean(globalState.value[name]) + const prevKey = updateKey.value + const isRenaming = + flag.value === OPTION_TYPE.UPDATE && prevKey && prevKey !== name @@ - if (isUpdate) { + if (isUpdate) { await updateGlobalState(store) } else { await addGlobalState(store) } + + if (isRenaming) { + await deleteGlobalState(prevKey) + }
342-343: Good: awaitingupdateOrAddGlobalStatekeeps UI state in sync.This addresses prior feedback about swallowed rejections. LGTM.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/plugins/state/src/mcp/tools/getGlobalState.ts (6)
12-12: inputSchema should be the Zod schema (not.shape).Most tool runners accept the Zod object itself (or JSON Schema), not the internal
.shape. Passing.shapecan break validation/types at runtime.Apply this change:
- inputSchema: inputSchema.shape, + inputSchema,If your MCP layer requires JSON Schema instead, convert explicitly (e.g., via zod-to-json-schema) rather than using
.shape.Run the script in my other comment to confirm how other tools declare
inputSchemaacross the repo.
36-38: Support async service methods (safe even if they’re sync).Await the calls to prevent leaking Promises into responses when the service becomes async.
- const list = listGlobalState() + const list = await listGlobalState()- const item = getGlobalStateById(id) + const item = await getGlobalStateById(id)Please confirm whether
GlobalStateServicemethods are synchronous today. If they are, this change is still harmless.Also applies to: 52-53
4-6: Explicitly reject empty ids and avoid falsy checks.Treating
''as “not provided” can mask user errors. Enforce non-empty ids and check forundefined.-const inputSchema = z.object({ - id: z.string().optional().describe('可选。若提供则返回指定 id 的全局状态；不提供则返回全量') -}) +const inputSchema = z.object({ + id: z + .string() + .min(1, 'id must be non-empty') + .optional() + .describe('可选。若提供则返回指定 id 的全局状态；不提供则返回全量') +})- if (!id) { + if (id === undefined) {Also applies to: 36-37
33-33: Defend against malformed service registrations.If the registry returns an object without the expected methods, you’ll get a runtime error. Add a quick type check:
- const { getGlobalState: listGlobalState, getGlobalStateById } = apis + const { getGlobalState: listGlobalState, getGlobalStateById } = apis + if (typeof listGlobalState !== 'function' || typeof getGlobalStateById !== 'function') { + return { + content: [ + { + isError: true, + type: 'text', + text: JSON.stringify({ + status: 'error', + error: 'SERVICE_MALFORMED', + message: 'GlobalStateService methods not available' + }) + } + ] + } + }
37-87: Optional: factor a small response helper to reduce repetition.Consider a tiny util like
toTextContent(payload: unknown, isError = false)to build the{ content: [{ type: 'text', isError?, text }] }envelope uniformly across tools.
8-13: No changes needed for inputSchema.shape or awaits; optionally tighten id check
The use ofinputSchema.shapeand synchronous calls togetGlobalState()/getGlobalStateById()matches all other MCP tools, and the service methods are indeed synchronous. To avoid treating an empty string as “no id,” consider changing the runtime check toif (id !== undefined)or enforcing a non-empty string via the Zod schema (e.g.z.string().min(1).optional()).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugins/state/src/mcp/tools/getGlobalState.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/plugins/state/src/mcp/tools/getGlobalState.ts (2)
packages/register/src/common.ts (1)
getMetaApi(20-30)packages/register/src/constants.ts (1)
META_SERVICE(1-25)
⏰ 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 (1)
packages/plugins/state/src/mcp/tools/getGlobalState.ts (1)
17-31: Good: service null-guard and clear error payload.Nice defensive check with a structured error response. This also resolves the earlier feedback about avoiding destructuring from an undefined service and name shadowing.
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?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores