Skip to content

Commit ca8ea8f

Browse files
authored
fix(sdk): validate tool args at dispatch boundary and fix bun export (#2405)
1 parent f725f36 commit ca8ea8f

5 files changed

Lines changed: 187 additions & 7 deletions

File tree

apps/docs/document-api/reference/ranges/index.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,4 @@ Deterministic range construction from explicit document anchors.
1515
| Operation | Member path | Mutates | Idempotency | Tracked | Dry run |
1616
| --- | --- | --- | --- | --- | --- |
1717
| <span style={{ whiteSpace: 'nowrap', wordBreak: 'normal', overflowWrap: 'normal' }}><a href="/document-api/reference/ranges/resolve"><code>ranges.resolve</code></a></span> | `ranges.resolve` | No | `idempotent` | No | No |
18+

apps/docs/document-engine/sdks.mdx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -376,8 +376,8 @@ The SDKs expose all operations from the [Document API](/document-api/overview) p
376376
| `doc.markdownToFragment` | `markdown-to-fragment` | Convert a Markdown string into an SDM/1 structural fragment. |
377377
| `doc.info` | `info` | Return document metadata including revision, node count, and capabilities. |
378378
| `doc.clearContent` | `clear-content` | Clear all document body content, leaving a single empty paragraph. |
379-
| `doc.insert` | `insert` | Insert inline content at a text position within an existing block, or at the end of the document when target is omitted. This is NOT for creating sibling blocks — use create.paragraph, create.heading, or lists.insert for that. Accepts two input shapes: legacy string-based (value + type) or structural SDFragment (content). Supports text (default), markdown, and html content types via the `type` field in legacy mode. Structural mode accepts an SDFragment with typed nodes (paragraphs, tables, images, etc.). |
380-
| `doc.replace` | `replace` | Replace content at a contiguous document selection. Text path accepts a SelectionTarget or ref plus replacement text. Structural path accepts an SDAddress, SelectionTarget, or ref plus SDFragment content. |
379+
| `doc.insert` | `insert` | Insert content into the document. Two input shapes: legacy string-based (value + type) inserts inline content at a text position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target is omitted, content appends at the end of the document. Legacy mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. |
380+
| `doc.replace` | `replace` | Replace content at a contiguous document selection. Text path accepts a SelectionTarget or ref plus replacement text. Structural path accepts a BlockNodeAddress (replaces whole block), SelectionTarget (expands to full covered block boundaries), or ref plus SDFragment content. |
381381
| `doc.delete` | `delete` | Delete content at a contiguous document selection. Accepts a SelectionTarget or mutation-ready ref. Supports cross-block deletion and optional block-edge expansion via behavior mode. |
382382
| `doc.blocks.list` | `blocks list` | List top-level blocks in document order with IDs, types, and text previews. Supports pagination via offset/limit and optional nodeType filtering. |
383383
| `doc.blocks.delete` | `blocks delete` | Delete an entire block node (paragraph, heading, list item, table, image, or sdt) deterministically. |
@@ -811,8 +811,8 @@ The SDKs expose all operations from the [Document API](/document-api/overview) p
811811
| `doc.markdown_to_fragment` | `markdown-to-fragment` | Convert a Markdown string into an SDM/1 structural fragment. |
812812
| `doc.info` | `info` | Return document metadata including revision, node count, and capabilities. |
813813
| `doc.clear_content` | `clear-content` | Clear all document body content, leaving a single empty paragraph. |
814-
| `doc.insert` | `insert` | Insert inline content at a text position within an existing block, or at the end of the document when target is omitted. This is NOT for creating sibling blocks — use create.paragraph, create.heading, or lists.insert for that. Accepts two input shapes: legacy string-based (value + type) or structural SDFragment (content). Supports text (default), markdown, and html content types via the `type` field in legacy mode. Structural mode accepts an SDFragment with typed nodes (paragraphs, tables, images, etc.). |
815-
| `doc.replace` | `replace` | Replace content at a contiguous document selection. Text path accepts a SelectionTarget or ref plus replacement text. Structural path accepts an SDAddress, SelectionTarget, or ref plus SDFragment content. |
814+
| `doc.insert` | `insert` | Insert content into the document. Two input shapes: legacy string-based (value + type) inserts inline content at a text position within an existing block; structural SDFragment (content) inserts one or more blocks as siblings relative to a BlockNodeAddress target. When target is omitted, content appends at the end of the document. Legacy mode supports text (default), markdown, and html content types via the `type` field. Structural mode uses `placement` (before/after/insideStart/insideEnd) to position relative to the target block. |
815+
| `doc.replace` | `replace` | Replace content at a contiguous document selection. Text path accepts a SelectionTarget or ref plus replacement text. Structural path accepts a BlockNodeAddress (replaces whole block), SelectionTarget (expands to full covered block boundaries), or ref plus SDFragment content. |
816816
| `doc.delete` | `delete` | Delete content at a contiguous document selection. Accepts a SelectionTarget or mutation-ready ref. Supports cross-block deletion and optional block-edge expansion via behavior mode. |
817817
| `doc.blocks.list` | `blocks list` | List top-level blocks in document order with IDs, types, and text previews. Supports pagination via offset/limit and optional nodeType filtering. |
818818
| `doc.blocks.delete` | `blocks delete` | Delete an entire block node (paragraph, heading, list item, table, image, or sdt) deterministically. |

packages/sdk/codegen/src/generate-intent-tools.mjs

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,66 @@ function buildInputSchemaFromParams(operation) {
9494
return result;
9595
}
9696

97+
// ---------------------------------------------------------------------------
98+
// Extract required-field constraints for an operation.
99+
//
100+
// Two sources of truth exist:
101+
// 1. CLI params (via buildInputSchemaFromParams) — use param names the tool
102+
// schema exposes (e.g. "id", not contract's "commentId").
103+
// 2. Contract inputSchema — captures oneOf / discriminated-union constraints
104+
// that CLI params can't express.
105+
//
106+
// Strategy:
107+
// - Flat required → derive from CLI params (names match the grouped tool schema)
108+
// - oneOf required → derive from contract inputSchema (property names verified
109+
// to match CLI param names for all oneOf operations)
110+
//
111+
// Returns one of:
112+
// { required: string[] } — all listed keys must be present
113+
// { requiredOneOf: string[][] } — at least one branch must be fully satisfied
114+
// {} — no extractable constraints
115+
// ---------------------------------------------------------------------------
116+
117+
function extractRequiredConstraints(operation) {
118+
const cliSchema = buildInputSchemaFromParams(operation);
119+
const cliParamNames = new Set(Object.keys(cliSchema.properties ?? {}));
120+
const contractSchema = operation.inputSchema;
121+
122+
// oneOf in contract schema — collect per-branch required arrays.
123+
// (Verified: all oneOf operations use property names matching CLI params.)
124+
if (contractSchema && Array.isArray(contractSchema.oneOf)) {
125+
const branches = [];
126+
for (const branch of contractSchema.oneOf) {
127+
if (Array.isArray(branch.oneOf)) {
128+
for (const sub of branch.oneOf) {
129+
if (Array.isArray(sub.required) && sub.required.length > 0) {
130+
branches.push(sub.required);
131+
}
132+
}
133+
} else if (Array.isArray(branch.required) && branch.required.length > 0) {
134+
branches.push(branch.required);
135+
}
136+
}
137+
if (branches.length > 0) return { requiredOneOf: branches };
138+
}
139+
140+
// Flat required — union two sources:
141+
// 1. Contract inputSchema.required filtered to CLI param names only
142+
// (contract is authoritative for required-ness, but may use different
143+
// property names, e.g. "commentId" vs CLI "id")
144+
// 2. CLI params with required: true
145+
// (covers names that don't appear in the contract schema)
146+
const required = new Set(cliSchema.required ?? []);
147+
if (contractSchema && Array.isArray(contractSchema.required)) {
148+
for (const key of contractSchema.required) {
149+
if (cliParamNames.has(key)) required.add(key);
150+
}
151+
}
152+
if (required.size > 0) return { required: [...required] };
153+
154+
return {};
155+
}
156+
97157
// ---------------------------------------------------------------------------
98158
// Build intent tools from grouped operations
99159
// ---------------------------------------------------------------------------
@@ -134,7 +194,7 @@ function buildIntentTools(contract) {
134194
description: meta.description,
135195
inputSchema,
136196
mutates,
137-
operations: [{ operationId, intentAction: operation.intentAction }],
197+
operations: [{ operationId, intentAction: operation.intentAction, ...extractRequiredConstraints(operation) }],
138198
});
139199
} else {
140200
// Multi-op tool — add action discriminator
@@ -200,6 +260,7 @@ function buildIntentTools(contract) {
200260
operations: ops.map(({ operationId, operation }) => ({
201261
operationId,
202262
intentAction: operation.intentAction,
263+
...extractRequiredConstraints(operation),
203264
})),
204265
});
205266
}

packages/sdk/langs/node/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"types": "./dist/index.d.ts",
99
"exports": {
1010
".": {
11-
"bun": "./src/index.ts",
11+
"bun": "./dist/index.js",
1212
"types": "./dist/index.d.ts",
1313
"import": "./dist/index.js",
1414
"require": "./dist/index.cjs"

packages/sdk/langs/node/src/tools.ts

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,19 @@ export type ToolCatalog = {
2323
tools: ToolCatalogEntry[];
2424
};
2525

26+
type OperationEntry = {
27+
operationId: string;
28+
intentAction: string;
29+
required?: string[];
30+
requiredOneOf?: string[][];
31+
};
32+
2633
type ToolCatalogEntry = {
2734
toolName: string;
2835
description: string;
2936
inputSchema: Record<string, unknown>;
3037
mutates: boolean;
31-
operations: Array<{ operationId: string; intentAction: string }>;
38+
operations: OperationEntry[];
3239
};
3340

3441
function isRecord(value: unknown): value is Record<string, unknown> {
@@ -150,6 +157,106 @@ function resolveDocApiMethod(
150157
return cursor as (args: unknown, options?: InvokeOptions) => Promise<unknown>;
151158
}
152159

160+
// Cached catalog instance — loaded once per process.
161+
let _catalogCache: ToolCatalog | null = null;
162+
163+
async function getCachedCatalog(): Promise<ToolCatalog> {
164+
if (_catalogCache == null) {
165+
_catalogCache = await loadCatalog();
166+
}
167+
return _catalogCache;
168+
}
169+
170+
/**
171+
* Validate tool arguments against the catalog schema.
172+
*
173+
* Checks three things in order:
174+
* 1. No unknown keys (additionalProperties: false in merged schema)
175+
* 2. All universally-required keys present (merged schema `required`)
176+
* 3. All action-specific required keys present (per-operation `required`)
177+
*/
178+
function validateToolArgs(toolName: string, args: Record<string, unknown>, tool: ToolCatalogEntry): void {
179+
const schema = tool.inputSchema;
180+
const properties = isRecord(schema.properties) ? schema.properties : {};
181+
const required: string[] = Array.isArray(schema.required) ? (schema.required as string[]) : [];
182+
183+
// 1. Reject unknown keys
184+
const knownKeys = new Set(Object.keys(properties));
185+
const unknownKeys = Object.keys(args).filter((k) => !knownKeys.has(k));
186+
if (unknownKeys.length > 0) {
187+
throw new SuperDocCliError(`Unknown argument(s) for ${toolName}: ${unknownKeys.join(', ')}`, {
188+
code: 'INVALID_ARGUMENT',
189+
details: { toolName, unknownKeys, knownKeys: [...knownKeys] },
190+
});
191+
}
192+
193+
// 2. Reject missing universally-required keys
194+
const missingKeys = required.filter((k) => args[k] == null);
195+
if (missingKeys.length > 0) {
196+
throw new SuperDocCliError(`Missing required argument(s) for ${toolName}: ${missingKeys.join(', ')}`, {
197+
code: 'INVALID_ARGUMENT',
198+
details: { toolName, missingKeys },
199+
});
200+
}
201+
202+
// 3. Reject missing per-operation required keys.
203+
// For multi-action tools, resolve the operation by action; for single-op
204+
// tools, use the sole operation entry.
205+
const action = args.action;
206+
let op: OperationEntry | undefined;
207+
if (typeof action === 'string' && tool.operations.length > 1) {
208+
op = tool.operations.find((o) => o.intentAction === action);
209+
} else if (tool.operations.length === 1) {
210+
op = tool.operations[0];
211+
}
212+
213+
if (op) {
214+
validateOperationRequired(toolName, action, args, op);
215+
}
216+
}
217+
218+
/**
219+
* Check per-operation required constraints.
220+
*
221+
* Handles two shapes emitted by the codegen:
222+
* - `required: string[]` — all listed keys must be present
223+
* - `requiredOneOf: string[][]` — at least one branch must be fully satisfied
224+
* (mirrors JSON Schema `oneOf` with per-branch `required` arrays)
225+
*/
226+
function validateOperationRequired(
227+
toolName: string,
228+
action: unknown,
229+
args: Record<string, unknown>,
230+
op: OperationEntry,
231+
): void {
232+
const actionLabel = typeof action === 'string' ? ` action "${action}"` : '';
233+
234+
if (op.requiredOneOf && op.requiredOneOf.length > 0) {
235+
const satisfied = op.requiredOneOf.some((branch) => branch.every((k) => args[k] != null));
236+
if (!satisfied) {
237+
const options = op.requiredOneOf.map((b) => b.join(' + ')).join(' | ');
238+
throw new SuperDocCliError(
239+
`Missing required argument(s) for ${toolName}${actionLabel}: must provide one of: ${options}`,
240+
{
241+
code: 'INVALID_ARGUMENT',
242+
details: { toolName, action, requiredOneOf: op.requiredOneOf },
243+
},
244+
);
245+
}
246+
} else if (op.required && op.required.length > 0) {
247+
const missingActionKeys = op.required.filter((k) => args[k] == null);
248+
if (missingActionKeys.length > 0) {
249+
throw new SuperDocCliError(
250+
`Missing required argument(s) for ${toolName}${actionLabel}: ${missingActionKeys.join(', ')}`,
251+
{
252+
code: 'INVALID_ARGUMENT',
253+
details: { toolName, action, missingKeys: missingActionKeys },
254+
},
255+
);
256+
}
257+
}
258+
}
259+
153260
export async function dispatchSuperDocTool(
154261
client: { doc: Record<string, unknown> },
155262
toolName: string,
@@ -163,6 +270,17 @@ export async function dispatchSuperDocTool(
163270
});
164271
}
165272

273+
// Validate against the tool schema before dispatch.
274+
const catalog = await getCachedCatalog();
275+
const tool = catalog.tools.find((t) => t.toolName === toolName);
276+
if (tool == null) {
277+
throw new SuperDocCliError(`Unknown tool: ${toolName}`, {
278+
code: 'TOOL_DISPATCH_NOT_FOUND',
279+
details: { toolName },
280+
});
281+
}
282+
validateToolArgs(toolName, args, tool);
283+
166284
// Strip doc/sessionId — the SDK client manages session targeting after doc.open().
167285
const { doc: _doc, sessionId: _sid, ...cleanArgs } = args;
168286

0 commit comments

Comments
 (0)