Skip to content

[build-tools] unify workflows hook#3957

Open
hSATAC wants to merge 3 commits into
mainfrom
ash/eng-22614-unify-workflows-hook
Open

[build-tools] unify workflows hook#3957
hSATAC wants to merge 3 commits into
mainfrom
ash/eng-22614-unify-workflows-hook

Conversation

@hSATAC

@hSATAC hSATAC commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Why

Workflow hooks (before_install_node_modules, before_submit, …) are currently spliced into the generated steps[] on the server, with every job type wiring them by hand, and they silently don't work in managed native builds. We're unifying this so the server sends job.hooks on the payload and the worker inserts the steps itself — one implementation, one set of semantics (ENG-22614).

This PR is the worker half and ships dormant: insertion is gated on job.hooks being present, which the server doesn't send yet, so there is no behavior change until the server side switches over.

How

  • @expo/eas-build-job gains a HOOK_ANCHORS registry — the single source of hook anchor names. Function-bound anchors carry a functionId (e.g. eas/install_node_modules); the parser resolves anchors through a reverse map at parse time, so the functions themselves need no changes.
  • The few anchors that are server-generated shell steps are marked with internal stamp fields on ShellStepZ (__hook_id, or the __hook_before_id/__hook_after_id pair for split placements like maestro_cloud). The values are loose strings on purpose, never enums — an older worker must treat anchor names newer than itself as inert metadata.
  • StepsConfigParser takes a new required hooks option and splices hook steps around anchor occurrences. This also works inside function-group expansions (uses: eas/build), which required retaining the originating function on expanded steps (BuildStep.sourceFunction — expansion used to drop that identity).
  • New run semantics for after-hooks: after_x runs iff its anchor executed, whether it passed or failed. BuildStep gains a runAfterStep gate checked in shouldExecuteStep; a user-supplied if: is AND-ed with the gate, so success()/failure() keep their meaning and always() cannot bypass the gate.
  • Version-skew safety: hook keys the worker doesn't recognize are fully inert — skipped before validation and insertion, surfaced as warnings — so a newer server can never fail an older worker. Hook steps under known keys are validated like job steps and fail with a BuildConfigError naming the key.
  • Each insertion emits a structured log line (Inserting N hook step(s) for anchor …); that is the signal we'll build the silent-drop metric on before the server-side rollout.

One deliberate restriction: a function group inside an after hook is rejected at parse time — the group API has no channel to gate its expanded steps on the anchor, so they would run even when the anchor was skipped. Groups in before hooks are unaffected.

Test Plan

~40 new unit tests across the three packages: insertion positions for function and shell anchors (including hooks landing inside a real uses: eas/build expansion), split-pair gating, no-nesting, multi-occurrence, gate semantics, skew cases (unknown keys and anchors stay inert), and dormancy (no job.hooks → identical parse output). Registry consistency tests assert every functionId names a registered eas function and that no function-group id shadows one. Existing suites across all packages are unchanged and green.

hSATAC added 3 commits July 6, 2026 23:55
…ntion

Signed-off-by: Ash Wu <hsatac@gmail.com>
Signed-off-by: Ash Wu <hsatac@gmail.com>
@linear-code

linear-code Bot commented Jul 6, 2026

Copy link
Copy Markdown

ENG-22614

@hSATAC hSATAC changed the title Ash/eng 22614 unify workflows hook [build-tools] unify workflows hook Jul 6, 2026
@hSATAC hSATAC added the no changelog PR that doesn't require a changelog entry label Jul 6, 2026
@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown

⏩ The changelog entry check has been skipped since the "no changelog" label is present.

@codecov

codecov Bot commented Jul 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.43590% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.38%. Comparing base (a848411) to head (907fb19).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
packages/steps/src/StepsConfigParser.ts 97.65% 2 Missing ⚠️
...ackages/build-tools/src/__tests__/utils/context.ts 85.72% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3957      +/-   ##
==========================================
+ Coverage   59.28%   59.38%   +0.11%     
==========================================
  Files         935      936       +1     
  Lines       41093    41204     +111     
  Branches     8660     8689      +29     
==========================================
+ Hits        24356    24465     +109     
- Misses      16643    16645       +2     
  Partials       94       94              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@hSATAC hSATAC requested a review from sjchmiela July 7, 2026 07:26
@hSATAC hSATAC marked this pull request as ready for review July 7, 2026 07:26
@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown

Subscribed to pull request

File Patterns Mentions
**/* @douglowder

Generated by CodeMention

Warning: The preamble and epilogue options in commentConfiguration are deprecated. Use template instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no changelog PR that doesn't require a changelog entry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant