Skip to content

Commit e9d24a7

Browse files
mojazayeriCopilot
andcommitted
[rush-daemon] Fix phased request event routing
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 1084cf0 commit e9d24a7

10 files changed

Lines changed: 239 additions & 30 deletions
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
{
2+
"changes": [
3+
{
4+
"packageName": "@rushstack/rush-terminal-renderer",
5+
"comment": "Use authoritative daemon operation-header counters when collating partial warm iterations.",
6+
"type": "patch"
7+
}
8+
],
9+
"packageName": "@rushstack/rush-terminal-renderer",
10+
"email": "mojazayeri@users.noreply.github.com"
11+
}

common/reviews/api/rush-terminal-renderer.api.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import type { DaemonVerbosity } from '@rushstack/rush-daemon-protocol';
88
import type { IDaemonClientCaps } from '@rushstack/rush-daemon-protocol';
99
import type { IDaemonEventEnvelope } from '@rushstack/rush-daemon-protocol';
10+
import type { IDaemonOperationHeaderPayload } from '@rushstack/rush-daemon-protocol';
1011
import { ITerminalChunk } from '@rushstack/terminal';
1112
import { TerminalWritable } from '@rushstack/terminal';
1213

@@ -82,6 +83,7 @@ export class OperationStreamRegistry {
8283
constructor(options: IOperationStreamRegistryOptions);
8384
closeOperation(operationId: string): void;
8485
registerOperation(): void;
86+
setOperationHeader(header: IDaemonOperationHeaderPayload): void;
8587
writeChunk(operationId: string, chunk: ITerminalChunk): void;
8688
}
8789

libraries/rush-daemon/src/PhasedRequestEventSink.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ const EVENT_SOURCE_COMPONENT: string = 'OperationGraph';
3030
const TEXT_ENCODER: InstanceType<typeof TextEncoder> = new TextEncoder();
3131

3232
interface IObservedOperationResult {
33-
readonly errorMessage: string | undefined;
33+
readonly executionResult: IOperationExecutionResult;
3434
readonly status: string;
3535
}
3636

@@ -129,7 +129,7 @@ export class PhasedRequestEventSink implements _IOperationGraphEventSink {
129129
return;
130130
}
131131
this.#observedResults.set(result.operation, {
132-
errorMessage: result.error?.message,
132+
executionResult: result,
133133
status: result.status
134134
});
135135
this.#emitEvent('operationStatusChanged', {
@@ -141,10 +141,14 @@ export class PhasedRequestEventSink implements _IOperationGraphEventSink {
141141

142142
public onOperationHeader(operationId: string, completed: number, total: number): void {
143143
if (this.#activeOperationIds.has(operationId)) {
144-
this.#emitEvent('extension', {
145-
data: { completedOperations: completed, operationId, totalOperations: total },
146-
name: RUSHD_OPERATION_HEADER
147-
});
144+
this.#emitEvent(
145+
'extension',
146+
{
147+
data: { completedOperations: completed, operationId, totalOperations: total },
148+
name: RUSHD_OPERATION_HEADER
149+
},
150+
{ required: true }
151+
);
148152
}
149153
}
150154

@@ -172,13 +176,13 @@ export class PhasedRequestEventSink implements _IOperationGraphEventSink {
172176

173177
public onActivity(text: string, options?: _IOperationActivityOptions): void {
174178
const operationId: string | undefined = options?.operationId;
175-
if (!operationId || !this.#activeOperationIds.has(operationId)) {
179+
if (operationId !== undefined && !this.#activeOperationIds.has(operationId)) {
176180
return;
177181
}
178182
this.#emitEvent(
179183
'activityChanged',
180184
{ stream: options?.stderr === true ? 'stderr' : 'stdout', text },
181-
{ required: true, scope: { operationId } }
185+
{ required: true, scope: operationId === undefined ? undefined : { operationId } }
182186
);
183187
}
184188

libraries/rush-daemon/src/PhasedRequestRouter.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ function collectOperationResults(
358358
continue;
359359
}
360360
const errorMessage: string | undefined = observed
361-
? observed.errorMessage
361+
? observed.executionResult.error?.message
362362
: retained?.error?.message;
363363
results.push({ operationId: operation.name, status, errorMessage });
364364
}
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import type { IDaemonEventEnvelope } from '@rushstack/rush-daemon-protocol';
5+
6+
import { PhasedRequestEventSink } from '../PhasedRequestEventSink';
7+
import { TestPhasedRequestClient } from './PhasedRequestRouterTestUtilities';
8+
9+
const ACTIVE_OPERATION: string = 'project-a (_phase:test)';
10+
const OTHER_OPERATION: string = 'project-b (_phase:test)';
11+
12+
function createSink(client: TestPhasedRequestClient): PhasedRequestEventSink {
13+
return new PhasedRequestEventSink({
14+
activeOperationIds: new Set([ACTIVE_OPERATION]),
15+
client,
16+
getNextSequence: () => client.getNextEventSequence(),
17+
onWriteFailure: () => undefined,
18+
rushVersion: '5.178.1'
19+
});
20+
}
21+
22+
it('forwards unscoped and active activity while filtering other operation activity', async () => {
23+
const client: TestPhasedRequestClient = new TestPhasedRequestClient();
24+
const sink: PhasedRequestEventSink = createSink(client);
25+
sink.onActivity('request summary');
26+
sink.onActivity('active detail', { operationId: ACTIVE_OPERATION });
27+
sink.onActivity('other detail', { operationId: OTHER_OPERATION });
28+
29+
await sink.flushAsync();
30+
31+
const activities: IDaemonEventEnvelope[] = client.writes
32+
.map(({ event }) => event)
33+
.filter(
34+
(event: IDaemonEventEnvelope | undefined): event is IDaemonEventEnvelope =>
35+
event?.type === 'activityChanged'
36+
);
37+
expect(activities.map(({ payload }) => payload)).toEqual([
38+
{ stream: 'stdout', text: 'request summary' },
39+
{ stream: 'stdout', text: 'active detail' }
40+
]);
41+
expect(activities.map(({ scope }) => scope)).toEqual([
42+
undefined,
43+
{ operationId: ACTIVE_OPERATION }
44+
]);
45+
expect(activities.every(({ required }) => required)).toBe(true);
46+
});

libraries/rush-daemon/src/test/PhasedRequestRouter.test.ts

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@ import type {
77
IDaemonPhasedOperationSelection,
88
IDaemonPhasedRequest
99
} from '@rushstack/rush-daemon-protocol';
10-
import { RUSHD_OPERATION_STREAM_CLOSED } from '@rushstack/rush-daemon-protocol';
10+
import {
11+
RUSHD_OPERATION_HEADER,
12+
RUSHD_OPERATION_STREAM_CLOSED
13+
} from '@rushstack/rush-daemon-protocol';
1114
import { OperationStatus } from '@microsoft/rush-lib';
1215

1316
import { PhasedRequestRouter } from '../PhasedRequestRouter';
@@ -204,7 +207,7 @@ describe(PhasedRequestRouter.name, () => {
204207
const fixture: ITestRoutingFixture = createThreeOperationFixture({
205208
actionAAsync: async (terminal: ITerminal): Promise<void> => {
206209
terminal.writeLine('stdout-a');
207-
terminal.writeErrorLine('stderr-a');
210+
terminal.writeErrorLine('stderr-a', { doNotOverrideSgrCodes: true });
208211
}
209212
});
210213
const client: TestPhasedRequestClient = new TestPhasedRequestClient();
@@ -528,4 +531,40 @@ describe(PhasedRequestRouter.name, () => {
528531
expect(firstSequences.length).toBeGreaterThan(0);
529532
expect(secondSequences[0]).toBeGreaterThan(firstSequences[firstSequences.length - 1]);
530533
});
534+
535+
it('uses authoritative header totals after a partial warm iteration', async () => {
536+
const fixture: ITestRoutingFixture = createThreeOperationFixture();
537+
let iteration: number = 0;
538+
fixture.graph.hooks.configureIteration.tap('partial warm iteration', (records, previousResults) => {
539+
if (iteration++ === 0) {
540+
return;
541+
}
542+
for (const record of records.values()) {
543+
if (record.operation.name === OPERATION_A && previousResults.has(record.operation)) {
544+
record.enabled = false;
545+
}
546+
}
547+
});
548+
const router: PhasedRequestRouter = new PhasedRequestRouter(fixture.session);
549+
await router.executeAsync(createRequest([select(OPERATION_B)]), new TestPhasedRequestClient());
550+
const client: TestPhasedRequestClient = new TestPhasedRequestClient();
551+
552+
await router.executeAsync(
553+
{ ...createRequest([select(OPERATION_B)]), requestId: 'request-2' },
554+
client
555+
);
556+
557+
const headers: IDaemonEventEnvelope[] = client.writes
558+
.map(({ event }) => event)
559+
.filter(
560+
(event: IDaemonEventEnvelope | undefined): event is IDaemonEventEnvelope =>
561+
(event?.payload as { name?: unknown } | undefined)?.name === RUSHD_OPERATION_HEADER
562+
);
563+
expect(headers).toHaveLength(1);
564+
expect(headers[0]?.required).toBe(true);
565+
expect(headers[0]?.payload).toEqual({
566+
data: { completedOperations: 1, operationId: OPERATION_B, totalOperations: 1 },
567+
name: RUSHD_OPERATION_HEADER
568+
});
569+
});
531570
});

libraries/rush-terminal-renderer/src/HostEventRouter.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@ import {
55
type DaemonVerbosity,
66
type IDaemonEventEnvelope,
77
type IDaemonExtensionEventPayload,
8+
type IDaemonOperationHeaderPayload,
89
type IDaemonOperationRegisteredPayload,
910
type IDaemonOperationStreamClosedPayload,
11+
RUSHD_OPERATION_HEADER,
1012
RUSHD_OPERATION_STREAM_CLOSED,
1113
shouldSerializeDaemonEvent
1214
} from '@rushstack/rush-daemon-protocol';
@@ -20,11 +22,7 @@ function readScopeOperationId(envelope: IDaemonEventEnvelope): string | undefine
2022
return scope === undefined ? undefined : scope.operationId;
2123
}
2224

23-
/**
24-
* Routes decoded event envelopes between the collator (stream-affecting and
25-
* operation-scoped events) and the verbosity-filtered renderer.
26-
* @internal
27-
*/
25+
/** Routes decoded events between the operation collator and renderer. @internal */
2826
export class HostEventRouter {
2927
private readonly _streams: OperationStreamRegistry;
3028
private readonly _renderer: IDaemonRenderer;
@@ -40,7 +38,6 @@ export class HostEventRouter {
4038
this._verbosity = verbosity;
4139
}
4240

43-
/** Routes one decoded `0x05` event envelope. */
4441
public routeEvent(envelope: IDaemonEventEnvelope): void {
4542
this._trackOperationLifecycle(envelope);
4643
if (this._routeScopedActivity(envelope)) {
@@ -67,16 +64,17 @@ export class HostEventRouter {
6764
}
6865

6966
private _trackExtension(payload: IDaemonExtensionEventPayload): void {
67+
if (payload.name === RUSHD_OPERATION_HEADER) {
68+
this._streams.setOperationHeader(payload.data as IDaemonOperationHeaderPayload);
69+
return;
70+
}
7071
if (payload.name === RUSHD_OPERATION_STREAM_CLOSED) {
7172
const data: IDaemonOperationStreamClosedPayload =
7273
payload.data as IDaemonOperationStreamClosedPayload;
7374
this._streams.closeOperation(data.operationId);
7475
}
7576
}
7677

77-
// Operation-scoped activity lines are part of the operation's output block
78-
// (legacy writes them to the operation's collated stream, bypassing the
79-
// quiet-mode stdout discard), so they route to the collator, not the renderer.
8078
private _routeScopedActivity(envelope: IDaemonEventEnvelope): boolean {
8179
const operationId: string | undefined = readScopeOperationId(envelope);
8280
if (envelope.type !== 'activityChanged' || operationId === undefined) {
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved. Licensed under the MIT license.
2+
// See LICENSE in the project root for license information.
3+
4+
import type { IDaemonOperationHeaderPayload } from '@rushstack/rush-daemon-protocol';
5+
6+
const INITIAL_OPERATION_COUNT: number = 0;
7+
const OPERATION_COUNT_INCREMENT: number = 1;
8+
9+
export class OperationHeaderTracker {
10+
private readonly _headerByOperation: Map<string, IDaemonOperationHeaderPayload> = new Map();
11+
private _completedOperations: number = INITIAL_OPERATION_COUNT;
12+
private _totalOperations: number = INITIAL_OPERATION_COUNT;
13+
14+
public registerOperation(): void {
15+
this._totalOperations += OPERATION_COUNT_INCREMENT;
16+
}
17+
18+
public setOperationHeader(header: IDaemonOperationHeaderPayload): void {
19+
this._headerByOperation.set(header.operationId, header);
20+
}
21+
22+
public takeOperationHeader(operationId: string): IDaemonOperationHeaderPayload {
23+
const header: IDaemonOperationHeaderPayload | undefined =
24+
this._headerByOperation.get(operationId);
25+
if (header !== undefined) {
26+
this._headerByOperation.delete(operationId);
27+
this._completedOperations = header.completedOperations;
28+
this._totalOperations = header.totalOperations;
29+
return header;
30+
}
31+
this._completedOperations += OPERATION_COUNT_INCREMENT;
32+
return {
33+
completedOperations: this._completedOperations,
34+
operationId,
35+
totalOperations: this._totalOperations
36+
};
37+
}
38+
}

libraries/rush-terminal-renderer/src/OperationStreamRegistry.ts

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22
// See LICENSE in the project root for license information.
33

44
import { NewlineKind } from '@rushstack/node-core-library';
5+
import type { IDaemonOperationHeaderPayload } from '@rushstack/rush-daemon-protocol';
56
import { CollatedTerminal, StreamCollator } from '@rushstack/stream-collator';
67
import type { CollatedWriter } from '@rushstack/stream-collator';
78
import { TextRewriterTransform } from '@rushstack/terminal';
89
import type { ITerminalChunk, TerminalWritable } from '@rushstack/terminal';
910

11+
import { OperationHeaderTracker } from './OperationHeaderTracker';
1012
import { formatDaemonOperationHeader } from './RendererHeader';
1113

1214
/** Options for {@link OperationStreamRegistry}. @beta */
@@ -29,16 +31,12 @@ export interface IOperationStreamRegistryOptions {
2931
export class OperationStreamRegistry {
3032
private readonly _collator: StreamCollator;
3133
private readonly _collatedTerminal: CollatedTerminal;
32-
private readonly _writers: Map<string, CollatedWriter>;
34+
private readonly _headers: OperationHeaderTracker = new OperationHeaderTracker();
35+
private readonly _writers: Map<string, CollatedWriter> = new Map();
3336
private readonly _quiet: boolean;
34-
private _completedOperations: number;
35-
private _totalOperations: number;
3637

3738
public constructor(options: IOperationStreamRegistryOptions) {
38-
this._writers = new Map();
3939
this._quiet = options.quiet;
40-
this._completedOperations = 0;
41-
this._totalOperations = 0;
4240
const transform: TextRewriterTransform = new TextRewriterTransform({
4341
destination: options.destination,
4442
normalizeNewlines: NewlineKind.OsDefault,
@@ -53,7 +51,12 @@ export class OperationStreamRegistry {
5351

5452
/** Increments the total-operation count shown in headers. */
5553
public registerOperation(): void {
56-
this._totalOperations += 1;
54+
this._headers.registerOperation();
55+
}
56+
57+
/** Records engine-authoritative counters before an operation's stream activates. */
58+
public setOperationHeader(header: IDaemonOperationHeaderPayload): void {
59+
this._headers.setOperationHeader(header);
5760
}
5861

5962
/** Writes one raw chunk to the operation's collated stream. */
@@ -78,11 +81,13 @@ export class OperationStreamRegistry {
7881
if (writer === undefined) {
7982
return;
8083
}
81-
this._completedOperations += 1;
84+
const counters: IDaemonOperationHeaderPayload = this._headers.takeOperationHeader(
85+
writer.taskName
86+
);
8287
const header: string = formatDaemonOperationHeader(
8388
writer.taskName,
84-
this._completedOperations,
85-
this._totalOperations
89+
counters.completedOperations,
90+
counters.totalOperations
8691
);
8792
this._collatedTerminal.writeStdoutLine(`\n${header}`);
8893
if (!this._quiet) {

0 commit comments

Comments
 (0)