refactor(workflow): remove step internal ids

packages/eas-cli/src/__tests__/commands/workflow-logs-test.ts

@@ -1,6 +1,7 @@
 import {
   getMockEmptyWorkflowRunsFragment,
   getMockWorkflowCustomJobFragment,
+  getMockWorkflowRunWithBuildJobsFragment,
   getMockWorkflowRunWithJobsFragment,
   getMockWorkflowRunsFragment,
   mockCommandContext,
@@ -124,7 +125,8 @@ describe(WorkflowLogView, () => {
       .mocked(AppQuery.byIdWorkflowRunsFilteredByStatusAsync)
       .mockResolvedValue(getMockWorkflowRunsFragment({ withJobs: 1 }));
     let promptCalls = 0;
-    jest.mocked(promptAsync).mockImplementation(async () => {
+    let stepPrompt: any;
+    jest.mocked(promptAsync).mockImplementation(async prompt => {
       if (promptCalls === 0) {
         promptCalls++;
         return { selectedRun: 'build1' };
@@ -132,13 +134,14 @@ describe(WorkflowLogView, () => {
         promptCalls++;
         return { selectedJob: 0 };
       } else {
-        return { selectedStep: 'step1' };
+        stepPrompt = prompt;
+        return { selectedStep: 'step-id-1' };
       }
     });
     jest
       .mocked(fetchRawLogsForCustomJobAsync)
       .mockResolvedValue(
-        '{"result":"test","marker":"test","buildStepDisplayName":"step1","buildStepInternalId":"step1","time":"2022-01-01T00:00:00.000Z","msg":"test"}'
+        '{"result":"success","marker":"end-step","buildStepDisplayName":"Install dependencies","buildStepId":"step-id-1","time":"2022-01-01T00:00:00.000Z","msg":"test"}'
       );
     await cmd.run();
     expect(AppQuery.byIdWorkflowRunsFilteredByStatusAsync).toHaveBeenCalledWith(
@@ -148,6 +151,14 @@ describe(WorkflowLogView, () => {
       20
     );
     expect(promptAsync).toHaveBeenCalledTimes(3);
+    expect(stepPrompt.choices).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          title: 'Install dependencies - success',
+          value: 'step-id-1',
+        }),
+      ])
+    );
     expect(Log.error).not.toHaveBeenCalled();
     expect(Log.log).toHaveBeenCalledWith('  2022-01-01T00:00:00.000Z test');
   });
@@ -159,6 +170,9 @@ describe(WorkflowLogView, () => {
     jest
       .mocked(AppQuery.byIdWorkflowRunsFilteredByStatusAsync)
       .mockResolvedValue(getMockWorkflowRunsFragment({ withBuildJobs: 1 }));
+    jest
+      .mocked(WorkflowRunQuery.withJobsByIdAsync)
+      .mockResolvedValue(getMockWorkflowRunWithBuildJobsFragment());
     jest.mocked(BuildQuery.byIdAsync).mockResolvedValue({
       id: 'build1',
       status: BuildStatus.Finished,
@@ -181,7 +195,8 @@ describe(WorkflowLogView, () => {
       logFiles: ['https://example.com/log1'],
     });
     let promptCalls = 0;
-    jest.mocked(promptAsync).mockImplementation(async () => {
+    let stepPrompt: any;
+    jest.mocked(promptAsync).mockImplementation(async prompt => {
       if (promptCalls === 0) {
         promptCalls++;
         return { selectedRun: 'build1' };
@@ -189,13 +204,14 @@ describe(WorkflowLogView, () => {
         promptCalls++;
         return { selectedJob: 0 };
       } else {
-        return { selectedStep: 'step1' };
+        stepPrompt = prompt;
+        return { selectedStep: 'legacy-step-1' };
       }
     });
     jest
       .mocked(fetchRawLogsForBuildJobAsync)
       .mockResolvedValue(
-        '{"result":"test","marker":"test","buildStepDisplayName":"step1","buildStepInternalId":"step1","time":"2022-01-01T00:00:00.000Z","msg":"test"}'
+        '{"result":"success","marker":"end-step","buildStepDisplayName":"step1","buildStepInternalId":"legacy-step-1","time":"2022-01-01T00:00:00.000Z","msg":"test"}'
       );
     await cmd.run();
     expect(AppQuery.byIdWorkflowRunsFilteredByStatusAsync).toHaveBeenCalledWith(
@@ -205,9 +221,65 @@ describe(WorkflowLogView, () => {
       20
     );
     expect(promptAsync).toHaveBeenCalledTimes(3);
+    expect(stepPrompt.choices).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          title: 'step1 - success',
+          value: 'legacy-step-1',
+        }),
+      ])
+    );
     expect(Log.error).not.toHaveBeenCalled();
     expect(Log.log).toHaveBeenCalledWith('  2022-01-01T00:00:00.000Z test');
   });
+  test('view logs keeps duplicate display names separated by step id', async () => {
+    const ctx = mockCommandContext(WorkflowLogView, {
+      projectId: mockProjectId,
+    });
+    const cmd = mockTestCommand(WorkflowLogView, [], ctx);
+    jest
+      .mocked(AppQuery.byIdWorkflowRunsFilteredByStatusAsync)
+      .mockResolvedValue(getMockWorkflowRunsFragment({ withJobs: 1 }));
+    let promptCalls = 0;
+    let stepPrompt: any;
+    jest.mocked(promptAsync).mockImplementation(async prompt => {
+      if (promptCalls === 0) {
+        promptCalls++;
+        return { selectedRun: 'build1' };
+      } else if (promptCalls === 1) {
+        promptCalls++;
+        return { selectedJob: 0 };
+      } else {
+        stepPrompt = prompt;
+        return { selectedStep: 'step-id-2' };
+      }
+    });
+    jest
+      .mocked(fetchRawLogsForCustomJobAsync)
+      .mockResolvedValue(
+        [
+          '{"result":"success","marker":"end-step","buildStepDisplayName":"Install","buildStepId":"step-id-1","time":"2022-01-01T00:00:00.000Z","msg":"first"}',
+          '{"result":"fail","marker":"end-step","buildStepDisplayName":"Install","buildStepId":"step-id-2","time":"2022-01-01T00:00:01.000Z","msg":"second"}',
+        ].join('\n')
+      );
+
+    await cmd.run();
+
+    expect(promptAsync).toHaveBeenCalledTimes(3);
+    expect(stepPrompt.choices).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          title: 'Install - success',
+          value: 'step-id-1',
+        }),
+        expect.objectContaining({
+          title: 'Install - fail',
+          value: 'step-id-2',
+        }),
+      ])
+    );
+    expect(Log.log).toHaveBeenCalledWith('  2022-01-01T00:00:01.000Z second');
+  });
   test('view logs for a workflow job, passing in a job ID, all steps', async () => {
     const ctx = mockCommandContext(WorkflowLogView, {
       projectId: mockProjectId,

packages/eas-cli/src/commandUtils/workflow/__tests__/utils-test.ts

@@ -0,0 +1,35 @@
+import { getMockWorkflowRunWithJobsFragment } from '../../../__tests__/commands/utils';
+import { fetchRawLogsForCustomJobAsync } from '../fetchLogs';
+import { infoForActiveWorkflowRunAsync } from '../utils';
+import { WorkflowJobStatus } from '../../../graphql/generated';
+
+jest.mock('../fetchLogs');
+
+describe('workflow utils', () => {
+  afterEach(() => {
+    jest.clearAllMocks();
+  });
+
+  test('shows the display name for the current step while keying logs by step id', async () => {
+    const workflowRun = getMockWorkflowRunWithJobsFragment();
+    workflowRun.jobs = workflowRun.jobs.map(job => ({
+      ...job,
+      status: WorkflowJobStatus.InProgress,
+    }));
+
+    jest
+      .mocked(fetchRawLogsForCustomJobAsync)
+      .mockResolvedValue(
+        [
+          '{"buildStepId":"step-id-1","buildStepDisplayName":"Install dependencies","time":"2022-01-01T00:00:00.000Z","msg":"npm ci"}',
+          '{"buildStepId":"step-id-1","buildStepDisplayName":"Install dependencies","marker":"end-step","result":"success","time":"2022-01-01T00:00:01.000Z","msg":"done"}',
+        ].join('\n')
+      );
+
+    const output = await infoForActiveWorkflowRunAsync({} as any, workflowRun);
+
+    expect(output).toContain('Current step');
+    expect(output).toContain('Install dependencies');
+    expect(output).not.toContain('step-id-1');
+  });
+});

packages/eas-cli/src/commandUtils/workflow/types.ts

@@ -50,4 +50,10 @@ export type WorkflowLogLine = {
   err?: any;
 };
 
-export type WorkflowLogs = Map<string, WorkflowLogLine[]>;
+export type WorkflowLogGroup = {
+  key: string;
+  label: string;
+  logLines: WorkflowLogLine[];
+};
+
+export type WorkflowLogs = Map<string, WorkflowLogGroup>;

packages/eas-cli/src/commandUtils/workflow/utils.ts

@@ -4,6 +4,7 @@ import * as fs from 'node:fs';
 import { fetchRawLogsForBuildJobAsync, fetchRawLogsForCustomJobAsync } from './fetchLogs';
 import {
   WorkflowJobResult,
+  WorkflowLogGroup,
   WorkflowLogLine,
   WorkflowLogs,
   WorkflowRunResult,
@@ -84,18 +85,17 @@ export function choiceFromWorkflowJob(job: WorkflowJobResult, index: number): Ch
 export function choicesFromWorkflowLogs(
   logs: WorkflowLogs
 ): (Choice & { name: string; status: string; logLines: WorkflowLogLine[] | undefined })[] {
-  return Array.from(logs.keys())
-    .map(step => {
-      const logLines = logs.get(step);
+  return Array.from(logs.values())
+    .map(({ key, label, logLines }) => {
       const stepStatus =
         logLines?.filter(
           (line: WorkflowLogLine) => line.marker === 'end-step' || line.marker === 'END_PHASE'
         )[0]?.result ?? '';
       return {
-        title: `${step} - ${stepStatus}`,
-        name: step,
+        title: `${label} - ${stepStatus}`,
+        name: label,
         status: stepStatus,
-        value: step,
+        value: key,
         logLines,
       };
     })
@@ -141,20 +141,36 @@ export async function fetchAndProcessLogsFromJobAsync(
   }
   Log.debug(`rawLogs = ${JSON.stringify(rawLogs, null, 2)}`);
   const logs: WorkflowLogs = new Map();
-  const logKeys = new Set<string>();
   rawLogs.split('\n').forEach((line, index) => {
     Log.debug(`line ${index} = ${JSON.stringify(line, null, 2)}`);
     try {
       const parsedLine = JSON.parse(line);
-      const { buildStepDisplayName, buildStepInternalId, phase, time, msg, result, marker, err } =
-        parsedLine;
-      const stepId = buildStepDisplayName ?? buildStepInternalId ?? phase;
-      if (stepId) {
-        if (!logKeys.has(stepId)) {
-          logKeys.add(stepId);
-          logs.set(stepId, []);
+      const {
+        buildStepDisplayName,
+        buildStepId,
+        buildStepInternalId,
+        phase,
+        time,
+        msg,
+        result,
+        marker,
+        err,
+      } = parsedLine;
+      const stepKey = buildStepId ?? buildStepInternalId ?? buildStepDisplayName ?? phase;
+      const stepLabel = buildStepDisplayName ?? buildStepId ?? phase;
+      if (stepKey && stepLabel) {
+        if (!logs.has(stepKey)) {
+          logs.set(stepKey, {
+            key: stepKey,
+            label: stepLabel,
+            logLines: [],
+          });
         }
-        logs.get(stepId)?.push({ time, msg, result, marker, err });
+        const logGroup = logs.get(stepKey) as WorkflowLogGroup;
+        if (buildStepDisplayName) {
+          logGroup.label = buildStepDisplayName;
+        }
+        logGroup.logLines.push({ time, msg, result, marker, err });
       }
     } catch {}
   });

packages/eas-cli/src/commands/workflow/logs.ts

@@ -12,10 +12,9 @@ import Log from '../../log';
 import { enableJsonOutput, printJsonOnlyOutput } from '../../utils/json';
 
 function printLogsForAllSteps(logs: WorkflowLogs): void {
-  [...logs.keys()].forEach(step => {
-    const logLines = logs.get(step);
-    if (logLines) {
-      Log.log(`Step: ${step}`);
+  [...logs.values()].forEach(({ label, logLines }) => {
+    if (logLines.length > 0) {
+      Log.log(`Step: ${label}`);
       logLines.forEach(line => {
         Log.log(`  ${line.time} ${line.msg}`);
       });
@@ -89,21 +88,25 @@ export default class WorkflowLogView extends EasCommand {
     if (allSteps) {
       if (logs) {
         if (flags.json) {
-          printJsonOnlyOutput(Object.fromEntries(logs));
+          printJsonOnlyOutput(
+            Object.fromEntries(
+              Array.from(logs.entries()).map(([key, value]) => [key, value.logLines])
+            )
+          );
         } else {
           printLogsForAllSteps(logs);
         }
       }
     } else {
       const selectedStep = finalSelectionState?.step as unknown as string;
-      const logLines = logs?.get(selectedStep);
-      if (logLines) {
+      const logGroup = logs?.get(selectedStep);
+      if (logGroup) {
         if (flags.json) {
           const output: { [key: string]: WorkflowLogLine[] | null } = {};
-          output[selectedStep] = logLines ?? null;
+          output[selectedStep] = logGroup.logLines;
           printJsonOnlyOutput(output);
         } else {
-          logLines.forEach(line => {
+          logGroup.logLines.forEach(line => {
             Log.log(`  ${line.time} ${line.msg}`);
           });
         }

packages/local-build-plugin/src/logger.ts

@@ -86,7 +86,6 @@ class PrettyStream extends Writable {
       'pid',
       'hostname',
       'name',
-      'buildStepInternalId',
       'buildStepId',
       'buildStepDisplayName',
     ]);

packages/steps/src/BuildStep.ts

@@ -139,7 +139,6 @@ export class BuildStep extends BuildStepOutputAccessor {
   private readonly outputsDir: string;
   private readonly envsDir: string;
 
-  private readonly internalId: string;
   private readonly inputById: BuildStepInputById;
   protected executed = false;
 
@@ -198,10 +197,7 @@ export class BuildStep extends BuildStepOutputAccessor {
     this.__metricsId = __metricsId;
     this.status = BuildStepStatus.NEW;
 
-    this.internalId = uuidv4();
-
     const logger = ctx.baseLogger.child({
-      buildStepInternalId: this.internalId,
       buildStepId: this.id,
       buildStepDisplayName: this.displayName,
     });

packages/steps/src/__tests__/BuildStep-test.ts

@@ -170,7 +170,6 @@ describe(BuildStep, () => {
       });
       expect(ctx.baseLogger.child).toHaveBeenCalledWith(
         expect.objectContaining({
-          buildStepInternalId: expect.stringMatching(UUID_REGEX),
           buildStepId: 'test1',
           buildStepDisplayName: 'Test step',
         })