From 326ce5f17f98ad8a7f1076bcca47e71bf5c51948 Mon Sep 17 00:00:00 2001 From: yashwanth195 <59129034+yashwanth195@users.noreply.github.com> Date: Thu, 12 Nov 2026 16:00:46 -0800 Subject: [PATCH 1/2] Add parameter mode preflight --- apps/heft/src/cli/HeftActionRunner.ts | 2 + .../src/configuration/HeftPluginDefinition.ts | 12 + .../pluginFramework/HeftParameterManager.ts | 43 +++ .../tests/HeftParameterManager.test.ts | 271 ++++++++++++++++++ apps/heft/src/schemas/heft-plugin.schema.json | 26 +- apps/heft/src/test/ParameterPreflight.test.ts | 173 +++++++++++ .../test/fixtures/PreflightLifecyclePlugin.ts | 21 ++ .../src/test/fixtures/PreflightTaskPlugin.ts | 18 ++ ...meter-mode-preflight_2026-09-14-20-45.json | 10 + 9 files changed, 575 insertions(+), 1 deletion(-) create mode 100644 apps/heft/src/pluginFramework/tests/HeftParameterManager.test.ts create mode 100644 apps/heft/src/test/ParameterPreflight.test.ts create mode 100644 apps/heft/src/test/fixtures/PreflightLifecyclePlugin.ts create mode 100644 apps/heft/src/test/fixtures/PreflightTaskPlugin.ts create mode 100644 common/changes/@rushstack/heft/parameter-mode-preflight_2026-09-14-20-45.json diff --git a/apps/heft/src/cli/HeftActionRunner.ts b/apps/heft/src/cli/HeftActionRunner.ts index 1d61e1db0ac..55eac1744ee 100644 --- a/apps/heft/src/cli/HeftActionRunner.ts +++ b/apps/heft/src/cli/HeftActionRunner.ts @@ -313,6 +313,8 @@ export class HeftActionRunner { // executed, and the session should be populated with the executing parameters. this.#internalHeftSession.parameterManager = this.parameterManager; + this.parameterManager.validateParameters(); + initializeHeft(this.#heftConfiguration, terminal, this.parameterManager.defaultParameters.verbose); const operations: ReadonlySet> = diff --git a/apps/heft/src/configuration/HeftPluginDefinition.ts b/apps/heft/src/configuration/HeftPluginDefinition.ts index 822ef3453e3..786c1f06953 100644 --- a/apps/heft/src/configuration/HeftPluginDefinition.ts +++ b/apps/heft/src/configuration/HeftPluginDefinition.ts @@ -35,6 +35,18 @@ export interface IBaseParameterJson { * If true, then this parameter must be included on the command line. */ required?: boolean; + /** + * If false, reject an active parameter when running with `--clean`, before any plugins run or outputs + * are deleted. Defaults to true. A parameter is active when its flag is true, its list is nonempty, + * or its scalar value is defined, including default values. + */ + allowWithClean?: boolean; + /** + * If false, reject an active parameter in watch mode, before any plugins run or outputs are deleted. + * Defaults to true. A parameter is active when its flag is true, its list is nonempty, or its scalar + * value is defined, including default values. + */ + allowInWatchMode?: boolean; } /** diff --git a/apps/heft/src/pluginFramework/HeftParameterManager.ts b/apps/heft/src/pluginFramework/HeftParameterManager.ts index 17538c851f9..987a9156689 100644 --- a/apps/heft/src/pluginFramework/HeftParameterManager.ts +++ b/apps/heft/src/pluginFramework/HeftParameterManager.ts @@ -195,6 +195,49 @@ export class HeftParameterManager { } } + /** + * Validate effective parameter values after parsing and before executing the action. + */ + public validateParameters(): void { + const { clean, watch } = this.defaultParameters; + for (const [pluginDefinition, parameters] of this.#parametersByDefinition) { + for (const definition of pluginDefinition.pluginParameters) { + const disallowedWithClean: boolean = clean && definition.allowWithClean === false; + const disallowedInWatchMode: boolean = watch && definition.allowInWatchMode === false; + if (!disallowedWithClean && !disallowedInWatchMode) { + continue; + } + + const parameter: CommandLineParameter = parameters.get(definition.longName)!; + let isActive: boolean; + switch (parameter.kind) { + case CommandLineParameterKind.Flag: + isActive = parameter.value; + break; + case CommandLineParameterKind.ChoiceList: + case CommandLineParameterKind.IntegerList: + case CommandLineParameterKind.StringList: + isActive = parameter.values.length > 0; + break; + default: + isActive = parameter.value !== undefined; + break; + } + if (isActive) { + const parameterName: string = parameter.scopedLongName || parameter.longName; + const mode: string = disallowedWithClean ? '--clean' : 'watch mode'; + const remedy: string = disallowedWithClean ? 'without --clean' : 'in a non-watch action'; + throw new Error( + `Parameter ${JSON.stringify(parameterName)} from plugin ` + + `${JSON.stringify(pluginDefinition.pluginName)} in package ` + + `${JSON.stringify(pluginDefinition.pluginPackageName)} cannot be used with ${mode}. ` + + `Run ${remedy}, or remove the parameter (including any configured default value).` + ); + } + } + } + } + /** * Get the finalized parameters for the specified plugin definition. */ diff --git a/apps/heft/src/pluginFramework/tests/HeftParameterManager.test.ts b/apps/heft/src/pluginFramework/tests/HeftParameterManager.test.ts new file mode 100644 index 00000000000..12d40a20e8c --- /dev/null +++ b/apps/heft/src/pluginFramework/tests/HeftParameterManager.test.ts @@ -0,0 +1,271 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import { JsonSchema } from '@rushstack/node-core-library'; +import { + AliasCommandLineAction, + DynamicCommandLineAction, + DynamicCommandLineParser +} from '@rushstack/ts-command-line'; + +import { HeftParameterManager } from '../HeftParameterManager'; +import { HeftTaskPluginDefinition, type IParameterJson } from '../../configuration/HeftPluginDefinition'; +import heftPluginSchema from '../../schemas/heft-plugin.schema.json'; + +interface IParameterTestCase { + parameter: IParameterJson; + args: string[]; +} + +const PARAMETER_TEST_CASES: IParameterTestCase[] = [ + { + parameter: { parameterKind: 'flag', longName: '--value', description: 'A flag.' }, + args: ['--value'] + }, + { + parameter: { + parameterKind: 'choice', + longName: '--value', + description: 'A choice.', + alternatives: [{ name: 'first', description: 'First choice.' }] + }, + args: ['--value', 'first'] + }, + { + parameter: { + parameterKind: 'choiceList', + longName: '--value', + description: 'A choice list.', + alternatives: [{ name: 'first', description: 'First choice.' }] + }, + args: ['--value', 'first'] + }, + { + parameter: { + parameterKind: 'integer', + longName: '--value', + description: 'An integer.', + argumentName: 'VALUE' + }, + args: ['--value', '0'] + }, + { + parameter: { + parameterKind: 'integerList', + longName: '--value', + description: 'An integer list.', + argumentName: 'VALUE' + }, + args: ['--value', '0'] + }, + { + parameter: { + parameterKind: 'string', + longName: '--value', + description: 'A string.', + argumentName: 'VALUE' + }, + args: ['--value', ''] + }, + { + parameter: { + parameterKind: 'stringList', + longName: '--value', + description: 'A string list.', + argumentName: 'VALUE' + }, + args: ['--value', ''] + } +]; + +function createPlugin( + parameter: IParameterJson, + parameterScope: string = 'test-scope' +): HeftTaskPluginDefinition { + return HeftTaskPluginDefinition.loadFromObject({ + heftPluginDefinitionJson: { + pluginName: 'test-plugin', + parameterScope, + entryPoint: 'unused.js', + parameters: [parameter] + }, + packageName: 'test-package', + packageRoot: __dirname + }); +} + +async function parseAsync( + plugins: HeftTaskPluginDefinition[], + args: string[], + aliasDefaults?: string[] +): Promise { + const parser: DynamicCommandLineParser = new DynamicCommandLineParser({ + toolFilename: 'heft-test', + toolDescription: 'Test parameter preflight.' + }); + const action: DynamicCommandLineAction = new DynamicCommandLineAction({ + actionName: 'run', + summary: 'Run a test.', + documentation: 'Run a test.' + }); + parser.addAction(action); + const clean = action.defineFlagParameter({ parameterLongName: '--clean', description: 'Clean outputs.' }); + const watch = action.defineFlagParameter({ parameterLongName: '--watch', description: 'Watch files.' }); + const manager: HeftParameterManager = new HeftParameterManager({ + getIsClean: () => clean.value, + getIsWatch: () => watch.value, + getIsDebug: () => false, + getIsVerbose: () => false, + getIsProduction: () => false, + getLocales: () => [] + }); + for (const plugin of plugins) { + manager.addPluginParameters(plugin); + } + manager.finalizeParameters(action); + if (aliasDefaults) { + parser.addAction( + new AliasCommandLineAction({ + toolFilename: 'heft-test', + aliasName: 'alias', + targetAction: action, + defaultParameters: aliasDefaults + }) + ); + } + await parser.executeWithoutErrorHandlingAsync([aliasDefaults ? 'alias' : 'run', ...args]); + return manager; +} + +describe(HeftParameterManager.name, () => { + describe.each(PARAMETER_TEST_CASES)('$parameter.parameterKind', ({ parameter, args }) => { + it.each(['--clean', '--watch'])('rejects an active restricted value with %s', async (mode) => { + const manager: HeftParameterManager = await parseAsync( + [createPlugin({ ...parameter, allowWithClean: false, allowInWatchMode: false })], + [mode, ...args] + ); + expect(() => manager.validateParameters()).toThrow( + `Parameter "--test-scope:value" from plugin "test-plugin" in package "test-package" cannot be used with ${ + mode === '--clean' ? '--clean' : 'watch mode' + }. Run ${mode === '--clean' ? 'without --clean' : 'in a non-watch action'}` + ); + }); + + it.each(['--clean', '--watch'])('allows an inactive restricted value with %s', async (mode) => { + const manager: HeftParameterManager = await parseAsync( + [createPlugin({ ...parameter, allowWithClean: false, allowInWatchMode: false })], + [mode] + ); + expect(() => manager.validateParameters()).not.toThrow(); + }); + + it('allows an active restricted value without either mode', async () => { + const manager: HeftParameterManager = await parseAsync( + [createPlugin({ ...parameter, allowWithClean: false, allowInWatchMode: false })], + args + ); + expect(() => manager.validateParameters()).not.toThrow(); + }); + + it.each([undefined, true])('preserves behavior when restrictions are %s', async (restriction) => { + const manager: HeftParameterManager = await parseAsync( + [createPlugin({ ...parameter, allowWithClean: restriction, allowInWatchMode: restriction })], + ['--clean', '--watch', ...args] + ); + expect(() => manager.validateParameters()).not.toThrow(); + }); + + it('does not restrict the other mode', async () => { + const cleanOnly: HeftParameterManager = await parseAsync( + [createPlugin({ ...parameter, allowWithClean: false })], + ['--watch', ...args] + ); + const watchOnly: HeftParameterManager = await parseAsync( + [createPlugin({ ...parameter, allowInWatchMode: false })], + ['--clean', ...args] + ); + expect(() => cleanOnly.validateParameters()).not.toThrow(); + expect(() => watchOnly.validateParameters()).not.toThrow(); + }); + }); + + it.each([ + { + parameterKind: 'integer', + longName: '--value', + description: 'Default zero.', + argumentName: 'VALUE', + defaultValue: 0 + }, + { + parameterKind: 'string', + longName: '--value', + description: 'Default empty string.', + argumentName: 'VALUE', + defaultValue: '' + }, + { + parameterKind: 'choice', + longName: '--value', + description: 'Default choice.', + alternatives: [{ name: 'first', description: 'First choice.' }], + defaultValue: 'first' + } + ])('checks the effective default for $parameterKind', async (parameter) => { + for (const mode of ['--clean', '--watch']) { + const manager: HeftParameterManager = await parseAsync( + [createPlugin({ ...parameter, allowWithClean: false, allowInWatchMode: false })], + [mode] + ); + expect(() => manager.validateParameters()).toThrow('--test-scope:value'); + } + }); + + it('checks scoped values independently when long names collide', async () => { + const parameter: IParameterJson = { + parameterKind: 'flag', + longName: '--value', + description: 'A flag.' + }; + const plugins: HeftTaskPluginDefinition[] = [ + createPlugin({ ...parameter, allowWithClean: false }, 'restricted'), + createPlugin(parameter, 'unrestricted') + ]; + const allowed: HeftParameterManager = await parseAsync(plugins, ['--clean', '--unrestricted:value']); + expect(() => allowed.validateParameters()).not.toThrow(); + const rejected: HeftParameterManager = await parseAsync(plugins, ['--clean', '--restricted:value']); + expect(() => rejected.validateParameters()).toThrow('--restricted:value'); + }); + + it('checks short names and alias defaults using parsed values', async () => { + const plugin: HeftTaskPluginDefinition = createPlugin({ + parameterKind: 'flag', + longName: '--value', + shortName: '-x', + description: 'A flag.', + allowWithClean: false + }); + const manager: HeftParameterManager = await parseAsync([plugin], [], ['--clean', '-x']); + expect(() => manager.validateParameters()).toThrow('--test-scope:value'); + }); +}); + +describe('plugin parameter restriction schema', () => { + const schema: JsonSchema = JsonSchema.fromLoadedObject(heftPluginSchema); + it.each(PARAMETER_TEST_CASES)('validates metadata on $parameter.parameterKind', ({ parameter }) => { + const validate = (extra: object): void => + schema.validateObject( + { + taskPlugins: [ + { pluginName: 'test-plugin', entryPoint: 'unused.js', parameters: [{ ...parameter, ...extra }] } + ] + }, + 'heft-plugin.json' + ); + expect(() => validate({ allowWithClean: false, allowInWatchMode: false })).not.toThrow(); + expect(() => validate({ allowWithClean: true, allowInWatchMode: true })).not.toThrow(); + expect(() => validate({ allowWithClean: 'false' })).toThrow(); + expect(() => validate({ allowInWatchMode: 'false' })).toThrow(); + expect(() => validate({ unknownRestriction: false })).toThrow(); + }); +}); diff --git a/apps/heft/src/schemas/heft-plugin.schema.json b/apps/heft/src/schemas/heft-plugin.schema.json index 37dd45012d7..f701d696bd6 100644 --- a/apps/heft/src/schemas/heft-plugin.schema.json +++ b/apps/heft/src/schemas/heft-plugin.schema.json @@ -42,6 +42,16 @@ "title": "Required", "description": "If true, then this parameter must be included on the command line", "type": "boolean" + }, + "allowWithClean": { + "title": "Allow With Clean", + "description": "If false, reject an active parameter with --clean before plugins run or outputs are deleted. Defaults to true. Flags are active when true, lists when nonempty, and scalars when defined, including default values.", + "type": "boolean" + }, + "allowInWatchMode": { + "title": "Allow In Watch Mode", + "description": "If false, reject an active parameter in watch mode before plugins run or outputs are deleted. Defaults to true. Flags are active when true, lists when nonempty, and scalars when defined, including default values.", + "type": "boolean" } } }, @@ -99,6 +109,8 @@ "shortName": { "$ref": "#/definitions/anything" }, "description": { "$ref": "#/definitions/anything" }, "required": { "$ref": "#/definitions/anything" }, + "allowWithClean": { "$ref": "#/definitions/anything" }, + "allowInWatchMode": { "$ref": "#/definitions/anything" }, "alternatives": { "$ref": "#/definitions/anything" }, "defaultValue": { "$ref": "#/definitions/anything" } @@ -155,6 +167,8 @@ "shortName": { "$ref": "#/definitions/anything" }, "description": { "$ref": "#/definitions/anything" }, "required": { "$ref": "#/definitions/anything" }, + "allowWithClean": { "$ref": "#/definitions/anything" }, + "allowInWatchMode": { "$ref": "#/definitions/anything" }, "alternatives": { "$ref": "#/definitions/anything" } } @@ -185,7 +199,9 @@ "longName": { "$ref": "#/definitions/anything" }, "shortName": { "$ref": "#/definitions/anything" }, "description": { "$ref": "#/definitions/anything" }, - "required": { "$ref": "#/definitions/anything" } + "required": { "$ref": "#/definitions/anything" }, + "allowWithClean": { "$ref": "#/definitions/anything" }, + "allowInWatchMode": { "$ref": "#/definitions/anything" } } } ] @@ -226,6 +242,8 @@ "shortName": { "$ref": "#/definitions/anything" }, "description": { "$ref": "#/definitions/anything" }, "required": { "$ref": "#/definitions/anything" }, + "allowWithClean": { "$ref": "#/definitions/anything" }, + "allowInWatchMode": { "$ref": "#/definitions/anything" }, "argumentName": { "$ref": "#/definitions/anything" }, "defaultValue": { "$ref": "#/definitions/anything" } @@ -264,6 +282,8 @@ "shortName": { "$ref": "#/definitions/anything" }, "description": { "$ref": "#/definitions/anything" }, "required": { "$ref": "#/definitions/anything" }, + "allowWithClean": { "$ref": "#/definitions/anything" }, + "allowInWatchMode": { "$ref": "#/definitions/anything" }, "argumentName": { "$ref": "#/definitions/anything" } } @@ -306,6 +326,8 @@ "shortName": { "$ref": "#/definitions/anything" }, "description": { "$ref": "#/definitions/anything" }, "required": { "$ref": "#/definitions/anything" }, + "allowWithClean": { "$ref": "#/definitions/anything" }, + "allowInWatchMode": { "$ref": "#/definitions/anything" }, "argumentName": { "$ref": "#/definitions/anything" }, "defaultValue": { "$ref": "#/definitions/anything" } @@ -344,6 +366,8 @@ "shortName": { "$ref": "#/definitions/anything" }, "description": { "$ref": "#/definitions/anything" }, "required": { "$ref": "#/definitions/anything" }, + "allowWithClean": { "$ref": "#/definitions/anything" }, + "allowInWatchMode": { "$ref": "#/definitions/anything" }, "argumentName": { "$ref": "#/definitions/anything" } } diff --git a/apps/heft/src/test/ParameterPreflight.test.ts b/apps/heft/src/test/ParameterPreflight.test.ts new file mode 100644 index 00000000000..fdc71040cf2 --- /dev/null +++ b/apps/heft/src/test/ParameterPreflight.test.ts @@ -0,0 +1,173 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import * as fs from 'node:fs'; +import * as path from 'node:path'; +import { spawnSync, type SpawnSyncReturns } from 'node:child_process'; + +import { JsonFile } from '@rushstack/node-core-library'; + +describe('Heft parameter preflight', () => { + let projectFolder: string; + const packageName: string = 'parameter-preflight-fixture'; + + beforeEach(() => { + projectFolder = fs.mkdtempSync(path.join(__dirname, 'parameter-preflight-')); + fs.mkdirSync(path.join(projectFolder, 'config')); + JsonFile.save({ name: packageName, version: '1.0.0' }, path.join(projectFolder, 'package.json')); + JsonFile.save( + { + lifecyclePlugins: [ + { + pluginName: 'lifecycle', + entryPoint: '../fixtures/PreflightLifecyclePlugin.js', + parameters: [ + { + parameterKind: 'flag', + longName: '--restricted-lifecycle', + description: 'A restricted lifecycle parameter.', + allowWithClean: false, + allowInWatchMode: false + } + ] + } + ], + taskPlugins: [ + { pluginName: 'build', entryPoint: '../fixtures/PreflightTaskPlugin.js' }, + { + pluginName: 'test', + entryPoint: '../fixtures/PreflightTaskPlugin.js', + parameterScope: 'test-scope', + parameters: [ + { + parameterKind: 'string', + longName: '--restricted', + description: 'A restricted parameter.', + argumentName: 'VALUE', + allowWithClean: false, + allowInWatchMode: false + } + ] + } + ] + }, + path.join(projectFolder, 'heft-plugin.json') + ); + JsonFile.save( + { + heftPlugins: [{ pluginPackage: packageName, pluginName: 'lifecycle' }], + aliasesByName: { + 'restricted-clean': { + actionName: 'run', + defaultParameters: ['--to', 'test', '--', '--clean', '--restricted', 'value'] + } + }, + phasesByName: { + build: { + cleanFiles: [{ sourcePath: 'build-output' }], + tasksByName: { + compile: { taskPlugin: { pluginPackage: packageName, pluginName: 'build' } } + } + }, + test: { + phaseDependencies: ['build'], + tasksByName: { + check: { taskPlugin: { pluginPackage: packageName, pluginName: 'test' } } + } + } + } + }, + path.join(projectFolder, 'config', 'heft.json') + ); + fs.mkdirSync(path.join(projectFolder, 'build-output')); + fs.writeFileSync(path.join(projectFolder, 'build-output', 'sentinel'), 'keep build output'); + fs.writeFileSync(path.join(projectFolder, 'lifecycle-output'), 'keep lifecycle output'); + }); + + afterEach(() => { + fs.rmSync(projectFolder, { recursive: true, force: true }); + }); + + function runHeft(args: string[]): SpawnSyncReturns { + const result: SpawnSyncReturns = spawnSync( + process.execPath, + [path.join(__dirname, '..', 'start.js'), ...args], + { cwd: projectFolder, encoding: 'utf8', timeout: 15000 } + ); + // A watch loop must exit on its own, rather than requiring the timeout to kill it. + expect(result.error).toBeUndefined(); + expect(result.signal).toBeNull(); + return result; + } + + it.each([ + ['run', '--to', 'test', '--', '--clean', '--restricted', 'value'], + ['run', '--to', 'test', '--', '--clean', '--test-scope:restricted', 'value'], + ['restricted-clean'], + ['run-watch', '--to', 'test', '--', '--restricted', 'value'], + ['test-watch', '--restricted', 'value'], + ['run-watch', '--to', 'test', '--', '--clean', '--restricted', 'value'] + ])('rejects before lifecycle, dependency cleaning, plugin application, or watching: %j', (...args) => { + const result: SpawnSyncReturns = runHeft(args); + expect(result.status).toBe(1); + expect(result.stderr).toContain( + 'Parameter "--test-scope:restricted" from plugin "test" in package "parameter-preflight-fixture"' + ); + expect(result.stderr).toContain( + args.includes('--clean') || args[0] === 'restricted-clean' + ? 'cannot be used with --clean' + : 'cannot be used with watch mode' + ); + expect(result.stdout).not.toContain('Waiting for changes'); + expect(fs.readFileSync(path.join(projectFolder, 'build-output', 'sentinel'), 'utf8')).toBe( + 'keep build output' + ); + expect(fs.readFileSync(path.join(projectFolder, 'lifecycle-output'), 'utf8')).toBe( + 'keep lifecycle output' + ); + for (const marker of ['lifecycle-applied', 'lifecycle-started', 'compile-applied', 'check-applied']) { + expect(fs.existsSync(path.join(projectFolder, marker))).toBe(false); + } + }); + + it('also preflights lifecycle parameters', () => { + const result: SpawnSyncReturns = runHeft([ + 'run', + '--to', + 'test', + '--', + '--clean', + '--restricted-lifecycle' + ]); + expect(result.status).toBe(1); + expect(result.stderr).toContain('--lifecycle:restricted-lifecycle'); + expect(fs.existsSync(path.join(projectFolder, 'lifecycle-applied'))).toBe(false); + expect(fs.existsSync(path.join(projectFolder, 'build-output', 'sentinel'))).toBe(true); + }); + + it('cleans dependency outputs and runs both phases when the restricted parameter is inactive', () => { + const result: SpawnSyncReturns = runHeft(['run', '--to', 'test', '--', '--clean']); + expect(result.stderr).toBe(''); + expect(result.status).toBe(0); + expect(fs.existsSync(path.join(projectFolder, 'build-output', 'sentinel'))).toBe(false); + expect(fs.existsSync(path.join(projectFolder, 'lifecycle-output'))).toBe(false); + expect(fs.existsSync(path.join(projectFolder, 'compile-ran'))).toBe(true); + expect(fs.existsSync(path.join(projectFolder, 'check-ran'))).toBe(true); + }); + + it('allows an active restricted parameter in an ordinary run without cleaning', () => { + const result: SpawnSyncReturns = runHeft(['test', '--restricted', 'value']); + expect(result.stderr).toBe(''); + expect(result.status).toBe(0); + expect(fs.existsSync(path.join(projectFolder, 'check-ran'))).toBe(true); + expect(fs.existsSync(path.join(projectFolder, 'build-output', 'sentinel'))).toBe(true); + }); + + it('leaves the standalone clean command unchanged', () => { + const result: SpawnSyncReturns = runHeft(['clean', '--to', 'test']); + expect(result.stderr).toBe(''); + expect(result.status).toBe(0); + expect(fs.existsSync(path.join(projectFolder, 'build-output', 'sentinel'))).toBe(false); + expect(fs.existsSync(path.join(projectFolder, 'lifecycle-applied'))).toBe(false); + }); +}); diff --git a/apps/heft/src/test/fixtures/PreflightLifecyclePlugin.ts b/apps/heft/src/test/fixtures/PreflightLifecyclePlugin.ts new file mode 100644 index 00000000000..77521dc0041 --- /dev/null +++ b/apps/heft/src/test/fixtures/PreflightLifecyclePlugin.ts @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +import type { IHeftLifecyclePlugin } from '../../pluginFramework/IHeftPlugin'; +import type { IHeftLifecycleSession } from '../../pluginFramework/HeftLifecycleSession'; +import type { HeftConfiguration } from '../../configuration/HeftConfiguration'; + +export default class PreflightLifecyclePlugin implements IHeftLifecyclePlugin { + public apply(session: IHeftLifecycleSession, configuration: HeftConfiguration): void { + fs.writeFileSync(path.join(configuration.buildFolderPath, 'lifecycle-applied'), ''); + session.hooks.toolStart.tap('preflight-test', () => { + fs.writeFileSync(path.join(configuration.buildFolderPath, 'lifecycle-started'), ''); + }); + session.hooks.clean.tap('preflight-test', () => { + fs.unlinkSync(path.join(configuration.buildFolderPath, 'lifecycle-output')); + }); + } +} diff --git a/apps/heft/src/test/fixtures/PreflightTaskPlugin.ts b/apps/heft/src/test/fixtures/PreflightTaskPlugin.ts new file mode 100644 index 00000000000..4a38c90d8e0 --- /dev/null +++ b/apps/heft/src/test/fixtures/PreflightTaskPlugin.ts @@ -0,0 +1,18 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license. +// See LICENSE in the project root for license information. + +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +import type { IHeftTaskPlugin } from '../../pluginFramework/IHeftPlugin'; +import type { IHeftTaskSession } from '../../pluginFramework/HeftTaskSession'; +import type { HeftConfiguration } from '../../configuration/HeftConfiguration'; + +export default class PreflightTaskPlugin implements IHeftTaskPlugin { + public apply(session: IHeftTaskSession, configuration: HeftConfiguration): void { + fs.writeFileSync(path.join(configuration.buildFolderPath, `${session.taskName}-applied`), ''); + session.hooks.run.tap('preflight-test', () => { + fs.writeFileSync(path.join(configuration.buildFolderPath, `${session.taskName}-ran`), ''); + }); + } +} diff --git a/common/changes/@rushstack/heft/parameter-mode-preflight_2026-09-14-20-45.json b/common/changes/@rushstack/heft/parameter-mode-preflight_2026-09-14-20-45.json new file mode 100644 index 00000000000..e84d1344ba9 --- /dev/null +++ b/common/changes/@rushstack/heft/parameter-mode-preflight_2026-09-14-20-45.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/heft", + "comment": "Add allowWithClean and allowInWatchMode plugin parameter metadata to reject incompatible active parameters before running lifecycle hooks, cleaning outputs, or entering watch mode.", + "type": "minor" + } + ], + "packageName": "@rushstack/heft" +} From 3eec263b02fdef7b4242441b8d0c0cca15641d9c Mon Sep 17 00:00:00 2001 From: yashwanth195 <59129034+yashwanth195@users.noreply.github.com> Date: Thu, 12 Nov 2026 17:28:40 -0800 Subject: [PATCH 2/2] Stabilize daemon activity tests --- ...aemon-activity-tests_2026-11-12-17-00.json | 10 ++ .../src/test/ConnectingClientTracker.test.ts | 20 +++ .../src/test/DaemonRequestWirePhased.test.ts | 163 ++++++++---------- 3 files changed, 99 insertions(+), 94 deletions(-) create mode 100644 common/changes/@rushstack/rush-daemon/daemon-activity-tests_2026-11-12-17-00.json diff --git a/common/changes/@rushstack/rush-daemon/daemon-activity-tests_2026-11-12-17-00.json b/common/changes/@rushstack/rush-daemon/daemon-activity-tests_2026-11-12-17-00.json new file mode 100644 index 00000000000..5c48a2320d1 --- /dev/null +++ b/common/changes/@rushstack/rush-daemon/daemon-activity-tests_2026-11-12-17-00.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/rush-daemon", + "comment": "Make connection activity regression tests independent of wall-clock timing and socket buffer sizes.", + "type": "none" + } + ], + "packageName": "@rushstack/rush-daemon" +} diff --git a/libraries/rush-daemon/src/test/ConnectingClientTracker.test.ts b/libraries/rush-daemon/src/test/ConnectingClientTracker.test.ts index a5b2fd2d71d..1df417e5c63 100644 --- a/libraries/rush-daemon/src/test/ConnectingClientTracker.test.ts +++ b/libraries/rush-daemon/src/test/ConnectingClientTracker.test.ts @@ -110,6 +110,26 @@ describe(ConnectingClientTracker.name, () => { expect(wait.done).toBe(true); }); + it('counts an idle connection as connecting again after its next activity', async () => { + const tracker: ConnectingClientTracker = new ConnectingClientTracker({ + idleLimitMs: 100, + waitLimitMs: 1000 + }); + const client: IConnectingClient = tracker.add(); + await jest.advanceTimersByTimeAsync(150); + const idleWait: IObservedWait = observe(tracker.waitAsync()); + await jest.advanceTimersByTimeAsync(5); + expect(idleWait.done).toBe(true); + + client.touch(); + const activeWait: IObservedWait = observe(tracker.waitAsync()); + await jest.advanceTimersByTimeAsync(5); + expect(activeWait.done).toBe(false); + await jest.advanceTimersByTimeAsync(100); + expect(activeWait.done).toBe(true); + expect(jest.getTimerCount()).toBe(0); + }); + it('reads the frames that already arrived before it decides that another connection is idle', async () => { const tracker: ConnectingClientTracker = new ConnectingClientTracker({ idleLimitMs: 100, diff --git a/libraries/rush-daemon/src/test/DaemonRequestWirePhased.test.ts b/libraries/rush-daemon/src/test/DaemonRequestWirePhased.test.ts index 0951837e4df..2b00e668da2 100644 --- a/libraries/rush-daemon/src/test/DaemonRequestWirePhased.test.ts +++ b/libraries/rush-daemon/src/test/DaemonRequestWirePhased.test.ts @@ -16,17 +16,19 @@ import { } from '@rushstack/rush-daemon-protocol'; import type { DaemonControlMessage, + IDaemonFrame, IDaemonPhasedRequest, IDaemonRequestEnvelope } from '@rushstack/rush-daemon-protocol'; +import { DaemonFrameConnection } from '@rushstack/rush-daemon-transport'; import type { ITerminal } from '@rushstack/terminal'; import { ConnectingClientTracker } from '../ConnectingClientTracker'; +import type { IConnectingClient } from '../ConnectingClientTracker'; import type { IDaemonRequestResolver } from '../DaemonRequestDispatcher'; import { RushDaemonHost } from '../RushDaemonHost'; import type { IRushDaemonHostOptions } from '../RushDaemonHost'; import { WorkspaceEngineRecreationRequiredError } from '../WorkspaceEngineComponentFactory'; -import type { IWorkspaceWarmSetStatus } from '../WorkspaceWarmSet'; import { TEST_ENGINE_SHAPE, TestOperationRunner, @@ -39,10 +41,7 @@ import { createDeferred, createWireEnvelope } from './DaemonRequestWireTestUtilities'; -import type { - IDeferred, - ITerminalExchange -} from './DaemonRequestWireTestUtilities'; +import type { IDeferred, ITerminalExchange } from './DaemonRequestWireTestUtilities'; const OPERATION_A: string = 'project-a (_phase:test)'; const OPERATION_B: string = 'project-b (_phase:test)'; @@ -102,9 +101,7 @@ async function startHostAsync( } async function connectAsync(host: RushDaemonHost): Promise { - const client: DaemonRequestWireClient = await DaemonRequestWireClient.connectAsync( - host.paths.socketPath - ); + const client: DaemonRequestWireClient = await DaemonRequestWireClient.connectAsync(host.paths.socketPath); await client.handshakeAsync(); return client; } @@ -156,26 +153,20 @@ describe('daemon phased request wire integration', () => { const host: RushDaemonHost = await startHostAsync(repoRoot, fixture); const client: DaemonRequestWireClient = await connectAsync(host); try { - const first: ITerminalExchange = await startAsync( - client, - { - ...phasedEnvelope(repoRoot, 'subtree', OPERATION_B), - environment: { RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD: '1' } - } - ); + const first: ITerminalExchange = await startAsync(client, { + ...phasedEnvelope(repoRoot, 'subtree', OPERATION_B), + environment: { RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD: '1' } + }); expect(first.terminal).toMatchObject({ kind: 'requestResult', payload: { exitCode: 0, outcome: 'success-with-warning', scheduled: true } }); expect(readOperationIds(first)).toEqual(new Set([OPERATION_A, OPERATION_B])); expect(readLogText(first)).toContain('warning-output'); - const warm: ITerminalExchange = await startAsync( - client, - { - ...phasedEnvelope(repoRoot, 'warm', OPERATION_B), - environment: { RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD: '1' } - } - ); + const warm: ITerminalExchange = await startAsync(client, { + ...phasedEnvelope(repoRoot, 'warm', OPERATION_B), + environment: { RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD: '1' } + }); expect(warm.terminal).toMatchObject({ kind: 'requestResult', payload: { scheduled: false } @@ -340,48 +331,66 @@ describe('daemon phased request wire integration', () => { } }); - it('counts a connection as connecting again once the daemon finishes writing a reply that waited', async () => { - const addSpy: jest.SpyInstance = jest.spyOn(ConnectingClientTracker.prototype, 'add'); + it('refreshes connection activity only after a delayed reply finishes writing', async () => { + const addSpy: jest.SpyInstance = jest.spyOn( + ConnectingClientTracker.prototype, + 'add' + ); const repoRoot: string = createRepoRoot(); const fixture: ITestRoutingFixture = createRoutingFixture( new Map([[OPERATION_A, new TestOperationRunner(OPERATION_A)]]) ); + const order: string[] = []; + const writeStarted: IDeferred = createDeferred(); + const release: IDeferred = createDeferred(); + const activityRefreshed: IDeferred = createDeferred(); + const sendFrameAsync: (frame: IDaemonFrame) => Promise = + DaemonFrameConnection.prototype.sendFrameAsync; + let sendSpy: jest.SpyInstance | undefined; + let touchSpy: jest.SpyInstance | undefined; const host: RushDaemonHost = await startHostAsync(repoRoot, fixture); - const clients: DaemonRequestWireClient[] = []; + const client: DaemonRequestWireClient = await DaemonRequestWireClient.connectAsync(host.paths.socketPath); try { - clients.push(await connectAsync(host)); - const tracker: ConnectingClientTracker = addSpy.mock.contexts[0] as ConnectingClientTracker; - // The first request installs the session, whose warm set status each pong then carries. As in a large repo, - // that makes the pong larger than the socket's buffers. - const first: ITerminalExchange = await startAsync( - clients[0], - phasedEnvelope(repoRoot, 'first', OPERATION_A) - ); - expect(first.terminal).toMatchObject({ kind: 'requestResult', payload: { exitCode: 0 } }); - fixture.session.warmSetStatus = createLargeWarmSetStatus(); - - const client: DaemonRequestWireClient = await DaemonRequestWireClient.connectAsync( - host.paths.socketPath - ); - clients.push(client); await client.sendControlAsync(createDaemonHello(DAEMON_PROTOCOL_VERSION)); expect((await client.readControlAsync()).kind).toBe('helloAck'); - await settleAsync(); - client.pauseReading(); + const connectingClient: IConnectingClient = addSpy.mock.results[0].value; + sendSpy = jest + .spyOn(DaemonFrameConnection.prototype, 'sendFrameAsync') + .mockImplementation(async function (this: DaemonFrameConnection, frame: IDaemonFrame): Promise { + if ( + frame.kind === DaemonFrameType.controlJson && + decodeDaemonControlMessage(frame.payload).kind === 'pong' + ) { + const touch: () => void = connectingClient.touch; + touchSpy = jest.spyOn(connectingClient, 'touch').mockImplementation(() => { + touch(); + order.push('touch'); + activityRefreshed.resolve(); + }); + order.push('write'); + writeStarted.resolve(); + // Hold the send at the same boundary as a backpressured socket, without depending on buffer sizes. + await release.promise; + await sendFrameAsync.call(this, frame); + order.push('written'); + } else { + await sendFrameAsync.call(this, frame); + } + }); await client.sendControlAsync({ kind: 'ping', payload: {} }); - // The daemon read the ping but cannot finish writing the pong. A client that stops reading must not hold - // batches, so its connection still counts as idle once the frame it sent last is old enough. - await sleepAsync(150); - expect(await isWaitingAsync(tracker)).toBe(false); + await writeStarted.promise; + expect(order).toEqual(['write']); - // As when the daemon was too busy to finish writing the pong, the client gets the pong only now, and has not - // sent its request yet. - client.resumeReading(); + release.resolve(); expect((await client.readControlAsync()).kind).toBe('pong'); - expect(await isWaitingAsync(tracker)).toBe(true); + await activityRefreshed.promise; + expect(order).toEqual(['write', 'written', 'touch']); } finally { + release.resolve(); + touchSpy?.mockRestore(); + sendSpy?.mockRestore(); addSpy.mockRestore(); - await Promise.all(clients.map((client: DaemonRequestWireClient) => client.closeAsync())); + await client.closeAsync(); await host.closeAsync(); } }); @@ -391,8 +400,7 @@ describe('daemon phased request wire integration', () => { const fixture: ITestRoutingFixture = createRoutingFixture( new Map([[OPERATION_A, new TestOperationRunner(OPERATION_A)]]) ); - fixture.session.onReconcileAsync = () => - Promise.reject(new WorkspaceEngineRecreationRequiredError()); + fixture.session.onReconcileAsync = () => Promise.reject(new WorkspaceEngineRecreationRequiredError()); const scheduleSpy: jest.SpyInstance = jest.spyOn(fixture.graph, 'scheduleIterationAsync'); const host: RushDaemonHost = await startHostAsync(repoRoot, fixture); const client: DaemonRequestWireClient = await connectAsync(host); @@ -421,17 +429,14 @@ describe('daemon phased request wire integration', () => { const host: RushDaemonHost = await startHostAsync(repoRoot, fixture); const client: DaemonRequestWireClient = await connectAsync(host); try { - const exchange: ITerminalExchange = await startAsync( - client, - { - ...phasedEnvelope(repoRoot, 'fallback', OPERATION_A), - terminal: { - isTTY: true, - supportsColor: true, - terminalRequirement: 'controllingTerminal' - } + const exchange: ITerminalExchange = await startAsync(client, { + ...phasedEnvelope(repoRoot, 'fallback', OPERATION_A), + terminal: { + isTTY: true, + supportsColor: true, + terminalRequirement: 'controllingTerminal' } - ); + }); expect(exchange.terminal).toMatchObject({ kind: 'terminalPolicy', payload: { decision: 'requiresInProcess', requestId: 'fallback' } @@ -499,10 +504,7 @@ function readOperationIds(exchange: ITerminalExchange): ReadonlySet { function readLogText(exchange: ITerminalExchange): string { return exchange.frames - .filter( - (frame) => - frame.kind === DaemonFrameType.logStdout || frame.kind === DaemonFrameType.logStderr - ) + .filter((frame) => frame.kind === DaemonFrameType.logStdout || frame.kind === DaemonFrameType.logStderr) .map((frame) => new TextDecoder().decode(decodeDaemonLogChunk(frame.payload).chunk)) .join(''); } @@ -533,30 +535,3 @@ function sleepAsync(durationMs: number): Promise { async function isWaitingAsync(tracker: ConnectingClientTracker): Promise { return await Promise.race([tracker.waitAsync().then(() => false), settleAsync().then(() => true)]); } - -/** A warm set status of about 2 MB, like a large repo's, which is larger than any platform's socket buffers. */ -function createLargeWarmSetStatus(): IWorkspaceWarmSetStatus { - const projectNames: string[] = Array.from( - { length: 20_000 }, - (unused, index) => `@example/${'large-project-name-'.repeat(5)}${index}` - ); - return { - cleanupFailures: [], - configuration: { - autoWarmByTelemetry: false, - warmIdleTimeoutSeconds: 60, - warmMemoryBudgetMB: 1024, - warmSetMaxProjects: 20_000 - }, - daemonResidentMemoryBytes: 0, - deferredReason: undefined, - maintenanceState: 'running', - measuredRunnerMemoryBytes: 0, - overMemoryBudget: false, - overProjectLimit: false, - protectedProjectNames: [], - retainedProjectNames: projectNames, - unmeasuredRunnerCount: 0, - watchedProjectNames: [] - }; -}