Skip to content

Commit a31c298

Browse files
Copilothotlong
andauthored
refactor: address code review feedback — extract helper, use Zod schemas, fix type safety
- Extract formatToolOutput() static helper for output transformation - Replace `as any` casts in tool handler with explicit Record<string,unknown> - Use Zod schemas for prompt argsSchema (z.string().describe() vs raw objects) - Add zod@^4.3.6 dependency aligned with @objectstack/spec - Document duck-typing pattern for IAIService.toolRegistry access Agent-Logs-Url: https://github.com/objectstack-ai/framework/sessions/5eb90a65-846f-4f24-947f-1c0e7ea5090e Co-authored-by: hotlong <50353452+hotlong@users.noreply.github.com>
1 parent 57a1c9b commit a31c298

File tree

4 files changed

+36
-24
lines changed

4 files changed

+36
-24
lines changed

packages/plugins/plugin-mcp-server/package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@
2020
"dependencies": {
2121
"@modelcontextprotocol/sdk": "^1.29.0",
2222
"@objectstack/core": "workspace:*",
23-
"@objectstack/spec": "workspace:*"
23+
"@objectstack/spec": "workspace:*",
24+
"zod": "^4.3.6"
2425
},
2526
"devDependencies": {
2627
"@types/node": "^25.5.2",

packages/plugins/plugin-mcp-server/src/mcp-server-runtime.ts

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ import { McpServer, ResourceTemplate } from '@modelcontextprotocol/sdk/server/mc
44
import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js';
55
import type { Logger, IMetadataService, IDataEngine, AIToolDefinition } from '@objectstack/spec/contracts';
66
import type { Agent } from '@objectstack/spec';
7-
import type { ToolRegistry } from './types.js';
7+
import type { ToolRegistry, ToolExecutionResult } from './types.js';
8+
import { z } from 'zod';
89

910
/**
1011
* Configuration for the MCP Server Runtime.
@@ -91,6 +92,22 @@ export class MCPServerRuntime {
9192
return this.started;
9293
}
9394

95+
// ── Helpers ─────────────────────────────────────────────────────
96+
97+
/**
98+
* Extract the text value from a ToolExecutionResult's output.
99+
*
100+
* The output may be a `{ type: 'text', value: string }` object (from the
101+
* Vercel AI SDK ToolResultPart) or any serialisable value.
102+
*/
103+
private static formatToolOutput(result: ToolExecutionResult): string {
104+
const output = result.output;
105+
if (output && typeof output === 'object' && 'value' in output) {
106+
return String((output as { value: unknown }).value);
107+
}
108+
return JSON.stringify(output ?? '');
109+
}
110+
94111
// ── Tool Bridge ────────────────────────────────────────────────
95112

96113
/**
@@ -133,9 +150,10 @@ export class MCPServerRuntime {
133150
},
134151
},
135152
async (extra) => {
136-
// MCP SDK passes the raw arguments when no inputSchema is defined
137-
// We need to extract them from the request
138-
const args = (extra as any)?.arguments ?? (extra as any)?.params?.arguments ?? {};
153+
// The MCP SDK passes tool arguments via the extra.arguments property
154+
// when registerTool is called without an inputSchema.
155+
const rawExtra = extra as Record<string, unknown>;
156+
const args = (rawExtra.arguments ?? {}) as Record<string, unknown>;
139157

140158
try {
141159
const result = await toolRegistry.execute({
@@ -145,9 +163,7 @@ export class MCPServerRuntime {
145163
input: args,
146164
});
147165

148-
const outputText = result.output && typeof result.output === 'object' && 'value' in result.output
149-
? String(result.output.value)
150-
: JSON.stringify(result.output ?? '');
166+
const outputText = MCPServerRuntime.formatToolOutput(result);
151167

152168
if (result.isError) {
153169
return {
@@ -370,22 +386,10 @@ export class MCPServerRuntime {
370386
description: 'Load an agent\'s system prompt with optional UI context. ' +
371387
'Use the agentName argument to select which agent\'s instructions to use.',
372388
argsSchema: {
373-
agentName: {
374-
type: 'string' as any,
375-
description: 'Name of the agent to load (e.g. "data_chat", "metadata_assistant")',
376-
},
377-
objectName: {
378-
type: 'string' as any,
379-
description: 'Current object the user is viewing',
380-
},
381-
recordId: {
382-
type: 'string' as any,
383-
description: 'Currently selected record ID',
384-
},
385-
viewName: {
386-
type: 'string' as any,
387-
description: 'Current view name',
388-
},
389+
agentName: z.string().describe('Name of the agent to load (e.g. "data_chat", "metadata_assistant")'),
390+
objectName: z.string().optional().describe('Current object the user is viewing'),
391+
recordId: z.string().optional().describe('Currently selected record ID'),
392+
viewName: z.string().optional().describe('Current view name'),
389393
},
390394
},
391395
async (args) => {

packages/plugins/plugin-mcp-server/src/plugin.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ export class MCPServerPlugin implements Plugin {
7979
if (!this.runtime) return;
8080

8181
// ── Bridge tools from AIService ──
82+
// The IAIService contract does not formally include `toolRegistry` because
83+
// it is an implementation detail of AIService. We use duck-typing here to
84+
// avoid a hard dependency on @objectstack/service-ai while still bridging
85+
// tools when the full AIService implementation is present.
8286
try {
8387
const aiService = ctx.getService<IAIService & { toolRegistry?: ToolRegistry }>('ai');
8488
if (aiService?.toolRegistry) {

pnpm-lock.yaml

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)