From 80fac08a2dd183a449b52cbd2ff623e9d4ca310b Mon Sep 17 00:00:00 2001 From: Martin Lingstuyl Date: Wed, 10 Sep 2025 12:33:57 +0200 Subject: [PATCH 1/2] Checkpoint from VS Code for coding agent session --- .../prompts/create-new-list-command.prompt.md | 4 + convert-validation-tests.js | 29 ++ .../commands/m365group/m365group-set.spec.ts | 67 +++-- .../entra/commands/m365group/m365group-set.ts | 274 +++++++----------- 4 files changed, 169 insertions(+), 205 deletions(-) create mode 100644 .github/prompts/create-new-list-command.prompt.md create mode 100644 convert-validation-tests.js diff --git a/.github/prompts/create-new-list-command.prompt.md b/.github/prompts/create-new-list-command.prompt.md new file mode 100644 index 00000000000..0850ec66631 --- /dev/null +++ b/.github/prompts/create-new-list-command.prompt.md @@ -0,0 +1,4 @@ +--- +mode: agent +--- +Create a new command \ No newline at end of file diff --git a/convert-validation-tests.js b/convert-validation-tests.js new file mode 100644 index 00000000000..f87eb740874 --- /dev/null +++ b/convert-validation-tests.js @@ -0,0 +1,29 @@ +import fs from 'fs'; + +const files = [ + 'src/m365/entra/commands/app/app-role-add.spec.ts', + 'src/m365/entra/commands/app/app-role-list.spec.ts' +]; + +files.forEach(filePath => { + if (!fs.existsSync(filePath)) { + console.log(`File not found: ${filePath}`); + return; + } + + let content = fs.readFileSync(filePath, 'utf-8'); + + // Replace validation test patterns + content = content.replace( + /const actual = await command\.validate\(\{ options: \{([^}]*)\} \}, commandInfo\);\s*assert\.notStrictEqual\(actual, true\);/g, + 'const schema = command.getRefinedSchema(commandOptionsSchema as any);\n const actual = schema?.safeParse({$1});\n assert.strictEqual(actual?.success, false);' + ); + + content = content.replace( + /const actual = await command\.validate\(\{ options: \{([^}]*)\} \}, commandInfo\);\s*assert\.strictEqual\(actual, true\);/g, + 'const schema = command.getRefinedSchema(commandOptionsSchema as any);\n const actual = schema?.safeParse({$1});\n assert.strictEqual(actual?.success, true);' + ); + + fs.writeFileSync(filePath, content); + console.log(`Conversion completed for ${filePath}`); +}); diff --git a/src/m365/entra/commands/m365group/m365group-set.spec.ts b/src/m365/entra/commands/m365group/m365group-set.spec.ts index 51b3e4345e8..f718c5aa044 100644 --- a/src/m365/entra/commands/m365group/m365group-set.spec.ts +++ b/src/m365/entra/commands/m365group/m365group-set.spec.ts @@ -2,6 +2,7 @@ import { Group } from '@microsoft/microsoft-graph-types'; import assert from 'assert'; import fs from 'fs'; import sinon from 'sinon'; +import { z } from 'zod'; import auth from '../../../../Auth.js'; import { cli } from '../../../../cli/cli.js'; import { CommandInfo } from '../../../../cli/CommandInfo.js'; @@ -53,6 +54,7 @@ describe(commands.M365GROUP_SET, () => { let logger: Logger; let loggerLogSpy: sinon.SinonSpy; let commandInfo: CommandInfo; + let commandOptionsSchema: z.ZodTypeAny; let loggerLogToStderrSpy: sinon.SinonSpy; const groupId = 'f3db5c2b-068f-480d-985b-ec78b9fa0e76'; @@ -70,6 +72,7 @@ describe(commands.M365GROUP_SET, () => { }; } commandInfo = cli.getCommandInfo(command); + commandOptionsSchema = commandInfo.command.getSchemaToParse()!; }); beforeEach(() => { @@ -671,94 +674,94 @@ describe(commands.M365GROUP_SET, () => { }); it('fails validation if the id is not a valid GUID', async () => { - const actual = await command.validate({ options: { id: 'invalid', description: 'My awesome group' } }, commandInfo); - assert.notStrictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: 'invalid', description: 'My awesome group' }); + assert.strictEqual(actual.success, false); }); it('passes validation when the id is a valid GUID and displayName specified', async () => { - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', newDisplayName: 'My group' } }, commandInfo); - assert.strictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', newDisplayName: 'My group' }); + assert.strictEqual(actual.success, true); }); it('passes validation when the id is a valid GUID and description specified', async () => { - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', description: 'My awesome group' } }, commandInfo); - assert.strictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', description: 'My awesome group' }); + assert.strictEqual(actual.success, true); }); it('fails validation if no property to update is specified', async () => { - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848' } }, commandInfo); - assert.notStrictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848' }); + assert.strictEqual(actual.success, false); }); it('fails validation if ownerIds contains invalid GUID', async () => { const ownerIds = ['7167b488-1ffb-43f1-9547-35969469bada', 'foo']; - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', ownerIds: ownerIds.join(',') } }, commandInfo); - assert.notStrictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', ownerIds: ownerIds.join(',') }); + assert.strictEqual(actual.success, false); }); it('fails validation if ownerUserNames contains invalid user principal name', async () => { const ownerUserNames = ['john.doe@contoso.com', 'foo']; - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', ownerUserNames: ownerUserNames.join(',') } }, commandInfo); - assert.notStrictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', ownerUserNames: ownerUserNames.join(',') }); + assert.strictEqual(actual.success, false); }); it('fails validation if memberIds contains invalid GUID', async () => { const memberIds = ['7167b488-1ffb-43f1-9547-35969469bada', 'foo']; - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', memberIds: memberIds.join(',') } }, commandInfo); - assert.notStrictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', memberIds: memberIds.join(',') }); + assert.strictEqual(actual.success, false); }); it('fails validation if memberUserNames contains invalid user principal name', async () => { const memberUserNames = ['john.doe@contoso.com', 'foo']; - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', memberUserNames: memberUserNames.join(',') } }, commandInfo); - assert.notStrictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', memberUserNames: memberUserNames.join(',') }); + assert.strictEqual(actual.success, false); }); it('passes validation if isPrivate is true', async () => { - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', isPrivate: true } }, commandInfo); - assert.strictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', isPrivate: true }); + assert.strictEqual(actual.success, true); }); it('passes validation if isPrivate is false', async () => { - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', isPrivate: false } }, commandInfo); - assert.strictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', isPrivate: false }); + assert.strictEqual(actual.success, true); }); it('passes validation when all required parameters are valid with ids', async () => { - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', ownerIds: userIds.join(',') } }, commandInfo); - assert.strictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', ownerIds: userIds.join(',') }); + assert.strictEqual(actual.success, true); }); it('passes validation when all required parameters are valid with user names', async () => { - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', memberUserNames: userUpns.join(',') } }, commandInfo); - assert.strictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', memberUserNames: userUpns.join(',') }); + assert.strictEqual(actual.success, true); }); it('fails validation if logoPath points to a non-existent file', async () => { sinon.stub(fs, 'existsSync').returns(false); - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', logoPath: 'invalid' } }, commandInfo); - assert.notStrictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', logoPath: 'invalid' }); + assert.strictEqual(actual.success, false); }); it('fails validation if logoPath points to a folder', async () => { const stats = { ...fsStats, isDirectory: () => true }; sinon.stub(fs, 'existsSync').returns(true); sinon.stub(fs, 'lstatSync').returns(stats); - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', logoPath: 'folder' } }, commandInfo); - assert.notStrictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', logoPath: 'folder' }); + assert.strictEqual(actual.success, false); }); it('passes validation if logoPath points to an existing file', async () => { sinon.stub(fs, 'existsSync').returns(true); sinon.stub(fs, 'lstatSync').returns(fsStats); - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', logoPath: 'folder' } }, commandInfo); - assert.strictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', logoPath: 'folder' }); + assert.strictEqual(actual.success, true); }); it('passes validation if all options are being set', async () => { sinon.stub(fs, 'existsSync').returns(true); sinon.stub(fs, 'lstatSync').returns(fsStats); - const actual = await command.validate({ options: { id: '28beab62-7540-4db1-a23f-29a6018a3848', newDisplayName: 'Title', description: 'Description', logoPath: 'logo.png', ownerIds: userIds.join(','), memberIds: userIds.join(','), isPrivate: false, allowExternalSenders: false, autoSubscribeNewMembers: false, hideFromAddressLists: false, hideFromOutlookClients: false } }, commandInfo); - assert.strictEqual(actual, true); + const actual = commandOptionsSchema.safeParse({ id: '28beab62-7540-4db1-a23f-29a6018a3848', newDisplayName: 'Title', description: 'Description', logoPath: 'logo.png', ownerIds: userIds.join(','), memberIds: userIds.join(','), isPrivate: false, allowExternalSenders: false, autoSubscribeNewMembers: false, hideFromAddressLists: false, hideFromOutlookClients: false }); + assert.strictEqual(actual.success, true); }); }); diff --git a/src/m365/entra/commands/m365group/m365group-set.ts b/src/m365/entra/commands/m365group/m365group-set.ts index 4d0d876d257..a4758503a51 100644 --- a/src/m365/entra/commands/m365group/m365group-set.ts +++ b/src/m365/entra/commands/m365group/m365group-set.ts @@ -2,10 +2,12 @@ import { Group } from '@microsoft/microsoft-graph-types'; import { setTimeout } from 'timers/promises'; import fs from 'fs'; import path from 'path'; +import { z } from 'zod'; import { Logger } from '../../../../cli/Logger.js'; -import GlobalOptions from '../../../../GlobalOptions.js'; +import { globalOptionsZod } from '../../../../Command.js'; import request, { CliRequestOptions } from '../../../../request.js'; import { validation } from '../../../../utils/validation.js'; +import { zod } from '../../../../utils/zod.js'; import GraphCommand from '../../../base/GraphCommand.js'; import commands from '../../commands.js'; import { entraGroup } from '../../../../utils/entraGroup.js'; @@ -16,27 +18,31 @@ import { formatting } from '../../../../utils/formatting.js'; import { odata } from '../../../../utils/odata.js'; import { User } from '@microsoft/microsoft-graph-types'; +const options = globalOptionsZod + .extend({ + id: zod.alias('i', z.string().uuid().optional()), + displayName: zod.alias('n', z.string().optional()), + newDisplayName: zod.alias('newDisplayName', z.string().optional()), + description: zod.alias('d', z.string().optional()), + ownerIds: zod.alias('ownerIds', z.string().optional()), + ownerUserNames: zod.alias('ownerUserNames', z.string().optional()), + memberIds: zod.alias('memberIds', z.string().optional()), + memberUserNames: zod.alias('memberUserNames', z.string().optional()), + isPrivate: zod.alias('isPrivate', z.boolean().optional()), + logoPath: zod.alias('l', z.string().optional()), + allowExternalSenders: zod.alias('allowExternalSenders', z.boolean().optional()), + autoSubscribeNewMembers: zod.alias('autoSubscribeNewMembers', z.boolean().optional()), + hideFromAddressLists: zod.alias('hideFromAddressLists', z.boolean().optional()), + hideFromOutlookClients: zod.alias('hideFromOutlookClients', z.boolean().optional()) + }) + .strict(); + +declare type Options = z.infer; + interface CommandArgs { options: Options; } -export interface Options extends GlobalOptions { - id?: string; - displayName?: string; - newDisplayName?: string; - description?: string; - ownerIds?: string; - ownerUserNames?: string; - memberIds?: string; - memberUserNames?: string; - isPrivate?: boolean; - logoPath?: string; - allowExternalSenders?: boolean; - autoSubscribeNewMembers?: boolean; - hideFromAddressLists?: boolean; - hideFromOutlookClients?: boolean; -} - class EntraM365GroupSetCommand extends GraphCommand { private static numRepeat: number = 15; private pollingInterval: number = 500; @@ -49,175 +55,97 @@ class EntraM365GroupSetCommand extends GraphCommand { return 'Updates Microsoft 365 Group properties'; } - constructor() { - super(); - - this.#initTelemetry(); - this.#initOptions(); - this.#initValidators(); - this.#initOptionSets(); - this.#initTypes(); - } - - #initTelemetry(): void { - this.telemetry.push((args: CommandArgs) => { - Object.assign(this.telemetryProperties, { - id: typeof args.options.id !== 'undefined', - displayName: typeof args.options.displayName !== 'undefined', - newDisplayName: typeof args.options.newDisplayName !== 'undefined', - description: typeof args.options.description !== 'undefined', - ownerIds: typeof args.options.ownerIds !== 'undefined', - ownerUserNames: typeof args.options.ownerUserNames !== 'undefined', - memberIds: typeof args.options.memberIds !== 'undefined', - memberUserNames: typeof args.options.memberUserNames !== 'undefined', - isPrivate: !!args.options.isPrivate, - logoPath: typeof args.options.logoPath !== 'undefined', - allowExternalSenders: !!args.options.allowExternalSenders, - autoSubscribeNewMembers: !!args.options.autoSubscribeNewMembers, - hideFromAddressLists: !!args.options.hideFromAddressLists, - hideFromOutlookClients: !!args.options.hideFromOutlookClients - }); - }); - } - - #initOptions(): void { - this.options.unshift( - { - option: '-i, --id [id]' - }, - { - option: '-n, --displayName [displayName]' - }, - { - option: '--newDisplayName [newDisplayName]' - }, - { - option: '-d, --description [description]' - }, - { - option: '--ownerIds [ownerIds]' - }, - { - option: '--ownerUserNames [ownerUserNames]' - }, - { - option: '--memberIds [memberIds]' - }, - { - option: '--memberUserNames [memberUserNames]' - }, - { - option: '--isPrivate [isPrivate]', - autocomplete: ['true', 'false'] - }, - { - option: '-l, --logoPath [logoPath]' - }, - { - option: '--allowExternalSenders [allowExternalSenders]', - autocomplete: ['true', 'false'] - }, - { - option: '--autoSubscribeNewMembers [autoSubscribeNewMembers]', - autocomplete: ['true', 'false'] - }, - { - option: '--hideFromAddressLists [hideFromAddressLists]', - autocomplete: ['true', 'false'] - }, - { - option: '--hideFromOutlookClients [hideFromOutlookClients]', - autocomplete: ['true', 'false'] - } - ); - } - - #initOptionSets(): void { - this.optionSets.push({ options: ['id', 'displayName'] }); - this.optionSets.push({ - options: ['ownerIds', 'ownerUserNames'], - runsWhen: (args) => { - return args.options.ownerIds !== undefined || args.options.ownerUserNames !== undefined; - } - }); - this.optionSets.push({ - options: ['memberIds', 'memberUserNames'], - runsWhen: (args) => { - return args.options.memberIds !== undefined || args.options.memberUserNames !== undefined; - } - }); - } - - #initTypes(): void { - this.types.boolean.push('isPrivate', 'allowEternalSenders', 'autoSubscribeNewMembers', 'hideFromAddressLists', 'hideFromOutlookClients'); - this.types.string.push('id', 'displayName', 'newDisplayName', 'description', 'ownerIds', 'ownerUserNames', 'memberIds', 'memberUserNames', 'logoPath'); + public get schema(): z.ZodTypeAny | undefined { + return options; } - #initValidators(): void { - this.validators.push( - async (args: CommandArgs) => { - if (!args.options.newDisplayName && - args.options.description === undefined && - args.options.ownerIds === undefined && - args.options.ownerUserNames === undefined && - args.options.memberIds === undefined && - args.options.memberUserNames === undefined && - args.options.isPrivate === undefined && - args.options.logoPath === undefined && - args.options.allowExternalSenders === undefined && - args.options.autoSubscribeNewMembers === undefined && - args.options.hideFromAddressLists === undefined && - args.options.hideFromOutlookClients === undefined) { - return 'Specify at least one option to update.'; + public getRefinedSchema(schema: typeof options): z.ZodEffects | undefined { + return schema + .refine(options => !!(options.id || options.displayName), { + message: 'Specify either id or displayName' + }) + .refine(options => { + return !!(options.newDisplayName || + options.description !== undefined || + options.ownerIds !== undefined || + options.ownerUserNames !== undefined || + options.memberIds !== undefined || + options.memberUserNames !== undefined || + options.isPrivate !== undefined || + options.logoPath !== undefined || + options.allowExternalSenders !== undefined || + options.autoSubscribeNewMembers !== undefined || + options.hideFromAddressLists !== undefined || + options.hideFromOutlookClients !== undefined); + }, { + message: 'Specify at least one option to update' + }) + .refine(options => { + if (options.ownerIds && options.ownerUserNames) { + return false; } - - if (args.options.id && !validation.isValidGuid(args.options.id)) { - return `${args.options.id} is not a valid GUID`; + return true; + }, { + message: 'Specify either ownerIds or ownerUserNames but not both' + }) + .refine(options => { + if (options.memberIds && options.memberUserNames) { + return false; } - - if (args.options.ownerIds) { - const isValidGUIDArrayResult = validation.isValidGuidArray(args.options.ownerIds); - if (isValidGUIDArrayResult !== true) { - return `The following GUIDs are invalid for the option 'ownerIds': ${isValidGUIDArrayResult}.`; - } + return true; + }, { + message: 'Specify either memberIds or memberUserNames but not both' + }) + .refine(options => { + if (options.ownerIds) { + const isValidGUIDArrayResult = validation.isValidGuidArray(options.ownerIds); + return isValidGUIDArrayResult === true; } - - if (args.options.ownerUserNames) { - const isValidUPNArrayResult = validation.isValidUserPrincipalNameArray(args.options.ownerUserNames); - if (isValidUPNArrayResult !== true) { - return `The following user principal names are invalid for the option 'ownerUserNames': ${isValidUPNArrayResult}.`; - } + return true; + }, { + message: 'The following GUIDs are invalid for the option \'ownerIds\'' + }) + .refine(options => { + if (options.ownerUserNames) { + const isValidUPNArrayResult = validation.isValidUserPrincipalNameArray(options.ownerUserNames); + return isValidUPNArrayResult === true; } - - if (args.options.memberIds) { - const isValidGUIDArrayResult = validation.isValidGuidArray(args.options.memberIds); - if (isValidGUIDArrayResult !== true) { - return `The following GUIDs are invalid for the option 'memberIds': ${isValidGUIDArrayResult}.`; - } + return true; + }, { + message: 'The following user principal names are invalid for the option \'ownerUserNames\'' + }) + .refine(options => { + if (options.memberIds) { + const isValidGUIDArrayResult = validation.isValidGuidArray(options.memberIds); + return isValidGUIDArrayResult === true; } - - if (args.options.memberUserNames) { - const isValidUPNArrayResult = validation.isValidUserPrincipalNameArray(args.options.memberUserNames); - if (isValidUPNArrayResult !== true) { - return `The following user principal names are invalid for the option 'memberUserNames': ${isValidUPNArrayResult}.`; - } + return true; + }, { + message: 'The following GUIDs are invalid for the option \'memberIds\'' + }) + .refine(options => { + if (options.memberUserNames) { + const isValidUPNArrayResult = validation.isValidUserPrincipalNameArray(options.memberUserNames); + return isValidUPNArrayResult === true; } - - if (args.options.logoPath) { - const fullPath: string = path.resolve(args.options.logoPath); - + return true; + }, { + message: 'The following user principal names are invalid for the option \'memberUserNames\'' + }) + .refine(options => { + if (options.logoPath) { + const fullPath: string = path.resolve(options.logoPath); if (!fs.existsSync(fullPath)) { - return `File '${fullPath}' not found`; + return false; } - if (fs.lstatSync(fullPath).isDirectory()) { - return `Path '${fullPath}' points to a directory`; + return false; } } - return true; - } - ); + }, { + message: 'File not found or path points to a directory' + }); } public async commandAction(logger: Logger, args: CommandArgs): Promise { From 2797a22ff6e1d7860cf5afdf8193705a3e96ffee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 10 Sep 2025 10:34:40 +0000 Subject: [PATCH 2/2] Initial plan