Skip to content

Commit ff328b7

Browse files
Adds prompting on invalid option sets in Zod-enabled commands. Closes #7060
1 parent 960ec81 commit ff328b7

53 files changed

Lines changed: 676 additions & 122 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

src/cli/cli.spec.ts

Lines changed: 155 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,66 @@ class MockCommandWithSchemaAndBoolRequiredOption extends AnonymousCommand {
297297
}
298298
}
299299

300+
const refinedSchemaOptions = z.strictObject({
301+
...globalOptionsZod.shape,
302+
authType: z.string().optional(),
303+
userName: z.string().optional(),
304+
password: z.string().optional(),
305+
certificateFile: z.string().optional(),
306+
certificateBase64Encoded: z.string().optional()
307+
});
308+
309+
class MockCommandWithRefinedSchema extends AnonymousCommand {
310+
public get name(): string {
311+
return 'cli mock schema refined';
312+
}
313+
public get description(): string {
314+
return 'Mock command with refined schema';
315+
}
316+
public get schema(): z.ZodType {
317+
return refinedSchemaOptions;
318+
}
319+
public getRefinedSchema(schema: typeof refinedSchemaOptions): z.ZodObject<any> | undefined {
320+
return schema
321+
.refine(options => options.authType !== 'password' || options.userName, {
322+
error: 'Username is required when using password authentication.',
323+
path: ['userName'],
324+
params: {
325+
customCode: 'required'
326+
}
327+
})
328+
.refine(options => options.authType !== 'password' || options.password, {
329+
error: 'Password is required when using password authentication.',
330+
path: ['password'],
331+
params: {
332+
customCode: 'required'
333+
}
334+
})
335+
.refine(options => options.authType !== 'certificate' || !(options.certificateFile && options.certificateBase64Encoded), {
336+
error: 'Specify either certificateFile or certificateBase64Encoded, but not both.',
337+
path: ['certificateBase64Encoded'],
338+
params: {
339+
customCode: 'optionSet',
340+
options: ['certificateFile', 'certificateBase64Encoded']
341+
}
342+
})
343+
.refine(options => options.authType !== 'certificate' || options.certificateFile || options.certificateBase64Encoded, {
344+
error: 'Specify either certificateFile or certificateBase64Encoded.',
345+
path: ['certificateFile'],
346+
params: {
347+
customCode: 'optionSet',
348+
options: ['certificateFile', 'certificateBase64Encoded']
349+
}
350+
})
351+
.refine(options => options.authType !== 'invalid' || false, {
352+
error: 'Invalid authentication type.',
353+
path: ['authType']
354+
});
355+
}
356+
public async commandAction(): Promise<void> {
357+
}
358+
}
359+
300360
describe('cli', () => {
301361
let rootFolder: string;
302362
let cliLogStub: sinon.SinonStub;
@@ -313,6 +373,7 @@ describe('cli', () => {
313373
let mockCommandWithSchema: Command;
314374
let mockCommandWithSchemaAndRequiredOptions: Command;
315375
let mockCommandWithSchemaAndBoolRequiredOption: Command;
376+
let mockCommandWithRefinedSchema: Command;
316377
let log: string[] = [];
317378
let mockCommandWithBooleanRewrite: Command;
318379

@@ -337,6 +398,7 @@ describe('cli', () => {
337398
mockCommandWithSchema = new MockCommandWithSchema();
338399
mockCommandWithSchemaAndRequiredOptions = new MockCommandWithSchemaAndRequiredOptions();
339400
mockCommandWithSchemaAndBoolRequiredOption = new MockCommandWithSchemaAndBoolRequiredOption();
401+
mockCommandWithRefinedSchema = new MockCommandWithRefinedSchema();
340402
mockCommandWithOptionSets = new MockCommandWithOptionSets();
341403
mockCommandActionSpy = sinon.spy(mockCommand, 'action');
342404

@@ -359,6 +421,7 @@ describe('cli', () => {
359421
cli.getCommandInfo(mockCommandWithSchema, 'cli-schema-mock.js', 'help.mdx'),
360422
cli.getCommandInfo(mockCommandWithSchemaAndRequiredOptions, 'cli-schema-mock.js', 'help.mdx'),
361423
cli.getCommandInfo(mockCommandWithSchemaAndBoolRequiredOption, 'cli-schema-mock.js', 'help.mdx'),
424+
cli.getCommandInfo(mockCommandWithRefinedSchema, 'cli-schema-refined-mock.js', 'help.mdx'),
362425
cli.getCommandInfo(cliCompletionUpdateCommand, 'cli/commands/completion/completion-clink-update.js', 'cli/completion/completion-clink-update.mdx'),
363426
cli.getCommandInfo(mockCommandWithBooleanRewrite, 'cli-boolean-rewrite-mock.js', 'help.mdx')
364427
];
@@ -395,6 +458,7 @@ describe('cli', () => {
395458
cli.loadAllCommandsInfo,
396459
cli.getConfig().get,
397460
cli.loadCommandFromFile,
461+
cli.promptForValue,
398462
browserUtil.open
399463
]);
400464
});
@@ -1154,6 +1218,94 @@ describe('cli', () => {
11541218
});
11551219
});
11561220

1221+
it(`prompts for missing required options from refined schema when prompting enabled`, async () => {
1222+
cli.commandToExecute = cli.commands.find(c => c.name === 'cli mock schema refined');
1223+
const promptInputStub: sinon.SinonStub = sinon.stub(prompt, 'forInput')
1224+
.onFirstCall().resolves('user@contoso.com')
1225+
.onSecondCall().resolves('pass@word1');
1226+
sinon.stub(cli, 'getSettingWithDefaultValue').callsFake((settingName, defaultValue) => {
1227+
if (settingName === settingsNames.prompt) {
1228+
return true;
1229+
}
1230+
return defaultValue;
1231+
});
1232+
const executeCommandSpy = sinon.spy(cli, 'executeCommand');
1233+
1234+
await cli.execute(['cli', 'mock', 'schema', 'refined', '--authType', 'password']);
1235+
assert(cliErrorStub.calledWith('🌶️ Provide values for the following parameters:'));
1236+
assert.strictEqual(promptInputStub.callCount, 2);
1237+
assert(executeCommandSpy.called);
1238+
});
1239+
1240+
it(`prompts for option set selection from refined schema when prompting enabled`, async () => {
1241+
cli.commandToExecute = cli.commands.find(c => c.name === 'cli mock schema refined');
1242+
const promptSelectionStub: sinon.SinonStub = sinon.stub(prompt, 'forSelection').resolves('certificateFile');
1243+
const promptInputStub: sinon.SinonStub = sinon.stub(prompt, 'forInput').resolves('/path/to/cert.pem');
1244+
sinon.stub(cli, 'getSettingWithDefaultValue').callsFake((settingName, defaultValue) => {
1245+
if (settingName === settingsNames.prompt) {
1246+
return true;
1247+
}
1248+
return defaultValue;
1249+
});
1250+
const executeCommandSpy = sinon.spy(cli, 'executeCommand');
1251+
1252+
await cli.execute(['cli', 'mock', 'schema', 'refined', '--authType', 'certificate']);
1253+
assert(cliErrorStub.calledWith('🌶️ Please specify one of the following options:'));
1254+
assert(promptSelectionStub.calledOnce);
1255+
assert.deepStrictEqual(promptSelectionStub.firstCall.args[0].choices, [
1256+
{ name: 'certificateFile', value: 'certificateFile' },
1257+
{ name: 'certificateBase64Encoded', value: 'certificateBase64Encoded' }
1258+
]);
1259+
assert(promptInputStub.calledOnce);
1260+
assert(executeCommandSpy.called);
1261+
});
1262+
1263+
it(`exits with error for non-required/non-optionSet errors in refined schema when prompting enabled`, (done) => {
1264+
cli.commandToExecute = cli.commands.find(c => c.name === 'cli mock schema refined');
1265+
sinon.stub(cli, 'getSettingWithDefaultValue').callsFake((settingName, defaultValue) => {
1266+
if (settingName === settingsNames.prompt) {
1267+
return true;
1268+
}
1269+
return defaultValue;
1270+
});
1271+
const executeCommandSpy = sinon.spy(cli, 'executeCommand');
1272+
1273+
cli
1274+
.execute(['cli', 'mock', 'schema', 'refined', '--authType', 'invalid'])
1275+
.then(_ => done('Promise fulfilled while error expected'), _ => {
1276+
try {
1277+
assert(executeCommandSpy.notCalled);
1278+
done();
1279+
}
1280+
catch (e) {
1281+
done(e);
1282+
}
1283+
});
1284+
});
1285+
1286+
it(`exits with proper error when prompting disabled and refined schema validation fails`, (done) => {
1287+
cli.commandToExecute = cli.commands.find(c => c.name === 'cli mock schema refined');
1288+
sinon.stub(cli, 'getSettingWithDefaultValue').callsFake((settingName, defaultValue) => {
1289+
if (settingName === settingsNames.prompt) {
1290+
return false;
1291+
}
1292+
return defaultValue;
1293+
});
1294+
const executeCommandSpy = sinon.spy(cli, 'executeCommand');
1295+
1296+
cli
1297+
.execute(['cli', 'mock', 'schema', 'refined', '--authType', 'password'])
1298+
.then(_ => done('Promise fulfilled while error expected'), _ => {
1299+
try {
1300+
assert(executeCommandSpy.notCalled);
1301+
done();
1302+
}
1303+
catch (e) {
1304+
done(e);
1305+
}
1306+
});
1307+
});
1308+
11571309
it(`executes command when validation passed`, async () => {
11581310
cli.commandToExecute = cli.commands.find(c => c.name === 'cli mock');
11591311

@@ -1725,7 +1877,7 @@ describe('cli', () => {
17251877
await cli.loadCommandFromArgs(['spo', 'site', 'list']);
17261878
cli.printAvailableCommands();
17271879

1728-
assert(cliLogStub.calledWith(' cli * 11 commands'));
1880+
assert(cliLogStub.calledWith(' cli * 12 commands'));
17291881
});
17301882

17311883
it(`prints commands from the specified group`, async () => {
@@ -1738,7 +1890,7 @@ describe('cli', () => {
17381890
};
17391891
cli.printAvailableCommands();
17401892

1741-
assert(cliLogStub.calledWith(' cli mock * 8 commands'));
1893+
assert(cliLogStub.calledWith(' cli mock * 9 commands'));
17421894
});
17431895

17441896
it(`prints commands from the root group when the specified string doesn't match any group`, async () => {
@@ -1751,7 +1903,7 @@ describe('cli', () => {
17511903
};
17521904
cli.printAvailableCommands();
17531905

1754-
assert(cliLogStub.calledWith(' cli * 11 commands'));
1906+
assert(cliLogStub.calledWith(' cli * 12 commands'));
17551907
});
17561908

17571909
it(`runs properly when context file not found`, async () => {

src/cli/cli.ts

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import os from 'os';
55
import path from 'path';
66
import { fileURLToPath, pathToFileURL } from 'url';
77
import yargs from 'yargs-parser';
8-
import { ZodError } from 'zod';
8+
import z, { ZodError } from 'zod';
99
import Command, { CommandArgs, CommandError } from '../Command.js';
1010
import GlobalOptions from '../GlobalOptions.js';
1111
import config from '../config.js';
@@ -186,15 +186,35 @@ async function execute(rawArgs: string[]): Promise<void> {
186186
break;
187187
}
188188
else {
189-
const hasNonRequiredErrors = result.error.issues.some(i => i.code !== 'invalid_type');
190189
const shouldPrompt = cli.getSettingWithDefaultValue<boolean>(settingsNames.prompt, true);
191190

192-
if (hasNonRequiredErrors === false &&
193-
shouldPrompt) {
191+
if (!shouldPrompt) {
192+
result.error.issues.forEach(e => {
193+
if (e.code === 'invalid_type' &&
194+
e.input === undefined) {
195+
(e.message as any) = `Required option not specified`;
196+
}
197+
});
198+
return cli.closeWithError(result.error, cli.optionsFromArgs, true);
199+
}
200+
201+
const missingRequiredValuesErrors: z.core.$ZodIssue[] = result.error.issues
202+
.filter(e => (e.code === 'invalid_type' && e.input === undefined) ||
203+
(e.code === 'custom' && e.params?.customCode === 'required'));
204+
const optionSetErrors: z.core.$ZodIssueCustom[] = result.error.issues
205+
.filter(e => e.code === 'custom' && e.params?.customCode === 'optionSet') as z.core.$ZodIssueCustom[];
206+
const otherErrors: z.core.$ZodIssue[] = result.error.issues
207+
.filter(e => !missingRequiredValuesErrors.includes(e) && !optionSetErrors.includes(e as z.core.$ZodIssueCustom));
208+
209+
if (otherErrors.some(e => e)) {
210+
return cli.closeWithError(result.error, cli.optionsFromArgs, true);
211+
}
212+
213+
if (missingRequiredValuesErrors.some(e => e)) {
194214
await cli.error('🌶️ Provide values for the following parameters:');
195215

196-
for (const issue of result.error.issues) {
197-
const optionName = issue.path.join('.');
216+
for (const error of missingRequiredValuesErrors) {
217+
const optionName = error.path.join('.');
198218
const optionInfo = cli.commandToExecute.options.find(o => o.name === optionName);
199219
const answer = await cli.promptForValue(optionInfo!);
200220
// coerce the answer to the correct type
@@ -206,15 +226,14 @@ async function execute(rawArgs: string[]): Promise<void> {
206226
return cli.closeWithError(e.message, cli.optionsFromArgs, true);
207227
}
208228
}
229+
230+
continue;
209231
}
210-
else {
211-
result.error.issues.forEach(i => {
212-
if (i.code === 'invalid_type' &&
213-
i.input === undefined) {
214-
(i.message as any) = `Required option not specified`;
215-
}
216-
});
217-
return cli.closeWithError(result.error, cli.optionsFromArgs, true);
232+
233+
if (optionSetErrors.some(e => e)) {
234+
for (const error of optionSetErrors) {
235+
await promptForOptionSetNameAndValue(cli.optionsFromArgs, error.params?.options);
236+
}
218237
}
219238
}
220239
}
@@ -1057,6 +1076,16 @@ function shouldTrimOutput(output: string | undefined): boolean {
10571076
return output === 'text';
10581077
}
10591078

1079+
async function promptForOptionSetNameAndValue(args: CommandArgs, options: string[]): Promise<void> {
1080+
await cli.error(`🌶️ Please specify one of the following options:`);
1081+
1082+
const selectedOptionName = await prompt.forSelection<string>({ message: `Option to use:`, choices: options.map((choice: any) => { return { name: choice, value: choice }; }) });
1083+
const optionValue = await prompt.forInput({ message: `${selectedOptionName}:` });
1084+
1085+
args.options[selectedOptionName] = optionValue;
1086+
await cli.error('');
1087+
}
1088+
10601089
export const cli = {
10611090
closeWithError,
10621091
commands,

src/index.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,14 @@ await (async () => {
1717
updateNotifier.default({ pkg: app.packageJson() as any }).notify({ defer: false });
1818
}
1919

20-
await cli.execute(process.argv.slice(2));
20+
try {
21+
await cli.execute(process.argv.slice(2));
22+
}
23+
catch (err) {
24+
if (err instanceof Error && err.name === 'ExitPromptError') {
25+
process.exit(1);
26+
}
27+
28+
await cli.closeWithError(err, cli.optionsFromArgs || { options: {} });
29+
}
2130
})();

src/m365/adaptivecard/commands/adaptivecard-send.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ class AdaptiveCardSendCommand extends AnonymousCommand {
4242
return schema
4343
.refine(options => !options.cardData || options.card, {
4444
error: 'When you specify cardData, you must also specify card.',
45-
path: ['cardData']
45+
path: ['cardData'],
46+
params: {
47+
customCode: 'required'
48+
}
4649
})
4750
.refine(options => {
4851
if (options.card) {

src/m365/app/commands/permission/permission-add.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ class AppPermissionAddCommand extends AppCommand {
4949
return schema
5050
.refine(options => options.applicationPermissions || options.delegatedPermissions, {
5151
error: 'Specify at least one of applicationPermissions or delegatedPermissions, or both.',
52-
path: ['delegatedPermissions']
52+
path: ['delegatedPermissions'],
53+
params: {
54+
customCode: 'required'
55+
}
5356
});
5457
}
5558

src/m365/booking/commands/business/business-get.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,11 @@ class BookingBusinessGetCommand extends GraphCommand {
3636
public getRefinedSchema(schema: typeof options): z.ZodObject<any> | undefined {
3737
return schema
3838
.refine(options => options.id || options.name, {
39-
error: 'Specify either id or name'
39+
error: 'Specify either id or name',
40+
params: {
41+
customCode: 'optionSet',
42+
options: ['id', 'name']
43+
}
4044
});
4145
}
4246

0 commit comments

Comments
 (0)