Skip to content

[steps][3/4] Interpolate composite function inputs - #4014

Merged
sswrk merged 6 commits into
mainfrom
szymonswierk/eng-22387-steps-interpolate-action-inputs
Jul 24, 2026
Merged

[steps][3/4] Interpolate composite function inputs#4014
sswrk merged 6 commits into
mainfrom
szymonswierk/eng-22387-steps-interpolate-action-inputs

Conversation

@sswrk

@sswrk sswrk commented Jul 14, 2026

Copy link
Copy Markdown
Member

Why

This PR stack adds support for local composite functions in workflows. After structural expansion, inner steps still could not see ${{ inputs.* }}, and caller env did not yet reach them at runtime.

This PR adds the data layer: input declaration, validation and interpolation, and runtime resolution of the caller context (env, templated working directories).

How

  • CompositeFunctionExpander builds declared composite function inputs as real BuildStepInput instances
  • BuildStepCompositeFunctionScope exposes ${{ inputs.* }} as lazy getters
  • Caller env and working directory templating resolve through the composite function scope

Test Plan

Added inputs and scoping unit tests.

@linear-code

linear-code Bot commented Jul 14, 2026

Copy link
Copy Markdown

ENG-22387

sswrk commented Jul 14, 2026

Copy link
Copy Markdown
Member Author

@sswrk sswrk changed the title [steps] Interpolate action inputs [steps][4/5] Interpolate action inputs Jul 14, 2026
@codecov

codecov Bot commented Jul 14, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.31461% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.45%. Comparing base (a8e8c79) to head (6c9c8dc).

Files with missing lines Patch % Lines
packages/steps/src/CompositeFunctionExpander.ts 95.46% 1 Missing and 1 partial ⚠️
.../steps/src/utils/compositeFunctionInterpolation.ts 93.75% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4014      +/-   ##
==========================================
+ Coverage   61.29%   61.45%   +0.16%     
==========================================
  Files         977      978       +1     
  Lines       43687    43829     +142     
  Branches     9186     9216      +30     
==========================================
+ Hits        26774    26931     +157     
+ Misses      15465    15450      -15     
  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 changed the base branch from szymonswierk/eng-22387-steps-expand-custom-actions-at-parse-time to graphite-base/4014 July 14, 2026 13:02
@sswrk
sswrk force-pushed the graphite-base/4014 branch from 66faed4 to 6ac7f96 Compare July 14, 2026 13:02
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-interpolate-action-inputs branch from c5be755 to 68b6cdf Compare July 14, 2026 13:02
@sswrk
sswrk changed the base branch from graphite-base/4014 to szymonswierk/eng-22387-steps-action-input-interpolation July 14, 2026 13:02
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-interpolate-action-inputs branch from 68b6cdf to e31b8bc Compare July 14, 2026 13:25
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-action-input-interpolation branch from 6ac7f96 to 42ee777 Compare July 14, 2026 13:25
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-interpolate-action-inputs branch from e31b8bc to 7937bd7 Compare July 14, 2026 13:35
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-action-input-interpolation branch 2 times, most recently from 30592bd to ff1c31a Compare July 14, 2026 13:44
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-interpolate-action-inputs branch 2 times, most recently from 709a789 to 1b037d3 Compare July 14, 2026 14:12
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-action-input-interpolation branch 2 times, most recently from 4ed949b to 7428b06 Compare July 14, 2026 14:28
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-interpolate-action-inputs branch from 1b037d3 to 051d1fa Compare July 14, 2026 14:28
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-action-input-interpolation branch from 7428b06 to 83c2e00 Compare July 14, 2026 14:57
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-interpolate-action-inputs branch from 051d1fa to 515cccd Compare July 14, 2026 14:57
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-action-input-interpolation branch from 83c2e00 to 7082ec5 Compare July 14, 2026 16:04
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-interpolate-action-inputs branch from 515cccd to 901e44a Compare July 14, 2026 16:04
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-action-input-interpolation branch from 7082ec5 to d5d55e3 Compare July 14, 2026 17:37
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-interpolate-action-inputs branch from 8c24784 to 6613344 Compare July 17, 2026 14:23
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-expand-custom-actions-at-parse-time branch from a41b6a3 to 26aa626 Compare July 17, 2026 14:23
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-interpolate-action-inputs branch from 6613344 to c92328e Compare July 17, 2026 14:33
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-expand-custom-actions-at-parse-time branch from 26aa626 to 73e1e7e Compare July 17, 2026 14:33
@sswrk
sswrk marked this pull request as ready for review July 17, 2026 16:17

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.

Tip: disable this comment in your organization's Code Review settings.

@sswrk
sswrk requested review from hSATAC and sjchmiela July 17, 2026 16:17

@hSATAC hSATAC left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great — the scoping (lazy inputs, caller/callee resolution, env boundary, the if memoization) is subtle but holds together, and the tests are really thorough. One naming nit inline, not blocking.


const compositeFunction = this.lookupCompositeFunction(compositeFunctionPath);
const innerSteps = compositeFunction.runs.steps;
const action = this.lookupCompositeFunction(compositeFunctionPath);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small naming thing: this renames the compositeFunction local to action (also in lookupCompositeFunction) and adds buildActionInputs. Everything else sticks to "composite function", so "action" stands out a bit — mind keeping these as compositeFunction / buildCompositeFunctionInputs?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes yes thank you for catching this! I originally implememented everything with the "actions" naming but then decided that "composite functions" fits this better and renamed. This one got away.

@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-expand-custom-actions-at-parse-time branch from 73e1e7e to 7a0d089 Compare July 21, 2026 14:19
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-interpolate-action-inputs branch 2 times, most recently from 672cc04 to 7e50713 Compare July 22, 2026 14:16
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-expand-custom-actions-at-parse-time branch from 7a0d089 to c50f0a3 Compare July 22, 2026 14:16
@sswrk
sswrk force-pushed the szymonswierk/eng-22387-steps-interpolate-action-inputs branch from 7e50713 to c88f637 Compare July 23, 2026 14:40
/** Merge call-site env from each scope level, outer to inner; inner keys win. */
public resolveInheritedEnv(base: JobInterpolationContext): BuildStepEnv {
const parentEnv = this.parent?.resolveInheritedEnv(base) ?? {};
return { ...parentEnv, ...this.resolveScopeEnv(base) };

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

codex tells me

# Workflow
- uses: ./outer
  env:
    OUTER: visible

and

# outer/function.yml
runs:
  steps:
    - uses: ./inner
      env:
        COPIED: ${{ env.OUTER }}

won't work if we don't add parentEnv to resolveScopeEnv call

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 81acb14

expect(error.message).toMatch(/Invalid identifier "name"/);
});

it('skips the allowed_values check for a reference-valued constrained input', async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this right? should we skip the allowed values check for interpolated values?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test documents a deliberate decision to not implement this check.

The reason behind this decision is to stay consistent with how things work currently in workflows (and custom builds), e.g. this

jobs:
  build:
    steps:
      - id: pick
        run: set-output artifact_type application-archive
        outputs: [artifact_type]
      - uses: eas/upload_artifact
        with:
          type: ${{ steps.pick.outputs.artifact_type }}

Even though type input in upload_artifact has a set of "allowed values", it's not validated for interpolated values.

We could add the runtime check globally in BuildStepInput, but I worry we would break things for users that depend on this behavior.

Alternatively, we can add this check just for reusable functions, so that there's validation from day one in this new feature.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

of course it's "already broken", what was i thinking

).toBe('');
});

it('does not error for a missing required input that is never referenced', async () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a bit weird too

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually let's harden it. Made it eagerly require something present when required: true in parse time. But not adding validations that the interpolation resolves to something (not undefined) for the same reasons as in the previous comment, documented it in a test too.

@github-actions

Copy link
Copy Markdown

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

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.

3 participants