Uh oh!
There was an error while loading. Please reload this page.
Setup Job: announce when running with locked dependencies - #4546
Merged
ericsciple merged 1 commit intoJul 9, 2026
Merged
Conversation
nodeselectorforce-pushed
the
nodeselector-setup-job-lockfile-feedback
branch
from
July 9, 2026 02:57
ea074f8 to
f1ad95eComparenodeselector
marked this pull request as ready for review
July 9, 2026 17:34
ericsciple
approved these changes
Jul 9, 2026
ericsciple
enabled auto-merge (squash)
July 9, 2026 17:35
When a job arrives with lockfile-pinned action dependencies (message.ActionsDependencies is non-empty), the 'Set up job' log now prints 'Running with locked dependencies' so users get clear feedback that pinning is in effect. Gated purely on the presence of dependency data — no lockfile means no line, so no feature flag is needed. Also fixes a pre-existing env-var leak in JobExtensionBuildFailsWithoutContainerIfRequired, which set RequireJobContainer=true without resetting it and could fail unrelated tests that run afterward. Towards: github/actions-dispatch#567
ericscipleforce-pushed
the
nodeselector-setup-job-lockfile-feedback
branch
from
July 9, 2026 17:36
f1ad95e to
a7510caCompareContributor
There was a problem hiding this comment.
Pull request overview
This PR adds a user-visible cue in the Set up job log when the server provides lockfile-pinned action dependency data, and updates L0 coverage to validate the presence/absence of that cue. It also fixes a pre-existing test process-wide environment variable leak that was making the new tests order-dependent.
Changes:
- Emit
Running with locked dependenciesduring job initialization whenmessage.ActionsDependenciesis non-empty. - Add L0 tests asserting the new log line appears only when dependency data is present.
- Reset
RequireJobContainerenv var in the existing L0 test to avoid cross-test leakage.
Show a summary per file
| File | Description |
|---|---|
| src/Runner.Worker/JobExtension.cs | Outputs a single “locked dependencies” announcement during setup when the job message includes dependency pins. |
| src/Test/L0/Worker/JobExtensionL0.cs | Adds L0 coverage for the new setup output and fixes an env-var leak that could break tests depending on execution order. |
Review details
Comments suppressed due to low confidence (1)
src/Test/L0/Worker/JobExtensionL0.cs:298
- The test now clears RequireJobContainer by setting the env var to null in finally. If the variable was already set (e.g., in CI or when running tests with custom env), this changes global process state and can cause ordering-dependent failures. Capture the original value before setting it and restore that exact value in the finally block.
[InlineData("write")]
[InlineData("write-only")]
public async Task InitializeJob_LogsCacheMode_WhenVariableSet(string mode)
{
using (TestHostContext hc = CreateTestContext())
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Low
Uh oh!
There was an error while loading. Please reload this page.
thboop
reviewed
Jul 9, 2026
| // action dependencies (a lockfile is in effect). | ||
| if (message.ActionsDependencies != null && message.ActionsDependencies.Count > 0) | ||
| { | ||
| context.Output("Running with locked dependencies"); |
Collaborator
There was a problem hiding this comment.
Suggested change
| context.Output("Running with locked dependencies"); | |
| context.Output("Using pinned Action versions"); |
Collaborator
There was a problem hiding this comment.
This reads a little clearer to me but its mostly a nit comment
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
When a job runs with lockfile-pinned action dependencies, the runner already receives the resolved pins (via
#4372 propagate actions dependencies) but gives the user no signal that pinning is in effect. This closes that feedback gap in the most visible place: the Set up job log.What
In
JobExtension.InitializeJob, the "Set up job" block now prints a single line whenever the job message carries a non-emptyActionsDependencieslist:No lockfile means an empty list means no line, so the behavior is gated purely on the presence of dependency data. No feature flag is needed. This is intentionally a single announcement line, not an enumerated list, per the product decision for this iteration.
Note on the tracking issue title
The tracking issue's title says "show pinned dependencies" (i.e. enumerate them). The shipped behavior here is the minimal boolean-style line, not an enumeration, per the product owner's direction. The issue may be retitled separately. Flagging so reviewers aren't confused by the title vs. behavior mismatch.
Bundled unrelated fix (please note)
This PR also fixes a pre-existing env-var leak in the test
JobExtensionBuildFailsWithoutContainerIfRequired: it setRequireJobContainer=trueviaEnvironment.SetEnvironmentVariableand never reset it. Because L0 tests share a process, that leak caused the two new lockfile tests to fail depending on ordering (they hitValidateJobContainerand threw). I wrapped that test's body in try/finally to reset the variable.It is unrelated to the feature but was necessary to make the new tests reliable, which is why an otherwise-untouched test changed. It is a small, self-contained change; happy to split it into its own PR if reviewers prefer.
Testing
JobExtensionL0suite: 27/27 passing via the repo's local SDK (./dev.sh)../dev.sh formatcrashes in my local environment (a Mono build-host crash insidedotnet format, reproducible on a clean tree too), so I matched the surrounding code style manually. Please rely on the CI format check for formatting validation rather than assuming local format was run.Changes
src/Runner.Worker/JobExtension.cs- emitRunning with locked dependencieswhenmessage.ActionsDependenciesis non-empty.src/Test/L0/Worker/JobExtensionL0.cs- two L0 tests (line present when deps present, absent when empty) plus the env-var leak fix described above.Towards: github/actions-dispatch#567