Skip to content

Commit ad856e2

Browse files
feat: fixing response envelope key for SDK operations (#2542)
* feat: fixing response envelope key for SDK operations - Added `responseEnvelopeKey` to SDK operations to specify which property to unwrap from CLI responses. - Updated the `buildSdkContract` function to include the new key. - Adjusted client generation for Node and Python to utilize the `responseEnvelopeKey` for unwrapping responses. - Enhanced tests to ensure integrity of the `responseEnvelopeKey` across operations, verifying its presence and correctness. * fix(cli): preserve doc.find selector shorthand with schema validation
1 parent ca7de73 commit ad856e2

7 files changed

Lines changed: 319 additions & 54 deletions

File tree

apps/cli/scripts/export-sdk-contract.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
} from '../src/cli/operation-set';
3030
import type { CliOnlyOperation } from '../src/cli/types';
3131
import { CLI_ONLY_OPERATION_DEFINITIONS } from '../src/cli/cli-only-operation-definitions';
32+
import { RESPONSE_ENVELOPE_KEY } from '../src/cli/operation-hints';
3233
import { HOST_PROTOCOL_VERSION, HOST_PROTOCOL_FEATURES, HOST_PROTOCOL_NOTIFICATIONS } from '../src/host/protocol';
3334

3435
// ---------------------------------------------------------------------------
@@ -105,6 +106,10 @@ function buildSdkContract() {
105106
requiresDocumentContext: cliRequiresDocumentContext(cliOpId),
106107
docRequirement: metadata.docRequirement,
107108

109+
// Response envelope key — tells SDKs which property to unwrap from the CLI response.
110+
// null means result is spread across top-level keys (no unwrapping needed).
111+
responseEnvelopeKey: docApiId ? (RESPONSE_ENVELOPE_KEY[docApiId] ?? null) : null,
112+
108113
// Transport plane
109114
params: metadata.params.map((p) => {
110115
const spec: Record<string, unknown> = {

apps/cli/src/__tests__/lib/find-query.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,4 +34,26 @@ describe('resolveFindQuery', () => {
3434
test('treats null --select-json as provided and validates the payload shape', async () => {
3535
await expect(resolveFindQuery(makeParsed({ 'select-json': 'null' }))).rejects.toThrow(CliError);
3636
});
37+
38+
test('normalizes shorthand node selector from --select-json', async () => {
39+
const query = await resolveFindQuery(makeParsed({ 'select-json': JSON.stringify({ type: 'paragraph' }) }));
40+
expect(query).toEqual({
41+
select: { type: 'node', nodeType: 'paragraph' },
42+
});
43+
});
44+
45+
test('passes through canonical node selector from --select-json', async () => {
46+
const query = await resolveFindQuery(
47+
makeParsed({ 'select-json': JSON.stringify({ type: 'node', nodeType: 'heading' }) }),
48+
);
49+
expect(query).toEqual({
50+
select: { type: 'node', nodeType: 'heading' },
51+
});
52+
});
53+
54+
test('rejects invalid shorthand node type from --select-json', async () => {
55+
await expect(resolveFindQuery(makeParsed({ 'select-json': JSON.stringify({ type: 'magic' }) }))).rejects.toThrow(
56+
CliError,
57+
);
58+
});
3759
});

apps/cli/src/__tests__/lib/validate-type-spec.test.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, test } from 'bun:test';
22
import { validateValueAgainstTypeSpec } from '../../lib/operation-args';
33
import { CliError } from '../../lib/errors';
44
import type { CliTypeSpec } from '../../cli/types';
5+
import { CLI_OPERATION_METADATA } from '../../cli/operation-params';
56

67
describe('validateValueAgainstTypeSpec – oneOf const enumeration', () => {
78
const schema: CliTypeSpec = {
@@ -91,3 +92,53 @@ describe('validateValueAgainstTypeSpec – enum branch', () => {
9192
}
9293
});
9394
});
95+
96+
// ---------------------------------------------------------------------------
97+
// doc.find select schema override
98+
// ---------------------------------------------------------------------------
99+
100+
describe('doc.find select schema — accepts canonical and shorthand forms', () => {
101+
const metadata = CLI_OPERATION_METADATA['doc.find'];
102+
const selectParam = metadata.params.find((p) => p.name === 'select');
103+
const schema = selectParam?.schema;
104+
105+
if (!schema) throw new Error('doc.find metadata missing select param with schema');
106+
107+
test('accepts canonical text selector', () => {
108+
expect(() => validateValueAgainstTypeSpec({ type: 'text', pattern: 'hello' }, schema, 'select')).not.toThrow();
109+
});
110+
111+
test('accepts canonical text selector with all optional fields', () => {
112+
expect(() =>
113+
validateValueAgainstTypeSpec(
114+
{ type: 'text', pattern: 'hello', mode: 'regex', caseSensitive: true },
115+
schema,
116+
'select',
117+
),
118+
).not.toThrow();
119+
});
120+
121+
test('accepts canonical node selector', () => {
122+
expect(() => validateValueAgainstTypeSpec({ type: 'node', nodeType: 'heading' }, schema, 'select')).not.toThrow();
123+
});
124+
125+
test('accepts canonical node selector with kind', () => {
126+
expect(() => validateValueAgainstTypeSpec({ type: 'node', kind: 'block' }, schema, 'select')).not.toThrow();
127+
});
128+
129+
test('accepts shorthand node selector', () => {
130+
expect(() => validateValueAgainstTypeSpec({ type: 'paragraph' }, schema, 'select')).not.toThrow();
131+
});
132+
133+
test('accepts shorthand for inline node type', () => {
134+
expect(() => validateValueAgainstTypeSpec({ type: 'hyperlink' }, schema, 'select')).not.toThrow();
135+
});
136+
137+
test('rejects invalid shorthand node type', () => {
138+
expect(() => validateValueAgainstTypeSpec({ type: 'magic' }, schema, 'select')).toThrow(CliError);
139+
});
140+
141+
test('rejects text selector missing required pattern', () => {
142+
expect(() => validateValueAgainstTypeSpec({ type: 'text' }, schema, 'select')).toThrow(CliError);
143+
});
144+
});

apps/cli/src/cli/operation-params.ts

Lines changed: 64 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import {
1313
buildInternalContractSchemas,
1414
COMMAND_CATALOG,
15+
NODE_TYPES,
1516
OPERATION_REQUIRES_DOCUMENT_CONTEXT_MAP,
1617
type OperationId,
1718
} from '@superdoc/document-api';
@@ -446,7 +447,47 @@ const PARAM_FLAG_OVERRIDES: Partial<Record<string, Record<string, { name?: strin
446447
// adjustment for CLI metadata (e.g. simplifying or enriching).
447448
// ---------------------------------------------------------------------------
448449

449-
const PARAM_SCHEMA_OVERRIDES: Partial<Record<string, Record<string, CliTypeSpec>>> = {};
450+
// The document-api contract schema for `select` is a strict oneOf(text, node)
451+
// that rejects shorthand selectors like `{ type: "paragraph" }`. The CLI
452+
// normalizes these shorthands in resolveFindQuery() → validateQuery(), so
453+
// the schema-level validation must accept all three supported forms.
454+
const DOC_FIND_SELECT_SCHEMA: CliTypeSpec = {
455+
oneOf: [
456+
// { type: 'text', pattern: '...', mode?, caseSensitive? }
457+
{
458+
type: 'object',
459+
properties: {
460+
type: { const: 'text' },
461+
pattern: { type: 'string' },
462+
mode: { oneOf: [{ const: 'contains' }, { const: 'regex' }] },
463+
caseSensitive: { type: 'boolean' },
464+
},
465+
required: ['type', 'pattern'],
466+
},
467+
// { type: 'node', nodeType?, kind? }
468+
{
469+
type: 'object',
470+
properties: {
471+
type: { const: 'node' },
472+
nodeType: { type: 'string' },
473+
kind: { oneOf: [{ const: 'block' }, { const: 'inline' }] },
474+
},
475+
required: ['type'],
476+
},
477+
// Shorthand: { type: '<NodeType>' } — normalized to { type: 'node', nodeType }
478+
{
479+
type: 'object',
480+
properties: {
481+
type: { oneOf: NODE_TYPES.map((t) => ({ const: t }) as CliTypeSpec) },
482+
},
483+
required: ['type'],
484+
},
485+
],
486+
};
487+
488+
const PARAM_SCHEMA_OVERRIDES: Partial<Record<string, Record<string, CliTypeSpec>>> = {
489+
'doc.find': { select: DOC_FIND_SELECT_SCHEMA },
490+
};
450491

451492
// ---------------------------------------------------------------------------
452493
// Schema-derived param exclusions
@@ -455,11 +496,7 @@ const PARAM_SCHEMA_OVERRIDES: Partial<Record<string, Record<string, CliTypeSpec>
455496
// exposed in CLI metadata because the CLI provides an alternative interface.
456497
// ---------------------------------------------------------------------------
457498

458-
const PARAM_EXCLUSIONS: Partial<Record<string, ReadonlySet<string>>> = {
459-
// CLI uses flat flags (--type, --pattern, --mode) or --query-json; `select`
460-
// is an internal document-api field that the invoker builds from flat flags.
461-
'doc.find': new Set(['select']),
462-
};
499+
const PARAM_EXCLUSIONS: Partial<Record<string, ReadonlySet<string>>> = {};
463500

464501
// ---------------------------------------------------------------------------
465502
// Extra CLI-specific params for doc-backed operations
@@ -494,36 +531,47 @@ const FORMAT_OPERATION_IDS = CLI_DOC_OPERATIONS.filter((operationId): operationI
494531
);
495532

496533
const EXTRA_CLI_PARAMS: Partial<Record<string, CliOperationParamSpec[]>> = {
534+
// Flat flags are CLI convenience alternatives to --select-json. Marked
535+
// agentVisible: false so that if doc.find is ever exposed as a tool
536+
// (currently skipAsATool), agents see only the structured `select` param.
497537
'doc.find': [
498538
{
499539
name: 'type',
500540
kind: 'flag',
501541
type: 'string',
502542
description: "Selector type: 'text' for text search or 'node' for node type search.",
543+
agentVisible: false,
503544
},
504545
{
505546
name: 'nodeType',
506547
kind: 'flag',
507548
flag: 'node-type',
508549
type: 'string',
509550
description: 'Node type to match (paragraph, heading, table, listItem, etc.).',
551+
agentVisible: false,
552+
},
553+
{ name: 'kind', kind: 'flag', type: 'string', description: "Filter: 'block' or 'inline'.", agentVisible: false },
554+
{
555+
name: 'pattern',
556+
kind: 'flag',
557+
type: 'string',
558+
description: 'Text or regex pattern to match.',
559+
agentVisible: false,
560+
},
561+
{
562+
name: 'mode',
563+
kind: 'flag',
564+
type: 'string',
565+
description: "Match mode: 'contains' (substring) or 'regex'.",
566+
agentVisible: false,
510567
},
511-
{ name: 'kind', kind: 'flag', type: 'string', description: "Filter: 'block' or 'inline'." },
512-
{ name: 'pattern', kind: 'flag', type: 'string', description: 'Text or regex pattern to match.' },
513-
{ name: 'mode', kind: 'flag', type: 'string', description: "Match mode: 'contains' (substring) or 'regex'." },
514568
{
515569
name: 'caseSensitive',
516570
kind: 'flag',
517571
flag: 'case-sensitive',
518572
type: 'boolean',
519573
description: 'Case-sensitive matching. Default: false.',
520-
},
521-
{
522-
name: 'select',
523-
kind: 'jsonFlag',
524-
flag: 'select-json',
525-
type: 'json',
526-
description: "Search selector as JSON: {type:'text', pattern:'...'} or {type:'node', nodeType:'...'}.",
574+
agentVisible: false,
527575
},
528576
{ name: 'query', kind: 'jsonFlag', flag: 'query-json', type: 'json', description: 'Query filter as JSON object.' },
529577
],

0 commit comments

Comments
 (0)