[rush reporter] Fix output ownership and idle rendering (stacked follow-up)

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');
     }
   );

apps/heft/src/pluginFramework/logging/HeftChildReporter.ts

@@ -46,7 +46,9 @@ function readDescriptorFd(env: Record<string, string | undefined>, 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;
   }

apps/rush/src/IRushFrontendLaunchOptions.ts

@@ -16,6 +16,7 @@ export interface IRushFrontendLaunchOptions extends ILaunchOptions {
   readonly reporterCloseAsync: () => Promise<void>;
   readonly reporterEnabled: boolean;
   readonly reporterStdoutIsMachineReadable?: boolean;
+  readonly reporterStdoutIsReserved?: boolean;
   readonly reporterSelectionReason:
     | 'explicit --reporter'
     | 'repository experiment'

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 ||

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,

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
   };
 

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');

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<void>) | 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();

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;