From 061d2e8e00b5d6dbd296b16f14d25efe8c3d5905 Mon Sep 17 00:00:00 2001 From: selarkin Date: Thu, 24 Sep 2026 14:34:15 -0700 Subject: [PATCH] [rush-daemon] Fix summary writer racing early coalesced results The early per-client result path (#6092) runs from onOperationCompleted, which the record's finalizeOperation() invokes synchronously before closing its StdioSummarizer. The summary writer (#6068) reads the failure tail from that summarizer, so it threw. Yield once before producing the early result. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...ix-main-summary-race_2026-09-24-21-45.json | 11 ++++ .../rush-daemon/src/PhasedRequestRouter.ts | 22 +++++--- .../src/test/PhasedRequestSummary.test.ts | 51 +++++++++++++++++++ 3 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 common/changes/@rushstack/rush-daemon/fix-main-summary-race_2026-09-24-21-45.json diff --git a/common/changes/@rushstack/rush-daemon/fix-main-summary-race_2026-09-24-21-45.json b/common/changes/@rushstack/rush-daemon/fix-main-summary-race_2026-09-24-21-45.json new file mode 100644 index 0000000000..8c97b33d05 --- /dev/null +++ b/common/changes/@rushstack/rush-daemon/fix-main-summary-race_2026-09-24-21-45.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@rushstack/rush-daemon", + "comment": "Fix an early coalesced phased result writing its summary before the failed operation's output summarizer was closed.", + "type": "patch" + } + ], + "packageName": "@rushstack/rush-daemon", + "email": "TheLarkInn@users.noreply.github.com" +} diff --git a/libraries/rush-daemon/src/PhasedRequestRouter.ts b/libraries/rush-daemon/src/PhasedRequestRouter.ts index 1a6e9bd0a7..c943eee794 100644 --- a/libraries/rush-daemon/src/PhasedRequestRouter.ts +++ b/libraries/rush-daemon/src/PhasedRequestRouter.ts @@ -572,15 +572,21 @@ class PhasedRequestBatchCoordinator { } entry.unsubscribe?.(); entry.unsubscribe = undefined; - entry.finishPromise = this.#produceResultAsync(entry, true, undefined, [], undefined, true).catch( - (error: unknown) => { - // Unlike a batch-wide failure, an early result's failure concerns only this client. - if (!entry.completed) { - this.#completeEntry(entry); - entry.reject(error); - } + entry.finishPromise = this.#produceEarlyResultAsync(entry).catch((error: unknown) => { + // Unlike a batch-wide failure, an early result's failure concerns only this client. + if (!entry.completed) { + this.#completeEntry(entry); + entry.reject(error); } - ); + }); + } + + async #produceEarlyResultAsync(entry: IBatchEntry): Promise { + // The sink is notified from the record's `finalizeOperation()`, which synchronously precedes the close of + // the record's StdioSummarizer and ProblemCollector. The summary reads the failure tail from the closed + // summarizer, so yield once to let the notifying record finish closing before the summary is written. + await Promise.resolve(); + await this.#produceResultAsync(entry, true, undefined, [], undefined, true); } #requestIterationAbort(): void { diff --git a/libraries/rush-daemon/src/test/PhasedRequestSummary.test.ts b/libraries/rush-daemon/src/test/PhasedRequestSummary.test.ts index a2f05f65af..5c3063d1ba 100644 --- a/libraries/rush-daemon/src/test/PhasedRequestSummary.test.ts +++ b/libraries/rush-daemon/src/test/PhasedRequestSummary.test.ts @@ -156,6 +156,57 @@ describe('phased request summary', () => { } }); + it('writes the failure summary before an early result while the coalesced batch continues', async () => { + let releaseC: () => void = () => undefined; + const cHeld: Promise = new Promise((resolve) => { + releaseC = resolve; + }); + const fixture: ITestRoutingFixture = createRoutingFixture( + new Map([ + [ + OPERATION_A, + new TestOperationRunner(OPERATION_A, OperationStatus.Failure, async (terminal) => + terminal.writeErrorLine('a-failure-detail') + ) + ], + [OPERATION_B, new TestOperationRunner(OPERATION_B)], + [OPERATION_C, new TestOperationRunner(OPERATION_C, OperationStatus.Success, () => cHeld)] + ]), + [[OPERATION_B, OPERATION_A]] + ); + fixture.graph.parallelism = 2; + try { + const router: PhasedRequestRouter = new PhasedRequestRouter(fixture.session); + const clientA: TestPhasedRequestClient = new TestPhasedRequestClient('one'); + const clientC: TestPhasedRequestClient = new TestPhasedRequestClient('two'); + const resultAPromise = router.executeAsync(createRequest('a', OPERATION_A), clientA); + const resultCPromise = router.executeAsync(createRequest('c', OPERATION_C), clientC); + + const resultA = await resultAPromise; + // The early result is published while the shared iteration still runs the other client's selection. + expect(fixture.graph.status).toBe(OperationStatus.Executing); + expect(resultA).toMatchObject({ exitCode: 1, outcome: 'failure' }); + const stdoutA: string = getActivity(clientA, 'stdout'); + const summaryA: string = getSummary(stdoutA); + expect(summaryA).toContain('==[ FAILURE: 1 operation ]=='); + expect(summaryA).toContain(`--[ FAILURE: ${OPERATION_A} ]--`); + expect(summaryA).toContain('a-failure-detail'); + expect(summaryA).not.toContain(OPERATION_C); + expect(stdoutA).toMatch(DURATION_LINE); + expect(getActivity(clientA, 'stderr')).toContain('Operations failed.'); + expect(clientA.writes[clientA.writes.length - 1].result).toBe(resultA); + + releaseC(); + const resultC = await resultCPromise; + expect(resultC).toMatchObject({ exitCode: 0, outcome: 'success' }); + const summaryC: string = getSummary(getActivity(clientC, 'stdout')); + expect(summaryC).toContain('==[ SUCCESS: 1 operation ]=='); + expect(summaryC).not.toContain(OPERATION_A); + } finally { + await fixture.session[Symbol.asyncDispose](); + } + }); + it('gives each coalesced request a summary of only its own selection', async () => { const fixture: ITestRoutingFixture = createFixture(); try {