diff --git a/apps/heft/src/pluginFramework/logging/HeftChildReporter.test.ts b/apps/heft/src/pluginFramework/logging/HeftChildReporter.test.ts index 686ee86c9e..a6dd904931 100644 --- a/apps/heft/src/pluginFramework/logging/HeftChildReporter.test.ts +++ b/apps/heft/src/pluginFramework/logging/HeftChildReporter.test.ts @@ -7,6 +7,8 @@ import * as os from 'node:os'; import * as path from 'node:path'; import type { Readable, Writable } from 'node:stream'; +import { PackageJsonLookup } from '@rushstack/node-core-library'; + import { HeftChildReporter } from './HeftChildReporter'; describe(HeftChildReporter.name, () => { @@ -36,6 +38,54 @@ describe(HeftChildReporter.name, () => { } }); + it('recognizes pipe mode bits when platform predicates return false', () => { + const fsModule: typeof fs = jest.requireActual('node:fs'); + const stats: fs.Stats = fs.statSync(__filename); + stats.mode = fs.constants.S_IFIFO; + stats.isFIFO = () => false; + stats.isSocket = () => false; + const acknowledgement: Buffer = Buffer.from( + `${JSON.stringify({ + kind: 'helloAck', + protocolVersion: { major: 1, minor: 2 }, + acceptedCapabilities: ['heft-child-events-v1'], + rejectedRequiredFeatures: [] + })}\n` + ); + const statSpy: jest.SpyInstance = jest.spyOn(fsModule, 'fstatSync').mockReturnValue(stats); + const packageSpy: jest.SpyInstance = jest + .spyOn(PackageJsonLookup.instance, 'tryLoadPackageJsonFor') + .mockReturnValue({ name: '@rushstack/heft', version: '1.0.0' }); + const writeSpy: jest.SpyInstance = jest.spyOn(fsModule, 'writeSync').mockReturnValue(1); + const readSpy: jest.SpyInstance = jest.spyOn(fsModule, 'readSync').mockImplementation((fd, buffer) => { + expect(fd).toBe(4); + if (!Buffer.isBuffer(buffer)) { + throw new Error('Expected the acknowledgement read buffer.'); + } + return acknowledgement.copy(buffer); + }); + const closeSpy: jest.SpyInstance = jest.spyOn(fsModule, 'closeSync').mockImplementation(() => {}); + + try { + const reporter: HeftChildReporter | undefined = HeftChildReporter.tryInitialize({ + _RUSH_REPORTER_CHILD_FD: '3', + _RUSH_REPORTER_CHILD_ACK_FD: '4' + }); + + expect(reporter).toBeDefined(); + expect(reporter?.parentReporterName).toBe('plaintext'); + expect(writeSpy).toHaveBeenCalledWith(3, expect.stringContaining('"kind":"hello"')); + expect(closeSpy).toHaveBeenCalledWith(4); + expect(closeSpy).not.toHaveBeenCalledWith(3); + } finally { + closeSpy.mockRestore(); + readSpy.mockRestore(); + writeSpy.mockRestore(); + packageSpy.mockRestore(); + statSpy.mockRestore(); + } + }); + it('negotiates context and emits ordered structured output and diagnostics', async () => { const modulePath: string = require.resolve('./HeftChildReporter'); const childScript: string = ` @@ -218,6 +268,7 @@ describe(HeftChildReporter.name, () => { }); expect(exitCode).toBe(0); + expect(acknowledgementSent).toBe(true); expect(stdout).toBe('context fallback'); } ); diff --git a/apps/heft/src/pluginFramework/logging/HeftChildReporter.ts b/apps/heft/src/pluginFramework/logging/HeftChildReporter.ts index d41e141897..471c053b88 100644 --- a/apps/heft/src/pluginFramework/logging/HeftChildReporter.ts +++ b/apps/heft/src/pluginFramework/logging/HeftChildReporter.ts @@ -46,7 +46,9 @@ function readDescriptorFd(env: Record, name: string) function isReporterPipe(fd: number): boolean { try { const stats: fs.Stats = fs.fstatSync(fd); - return stats.isFIFO() || stats.isSocket(); + // Node disables isFIFO() on Windows even when fstat reports a named pipe. + // eslint-disable-next-line no-bitwise -- Compare the file type without permission bits. + return (stats.mode & fs.constants.S_IFMT) === fs.constants.S_IFIFO || stats.isSocket(); } catch { return false; } diff --git a/apps/rush/src/IRushFrontendLaunchOptions.ts b/apps/rush/src/IRushFrontendLaunchOptions.ts index 34753e80af..9bf40d8980 100644 --- a/apps/rush/src/IRushFrontendLaunchOptions.ts +++ b/apps/rush/src/IRushFrontendLaunchOptions.ts @@ -16,6 +16,7 @@ export interface IRushFrontendLaunchOptions extends ILaunchOptions { readonly reporterCloseAsync: () => Promise; readonly reporterEnabled: boolean; readonly reporterStdoutIsMachineReadable?: boolean; + readonly reporterStdoutIsReserved?: boolean; readonly reporterSelectionReason: | 'explicit --reporter' | 'repository experiment' diff --git a/apps/rush/src/MinimalRushConfiguration.ts b/apps/rush/src/MinimalRushConfiguration.ts index 4d84b61e85..0687550128 100644 --- a/apps/rush/src/MinimalRushConfiguration.ts +++ b/apps/rush/src/MinimalRushConfiguration.ts @@ -75,7 +75,7 @@ export class MinimalRushConfiguration { explicitReporter === 'legacy' || process.env.RUSH_REPORTER?.trim().toLowerCase() === 'legacy' || _hasHelpControl(process.argv.slice(2)) || - effectiveRushVersion !== currentPackageVersion; + (effectiveRushVersion !== currentPackageVersion && explicitReporter === undefined); if ( showVerbose && (legacyFallbackRequested || diff --git a/apps/rush/src/RushCommandSelector.ts b/apps/rush/src/RushCommandSelector.ts index 8ecee361a3..2db3426593 100644 --- a/apps/rush/src/RushCommandSelector.ts +++ b/apps/rush/src/RushCommandSelector.ts @@ -57,7 +57,7 @@ export class RushCommandSelector { } ); let effectiveOptions: IRushFrontendLaunchOptions = options; - let restoreOldEngineOutput: (() => void) | undefined; + let restoreEngineOutput: (() => void) | undefined; if (compatibility.mode !== 'structured' && engineProtocolMajor !== undefined && options.reporterEnabled) { if (options.reporterSelectionReason === 'explicit --reporter') { throw new Error( @@ -75,8 +75,15 @@ export class RushCommandSelector { reporterEnabled: false, reporterSelectionReason: 'bootstrap compatibility fallback' }; - } else if (compatibility.mode === 'new-frontend-old-engine' && options.reporterEnabled) { - restoreOldEngineOutput = _observeOldEngineOutput(options, Rush.version); + } else if (options.reporterEnabled) { + restoreEngineOutput = _observeEngineOutput(options, Rush.version); + effectiveOptions = { + ...options, + reporterCloseAsync: async () => { + restoreEngineOutput?.(); + await options.reporterCloseAsync(); + } + }; } try { @@ -103,13 +110,13 @@ export class RushCommandSelector { Rush.launch(launcherVersion, effectiveOptions); } } catch (error) { - restoreOldEngineOutput?.(); + restoreEngineOutput?.(); throw error; } } } -function _observeOldEngineOutput(options: IRushFrontendLaunchOptions, engineVersion: string): () => void { +function _observeEngineOutput(options: IRushFrontendLaunchOptions, engineVersion: string): () => void { const adapter: OldEngineOutputAdapter = new OldEngineOutputAdapter({ sink: options.reporter.eventSink, sessionId: options.reporter.sessionId, @@ -120,7 +127,7 @@ function _observeOldEngineOutput(options: IRushFrontendLaunchOptions, engineVers 'stdout', adapter, process.stdout.write.bind(process.stdout), - options.reporterStdoutIsMachineReadable !== true + (options.reporterStdoutIsReserved ?? options.reporterStdoutIsMachineReadable) !== true ); const restoreStderr: () => void = _observeStream( process.stderr, diff --git a/apps/rush/src/RushFrontend.ts b/apps/rush/src/RushFrontend.ts index fd9b9f2e09..81c2bf6501 100644 --- a/apps/rush/src/RushFrontend.ts +++ b/apps/rush/src/RushFrontend.ts @@ -214,6 +214,11 @@ export async function launchRushFrontendAsync(options: IRushFrontendOptions): Pr reporterEnabled: reporterHost.selection.enabled, reporterStdoutIsMachineReadable: reporterHost.selection.reporter === 'ai' || reporterHost.selection.reporter === 'json', + reporterStdoutIsReserved: + !reporterHost.selection.commandJson && + (reporterHost.selection.reporter === 'ai' || + reporterHost.selection.reporter === 'json' || + reporterHost.selection.reporter === 'file'), reporterSelectionReason: reporterHost.selection.reason }; diff --git a/apps/rush/src/test/MinimalRushConfiguration.test.ts b/apps/rush/src/test/MinimalRushConfiguration.test.ts index 9894396f92..3f2e1e847e 100644 --- a/apps/rush/src/test/MinimalRushConfiguration.test.ts +++ b/apps/rush/src/test/MinimalRushConfiguration.test.ts @@ -12,6 +12,7 @@ describe(MinimalRushConfiguration.name, () => { const originalArgv: string[] = process.argv; const originalRushTempFolder: string | undefined = process.env.RUSH_TEMP_FOLDER; const originalRushPreviewVersion: string | undefined = process.env.RUSH_PREVIEW_VERSION; + const originalRushReporter: string | undefined = process.env.RUSH_REPORTER; afterEach(() => { jest.restoreAllMocks(); @@ -26,6 +27,11 @@ describe(MinimalRushConfiguration.name, () => { } else { process.env.RUSH_PREVIEW_VERSION = originalRushPreviewVersion; } + if (originalRushReporter === undefined) { + delete process.env.RUSH_REPORTER; + } else { + process.env.RUSH_REPORTER = originalRushReporter; + } EnvironmentConfiguration.reset(); }); @@ -143,11 +149,30 @@ describe(MinimalRushConfiguration.name, () => { ]); }); + it.each(['json', 'ai', 'file'])( + 'keeps discovery off stdout when an incompatible engine rejects %s', + (reporter) => { + const legacyRepo: string = path.join(__dirname, 'sandbox', 'legacy-repo'); + const consoleLog = jest.spyOn(console, 'log').mockImplementation(() => undefined); + jest.spyOn(PackageJsonLookup, 'loadOwnPackageJson').mockReturnValue({ + name: '@microsoft/rush', + version: '5.178.1' + }); + jest.spyOn(process, 'cwd').mockReturnValue(path.join(legacyRepo, 'project')); + delete process.env.RUSH_REPORTER; + process.argv = ['node', 'rush', 'build', `--reporter=${reporter}`]; + + MinimalRushConfiguration.loadFromDefaultLocation(); + + expect(consoleLog).not.toHaveBeenCalled(); + } + ); + it.each([ ['environment fallback', ['build', '--reporter=json'], 'legacy'], ['explicit legacy reporter', ['build', '--reporter=legacy'], undefined], ['help fallback', ['build', '--reporter=json', '--help'], undefined], - ['cross-version fallback', ['build', '--reporter=json'], undefined] + ['cross-version fallback', ['build'], undefined] ])('restores legacy discovery output in an opted-in repository for %s', (testName, args, envValue) => { void testName; const repo: string = path.join(__dirname, 'sandbox', 'repo'); diff --git a/apps/rush/src/test/RushCommandSelector.test.ts b/apps/rush/src/test/RushCommandSelector.test.ts index 9d33bb842b..eb7e9383e9 100644 --- a/apps/rush/src/test/RushCommandSelector.test.ts +++ b/apps/rush/src/test/RushCommandSelector.test.ts @@ -72,13 +72,13 @@ describe(RushCommandSelector.name, () => { ); }); - it('does not observe output from a matching structured engine', () => { + it('does not observe output from a matching structured engine when reporters are disabled', () => { const manager: ReporterManager = new ReporterManager(); const options: IRushFrontendLaunchOptions = { isManaged: true, reporter: { eventSink: manager, sessionId: 'test-session' }, reporterCloseAsync: async () => {}, - reporterEnabled: true, + reporterEnabled: false, reporterSelectionReason: 'explicit --reporter' }; const originalStdoutWrite: typeof process.stdout.write = process.stdout.write; @@ -275,62 +275,72 @@ describe(RushCommandSelector.name, () => { ]); }); - it('keeps old-engine stdout structured for machine reporters', async () => { - const manager: ReporterManager = new ReporterManager(); - const reporter: RecordingReporter = new RecordingReporter(); - manager.addReporter(reporter); - await manager.initializeAsync(); - - const originalArgv: string[] = process.argv; - const originalStdoutWrite: typeof process.stdout.write = process.stdout.write; - const originalStderrWrite: typeof process.stderr.write = process.stderr.write; - let stdoutText: string = ''; - const stdoutWrite: typeof process.stdout.write = ((text: string): boolean => { - stdoutText += text; - return true; - }) as typeof process.stdout.write; - process.argv = ['node', 'rush', 'build']; - process.stdout.write = stdoutWrite; - process.stderr.write = (() => true) as typeof process.stderr.write; - const previousBeforeExitListeners: readonly BeforeExitListener[] = process.listeners( - 'beforeExit' - ) as BeforeExitListener[]; - - try { - RushCommandSelector.execute( - '5.178.1', - { - Rush: { - version: '5.177.0', - launch: () => { - process.stdout.write('legacy stdout\n'); + it.each([undefined, REPORTER_PROTOCOL_VERSION.major])( + 'keeps engine stdout structured for machine reporters (protocol %s)', + async (protocolMajor) => { + const manager: ReporterManager = new ReporterManager(); + const reporter: RecordingReporter = new RecordingReporter(); + manager.addReporter(reporter); + await manager.initializeAsync(); + + const originalArgv: string[] = process.argv; + const originalStdoutWrite: typeof process.stdout.write = process.stdout.write; + const originalStderrWrite: typeof process.stderr.write = process.stderr.write; + let stdoutText: string = ''; + const stdoutWrite: typeof process.stdout.write = ((text: string): boolean => { + stdoutText += text; + return true; + }) as typeof process.stdout.write; + process.argv = ['node', 'rush', 'build']; + process.stdout.write = stdoutWrite; + process.stderr.write = (() => true) as typeof process.stderr.write; + const previousBeforeExitListeners: readonly BeforeExitListener[] = process.listeners( + 'beforeExit' + ) as BeforeExitListener[]; + let closeFromEngine: (() => Promise) | undefined; + + try { + RushCommandSelector.execute( + '5.178.1', + { + Rush: { + version: '5.177.0', + _reporterProtocolMajor: protocolMajor, + launch: (version: string, options: IRushFrontendLaunchOptions) => { + void version; + closeFromEngine = options.reporterCloseAsync; + process.stdout.write('legacy stdout\n'); + } } + } as unknown as typeof import('@microsoft/rush-lib'), + { + isManaged: true, + reporter: { eventSink: manager, sessionId: 'test-session' }, + reporterCloseAsync: async () => { + expect(process.stdout.write).toBe(stdoutWrite); + await manager.closeAsync(); + }, + reporterEnabled: true, + reporterStdoutIsMachineReadable: true, + reporterSelectionReason: 'explicit --reporter' } - } as unknown as typeof import('@microsoft/rush-lib'), - { - isManaged: true, - reporter: { eventSink: manager, sessionId: 'test-session' }, - reporterCloseAsync: async () => {}, - reporterEnabled: true, - reporterStdoutIsMachineReadable: true, - reporterSelectionReason: 'explicit --reporter' - } - ); - restoreObservedOutput(previousBeforeExitListeners); - await manager.flushAsync(); - } finally { - restoreObservedOutput(previousBeforeExitListeners, false); - process.stdout.write = originalStdoutWrite; - process.stderr.write = originalStderrWrite; - process.argv = originalArgv; - } + ); + expect(closeFromEngine).toBeDefined(); + await closeFromEngine!(); + } finally { + restoreObservedOutput(previousBeforeExitListeners, false); + process.stdout.write = originalStdoutWrite; + process.stderr.write = originalStderrWrite; + process.argv = originalArgv; + } - expect(stdoutText).toBe(''); - expect(reporter.events[0].payload).toEqual({ - stream: 'stdout', - text: 'legacy stdout\n' - }); - }); + expect(stdoutText).toBe(''); + expect(reporter.events[0].payload).toEqual({ + stream: 'stdout', + text: 'legacy stdout\n' + }); + } + ); it('preserves a UTF-8 code point split across old-engine buffer writes', async () => { const manager: ReporterManager = new ReporterManager(); diff --git a/apps/rush/src/test/RushFrontend.test.ts b/apps/rush/src/test/RushFrontend.test.ts index 910927efe0..22d9e5228d 100644 --- a/apps/rush/src/test/RushFrontend.test.ts +++ b/apps/rush/src/test/RushFrontend.test.ts @@ -187,6 +187,41 @@ function emitCommandStarted(sink: IReporterEventSink): void { } describe(launchRushFrontendAsync.name, () => { + it.each([ + ['file', false, true], + ['file', true, false], + ['json', false, true], + ['ai', false, true], + ['plaintext', false, false] + ] as const)('reserves stdout for %s with command JSON %s: %s', async (reporter, commandJson, reserved) => { + const initialized: IInitializedRushReporterHost = await createEnabledHostAsync(); + const originalArgv: string[] = process.argv; + process.argv = ['node', 'rush', 'list']; + try { + await launchRushFrontendAsync({ + currentPackageVersion: '5.178.1', + rushVersionToLoad: undefined, + configuration: undefined, + launchOptions: { isManaged: false }, + currentRushLib: rushLib, + initializeReporterHostAsync: async () => ({ + ...initialized, + selection: { ...initialized.selection, reporter, commandJson } + }), + executeCurrentRush: (version, selectedRushLib, options) => { + void version; + void selectedRushLib; + expect(options.reporterStdoutIsReserved).toBe(reserved); + return options.reporterCloseAsync(); + }, + processLifecycle: createTestProcessLifecycle() + }); + } finally { + await initialized.closeAsync(); + process.argv = originalArgv; + } + }); + it('creates the authoritative host before invoking the bundled rush-lib and passes only its channel', async () => { const order: string[] = []; let receivedOptions: IRushFrontendLaunchOptions | undefined; diff --git a/apps/rush/src/test/sandbox/reporter-demo/README.md b/apps/rush/src/test/sandbox/reporter-demo/README.md index 72c415bc11..e61f310ca4 100644 --- a/apps/rush/src/test/sandbox/reporter-demo/README.md +++ b/apps/rush/src/test/sandbox/reporter-demo/README.md @@ -17,7 +17,9 @@ plaintext, JSON, AI, file, and quiet modes, plus parser failure, help, and comma It verifies payload-only machine stdout, one visible writer, ordered/lossless plaintext grouping from a same-invocation JSON sidecar, final artifact completeness, owner-only log permissions, failure flushing, AI parser-error context, command-JSON ownership, exclusive sidecar destinations, and the -`RUSH_REPORTER=legacy` rollback transcript. Inherited `RUSH_REPORTER`, `RUSH_LOG_LEVEL`, and +`RUSH_REPORTER=legacy` rollback transcript. Non-phased `rush list` output must also remain structured in +JSON mode and reach the full-detail log without leaking onto stdout in file mode. +Inherited `RUSH_REPORTER`, `RUSH_LOG_LEVEL`, and `RUSH_QUIET_MODE` values are removed from the self-check matrix. The matrix sets `RUSH_PREVIEW_VERSION` to the locally built Rush package version so it exercises the integrated frontend and engine even when `rush.json` pins an older release; preview warnings remain on stderr. It also verifies CI plaintext output, diff --git a/apps/rush/src/test/sandbox/reporter-demo/run.mjs b/apps/rush/src/test/sandbox/reporter-demo/run.mjs index dcf5094662..33e58c661f 100644 --- a/apps/rush/src/test/sandbox/reporter-demo/run.mjs +++ b/apps/rush/src/test/sandbox/reporter-demo/run.mjs @@ -73,6 +73,8 @@ const flagOffHelp = run('help-flag-off', ['--help']).stdout; const help = run('help', ['--help', '--reporter=json'], { RUSH_REPORTER: 'legacy' }).stdout; const commandJson = run('command-json', ['list', '--json', '--reporter=file']); const commandJsonConflict = run('command-json-conflict', ['list', '--json', '--reporter=json'], {}, 1); +const listReporterJson = run('list-reporter-json', ['list', '--reporter=json', '--log-level=debug']); +const listReporterFile = run('list-reporter-file', ['list', '--reporter=file']); const heftChild = run('heft-child', [ 'rebuild', '--only', @@ -105,13 +107,16 @@ if ( throw new Error('RUSH_TEMP_FOLDER did not own the full-detail log path.'); } const tempPurge = run('temp-purge', ['purge', '--reporter=file'], { RUSH_TEMP_FOLDER: tempOverride }); -if (!tempPurge.stdout.includes(`Purging ${tempOverride}`)) { - throw new Error('rush purge did not use the same normalized RUSH_TEMP_FOLDER path as the reporter log.'); -} const purgeLogMatch = tempPurge.stderr.match(/^Rush full log: (.+)$/m); if (!purgeLogMatch || purgeLogMatch[1].startsWith(tempOverride) || !fs.existsSync(purgeLogMatch[1])) { throw new Error('The active purge reporter log was not preserved outside RUSH_TEMP_FOLDER.'); } +if ( + tempPurge.stdout !== '' || + !fs.readFileSync(purgeLogMatch[1], 'utf8').includes(`Purging ${tempOverride}`) +) { + throw new Error('File-mode purge must log the normalized RUSH_TEMP_FOLDER without writing to stdout.'); +} function parseNdjson(text, name) { if (text.includes('\u001b')) { @@ -129,6 +134,19 @@ const aiRecords = parseNdjson(ai, 'ai'); const failureJsonEvents = parseNdjson(failureJson, 'failure-json'); const failureAiRecords = parseNdjson(failureAi, 'failure-ai'); const plaintextEvents = parseNdjson(fs.readFileSync(plaintextEventsPath, 'utf8'), 'plaintext sidecar'); +const listingOutput = parseNdjson(listReporterJson.stdout, 'list reporter JSON') + .filter((event) => event.type === 'externalOutput') + .map((event) => event.payload.text) + .join(''); +const listingLogMatch = listReporterFile.stderr.match(/^Rush full log: (.+)$/m); +if ( + !listingOutput.includes('@rushstack/rush-reporter') || + listReporterFile.stdout !== '' || + !listingLogMatch || + !fs.readFileSync(listingLogMatch[1], 'utf8').includes('@rushstack/rush-reporter') +) { + throw new Error('Non-phased command output bypassed reporter stdout ownership or its full-detail log.'); +} for (const [name, events] of [ ['json', jsonEvents], diff --git a/common/changes/@microsoft/rush/reporter-dag-review-fixes_2026-09-07.json b/common/changes/@microsoft/rush/reporter-dag-review-fixes_2026-09-07.json new file mode 100644 index 0000000000..46d281e70a --- /dev/null +++ b/common/changes/@microsoft/rush/reporter-dag-review-fixes_2026-09-07.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@microsoft/rush", + "comment": "Preserve custom bootstrap controls and legacy environment overrides, keep direct command output within reporter-owned destinations, and suppress duplicate watch presentation.", + "type": "patch" + } + ], + "packageName": "@microsoft/rush", + "email": "223556219+Copilot@users.noreply.github.com" +} diff --git a/common/changes/@rushstack/heft/reporter-windows-pipes_2026-09-07.json b/common/changes/@rushstack/heft/reporter-windows-pipes_2026-09-07.json new file mode 100644 index 0000000000..2a0379cc6d --- /dev/null +++ b/common/changes/@rushstack/heft/reporter-windows-pipes_2026-09-07.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@rushstack/heft", + "comment": "Recognize inherited reporter pipes on Windows while continuing to reject regular-file descriptors.", + "type": "patch" + } + ], + "packageName": "@rushstack/heft", + "email": "223556219+Copilot@users.noreply.github.com" +} diff --git a/common/changes/@rushstack/rush-daemon/reporter-operation-forwarding_2026-09-07.json b/common/changes/@rushstack/rush-daemon/reporter-operation-forwarding_2026-09-07.json new file mode 100644 index 0000000000..24fd77d614 --- /dev/null +++ b/common/changes/@rushstack/rush-daemon/reporter-operation-forwarding_2026-09-07.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@rushstack/rush-daemon", + "comment": "Forward reporter operation completion and iteration identity through phased request event multiplexing.", + "type": "patch" + } + ], + "packageName": "@rushstack/rush-daemon", + "email": "223556219+Copilot@users.noreply.github.com" +} diff --git a/common/changes/@rushstack/rush-reporter/reporter-dag-review-fixes_2026-09-07.json b/common/changes/@rushstack/rush-reporter/reporter-dag-review-fixes_2026-09-07.json new file mode 100644 index 0000000000..e23eda9da3 --- /dev/null +++ b/common/changes/@rushstack/rush-reporter/reporter-dag-review-fixes_2026-09-07.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@rushstack/rush-reporter", + "comment": "Refresh quiet interactive operations and emit plaintext heartbeats using unrefed timers that stop when reporters close.", + "type": "patch" + } + ], + "packageName": "@rushstack/rush-reporter", + "email": "223556219+Copilot@users.noreply.github.com" +} diff --git a/common/reviews/api/rush-lib.api.md b/common/reviews/api/rush-lib.api.md index c5269b50b8..728b23c954 100644 --- a/common/reviews/api/rush-lib.api.md +++ b/common/reviews/api/rush-lib.api.md @@ -277,6 +277,8 @@ export class EnvironmentConfiguration { export const EnvironmentVariableNames: { readonly RUSH_TEMP_FOLDER: "RUSH_TEMP_FOLDER"; readonly RUSH_PREVIEW_VERSION: "RUSH_PREVIEW_VERSION"; + readonly RUSH_REPORTER: "RUSH_REPORTER"; + readonly RUSH_LOG_LEVEL: "RUSH_LOG_LEVEL"; readonly RUSH_ALLOW_UNSUPPORTED_NODEJS: "RUSH_ALLOW_UNSUPPORTED_NODEJS"; readonly RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD: "RUSH_ALLOW_WARNINGS_IN_SUCCESSFUL_BUILD"; readonly RUSH_VARIANT: "RUSH_VARIANT"; diff --git a/libraries/reporter/src/reporters/DefaultInteractiveReporter.ts b/libraries/reporter/src/reporters/DefaultInteractiveReporter.ts index 613269b9e6..eefc9e7576 100644 --- a/libraries/reporter/src/reporters/DefaultInteractiveReporter.ts +++ b/libraries/reporter/src/reporters/DefaultInteractiveReporter.ts @@ -127,6 +127,7 @@ export class DefaultInteractiveReporter implements IReporter { private _paintedRowCount: number; private _cursorHidden: boolean; private _finalized: boolean; + private _refreshTimer: ReturnType | undefined; public constructor(options: IDefaultInteractiveReporterOptions) { this._terminal = options.terminal; @@ -153,7 +154,22 @@ export class DefaultInteractiveReporter implements IReporter { } public async initializeAsync(): Promise { - /* The cursor is hidden lazily on the first paint. */ + if (!this._refreshTimer && this._terminal.isTTY && !this._finalized) { + this._refreshTimer = setInterval( + () => { + if ( + this._terminal.isTTY && + this._cursorHidden && + !this._finalized && + shouldRefresh(this._lastPaintMs, this._nowMs(), this._minRefreshIntervalMs) + ) { + this._paint(); + } + }, + Math.max(MIN_REFRESH_INTERVAL_MS, this._minRefreshIntervalMs) + ); + this._refreshTimer.unref(); + } } public report(event: IReporterEventEnvelope): void { @@ -174,6 +190,10 @@ export class DefaultInteractiveReporter implements IReporter { } public async closeAsync(): Promise { + if (this._refreshTimer) { + clearInterval(this._refreshTimer); + this._refreshTimer = undefined; + } this._finalize(); } diff --git a/libraries/reporter/src/reporters/PlaintextReporter.ts b/libraries/reporter/src/reporters/PlaintextReporter.ts index 393c253a72..585414fd3d 100644 --- a/libraries/reporter/src/reporters/PlaintextReporter.ts +++ b/libraries/reporter/src/reporters/PlaintextReporter.ts @@ -117,6 +117,7 @@ export class PlaintextReporter implements IReporter { private _nextSpoolId: number; private _legacyIterationId: number; private _latestIterationId: number; + private _heartbeatTimer: ReturnType | undefined; public constructor(options: IPlaintextReporterOptions) { this._write = options.write; @@ -138,7 +139,17 @@ export class PlaintextReporter implements IReporter { } public async initializeAsync(): Promise { - /* no-op */ + if (!this._heartbeatTimer && this._logLevel !== 'quiet') { + this._heartbeatTimer = setInterval( + () => { + if (this._commandName !== undefined) { + this.emitHeartbeatIfDue(); + } + }, + Math.max(1, this._heartbeatIntervalMs) + ); + this._heartbeatTimer.unref(); + } } public report(event: IReporterEventEnvelope): void { @@ -245,6 +256,7 @@ export class PlaintextReporter implements IReporter { break; } case 'commandResult': { + this._stopHeartbeat(); this._onResult(event.payload as { commandName: string; succeeded: boolean; exitCode: number }); break; } @@ -258,6 +270,7 @@ export class PlaintextReporter implements IReporter { } public async closeAsync(): Promise { + this._stopHeartbeat(); for (const cycle of this._watchCycles.values()) { for (const [operationId, record] of cycle.operations) { if (!record.silent && this._variant === 'detailed') { @@ -291,6 +304,13 @@ export class PlaintextReporter implements IReporter { return false; } + private _stopHeartbeat(): void { + if (this._heartbeatTimer) { + clearInterval(this._heartbeatTimer); + this._heartbeatTimer = undefined; + } + } + private _onOperationCompleted(event: IReporterEventEnvelope): void { const payload: { operationId: string; status: string; iterationId?: number } = event.payload as { operationId: string; diff --git a/libraries/reporter/src/test/DefaultInteractiveReporter.test.ts b/libraries/reporter/src/test/DefaultInteractiveReporter.test.ts index e7e2f5367e..a464913f9f 100644 --- a/libraries/reporter/src/test/DefaultInteractiveReporter.test.ts +++ b/libraries/reporter/src/test/DefaultInteractiveReporter.test.ts @@ -97,6 +97,33 @@ describe('interactive rendering helpers', () => { }); describe('DefaultInteractiveReporter', () => { + it('repaints quiet operations at most ten times per second and clears its unrefed timer', async () => { + jest.useFakeTimers(); + const intervalSpy = jest.spyOn(global, 'setInterval'); + const terminal: FakeTerminal = new FakeTerminal(); + const reporter: DefaultInteractiveReporter = new DefaultInteractiveReporter({ terminal, color: false }); + try { + await reporter.initializeAsync(); + expect(intervalSpy.mock.results[0].value.hasRef()).toBe(false); + jest.advanceTimersByTime(1000); + expect(terminal.output).toBe(''); + reporter.report(ev('commandStarted', { commandName: 'quiet-build' })); + terminal.output = ''; + jest.advanceTimersByTime(1000); + expect(terminal.output.split('quiet-build').length - 1).toBe(10); + + await reporter.closeAsync(); + const afterClose: string = terminal.output; + jest.advanceTimersByTime(1000); + expect(terminal.output).toBe(afterClose); + expect(jest.getTimerCount()).toBe(0); + } finally { + await reporter.closeAsync(); + intervalSpy.mockRestore(); + jest.useRealTimers(); + } + }); + it('honors NO_COLOR and FORCE_COLOR when color is not explicit', async () => { const noColorTerminal: FakeTerminal = new FakeTerminal(); const noColorReporter: DefaultInteractiveReporter = new DefaultInteractiveReporter({ diff --git a/libraries/reporter/src/test/PlaintextReporter.test.ts b/libraries/reporter/src/test/PlaintextReporter.test.ts index a5c2f2056f..200003f150 100644 --- a/libraries/reporter/src/test/PlaintextReporter.test.ts +++ b/libraries/reporter/src/test/PlaintextReporter.test.ts @@ -278,4 +278,64 @@ describe('PlaintextReporter', () => { expect(output).toContain('still running'); }); + + it.each(['normal', 'quiet'] as const)( + 'schedules unrefed heartbeats at %s level and stops on close', + async (logLevel) => { + jest.useFakeTimers(); + const intervalSpy = jest.spyOn(global, 'setInterval'); + let output: string = ''; + const reporter: PlaintextReporter = new PlaintextReporter({ + write: (text: string) => { + output += text; + }, + logLevel + }); + try { + await reporter.initializeAsync(); + if (logLevel === 'normal') { + expect(intervalSpy.mock.results[0].value.hasRef()).toBe(false); + } else { + expect(intervalSpy).not.toHaveBeenCalled(); + } + reporter.report(ev('commandStarted', { commandName: 'build' })); + jest.advanceTimersByTime(29999); + expect(output).not.toContain('still running'); + jest.advanceTimersByTime(1); + expect(output.includes('still running')).toBe(logLevel !== 'quiet'); + + await reporter.closeAsync(); + const afterClose: string = output; + jest.advanceTimersByTime(60000); + expect(output).toBe(afterClose); + expect(jest.getTimerCount()).toBe(0); + } finally { + await reporter.closeAsync(); + intervalSpy.mockRestore(); + jest.useRealTimers(); + } + } + ); + + it('stops automatic heartbeats when the command result arrives', async () => { + jest.useFakeTimers(); + let output: string = ''; + const reporter: PlaintextReporter = new PlaintextReporter({ + write: (text: string) => { + output += text; + } + }); + try { + await reporter.initializeAsync(); + reporter.report(ev('commandStarted', { commandName: 'build' })); + reporter.report(ev('commandResult', { commandName: 'build', succeeded: true, exitCode: 0 })); + const finalOutput: string = output; + jest.advanceTimersByTime(60000); + expect(output).toBe(finalOutput); + expect(jest.getTimerCount()).toBe(0); + } finally { + await reporter.closeAsync(); + jest.useRealTimers(); + } + }); }); diff --git a/libraries/rush-lib/src/api/EnvironmentConfiguration.ts b/libraries/rush-lib/src/api/EnvironmentConfiguration.ts index e2b461d199..bc52ac769f 100644 --- a/libraries/rush-lib/src/api/EnvironmentConfiguration.ts +++ b/libraries/rush-lib/src/api/EnvironmentConfiguration.ts @@ -37,6 +37,17 @@ export const EnvironmentVariableNames = { */ RUSH_PREVIEW_VERSION: 'RUSH_PREVIEW_VERSION', + /** + * Frontend-owned reporter selection, including the legacy emergency override. + * Engines recognize this variable without consuming its value. + */ + RUSH_REPORTER: 'RUSH_REPORTER', + + /** + * Frontend-owned reporter verbosity. Engines recognize this variable without consuming its value. + */ + RUSH_LOG_LEVEL: 'RUSH_LOG_LEVEL', + /** * If this variable is set to "1", Rush will not fail the build when running a version * of Node that does not match the criteria specified in the "nodeSupportedVersionRange" @@ -653,6 +664,8 @@ export class EnvironmentConfiguration { case EnvironmentVariableNames.RUSH_PARALLELISM: case EnvironmentVariableNames.RUSH_PREVIEW_VERSION: + case EnvironmentVariableNames.RUSH_REPORTER: + case EnvironmentVariableNames.RUSH_LOG_LEVEL: case EnvironmentVariableNames.RUSH_VARIANT: case EnvironmentVariableNames.RUSH_DEPLOY_TARGET_FOLDER: // Handled by @microsoft/rush front end diff --git a/libraries/rush-lib/src/api/test/EnvironmentConfiguration.test.ts b/libraries/rush-lib/src/api/test/EnvironmentConfiguration.test.ts index 78f597c2a3..0dc982bb2e 100644 --- a/libraries/rush-lib/src/api/test/EnvironmentConfiguration.test.ts +++ b/libraries/rush-lib/src/api/test/EnvironmentConfiguration.test.ts @@ -27,6 +27,16 @@ describe(EnvironmentConfiguration.name, () => { expect(EnvironmentConfiguration.validate).not.toThrow(); }); + it.each([ + { RUSH_REPORTER: 'legacy', RUSH_LOG_LEVEL: 'debug' }, + { RUSH_REPORTER: 'json', RUSH_LOG_LEVEL: 'frontend-owned-value' } + ])('recognizes frontend-owned reporter controls without interpreting them: %p', (env) => { + Object.assign(process.env, env); + expect(EnvironmentConfiguration.validate).not.toThrow(); + expect(process.env.RUSH_REPORTER).toBe(env.RUSH_REPORTER); + expect(process.env.RUSH_LOG_LEVEL).toBe(env.RUSH_LOG_LEVEL); + }); + it('does not allow unknown environment variables', () => { process.env['rush_foobar'] = 'asdf'; // eslint-disable-line dot-notation expect(EnvironmentConfiguration.validate).toThrow(); diff --git a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts index a8ccf98304..39f3c257c5 100644 --- a/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts +++ b/libraries/rush-lib/src/cli/scriptActions/PhasedScriptAction.ts @@ -725,7 +725,7 @@ export class PhasedScriptAction extends BaseScriptAction i rushConfiguration: this.rushConfiguration, graph, initialSnapshot, - terminal, + terminal: presentationTerminal, debounceMs: this._watchDebounceMs, renderStatusInPlace: !_isRushSessionOperationStreamEnabled(this.rushSession) }); diff --git a/libraries/rush-lib/src/scripts/InstallRunRushBootstrap.ts b/libraries/rush-lib/src/scripts/InstallRunRushBootstrap.ts index c1dc3eb09e..6bb8740f96 100644 --- a/libraries/rush-lib/src/scripts/InstallRunRushBootstrap.ts +++ b/libraries/rush-lib/src/scripts/InstallRunRushBootstrap.ts @@ -80,8 +80,8 @@ export interface IInstallRunRushBootstrap { readonly prepareToRun: (() => void) | undefined; } -function readSingleFlagValue(argv: readonly string[], flag: string): string | undefined { - let result: string | undefined; +function readFlagValues(argv: readonly string[], flag: string, strict: boolean): string[] { + const result: string[] = []; const prefix: string = `${flag}=`; for (let index: number = 0; index < argv.length; index++) { const argument: string = argv[index]; @@ -94,19 +94,25 @@ function readSingleFlagValue(argv: readonly string[], flag: string): string | un } else if (argument === flag) { value = argv[index + 1]; if (!value || value.startsWith('-')) { - throw new Error(`${flag} requires a value.`); + if (strict) { + throw new Error(`${flag} requires a value.`); + } + continue; } index++; } if (value !== undefined) { if (!value) { - throw new Error(`${flag} requires a value.`); + if (strict) { + throw new Error(`${flag} requires a value.`); + } + continue; } - if (result !== undefined) { + if (strict && result.length > 0) { throw new Error(`${flag} may be specified only once.`); } - result = value; + result.push(value); } } return result; @@ -551,14 +557,32 @@ export function createInstallRunRushBootstrap( return createLegacyBootstrap(options); } - const explicitReporter: string | undefined = readSingleFlagValue(options.argv, '--reporter'); - const explicitLogLevel: string | undefined = readSingleFlagValue(options.argv, '--log-level'); + const repositoryOptIn: boolean = repositoryUsesRushReporter(options.rushJsonFolder); + const reporterControlsOwned: boolean = + repositoryOptIn || + readFlagValues(options.argv, '--reporter', false).some((value: string) => SUPPORTED_REPORTERS.has(value)); + if (!reporterControlsOwned) { + return createLegacyBootstrap(options); + } + + const explicitReporter: string | undefined = readFlagValues(options.argv, '--reporter', true)[0]; if (explicitReporter !== undefined && !SUPPORTED_REPORTERS.has(explicitReporter)) { throw new Error( `Unsupported reporter ${JSON.stringify(explicitReporter)}. ` + 'Supported values are default, ai, json, plaintext, file, and legacy.' ); } + if (explicitReporter === 'legacy') { + return createLegacyBootstrap(options); + } + + const logLevelProbe: string[] = readFlagValues(options.argv, '--log-level', false); + const logLevelOwned: boolean = + explicitReporter !== undefined || + (logLevelProbe.length > 0 && logLevelProbe.every((value: string) => SUPPORTED_LOG_LEVELS.has(value))); + const explicitLogLevel: string | undefined = logLevelOwned + ? readFlagValues(options.argv, '--log-level', true)[0] + : undefined; if (explicitLogLevel !== undefined && !SUPPORTED_LOG_LEVELS.has(explicitLogLevel)) { throw new Error( `Unsupported log level ${JSON.stringify(explicitLogLevel)}. ` + @@ -566,15 +590,7 @@ export function createInstallRunRushBootstrap( ); } - if (explicitReporter === 'legacy') { - return createLegacyBootstrap(options); - } - - const repositoryOptIn: boolean = repositoryUsesRushReporter(options.rushJsonFolder); const explicitOptIn: boolean = explicitReporter !== undefined; - if (!explicitOptIn && !repositoryOptIn) { - return createLegacyBootstrap(options); - } if (!supportsBootstrapHandoff(options.rushVersion, options.bootstrapVersion)) { if (explicitOptIn) { @@ -587,5 +603,13 @@ export function createInstallRunRushBootstrap( return createLegacyBootstrap(options); } - return new InstallRunRushBootstrap(options, explicitReporter !== 'json' && explicitReporter !== 'ai'); + const separatorIndex: number = options.argv.indexOf('--'); + const commandArgs: readonly string[] = + separatorIndex < 0 ? options.argv : options.argv.slice(0, separatorIndex); + const stdoutReserved: boolean = + explicitReporter === 'json' || + explicitReporter === 'ai' || + explicitReporter === 'file' || + commandArgs.includes('--json'); + return new InstallRunRushBootstrap(options, !stdoutReserved); } diff --git a/libraries/rush-lib/src/scripts/test/InstallRunRushBootstrap.test.ts b/libraries/rush-lib/src/scripts/test/InstallRunRushBootstrap.test.ts index 3dfa051b0a..3209d7e110 100644 --- a/libraries/rush-lib/src/scripts/test/InstallRunRushBootstrap.test.ts +++ b/libraries/rush-lib/src/scripts/test/InstallRunRushBootstrap.test.ts @@ -81,6 +81,79 @@ function readHandoff(env: Record): { } describe(createInstallRunRushBootstrap.name, () => { + it.each([ + { argv: ['custom', '--reporter'] }, + { argv: ['custom', '--reporter='] }, + { argv: ['custom', '--reporter=custom'] }, + { argv: ['custom', '--reporter', '--log-level=custom'] }, + { argv: ['custom', '--log-level'] }, + { argv: ['custom', '--log-level=custom'] }, + { argv: ['custom', '--reporter=one', '--reporter=two'] }, + { argv: ['custom', '--reporter', '--', '--reporter=json'] }, + { argv: ['custom', '--reporter=legacy', '--log-level'] } + ])('preserves command-owned reporter controls without opt-in: $argv', async ({ argv }) => { + await withTempDir(async (directory: string) => { + const { options, env } = makeOptions(directory, { argv }); + expect(createInstallRunRushBootstrap(options).enabled).toBe(false); + expect(env[RUSH_REPORTER_BOOTSTRAP_HANDOFF_ENV_VAR]).toBeUndefined(); + }); + }); + + it.each([ + { argv: ['build', '--reporter=json', '--reporter=custom'] }, + { argv: ['build', '--reporter=custom', '--reporter=json'] }, + { argv: ['build', '--reporter=json', '--log-level'] }, + { argv: ['build', '--reporter=json', '--log-level=custom'] } + ])('validates controls once reporter ownership is established: $argv', async ({ argv }) => { + await withTempDir(async (directory: string) => { + expect(() => createInstallRunRushBootstrap(makeOptions(directory, { argv }).options)).toThrow(); + }); + }); + + it.each([{ argv: ['custom', '--log-level'] }, { argv: ['custom', '--log-level=custom'] }])( + 'preserves command-owned log-level flags under repository opt-in: $argv', + async ({ argv }) => { + await withTempDir(async (directory: string) => { + const configFolder: string = path.join(directory, 'common', 'config', 'rush'); + await fs.promises.mkdir(configFolder, { recursive: true }); + await fs.promises.writeFile(path.join(configFolder, 'experiments.json'), '{"useRushReporter":true}'); + expect(createInstallRunRushBootstrap(makeOptions(directory, { argv }).options).enabled).toBe(true); + }); + } + ); + + it.each([ + { argv: ['build', '--reporter=file'], repositoryOptIn: false }, + { argv: ['list', '--json', '--reporter=file'], repositoryOptIn: false }, + { argv: ['list', '--json'], repositoryOptIn: true } + ])('keeps bootstrap output out of exclusive stdout: $argv', async ({ argv, repositoryOptIn }) => { + await withTempDir(async (directory: string) => { + if (repositoryOptIn) { + const configFolder: string = path.join(directory, 'common', 'config', 'rush'); + await fs.promises.mkdir(configFolder, { recursive: true }); + await fs.promises.writeFile(path.join(configFolder, 'experiments.json'), '{"useRushReporter":true}'); + } + const { options, env, stdout, stderr } = makeOptions(directory, { argv }); + const bootstrap: IInstallRunRushBootstrap = createInstallRunRushBootstrap(options); + bootstrap.logger.info('installing Rush'); + bootstrap.externalOutputHandler?.('stdout', 'npm stdout\n', false); + bootstrap.prepareToRun?.(); + + expect(bootstrap.externalOutputLiveStreams).toEqual({ stdout: false, stderr: true }); + expect(readHandoff(env).records).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + type: 'externalOutput', + payload: { stream: 'stdout', text: 'npm stdout\n' } + }) + ]) + ); + bootstrap.logger.error('bootstrap failed'); + expect(stdout).toEqual([]); + expect(stderr.join('')).toBe('installing Rush\nnpm stdout\nbootstrap failed\n'); + }); + }); + it('preserves direct legacy bootstrap output without an opt-in', async () => { await withTempDir(async (directory: string) => { const { options, env, stdout } = makeOptions(directory); @@ -262,6 +335,13 @@ describe(createInstallRunRushBootstrap.name, () => { }).options ).enabled ).toBe(true); + expect( + createInstallRunRushBootstrap( + makeOptions(directory, { + argv: ['build', '--reporter=plaintext', '--', '--json'] + }).options + ).externalOutputLiveStreams + ).toEqual({ stdout: true, stderr: true }); }); }); @@ -319,6 +399,9 @@ describe(createInstallRunRushBootstrap.name, () => { it('fails unsupported explicit requests and explicit requests for an old frontend', async () => { await withTempDir(async (directory: string) => { + const configFolder: string = path.join(directory, 'common', 'config', 'rush'); + await fs.promises.mkdir(configFolder, { recursive: true }); + await fs.promises.writeFile(path.join(configFolder, 'experiments.json'), '{"useRushReporter":true}'); expect(() => createInstallRunRushBootstrap( makeOptions(directory, { argv: ['build', '--reporter=unknown'] }).options @@ -388,10 +471,10 @@ describe(createInstallRunRushBootstrap.name, () => { }); }); - it('keeps machine-reporter failure fallback off stdout', async () => { + it.each(['json', 'ai', 'file'])('keeps %s reporter failure fallback off stdout', async (reporter) => { await withTempDir(async (directory: string) => { const { options, stdout, stderr } = makeOptions(directory, { - argv: ['build', '--reporter=json'] + argv: ['build', `--reporter=${reporter}`] }); const bootstrap: IInstallRunRushBootstrap = createInstallRunRushBootstrap(options); bootstrap.logger.info('installing Rush');