[steps] Support local composite functions in hook steps

packages/steps/src/BuildWorkflow.ts

@@ -5,6 +5,7 @@ import { BuildFunctionById } from './BuildFunction';
 import { BuildRuntimePlatform } from './BuildRuntimePlatform';
 import { BuildStep } from './BuildStep';
 import { BuildStepGlobalContext } from './BuildStepContext';
+import { CompositeBuildStep } from './CompositeBuildStep';
 import { StepMetricResult } from './StepMetrics';
 import { AnchorHooks, HookEntry } from './hooks';
 import { evaluateIfCondition } from './utils/jsepEval';
@@ -165,8 +166,9 @@ export class BuildWorkflow {
  * Default-run rules (a step or entry with no `if:`):
  * - `before`: runs unless a failure occurred within THIS hook sequence;
  *   failures predating the call are ignored — "runs iff the anchor runs".
- * - `after`: runs unconditionally — past the anchor's own failure AND past an
- *   earlier after-entry's failure.
+ * - `after`: each entry starts fresh, past the anchor's own failure AND past
+ *   an earlier after-entry's failure, but a failure within the entry skips
+ *   its later no-`if:` steps.
  * Passed as `runByDefault` so composite scopes share the same missing-`if:` rule.
  * A user `if:` is always evaluated against the real global context, so
  * `failure()` / `success()` keep their global meaning on both sides.
@@ -191,7 +193,7 @@ export async function executeHookStepsAsync(
     ctx.markAsFailed();
   };
 
-  let anyStepExecuted = false;
+  let anyAuthoredStepExecuted = false;
   for (const entry of entries) {
     // Truthiness, not presence: an empty `if:` means "no condition", the same
     // as BuildStep.shouldExecuteStep treats it.
@@ -224,10 +226,11 @@ export async function executeHookStepsAsync(
 
     let entryFailed = false;
     for (const step of entry.steps) {
-      // before skips on in-sequence failure; an entry with a passed if: only skips
-      // on within-entry failure; after always runs.
-      const runByDefault =
-        options.timing === 'after' || (entryHasExplicitCondition ? !entryFailed : !failedLocally);
+      // after and an entry with a passed if: skip on within-entry failure;
+      // before without an entry if: skips on any in-sequence failure.
+      const scopedFailure =
+        options.timing === 'after' || entryHasExplicitCondition ? entryFailed : failedLocally;
+      const runByDefault = !scopedFailure;
       let shouldExecuteStep = false;
       try {
         shouldExecuteStep = step.shouldExecuteStep({ runByDefault });
@@ -245,7 +248,11 @@ export async function executeHookStepsAsync(
         step.skip();
         continue;
       }
-      anyStepExecuted = true;
+      // The synthetic composite outputs node runs under always(), so it alone
+      // must not make a fully-skipped hook side report a metric.
+      if (!(step instanceof CompositeBuildStep)) {
+        anyAuthoredStepExecuted = true;
+      }
       const startTime = performance.now();
       let stepResult: StepMetricResult = 'success';
       try {
@@ -260,7 +267,7 @@ export async function executeHookStepsAsync(
     }
   }
 
-  if (anyStepExecuted) {
+  if (anyAuthoredStepExecuted) {
     ctx.reportWorkflowHookMetric({
       anchor: options.anchor,
       timing: options.timing,
@@ -272,8 +279,9 @@ export async function executeHookStepsAsync(
   return { failedLocally, firstError };
 }
 
-// The one wording for an `if:` that could not be evaluated — anchor steps,
-// hook steps, and hook group entries all log through here.
+// Shared wording for unevaluable `if:` gates. Callers pass the step's own
+// `if:`, but the throw may come from a composite call-site `if:` evaluated via
+// the step's scope. In that case the quoted condition is not the one that failed.
 function logConditionEvaluationError(
   logger: bunyan,
   err: unknown,
@@ -282,6 +290,8 @@ function logConditionEvaluationError(
 ): void {
   logger.error({ err });
   logger.error(
-    `Runner failed to evaluate if it should execute ${subject}, using its if condition "${ifCondition}". This can be caused by trying to access non-existing object property. If you think this is a bug report it here: https://github.com/expo/eas-cli/issues.`
+    `Runner failed to evaluate if it should execute ${subject}${
+      ifCondition ? `, using its if condition "${ifCondition}"` : ''
+    }. This can be caused by trying to access non-existing object property. If you think this is a bug report it here: https://github.com/expo/eas-cli/issues.`
   );
 }

packages/steps/src/CompositeFunctionExpander.ts

@@ -70,6 +70,14 @@ export class CompositeFunctionExpander {
     private readonly functionMaps: FunctionMaps
   ) {}
 
+  public get buildFunctionById(): BuildFunctionById {
+    return this.functionMaps.buildFunctionById;
+  }
+
+  public get buildFunctionGroupById(): BuildFunctionGroupById {
+    return this.functionMaps.buildFunctionGroupById;
+  }
+
   public expandCompositeFunctionStep(
     step: FunctionStep,
     compositeFunctionPath: string,

packages/steps/src/StepsConfigParser.ts

@@ -13,7 +13,7 @@ import {
 import assert from 'node:assert';
 
 import { AbstractConfigParser } from './AbstractConfigParser';
-import { CompositeFunctionExpander, FunctionMaps } from './CompositeFunctionExpander';
+import { CompositeFunctionExpander } from './CompositeFunctionExpander';
 import { BuildFunction, BuildFunctionById, createBuildFunctionByIdMapping } from './BuildFunction';
 import {
   BuildFunctionGroup,
@@ -122,26 +122,22 @@ export class StepsConfigParser extends AbstractConfigParser {
           if (anchorId === undefined) {
             continue;
           }
-          const anchorHooks = this.constructAnchorHooks(anchorId, validatedHooks, {
-            buildFunctionById,
-            buildFunctionGroupById,
-          });
+          const anchorHooks = this.constructAnchorHooks(
+            anchorId,
+            validatedHooks,
+            compositeFunctionExpander
+          );
           if (anchorHooks !== undefined) {
             hooksByAnchorStep.set(expandedStep, anchorHooks);
           }
         }
         continue;
       }
 
-      const maps = { buildFunctionById, buildFunctionGroupById };
       const anchorId = StepsConfigParser.resolveStepAnchor(stepConfig, buildFunctionById);
       if (anchorId === undefined) {
         buildSteps.push(
-          ...this.createBuildStepsFromNonGroupStepConfig(
-            stepConfig,
-            maps,
-            compositeFunctionExpander
-          )
+          ...this.createBuildStepsFromNonGroupStepConfig(stepConfig, compositeFunctionExpander)
         );
         continue;
       }
@@ -152,10 +148,14 @@ export class StepsConfigParser extends AbstractConfigParser {
           'Hook anchors are not supported on local composite function steps.'
         );
       }
-      const before = this.constructHookSideEntries(anchorId, 'before', validatedHooks, maps);
+      const before = this.constructHookSideEntries(
+        anchorId,
+        'before',
+        validatedHooks,
+        compositeFunctionExpander
+      );
       const createdSteps = this.createBuildStepsFromNonGroupStepConfig(
         stepConfig,
-        maps,
         compositeFunctionExpander
       );
       assert(
@@ -164,7 +164,12 @@ export class StepsConfigParser extends AbstractConfigParser {
       );
       const anchorStep = createdSteps[0];
       buildSteps.push(anchorStep);
-      const after = this.constructHookSideEntries(anchorId, 'after', validatedHooks, maps);
+      const after = this.constructHookSideEntries(
+        anchorId,
+        'after',
+        validatedHooks,
+        compositeFunctionExpander
+      );
       if (before.length > 0 || after.length > 0) {
         hooksByAnchorStep.set(anchorStep, { anchor: anchorId, before, after });
       }
@@ -227,13 +232,20 @@ export class StepsConfigParser extends AbstractConfigParser {
   private constructAnchorHooks(
     anchorId: HookAnchorId,
     validatedHooks: Record<string, Step[]>,
-    maps: {
-      buildFunctionById: BuildFunctionById;
-      buildFunctionGroupById: BuildFunctionGroupById;
-    }
+    compositeFunctionExpander: CompositeFunctionExpander
   ): AnchorHooks | undefined {
-    const before = this.constructHookSideEntries(anchorId, 'before', validatedHooks, maps);
-    const after = this.constructHookSideEntries(anchorId, 'after', validatedHooks, maps);
+    const before = this.constructHookSideEntries(
+      anchorId,
+      'before',
+      validatedHooks,
+      compositeFunctionExpander
+    );
+    const after = this.constructHookSideEntries(
+      anchorId,
+      'after',
+      validatedHooks,
+      compositeFunctionExpander
+    );
     if (before.length === 0 && after.length === 0) {
       return undefined;
     }
@@ -244,32 +256,24 @@ export class StepsConfigParser extends AbstractConfigParser {
     anchorId: HookAnchorId,
     side: 'before' | 'after',
     validatedHooks: Record<string, Step[]>,
-    maps: {
-      buildFunctionById: BuildFunctionById;
-      buildFunctionGroupById: BuildFunctionGroupById;
-    }
+    compositeFunctionExpander: CompositeFunctionExpander
   ): HookEntry[] {
     const hookSteps = validatedHooks[`${side}_${anchorId}`];
     if (hookSteps === undefined) {
       return [];
     }
-    return constructHookEntriesFromValidatedSteps(this.ctx, hookSteps, maps);
+    return constructHookEntriesFromValidatedSteps(this.ctx, hookSteps, compositeFunctionExpander);
   }
 
   private createBuildStepsFromNonGroupStepConfig(
     stepConfig: Step,
-    maps: FunctionMaps,
     compositeFunctionExpander: CompositeFunctionExpander
   ): BuildStep[] {
     if (isStepShellStep(stepConfig)) {
       return [createBuildStepFromShellStep(this.ctx, stepConfig)];
     }
     if (isStepFunctionStep(stepConfig)) {
-      return this.createBuildStepsFromFunctionStepConfig(
-        stepConfig,
-        maps,
-        compositeFunctionExpander
-      );
+      return this.createBuildStepsFromFunctionStepConfig(stepConfig, compositeFunctionExpander);
     }
     throw new BuildConfigError(
       'Invalid job step configuration detected. Step must be shell or function step'
@@ -278,7 +282,6 @@ export class StepsConfigParser extends AbstractConfigParser {
 
   private createBuildStepsFromFunctionStepConfig(
     step: FunctionStep,
-    { buildFunctionById }: FunctionMaps,
     compositeFunctionExpander: CompositeFunctionExpander
   ): BuildStep[] {
     if (isLocalCompositeFunctionPath(step.uses)) {
@@ -291,7 +294,7 @@ export class StepsConfigParser extends AbstractConfigParser {
         .getFlattenedSteps();
     }
 
-    const buildFunction = buildFunctionById[step.uses];
+    const buildFunction = compositeFunctionExpander.buildFunctionById[step.uses];
     assert(buildFunction, 'function ID must be ID of function or function group');
 
     return [