Skip to content

[eas-cli][build-tools][steps] Validate local composite functions referenced from workflow hooks; extract the shared loader into steps - #4064

Open
sswrk wants to merge 3 commits into
szymonswierk/eng-22387-build-tools-hook-composite-catalogfrom
szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions
Open

[eas-cli][build-tools][steps] Validate local composite functions referenced from workflow hooks; extract the shared loader into steps#4064
sswrk wants to merge 3 commits into
szymonswierk/eng-22387-build-tools-hook-composite-catalogfrom
szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions

Conversation

@sswrk

@sswrk sswrk commented Jul 23, 2026

Copy link
Copy Markdown
Member

Why

Workflow validation already checks local composite functions referenced from job steps, but hooks can also use uses: ./..... eas workflow:validate don't catch problems with composite functions in hooks.

This PR also includes an extraction of duplicated (eas-cli and build-tools) composite function loader to @expo/steps.

How

Validation of workflows in eas-cli now reads each job's hooks steps too, not just job.steps.

Removed a duplicate loader from eas-cli and build-tools, moved it to steps.

Note: no validation when project_root_directory is set.

Test Plan

Added unit tests.

@linear-code

linear-code Bot commented Jul 23, 2026

Copy link
Copy Markdown

ENG-22387

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.21429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 62.66%. Comparing base (a6060b0) to head (d3a4b3a).
⚠️ Report is 1 commits behind head on szymonswierk/eng-22387-build-tools-hook-composite-catalog.

Files with missing lines Patch % Lines
...li/src/commandUtils/workflow/compositeFunctions.ts 95.66% 1 Missing ⚠️
Additional details and impacted files
@@                                      Coverage Diff                                      @@
##           szymonswierk/eng-22387-build-tools-hook-composite-catalog    #4064      +/-   ##
=============================================================================================
- Coverage                                                      62.66%   62.66%   -0.00%     
=============================================================================================
  Files                                                            999      998       -1     
  Lines                                                          45207    45190      -17     
  Branches                                                        9488     9492       +4     
=============================================================================================
- Hits                                                           28326    28314      -12     
+ Misses                                                         15433    15428       -5     
  Partials                                                        1448     1448              

☔ 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.

@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from a32c830 to cc4a09e Compare July 23, 2026 16:17
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from cc4a09e to 9f8972c Compare July 23, 2026 17:06
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from 9f8972c to 9a9c633 Compare July 24, 2026 11:43
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from 9a9c633 to a4a94b7 Compare July 24, 2026 11:49
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from a4a94b7 to 3cb953a Compare July 24, 2026 12:33
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from 3cb953a to bc5f0bb Compare July 24, 2026 13:02
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from bc5f0bb to bc5dee9 Compare July 24, 2026 14:12
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from bc5dee9 to 51157f3 Compare July 24, 2026 14:42
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from 51157f3 to 6f6441f Compare July 24, 2026 15:00
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from 6f6441f to e6a3b9a Compare July 24, 2026 15:18
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from c99eeea to 34605fd Compare July 29, 2026 13:40
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch 2 times, most recently from 2fa436b to ba6e97f Compare July 30, 2026 15:11
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from ba6e97f to eb63c1a Compare July 30, 2026 15:34
@sswrk

sswrk commented Jul 30, 2026

Copy link
Copy Markdown
Member Author

Also, moving an open discussion from the previous eas-cli PR in the stack: #4064

@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from eb63c1a to 5038af8 Compare July 30, 2026 15:44
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from 5038af8 to c3e6849 Compare July 30, 2026 16:12
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch 2 times, most recently from 51c7e4c to a78602b Compare July 31, 2026 13:26
@sswrk sswrk changed the title [eas-cli] Validate local composite functions referenced from workflow hooks [eas-cli][build-tools][steps] Validate local composite functions referenced from workflow hooks; extract the shared loader into steps Jul 31, 2026
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from 2c769a5 to f40e28c Compare July 31, 2026 14:46
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-eas-cli-validate-hook-composite-functions branch from f40e28c to d3a4b3a Compare July 31, 2026 17:05
@sswrk sswrk added the ai-review Commits pushed to PRs with this label be automatically reviewed. label Jul 31, 2026
@github-actions

Copy link
Copy Markdown

✅ Thank you for adding the changelog entry!

@github-actions

Copy link
Copy Markdown

🤖 AI code review

Decision: Approve with comments

The reviewers found warning-level issues in workflow validation and the shared loader. The CLI can miss static custom roots or fail on ignored hooks. The loader can misclassify bad config and echo control characters from a path.

🟡 Warning (4)

  • Validation skips jobs with a custom project rootpackages/eas-cli/src/commandUtils/workflow/compositeFunctions.ts:25 (correctness) · id:a6887c3cc9e3
    This branch skips all local composite function checks for the job. If project_root_directory is a literal path, the CLI can resolve it. A missing function then passes eas workflow:validate, but the build fails later when build-tools parses the job.
    Suggestion: For literal project_root_directory values, validate with path.join(projectDir, project_root_directory). Skip only interpolated roots.
  • Invalid function config uses the wrong error typepackages/steps/src/utils/localCompositeFunctions.ts:147 (correctness) · id:0cd549fea4a3
    The loader reads user workflow config. Sibling config paths throw BuildConfigError for invalid config. This code throws a plain Error for a bad function.yml file. Build tools can then classify it as UNKNOWN_ERROR instead of a user config error.
    Suggestion: Throw BuildConfigError for local composite function read, parse, validation, and missing-file errors.
  • Path text can inject terminal control characterspackages/steps/src/utils/localCompositeFunctions.ts:159 (security) · id:323547c146b0
    A workflow controls compositeFunctionPath through the uses value. If the file is missing, this error includes that value. An attacker can put ANSI or control characters in the path. The CLI or build log can then show spoofed output.
    Suggestion: Strip ANSI and control characters from compositeFunctionPath before you put it in errors or logs.
  • Validation loads composites from ignored hookspackages/eas-cli/src/commandUtils/workflow/compositeFunctions.ts:50 (correctness) · id:3eadab58048d
    stepsFromWorkflow adds steps from every hook key. The build parser only loads hook composites for anchors that the job runs. A workflow with an unused hook can now fail eas workflow:validate on a missing local composite function. The build would skip that hook and continue.
    Suggestion: Load hook composite functions only for hook anchors that the job can run. Otherwise keep the parser behavior and skip unused hooks.

This review is advisory — it never blocks a merge and never auto-approves.

@sswrk

sswrk commented Jul 31, 2026

Copy link
Copy Markdown
Member Author

Re. AI review:

Validation skips jobs with a custom project root

That's a deliberate skip. Validating it would need injecting the vcs client. Will change it if human reviewers advise doing so

Invalid function config uses the wrong error type

Not specific to this PR, worth considering as a follow up

Path text can inject terminal control characters

not specific to this PR, a general pattern

Validation loads composites from ignored hooks

It does hold up, but it was deliberate: we can only do static verification here, so that's the only way we can check those functions within workflow:validate. Will change it if human reviewers advise doing so.

@sswrk
sswrk marked this pull request as ready for review July 31, 2026 18:11
@sswrk
sswrk requested review from hSATAC and sjchmiela July 31, 2026 18:11
@github-actions

Copy link
Copy Markdown

Subscribed to pull request

File Patterns Mentions
packages/eas-cli/** @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

ai-review Commits pushed to PRs with this label be automatically reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant