[steps] Composite functions: skip the outputs step when the call never ran

packages/steps/src/BuildStep.ts

@@ -83,6 +83,10 @@ export class BuildStepOutputAccessor {
     return Object.values(this.outputById);
   }
 
+  public get wasExecuted(): boolean {
+    return this.executed;
+  }
+
   public getOutputValueByName(name: string): string | undefined {
     if (!this.executed) {
       throw new BuildStepRuntimeError(

packages/steps/src/CompositeBuildStep.ts

@@ -59,7 +59,7 @@ export class CompositeBuildStep extends BuildStep {
       displayName,
       outputs,
       fn,
-      // `scope` already applies the call-site `if`, so `always()` disables the default skip-on-failure.
+      // Call-site `if` lives on `scope`. `always()` survives a child failure; `shouldExecuteStep` still skips a call that never ran so outputs stay unset.
       ifCondition: '${{ always() }}',
       compositeFunctionScope: scope,
     });
@@ -87,6 +87,19 @@ export class CompositeBuildStep extends BuildStep {
     }
   }
 
+  public override shouldExecuteStep(options: { runByDefault: boolean }): boolean {
+    if (!super.shouldExecuteStep(options)) {
+      return false;
+    }
+    return options.runByDefault || this.anyAuthoredChildExecuted();
+  }
+
+  private anyAuthoredChildExecuted(): boolean {
+    return this.children.some(child =>
+      child instanceof CompositeBuildStep ? child.anyAuthoredChildExecuted() : child.wasExecuted
+    );
+  }
+
   public getFlattenedSteps(): BuildStep[] {
     const flattened = this.children.flatMap(child =>
       child instanceof CompositeBuildStep ? child.getFlattenedSteps() : [child]

packages/steps/src/__tests__/BuildWorkflow-hooks-test.ts

@@ -835,6 +835,80 @@ describe('BuildWorkflow hook execution', () => {
       expect(statuses['setup']).toBe(BuildStepStatus.SKIPPED);
     });
 
+    it('skips the outputs node of an always() composite entry whose children an earlier entry suppressed', async () => {
+      const workflow = await parseAsync({
+        steps: [{ uses: 'eas/install_node_modules' }],
+        hooks: {
+          before_install_node_modules: [
+            { uses: 'test/first-boom', id: 'first-boom' },
+            { uses: './.eas/functions/setup', id: 'setup', if: '${{ always() }}' },
+          ],
+        },
+        externalFunctions: [
+          anchorFunction(),
+          recordingFunction('first-boom', { failWith: new Error('first failed') }),
+          recordingFunction('plain-child'),
+          versionFunction('read-version', '1.2.3'),
+        ],
+        compositeFunctionCatalog: {
+          './.eas/functions/setup': {
+            outputs: { version: { value: '${{ steps.read.outputs.version }}' } },
+            runs: {
+              steps: [
+                { id: 'plain', uses: 'test/plain-child' },
+                { id: 'read', uses: 'test/read-version' },
+              ],
+            },
+          },
+        },
+      });
+      await expect(workflow.executeAsync()).rejects.toThrow('first failed');
+      expect(executionLog).toEqual(['first-boom']);
+      const statuses = hookStepStatuses(workflow);
+      expect(statuses['setup__plain']).toBe(BuildStepStatus.SKIPPED);
+      expect(statuses['setup__read']).toBe(BuildStepStatus.SKIPPED);
+      expect(statuses['setup']).toBe(BuildStepStatus.SKIPPED);
+      expect(metrics).toEqual([
+        { anchor: 'install_node_modules', timing: 'before', result: 'failed' },
+      ]);
+    });
+
+    it('skips the outputs node of an after composite entry whose first child has an unevaluable if', async () => {
+      const workflow = await parseAsync({
+        steps: [{ uses: 'eas/install_node_modules' }],
+        hooks: {
+          after_install_node_modules: [{ uses: './.eas/functions/cleanup', id: 'cleanup' }],
+        },
+        externalFunctions: [
+          anchorFunction(),
+          recordingFunction('broken-gate'),
+          recordingFunction('plain-child'),
+        ],
+        compositeFunctionCatalog: {
+          './.eas/functions/cleanup': {
+            outputs: { note: { value: 'done' } },
+            runs: {
+              steps: [
+                {
+                  id: 'broken',
+                  uses: 'test/broken-gate',
+                  if: '${{ nonexistent.object.property }}',
+                },
+                { id: 'plain', uses: 'test/plain-child' },
+              ],
+            },
+          },
+        },
+      });
+      await expect(workflow.executeAsync()).rejects.toThrow();
+      expect(executionLog).toEqual(['anchor']);
+      const statuses = hookStepStatuses(workflow);
+      expect(statuses['cleanup__broken']).toBe(BuildStepStatus.SKIPPED);
+      expect(statuses['cleanup__plain']).toBe(BuildStepStatus.SKIPPED);
+      expect(statuses['cleanup']).toBe(BuildStepStatus.SKIPPED);
+      expect(metrics).toEqual([]);
+    });
+
     it('a later hook step consumes the composite call outputs via steps.<call-id>', async () => {
       const captured: unknown[] = [];
       const workflow = await parseAsync({

packages/steps/src/__tests__/StepsConfigParser-composite-functions-outputs-test.ts

@@ -319,6 +319,172 @@ describe('StepsConfigParser local composite functions', () => {
       expect(outputsStep.getOutputValueByName('version_expr')).toBe('');
     });
 
+    it('skips the outputs step when an earlier failure suppressed every inner step', async () => {
+      const workflow = await parseCompositeFunctions({
+        catalog: {
+          [SETUP]: {
+            outputs: { version: { value: '${{ steps.read.outputs.version }}' } },
+            runs: {
+              steps: [{ id: 'read', uses: 'test/set-version' }],
+            },
+          },
+        },
+        steps: [
+          { id: 'boom', uses: 'test/fail' },
+          { uses: SETUP, id: 'setup', if: '${{ always() }}' },
+          {
+            id: 'after',
+            uses: 'test/passthrough',
+            if: '${{ always() }}',
+            with: { value: '${{ steps.setup.outputs.version }}' },
+          },
+        ],
+        externalFunctions: [setVersionFunction(), failingFunction(), passThroughFunction()],
+      });
+
+      const error = await getErrorAsync<Error>(() => workflow.executeAsync());
+      expect(error.message).toBe('inner failed');
+
+      const innerStep = workflow.buildSteps.find(s => s.id === 'setup__read');
+      const outputsStep = workflow.buildSteps.find(s => s.id === 'setup');
+      const afterStep = workflow.buildSteps.find(s => s.id === 'after');
+      expect(innerStep?.status).toBe(BuildStepStatus.SKIPPED);
+      expect(outputsStep?.status).toBe(BuildStepStatus.SKIPPED);
+      expect(afterStep?.getOutputValueByName('out')).toBe('');
+      expect(() => outputsStep?.getOutputValueByName('version')).toThrow(
+        'has not been executed yet'
+      );
+    });
+
+    it('runs the outputs step when an inner step opted past the earlier failure with always()', async () => {
+      const workflow = await parseCompositeFunctions({
+        catalog: {
+          [SETUP]: {
+            outputs: {
+              version: { value: '${{ steps.read.outputs.version }}' },
+              skipped: { value: '${{ steps.later.outputs.out }}' },
+            },
+            runs: {
+              steps: [
+                { id: 'read', uses: 'test/set-version', if: '${{ always() }}' },
+                { id: 'later', uses: 'test/passthrough', with: { value: 'x' } },
+              ],
+            },
+          },
+        },
+        steps: [
+          { id: 'boom', uses: 'test/fail' },
+          { uses: SETUP, id: 'setup', if: '${{ always() }}' },
+        ],
+        externalFunctions: [setVersionFunction(), failingFunction(), passThroughFunction()],
+      });
+
+      const error = await getErrorAsync<Error>(() => workflow.executeAsync());
+      expect(error.message).toBe('inner failed');
+
+      const outputsStep = workflow.buildSteps.find(s => s.id === 'setup');
+      expect(workflow.buildSteps.find(s => s.id === 'setup__read')?.status).toBe(
+        BuildStepStatus.SUCCESS
+      );
+      expect(workflow.buildSteps.find(s => s.id === 'setup__later')?.status).toBe(
+        BuildStepStatus.SKIPPED
+      );
+      expect(outputsStep?.status).toBe(BuildStepStatus.SUCCESS);
+      expect(outputsStep?.getOutputValueByName('version')).toBe('$(echo injected)');
+      expect(outputsStep?.getOutputValueByName('skipped')).toBe('');
+    });
+
+    it('skips the outer outputs step when a nested call without outputs was fully suppressed', async () => {
+      const workflow = await parseCompositeFunctions({
+        catalog: {
+          './.eas/functions/outer': {
+            outputs: { version: { value: '${{ steps.mid.outputs.version }}' } },
+            runs: { steps: [{ uses: './.eas/functions/inner', id: 'mid' }] },
+          },
+          './.eas/functions/inner': {
+            runs: { steps: [{ id: 'read', uses: 'test/set-version' }] },
+          },
+        },
+        steps: [
+          { id: 'boom', uses: 'test/fail' },
+          { uses: './.eas/functions/outer', id: 'top', if: '${{ always() }}' },
+        ],
+        externalFunctions: [setVersionFunction(), failingFunction()],
+      });
+
+      const error = await getErrorAsync<Error>(() => workflow.executeAsync());
+      expect(error.message).toBe('inner failed');
+
+      expect(workflow.buildSteps.find(s => s.id === 'top__mid__read')?.status).toBe(
+        BuildStepStatus.SKIPPED
+      );
+      expect(workflow.buildSteps.find(s => s.id === 'top')?.status).toBe(BuildStepStatus.SKIPPED);
+    });
+
+    it('runs the outer outputs step when a grandchild of a nested call executed', async () => {
+      const workflow = await parseCompositeFunctions({
+        catalog: {
+          './.eas/functions/outer': {
+            outputs: { version: { value: '${{ steps.mid.outputs.inner_version }}' } },
+            runs: {
+              steps: [{ uses: './.eas/functions/inner', id: 'mid', if: '${{ always() }}' }],
+            },
+          },
+          './.eas/functions/inner': {
+            outputs: { inner_version: { value: '${{ steps.read.outputs.version }}' } },
+            runs: { steps: [{ id: 'read', uses: 'test/set-version', if: '${{ always() }}' }] },
+          },
+        },
+        steps: [
+          { id: 'boom', uses: 'test/fail' },
+          { uses: './.eas/functions/outer', id: 'top', if: '${{ always() }}' },
+        ],
+        externalFunctions: [setVersionFunction(), failingFunction()],
+      });
+
+      const error = await getErrorAsync<Error>(() => workflow.executeAsync());
+      expect(error.message).toBe('inner failed');
+
+      expect(workflow.buildSteps.find(s => s.id === 'top__mid__read')?.status).toBe(
+        BuildStepStatus.SUCCESS
+      );
+      expect(workflow.buildSteps.find(s => s.id === 'top')?.getOutputValueByName('version')).toBe(
+        '$(echo injected)'
+      );
+    });
+
+    it('runs the outer outputs step when a grandchild of a nested call without outputs executed via always()', async () => {
+      const workflow = await parseCompositeFunctions({
+        catalog: {
+          './.eas/functions/outer': {
+            outputs: { version: { value: '${{ steps.mid.outputs.version }}' } },
+            runs: {
+              steps: [{ uses: './.eas/functions/inner', id: 'mid', if: '${{ always() }}' }],
+            },
+          },
+          './.eas/functions/inner': {
+            runs: { steps: [{ id: 'read', uses: 'test/set-version', if: '${{ always() }}' }] },
+          },
+        },
+        steps: [
+          { id: 'boom', uses: 'test/fail' },
+          { uses: './.eas/functions/outer', id: 'top', if: '${{ always() }}' },
+        ],
+        externalFunctions: [setVersionFunction(), failingFunction()],
+      });
+
+      const error = await getErrorAsync<Error>(() => workflow.executeAsync());
+      expect(error.message).toBe('inner failed');
+
+      expect(workflow.buildSteps.find(s => s.id === 'top__mid__read')?.status).toBe(
+        BuildStepStatus.SUCCESS
+      );
+      expect(workflow.buildSteps.find(s => s.id === 'top__mid')).toBeUndefined();
+      const outputsStep = workflow.buildSteps.find(s => s.id === 'top');
+      expect(outputsStep?.status).toBe(BuildStepStatus.SUCCESS);
+      expect(outputsStep?.getOutputValueByName('version')).toBe('');
+    });
+
     it('resolves an out-of-scope action output reference to an empty string', async () => {
       const workflow = await parseCompositeFunctions({
         catalog: {