fix(ai-chat): clearer tool-call validation error for dropped pageId in parallel batches (#190)
In-app AI-chat tools used bare zod schemas, so when the model dropped a required arg (typically pageId) in a parallel/batch tool call, the AI SDK forwarded zod's raw "expected string, received undefined" text to the model — not actionable. Add a centralized modelFriendlyInput(shape) wrapper that keeps the exact JSON Schema contract (required/description/constraints via z.toJSONSchema draft-7) but replaces the raw zod text with a message naming each missing/invalid parameter and reminding the model not to drop ids like pageId in parallel batches. No value is guessed/backfilled (cf. #159). Applied to every in-app tool: the sharedTool() builder and all inline inputSchema in ai-chat-tools.service.ts, plus public-share-chat-tools.service.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -120,18 +120,25 @@ describe('AiChatToolsService deletePage guardrail (H4)', () => {
|
|||||||
const tools = await buildTools();
|
const tools = await buildTools();
|
||||||
const deletePage = tools.deletePage;
|
const deletePage = tools.deletePage;
|
||||||
|
|
||||||
// The Zod input schema only allows `pageId`; parsing strips/ignores extra
|
// The wrapped input schema (modelFriendlyInput) only allows `pageId`;
|
||||||
// keys, so a permanent/force flag is never part of the validated input.
|
// validation strips/ignores extra keys, so a permanent/force flag is never
|
||||||
|
// part of the validated input handed to execute.
|
||||||
const schema = (deletePage as unknown as { inputSchema: unknown })
|
const schema = (deletePage as unknown as { inputSchema: unknown })
|
||||||
.inputSchema as {
|
.inputSchema as {
|
||||||
parse: (v: unknown) => Record<string, unknown>;
|
validate: (
|
||||||
|
v: unknown,
|
||||||
|
) =>
|
||||||
|
| { success: boolean; value?: Record<string, unknown> }
|
||||||
|
| Promise<{ success: boolean; value?: Record<string, unknown> }>;
|
||||||
};
|
};
|
||||||
const parsed = schema.parse({
|
const result = await schema.validate({
|
||||||
pageId: 'page-789',
|
pageId: 'page-789',
|
||||||
permanentlyDelete: true,
|
permanentlyDelete: true,
|
||||||
forceDelete: true,
|
forceDelete: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
expect(result.success).toBe(true);
|
||||||
|
const parsed = result.value as Record<string, unknown>;
|
||||||
expect(parsed).toHaveProperty('pageId', 'page-789');
|
expect(parsed).toHaveProperty('pageId', 'page-789');
|
||||||
expect(parsed).not.toHaveProperty('permanentlyDelete');
|
expect(parsed).not.toHaveProperty('permanentlyDelete');
|
||||||
expect(parsed).not.toHaveProperty('forceDelete');
|
expect(parsed).not.toHaveProperty('forceDelete');
|
||||||
@@ -207,19 +214,26 @@ describe('AiChatToolsService expanded toolset guardrails', () => {
|
|||||||
const tools = await buildTools();
|
const tools = await buildTools();
|
||||||
const transformPage = tools.transformPage;
|
const transformPage = tools.transformPage;
|
||||||
|
|
||||||
// The Zod input schema only allows pageId/transformJs/dryRun; parsing
|
// The wrapped input schema only allows pageId/transformJs/dryRun;
|
||||||
// strips unknown keys, so deleteComments can never reach the client.
|
// validation strips unknown keys, so deleteComments can never reach the
|
||||||
|
// client.
|
||||||
const schema = (transformPage as unknown as { inputSchema: unknown })
|
const schema = (transformPage as unknown as { inputSchema: unknown })
|
||||||
.inputSchema as {
|
.inputSchema as {
|
||||||
parse: (v: unknown) => Record<string, unknown>;
|
validate: (
|
||||||
|
v: unknown,
|
||||||
|
) =>
|
||||||
|
| { success: boolean; value?: Record<string, unknown> }
|
||||||
|
| Promise<{ success: boolean; value?: Record<string, unknown> }>;
|
||||||
};
|
};
|
||||||
const parsed = schema.parse({
|
const result = await schema.validate({
|
||||||
pageId: 'p',
|
pageId: 'p',
|
||||||
transformJs: '(d)=>d',
|
transformJs: '(d)=>d',
|
||||||
dryRun: true,
|
dryRun: true,
|
||||||
deleteComments: true,
|
deleteComments: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
expect(result.success).toBe(true);
|
||||||
|
const parsed = result.value as Record<string, unknown>;
|
||||||
expect(parsed).toHaveProperty('pageId', 'p');
|
expect(parsed).toHaveProperty('pageId', 'p');
|
||||||
expect(parsed).not.toHaveProperty('deleteComments');
|
expect(parsed).not.toHaveProperty('deleteComments');
|
||||||
});
|
});
|
||||||
@@ -395,3 +409,95 @@ describe('AiChatToolsService node-arg JSON-string coercion', () => {
|
|||||||
expect(updatePageJsonCalls).toHaveLength(0);
|
expect(updatePageJsonCalls).toHaveLength(0);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Model-friendly tool-call validation (#190): when the model drops a required
|
||||||
|
* `pageId` in a parallel/batch tool call, the built-in input schema must return
|
||||||
|
* a CLEAR, actionable message (naming the parameter, reminding it not to drop
|
||||||
|
* ids in batches) instead of zod's raw "expected string, received undefined" —
|
||||||
|
* while a valid call still validates. This is wired centrally via
|
||||||
|
* modelFriendlyInput, so it applies to every in-app tool; createComment (the
|
||||||
|
* tool from the bug report) and a sharedTool-built tool (getPage's sibling
|
||||||
|
* getOutline) are exercised here end-to-end through forUser().
|
||||||
|
*/
|
||||||
|
describe('AiChatToolsService model-friendly input validation (#190)', () => {
|
||||||
|
const fakeClient: Partial<DocmostClientLike> = {};
|
||||||
|
const tokenServiceStub = {
|
||||||
|
generateAccessToken: jest.fn().mockResolvedValue('access-token'),
|
||||||
|
generateCollabToken: jest.fn().mockResolvedValue('collab-token'),
|
||||||
|
};
|
||||||
|
let service: AiChatToolsService;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
jest.spyOn(loader, 'loadDocmostMcp').mockResolvedValue(
|
||||||
|
mockLoaded(function () {
|
||||||
|
return fakeClient as DocmostClientLike;
|
||||||
|
} as unknown as loader.DocmostClientCtor),
|
||||||
|
);
|
||||||
|
service = new AiChatToolsService(
|
||||||
|
tokenServiceStub as never,
|
||||||
|
{} as never,
|
||||||
|
{} as never,
|
||||||
|
{} as never,
|
||||||
|
{} as never,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => jest.restoreAllMocks());
|
||||||
|
|
||||||
|
function buildTools() {
|
||||||
|
return service.forUser(
|
||||||
|
{ id: 'user-1', email: 'u@example.com', workspaceId: 'ws-1' } as never,
|
||||||
|
'session-1',
|
||||||
|
'ws-1',
|
||||||
|
'chat-1',
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// The AI SDK Schema produced by modelFriendlyInput exposes `validate`.
|
||||||
|
type ValidatableSchema = {
|
||||||
|
validate: (
|
||||||
|
v: unknown,
|
||||||
|
) =>
|
||||||
|
| { success: boolean; value?: unknown; error?: Error }
|
||||||
|
| Promise<{ success: boolean; value?: unknown; error?: Error }>;
|
||||||
|
};
|
||||||
|
const inputSchemaOf = (t: unknown) =>
|
||||||
|
(t as { inputSchema: unknown }).inputSchema as ValidatableSchema;
|
||||||
|
|
||||||
|
it('createComment: a dropped pageId yields a clear, model-actionable message', async () => {
|
||||||
|
const tools = await buildTools();
|
||||||
|
// The exact failing shape from the bug report's second parallel batch:
|
||||||
|
// content + selection, but pageId silently dropped.
|
||||||
|
const result = await inputSchemaOf(tools.createComment).validate({
|
||||||
|
content: 'A remark',
|
||||||
|
selection: 'титановый проводник',
|
||||||
|
});
|
||||||
|
expect(result.success).toBe(false);
|
||||||
|
expect(result.error?.message).toContain('parameter "pageId": missing (required)');
|
||||||
|
expect(result.error?.message).toContain('parallel/batch tool calls');
|
||||||
|
// Not the raw zod text the model previously received.
|
||||||
|
expect(result.error?.message).not.toContain('received undefined');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('createComment: a valid call with pageId validates successfully', async () => {
|
||||||
|
const tools = await buildTools();
|
||||||
|
const result = await inputSchemaOf(tools.createComment).validate({
|
||||||
|
pageId: '019efe44-0000-0000-0000-000000000000',
|
||||||
|
content: 'A remark',
|
||||||
|
selection: 'титановый проводник',
|
||||||
|
});
|
||||||
|
expect(result.success).toBe(true);
|
||||||
|
expect(result.value).toMatchObject({
|
||||||
|
pageId: '019efe44-0000-0000-0000-000000000000',
|
||||||
|
content: 'A remark',
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it('sharedTool-built tools (getOutline) also get the friendly message on a dropped pageId', async () => {
|
||||||
|
const tools = await buildTools();
|
||||||
|
const result = await inputSchemaOf(tools.getOutline).validate({});
|
||||||
|
expect(result.success).toBe(false);
|
||||||
|
expect(result.error?.message).toContain('parameter "pageId": missing (required)');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import {
|
|||||||
} from './docmost-client.loader';
|
} from './docmost-client.loader';
|
||||||
import { resolveCurrentPageResult } from './current-page.util';
|
import { resolveCurrentPageResult } from './current-page.util';
|
||||||
import { parseNodeArg } from './parse-node-arg';
|
import { parseNodeArg } from './parse-node-arg';
|
||||||
|
import { modelFriendlyInput } from './model-friendly-input';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Per-user, per-request adapter that exposes Docmost READ operations to the
|
* Per-user, per-request adapter that exposes Docmost READ operations to the
|
||||||
@@ -102,9 +103,13 @@ export class AiChatToolsService {
|
|||||||
): Tool =>
|
): Tool =>
|
||||||
tool({
|
tool({
|
||||||
description: spec.description,
|
description: spec.description,
|
||||||
inputSchema: spec.buildShape
|
// Wrap via modelFriendlyInput so a dropped/invalid parameter (e.g. a
|
||||||
? z.object(spec.buildShape(z) as z.ZodRawShape)
|
// pageId omitted in a parallel batch, #190) yields a clear, actionable
|
||||||
: z.object({}),
|
// tool error instead of zod's raw text. No-arg specs still get an empty
|
||||||
|
// object schema.
|
||||||
|
inputSchema: modelFriendlyInput(
|
||||||
|
spec.buildShape ? (spec.buildShape(z) as z.ZodRawShape) : {},
|
||||||
|
),
|
||||||
execute,
|
execute,
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -118,7 +123,7 @@ export class AiChatToolsService {
|
|||||||
'and entities), not a full sentence. If the first results look weak ' +
|
'and entities), not a full sentence. If the first results look weak ' +
|
||||||
'or incomplete, search again with different wording or synonyms ' +
|
'or incomplete, search again with different wording or synonyms ' +
|
||||||
'before answering.',
|
'before answering.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
query: z.string().describe('The search query.'),
|
query: z.string().describe('The search query.'),
|
||||||
limit: z
|
limit: z
|
||||||
.number()
|
.number()
|
||||||
@@ -227,7 +232,7 @@ export class AiChatToolsService {
|
|||||||
'"the current page", or "here" refers to. Returns the page id and title, ' +
|
'"the current page", or "here" refers to. Returns the page id and title, ' +
|
||||||
'or null if the user is not currently on a page. Call this first whenever ' +
|
'or null if the user is not currently on a page. Call this first whenever ' +
|
||||||
'the user refers to the current page without giving an explicit id.',
|
'the user refers to the current page without giving an explicit id.',
|
||||||
inputSchema: z.object({}),
|
inputSchema: modelFriendlyInput({}),
|
||||||
execute: async () => resolveCurrentPageResult(openedPage),
|
execute: async () => resolveCurrentPageResult(openedPage),
|
||||||
}),
|
}),
|
||||||
|
|
||||||
@@ -235,7 +240,7 @@ export class AiChatToolsService {
|
|||||||
description:
|
description:
|
||||||
'Fetch a single page as Markdown by its page id. Returns the page ' +
|
'Fetch a single page as Markdown by its page id. Returns the page ' +
|
||||||
'title and its Markdown content.',
|
'title and its Markdown content.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id (or slugId) of the page.'),
|
pageId: z.string().describe('The id (or slugId) of the page.'),
|
||||||
}),
|
}),
|
||||||
execute: async ({ pageId }) => {
|
execute: async ({ pageId }) => {
|
||||||
@@ -259,7 +264,7 @@ export class AiChatToolsService {
|
|||||||
'Create a new page with a Markdown body in a space, optionally under ' +
|
'Create a new page with a Markdown body in a space, optionally under ' +
|
||||||
'a parent page. Returns the new page id and title. Reversible: a page ' +
|
'a parent page. Returns the new page id and title. Reversible: a page ' +
|
||||||
'can be moved to trash later.',
|
'can be moved to trash later.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
title: z.string().describe('The title of the new page.'),
|
title: z.string().describe('The title of the new page.'),
|
||||||
content: z
|
content: z
|
||||||
.string()
|
.string()
|
||||||
@@ -294,7 +299,7 @@ export class AiChatToolsService {
|
|||||||
description:
|
description:
|
||||||
"Replace a page's body with new Markdown content (and optionally its " +
|
"Replace a page's body with new Markdown content (and optionally its " +
|
||||||
'title). Reversible: the previous version is kept in page history.',
|
'title). Reversible: the previous version is kept in page history.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page to update.'),
|
pageId: z.string().describe('The id of the page to update.'),
|
||||||
content: z.string().describe('The new page body as Markdown.'),
|
content: z.string().describe('The new page body as Markdown.'),
|
||||||
title: z
|
title: z
|
||||||
@@ -316,7 +321,7 @@ export class AiChatToolsService {
|
|||||||
description:
|
description:
|
||||||
"Rename a page (change its title only; the body is untouched). " +
|
"Rename a page (change its title only; the body is untouched). " +
|
||||||
'Reversible: rename back at any time.',
|
'Reversible: rename back at any time.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page to rename.'),
|
pageId: z.string().describe('The id of the page to rename.'),
|
||||||
title: z.string().describe('The new title.'),
|
title: z.string().describe('The new title.'),
|
||||||
}),
|
}),
|
||||||
@@ -331,7 +336,7 @@ export class AiChatToolsService {
|
|||||||
description:
|
description:
|
||||||
'Move a page under a new parent page, or to the space root when no ' +
|
'Move a page under a new parent page, or to the space root when no ' +
|
||||||
'parent is given. Reversible: move it back at any time.',
|
'parent is given. Reversible: move it back at any time.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page to move.'),
|
pageId: z.string().describe('The id of the page to move.'),
|
||||||
parentPageId: z
|
parentPageId: z
|
||||||
.string()
|
.string()
|
||||||
@@ -353,7 +358,7 @@ export class AiChatToolsService {
|
|||||||
description:
|
description:
|
||||||
'Move a page to the trash (SOFT delete only — fully reversible; the ' +
|
'Move a page to the trash (SOFT delete only — fully reversible; the ' +
|
||||||
'page can be restored from trash). This NEVER permanently deletes.',
|
'page can be restored from trash). This NEVER permanently deletes.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page to move to trash.'),
|
pageId: z.string().describe('The id of the page to move to trash.'),
|
||||||
}),
|
}),
|
||||||
// GUARDRAIL (§14 H4): the only field ever passed to the client is
|
// GUARDRAIL (§14 H4): the only field ever passed to the client is
|
||||||
@@ -379,7 +384,7 @@ export class AiChatToolsService {
|
|||||||
'"selection not found" error, retry with a corrected EXACT selection ' +
|
'"selection not found" error, retry with a corrected EXACT selection ' +
|
||||||
'copied verbatim from a single paragraph/block. Reversible via the ' +
|
'copied verbatim from a single paragraph/block. Reversible via the ' +
|
||||||
'comment UI.',
|
'comment UI.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page to comment on.'),
|
pageId: z.string().describe('The id of the page to comment on.'),
|
||||||
content: z.string().describe('The comment body as Markdown.'),
|
content: z.string().describe('The comment body as Markdown.'),
|
||||||
selection: z
|
selection: z
|
||||||
@@ -428,7 +433,7 @@ export class AiChatToolsService {
|
|||||||
description:
|
description:
|
||||||
'Resolve or reopen a top-level comment thread (reversible — toggle ' +
|
'Resolve or reopen a top-level comment thread (reversible — toggle ' +
|
||||||
'the resolved flag). Only top-level comments can be resolved.',
|
'the resolved flag). Only top-level comments can be resolved.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
commentId: z
|
commentId: z
|
||||||
.string()
|
.string()
|
||||||
.describe('The id of the top-level comment to resolve/reopen.'),
|
.describe('The id of the top-level comment to resolve/reopen.'),
|
||||||
@@ -460,7 +465,7 @@ export class AiChatToolsService {
|
|||||||
'List the most recent pages, optionally scoped to a single space. ' +
|
'List the most recent pages, optionally scoped to a single space. ' +
|
||||||
'Returns a bounded list (default 50, max 100). Pass tree:true (with ' +
|
'Returns a bounded list (default 50, max 100). Pass tree:true (with ' +
|
||||||
"spaceId) to instead get the space's full page hierarchy as a nested tree.",
|
"spaceId) to instead get the space's full page hierarchy as a nested tree.",
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
spaceId: z
|
spaceId: z
|
||||||
.string()
|
.string()
|
||||||
.optional()
|
.optional()
|
||||||
@@ -488,7 +493,7 @@ export class AiChatToolsService {
|
|||||||
'List sidebar pages for a space. With no pageId, returns the ' +
|
'List sidebar pages for a space. With no pageId, returns the ' +
|
||||||
"space's ROOT pages; with a pageId, returns that page's direct " +
|
"space's ROOT pages; with a pageId, returns that page's direct " +
|
||||||
'CHILDREN.',
|
'CHILDREN.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
spaceId: z.string().describe('The id of the space.'),
|
spaceId: z.string().describe('The id of the space.'),
|
||||||
pageId: z
|
pageId: z
|
||||||
.string()
|
.string()
|
||||||
@@ -520,7 +525,7 @@ export class AiChatToolsService {
|
|||||||
description:
|
description:
|
||||||
'Read a table as a matrix of cell texts (plus a parallel cellIds ' +
|
'Read a table as a matrix of cell texts (plus a parallel cellIds ' +
|
||||||
'matrix so cells can be addressed for rich edits).',
|
'matrix so cells can be addressed for rich edits).',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page.'),
|
pageId: z.string().describe('The id of the page.'),
|
||||||
tableRef: z
|
tableRef: z
|
||||||
.string()
|
.string()
|
||||||
@@ -536,7 +541,7 @@ export class AiChatToolsService {
|
|||||||
listComments: tool({
|
listComments: tool({
|
||||||
description:
|
description:
|
||||||
'List all comments on a page (content as Markdown).',
|
'List all comments on a page (content as Markdown).',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page.'),
|
pageId: z.string().describe('The id of the page.'),
|
||||||
}),
|
}),
|
||||||
execute: async ({ pageId }) => await client.listComments(pageId),
|
execute: async ({ pageId }) => await client.listComments(pageId),
|
||||||
@@ -544,7 +549,7 @@ export class AiChatToolsService {
|
|||||||
|
|
||||||
getComment: tool({
|
getComment: tool({
|
||||||
description: 'Fetch a single comment by id (content as Markdown).',
|
description: 'Fetch a single comment by id (content as Markdown).',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
commentId: z.string().describe('The id of the comment.'),
|
commentId: z.string().describe('The id of the comment.'),
|
||||||
}),
|
}),
|
||||||
execute: async ({ commentId }) => await client.getComment(commentId),
|
execute: async ({ commentId }) => await client.getComment(commentId),
|
||||||
@@ -554,7 +559,7 @@ export class AiChatToolsService {
|
|||||||
description:
|
description:
|
||||||
'Find new comments across a space (optionally scoped to a subtree) ' +
|
'Find new comments across a space (optionally scoped to a subtree) ' +
|
||||||
'created after a given timestamp.',
|
'created after a given timestamp.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
spaceId: z.string().describe('The id of the space to scan.'),
|
spaceId: z.string().describe('The id of the space to scan.'),
|
||||||
since: z
|
since: z
|
||||||
.string()
|
.string()
|
||||||
@@ -586,7 +591,7 @@ export class AiChatToolsService {
|
|||||||
description:
|
description:
|
||||||
'Fetch a single page-history version including its lossless ' +
|
'Fetch a single page-history version including its lossless ' +
|
||||||
'ProseMirror content.',
|
'ProseMirror content.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
historyId: z.string().describe('The id of the history version.'),
|
historyId: z.string().describe('The id of the history version.'),
|
||||||
}),
|
}),
|
||||||
execute: async ({ historyId }) =>
|
execute: async ({ historyId }) =>
|
||||||
@@ -604,7 +609,7 @@ export class AiChatToolsService {
|
|||||||
'Export a page to a single self-contained Docmost-flavoured ' +
|
'Export a page to a single self-contained Docmost-flavoured ' +
|
||||||
'Markdown file (meta + body + comment threads). Lossless round-trip ' +
|
'Markdown file (meta + body + comment threads). Lossless round-trip ' +
|
||||||
'with importPageMarkdown.',
|
'with importPageMarkdown.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page to export.'),
|
pageId: z.string().describe('The id of the page to export.'),
|
||||||
}),
|
}),
|
||||||
execute: async ({ pageId }) => {
|
execute: async ({ pageId }) => {
|
||||||
@@ -630,7 +635,7 @@ export class AiChatToolsService {
|
|||||||
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node arg ' +
|
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node arg ' +
|
||||||
'may be a JSON object or a JSON string (both accepted). Reversible: ' +
|
'may be a JSON object or a JSON string (both accepted). Reversible: ' +
|
||||||
'the previous version is kept in page history.',
|
'the previous version is kept in page history.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page.'),
|
pageId: z.string().describe('The id of the page.'),
|
||||||
nodeId: z
|
nodeId: z
|
||||||
.string()
|
.string()
|
||||||
@@ -663,7 +668,7 @@ export class AiChatToolsService {
|
|||||||
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node arg ' +
|
'{"type":"text","text":"x","marks":[{"type":"bold"}]}. The node arg ' +
|
||||||
'may be a JSON object or a JSON string (both accepted). Reversible ' +
|
'may be a JSON object or a JSON string (both accepted). Reversible ' +
|
||||||
'via page history.',
|
'via page history.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page.'),
|
pageId: z.string().describe('The id of the page.'),
|
||||||
node: z
|
node: z
|
||||||
.any()
|
.any()
|
||||||
@@ -722,7 +727,7 @@ export class AiChatToolsService {
|
|||||||
'object or a JSON string (both accepted). Omit content for a ' +
|
'object or a JSON string (both accepted). Omit content for a ' +
|
||||||
'title-only update. Reversible: the previous version is kept in page ' +
|
'title-only update. Reversible: the previous version is kept in page ' +
|
||||||
'history.',
|
'history.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page to update.'),
|
pageId: z.string().describe('The id of the page to update.'),
|
||||||
content: z
|
content: z
|
||||||
.any()
|
.any()
|
||||||
@@ -753,7 +758,7 @@ export class AiChatToolsService {
|
|||||||
description:
|
description:
|
||||||
'Insert a row of plain-text cells into a table. Reversible via ' +
|
'Insert a row of plain-text cells into a table. Reversible via ' +
|
||||||
'page history.',
|
'page history.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page.'),
|
pageId: z.string().describe('The id of the page.'),
|
||||||
tableRef: z
|
tableRef: z
|
||||||
.string()
|
.string()
|
||||||
@@ -772,7 +777,7 @@ export class AiChatToolsService {
|
|||||||
tableDeleteRow: tool({
|
tableDeleteRow: tool({
|
||||||
description:
|
description:
|
||||||
'Delete a table row at a 0-based index. Reversible via page history.',
|
'Delete a table row at a 0-based index. Reversible via page history.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page.'),
|
pageId: z.string().describe('The id of the page.'),
|
||||||
tableRef: z
|
tableRef: z
|
||||||
.string()
|
.string()
|
||||||
@@ -787,7 +792,7 @@ export class AiChatToolsService {
|
|||||||
description:
|
description:
|
||||||
'Set the plain-text content of a table cell at [row, col] (0-based). ' +
|
'Set the plain-text content of a table cell at [row, col] (0-based). ' +
|
||||||
'Reversible via page history.',
|
'Reversible via page history.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page.'),
|
pageId: z.string().describe('The id of the page.'),
|
||||||
tableRef: z
|
tableRef: z
|
||||||
.string()
|
.string()
|
||||||
@@ -817,7 +822,7 @@ export class AiChatToolsService {
|
|||||||
'Make a page PUBLICLY accessible and return its public URL. ' +
|
'Make a page PUBLICLY accessible and return its public URL. ' +
|
||||||
'Reversible via unsharePage. Only share when the user explicitly ' +
|
'Reversible via unsharePage. Only share when the user explicitly ' +
|
||||||
'asked, since this exposes the page to anyone with the link.',
|
'asked, since this exposes the page to anyone with the link.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page to share.'),
|
pageId: z.string().describe('The id of the page to share.'),
|
||||||
searchIndexing: z
|
searchIndexing: z
|
||||||
.boolean()
|
.boolean()
|
||||||
@@ -844,7 +849,7 @@ export class AiChatToolsService {
|
|||||||
"page's ProseMirror document for complex/scripted rewrites. dryRun " +
|
"page's ProseMirror document for complex/scripted rewrites. dryRun " +
|
||||||
'(default true) previews a diff WITHOUT writing; set dryRun:false to ' +
|
'(default true) previews a diff WITHOUT writing; set dryRun:false to ' +
|
||||||
'apply. Reversible: applying creates a new page-history snapshot.',
|
'apply. Reversible: applying creates a new page-history snapshot.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z.string().describe('The id of the page to transform.'),
|
pageId: z.string().describe('The id of the page to transform.'),
|
||||||
transformJs: z
|
transformJs: z
|
||||||
.string()
|
.string()
|
||||||
|
|||||||
101
apps/server/src/core/ai-chat/tools/model-friendly-input.spec.ts
Normal file
101
apps/server/src/core/ai-chat/tools/model-friendly-input.spec.ts
Normal file
@@ -0,0 +1,101 @@
|
|||||||
|
import { z } from 'zod';
|
||||||
|
import {
|
||||||
|
modelFriendlyInput,
|
||||||
|
buildModelFriendlyMessage,
|
||||||
|
} from './model-friendly-input';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Unit tests for the centralized in-app tool input wrapper (#190). A dropped or
|
||||||
|
* invalid parameter must surface a clear, model-actionable message (naming the
|
||||||
|
* parameter and reminding the model not to drop ids in parallel batches), while
|
||||||
|
* a valid call validates cleanly and strips unknown keys — and the advertised
|
||||||
|
* JSON Schema keeps the unchanged required/description contract.
|
||||||
|
*/
|
||||||
|
describe('modelFriendlyInput', () => {
|
||||||
|
// Mirrors createComment's shape: pageId is the required id the model drops in
|
||||||
|
// parallel batches; selection is optional with a min length.
|
||||||
|
const shape = {
|
||||||
|
pageId: z.string().describe('The id of the page to comment on.'),
|
||||||
|
content: z.string().describe('The comment body as Markdown.'),
|
||||||
|
selection: z.string().min(1).max(250).optional(),
|
||||||
|
};
|
||||||
|
|
||||||
|
// Loose return type: the AI SDK ValidationResult is a discriminated union, but
|
||||||
|
// these tests assert on both branches, so a flat optional shape is simpler.
|
||||||
|
async function validate(
|
||||||
|
value: unknown,
|
||||||
|
): Promise<{ success: boolean; value?: unknown; error?: Error }> {
|
||||||
|
const schema = modelFriendlyInput(shape);
|
||||||
|
return await schema.validate!(value);
|
||||||
|
}
|
||||||
|
|
||||||
|
it('rejects a dropped required pageId with a clear, actionable message', async () => {
|
||||||
|
const result = await validate({
|
||||||
|
content: 'Looks off here',
|
||||||
|
selection: 'титановый проводник',
|
||||||
|
});
|
||||||
|
expect(result.success).toBe(false);
|
||||||
|
const msg = result.error?.message ?? '';
|
||||||
|
// Names the dropped parameter...
|
||||||
|
expect(msg).toContain('parameter "pageId": missing (required)');
|
||||||
|
// ...and gives an explicit, non-raw instruction (not zod's raw text).
|
||||||
|
expect(msg).toContain('parallel/batch tool calls');
|
||||||
|
expect(msg).not.toContain('expected string, received undefined');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('distinguishes a present-but-invalid parameter from a missing one', async () => {
|
||||||
|
// selection is present but too short (invalid), pageId is missing.
|
||||||
|
const result = await validate({ content: 'x', selection: '' });
|
||||||
|
expect(result.success).toBe(false);
|
||||||
|
const msg = result.error?.message ?? '';
|
||||||
|
expect(msg).toContain('parameter "pageId": missing (required)');
|
||||||
|
expect(msg).toContain('parameter "selection": invalid');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('accepts a valid call and strips unknown keys from the validated value', async () => {
|
||||||
|
const result = await validate({
|
||||||
|
pageId: 'page-1',
|
||||||
|
content: 'A comment',
|
||||||
|
selection: 'anchor text',
|
||||||
|
bogus: true,
|
||||||
|
});
|
||||||
|
expect(result.success).toBe(true);
|
||||||
|
if (!result.success) throw new Error('expected success');
|
||||||
|
expect(result.value).toEqual({
|
||||||
|
pageId: 'page-1',
|
||||||
|
content: 'A comment',
|
||||||
|
selection: 'anchor text',
|
||||||
|
});
|
||||||
|
expect(result.value).not.toHaveProperty('bogus');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('preserves the required/description contract in the advertised JSON Schema', async () => {
|
||||||
|
const schema = modelFriendlyInput(shape);
|
||||||
|
const json = (await schema.jsonSchema) as {
|
||||||
|
required?: string[];
|
||||||
|
properties?: Record<string, { description?: string }>;
|
||||||
|
};
|
||||||
|
// pageId + content stay required; selection stays optional.
|
||||||
|
expect(json.required).toEqual(expect.arrayContaining(['pageId', 'content']));
|
||||||
|
expect(json.required).not.toContain('selection');
|
||||||
|
expect(json.properties?.pageId.description).toBe(
|
||||||
|
'The id of the page to comment on.',
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('handles a no-arg tool (empty shape) without error', async () => {
|
||||||
|
const schema = modelFriendlyInput({});
|
||||||
|
const result = await schema.validate!({});
|
||||||
|
expect(result.success).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('buildModelFriendlyMessage', () => {
|
||||||
|
it('falls back to a generic message when issues carry an empty path', () => {
|
||||||
|
// safeParse on a non-object yields a root-level issue (empty path).
|
||||||
|
const error = z.object({ a: z.string() }).safeParse('not-an-object');
|
||||||
|
if (error.success) throw new Error('expected failure');
|
||||||
|
const msg = buildModelFriendlyMessage(error.error, 'not-an-object');
|
||||||
|
expect(msg).toContain('parameter "input"');
|
||||||
|
});
|
||||||
|
});
|
||||||
93
apps/server/src/core/ai-chat/tools/model-friendly-input.ts
Normal file
93
apps/server/src/core/ai-chat/tools/model-friendly-input.ts
Normal file
@@ -0,0 +1,93 @@
|
|||||||
|
import { jsonSchema, type Schema } from 'ai';
|
||||||
|
import type { JSONSchema7 } from '@ai-sdk/provider';
|
||||||
|
import { z } from 'zod';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Centralized input-schema wrapper for every in-app AI-chat tool.
|
||||||
|
*
|
||||||
|
* THE PROBLEM (#190): when the model issues PARALLEL / batch tool calls it
|
||||||
|
* sometimes drops an "obvious" repeated required argument (typically `pageId`)
|
||||||
|
* from some of the calls. zod v4 correctly rejects the missing value, but the
|
||||||
|
* AI SDK forwards zod's RAW message ("Invalid input: expected string, received
|
||||||
|
* undefined") straight back to the model, which is not actionable — the model
|
||||||
|
* cannot tell WHICH parameter it dropped or that it must re-send it.
|
||||||
|
*
|
||||||
|
* THE FIX: keep the exact same validation, but replace the raw zod text with a
|
||||||
|
* model-friendly message that names every problematic parameter and tells the
|
||||||
|
* model to re-issue the call with all required parameters present. We do NOT
|
||||||
|
* guess/backfill the value (a silently-assumed "current page" could comment on
|
||||||
|
* the wrong page — cf. #159); the model is simply told to retry correctly.
|
||||||
|
*
|
||||||
|
* HOW IT WORKS: we build the tool's JSON Schema from the zod shape via
|
||||||
|
* `z.toJSONSchema(..., { target: 'draft-7' })` (so the advertised contract —
|
||||||
|
* `required` / `description` / field constraints — is unchanged) and hand the
|
||||||
|
* AI SDK a custom `validate` that runs `z.object(shape).safeParse(value)`. On
|
||||||
|
* failure the AI SDK wraps our returned `Error` in `InvalidToolInputError`, so
|
||||||
|
* our clear text is what reaches the model as the tool error.
|
||||||
|
*/
|
||||||
|
export function modelFriendlyInput<T extends z.ZodRawShape>(
|
||||||
|
shape: T,
|
||||||
|
): Schema<z.output<z.ZodObject<T>>> {
|
||||||
|
const objectSchema = z.object(shape);
|
||||||
|
// draft-07 keeps required/description/constraints intact, matching what the
|
||||||
|
// model already saw — the tool contract does not change.
|
||||||
|
const json = z.toJSONSchema(objectSchema, {
|
||||||
|
target: 'draft-7',
|
||||||
|
}) as JSONSchema7;
|
||||||
|
|
||||||
|
return jsonSchema<z.output<z.ZodObject<T>>>(json, {
|
||||||
|
validate: (value) => {
|
||||||
|
const result = objectSchema.safeParse(value);
|
||||||
|
if (result.success) {
|
||||||
|
return { success: true, value: result.data };
|
||||||
|
}
|
||||||
|
return {
|
||||||
|
success: false,
|
||||||
|
error: new Error(buildModelFriendlyMessage(result.error, value)),
|
||||||
|
};
|
||||||
|
},
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Turn a zod validation failure into a clear, model-actionable message naming
|
||||||
|
* each problematic parameter (and whether it is missing vs. invalid), plus an
|
||||||
|
* explicit reminder not to drop required ids in parallel/batch tool calls.
|
||||||
|
*/
|
||||||
|
export function buildModelFriendlyMessage(
|
||||||
|
error: z.ZodError,
|
||||||
|
value: unknown,
|
||||||
|
): string {
|
||||||
|
const seen = new Set<string>();
|
||||||
|
const parts: string[] = [];
|
||||||
|
for (const issue of error.issues) {
|
||||||
|
const name = issue.path.length ? issue.path.map(String).join('.') : 'input';
|
||||||
|
// A parameter the model omitted entirely reads as `undefined` at its path;
|
||||||
|
// anything else is present-but-invalid (wrong type, too short, etc.).
|
||||||
|
const missing = valueAtPath(value, issue.path) === undefined;
|
||||||
|
const part = `parameter "${name}": ${missing ? 'missing (required)' : 'invalid'}`;
|
||||||
|
if (seen.has(part)) continue;
|
||||||
|
seen.add(part);
|
||||||
|
parts.push(part);
|
||||||
|
}
|
||||||
|
if (parts.length === 0) {
|
||||||
|
// Defensive: a ZodError always has issues, but never emit an empty list.
|
||||||
|
parts.push('input: invalid');
|
||||||
|
}
|
||||||
|
return (
|
||||||
|
`Invalid input for this tool — ${parts.join('; ')}. ` +
|
||||||
|
'Re-issue the call with EVERY required parameter present and valid. ' +
|
||||||
|
"Do not drop ids like pageId, even when making parallel/batch tool calls — " +
|
||||||
|
'each tool call must carry its own pageId.'
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Read the value at a zod issue path; returns undefined if any hop is absent. */
|
||||||
|
function valueAtPath(value: unknown, path: ReadonlyArray<PropertyKey>): unknown {
|
||||||
|
let current: unknown = value;
|
||||||
|
for (const key of path) {
|
||||||
|
if (current === null || typeof current !== 'object') return undefined;
|
||||||
|
current = (current as Record<PropertyKey, unknown>)[key];
|
||||||
|
}
|
||||||
|
return current;
|
||||||
|
}
|
||||||
@@ -5,6 +5,7 @@ import { ShareService } from '../../share/share.service';
|
|||||||
import { SearchService } from '../../search/search.service';
|
import { SearchService } from '../../search/search.service';
|
||||||
import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
import { PageRepo } from '@docmost/db/repos/page/page.repo';
|
||||||
import { jsonToMarkdown } from '../../../collaboration/collaboration.util';
|
import { jsonToMarkdown } from '../../../collaboration/collaboration.util';
|
||||||
|
import { modelFriendlyInput } from './model-friendly-input';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Isolated, READ-ONLY toolset for the ANONYMOUS public-share assistant.
|
* Isolated, READ-ONLY toolset for the ANONYMOUS public-share assistant.
|
||||||
@@ -52,7 +53,7 @@ export class PublicShareChatToolsService {
|
|||||||
'(key terms and entities), not a full sentence. If the first ' +
|
'(key terms and entities), not a full sentence. If the first ' +
|
||||||
'results look weak, search again with different wording before ' +
|
'results look weak, search again with different wording before ' +
|
||||||
'answering. Only pages inside this share are ever returned.',
|
'answering. Only pages inside this share are ever returned.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
query: z.string().describe('The search query.'),
|
query: z.string().describe('The search query.'),
|
||||||
limit: z
|
limit: z
|
||||||
.number()
|
.number()
|
||||||
@@ -87,7 +88,7 @@ export class PublicShareChatToolsService {
|
|||||||
'Markdown, by its page id. Returns the page title and its Markdown ' +
|
'Markdown, by its page id. Returns the page title and its Markdown ' +
|
||||||
'content. Only pages inside this share can be read; reading any ' +
|
'content. Only pages inside this share can be read; reading any ' +
|
||||||
'other page fails.',
|
'other page fails.',
|
||||||
inputSchema: z.object({
|
inputSchema: modelFriendlyInput({
|
||||||
pageId: z
|
pageId: z
|
||||||
.string()
|
.string()
|
||||||
.describe('The id (or slugId) of a page within this share.'),
|
.describe('The id (or slugId) of a page within this share.'),
|
||||||
@@ -142,7 +143,7 @@ export class PublicShareChatToolsService {
|
|||||||
'List the pages (titles + ids) that make up THIS published ' +
|
'List the pages (titles + ids) that make up THIS published ' +
|
||||||
'documentation share, so you can orient yourself before reading or ' +
|
'documentation share, so you can orient yourself before reading or ' +
|
||||||
'searching. Only pages inside this share are listed.',
|
'searching. Only pages inside this share are listed.',
|
||||||
inputSchema: z.object({}),
|
inputSchema: modelFriendlyInput({}),
|
||||||
execute: async () => {
|
execute: async () => {
|
||||||
// Reuse the same share-tree logic the public /shares/tree route uses:
|
// Reuse the same share-tree logic the public /shares/tree route uses:
|
||||||
// it validates the share + workspace, excludes restricted subtrees,
|
// it validates the share + workspace, excludes restricted subtrees,
|
||||||
|
|||||||
Reference in New Issue
Block a user