Add infrastructure for background step support: thread safety, SDK models, and results plumbing#4416
Open
lokesh755 wants to merge 4 commits into
Open
Add infrastructure for background step support: thread safety, SDK models, and results plumbing#4416lokesh755 wants to merge 4 commits into
lokesh755 wants to merge 4 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds first-class “background step” support to the runner by allowing action steps to run concurrently, introducing control steps (wait / wait-all / cancel), and surfacing related metadata to the Results service contracts.
Changes:
- Introduces new pipeline step types (Wait, WaitAll, Cancel) and runner step implementations to coordinate background execution.
- Extends
StepsRunnerto start background actions concurrently, provide wait/cancel semantics, and attempt to isolate per-step GitHub context. - Adds Results/RunService contract fields and L0 tests to validate concurrency and metadata propagation.
Show a summary per file
| File | Description |
|---|---|
| src/Test/L0/Worker/BackgroundStepsL0.cs | Adds L0 coverage for concurrent background steps, waits, cancels, and steps-context thread-safety. |
| src/Sdk/WebApi/WebApi/ResultsHttpClient.cs | Adds mapping of timeline record variables into workflow step payloads (but also includes debug file logging). |
| src/Sdk/WebApi/WebApi/Contracts.cs | Extends Results service Step contract with background/control-step metadata DTOs. |
| src/Sdk/RSWebApi/Contracts/StepResult.cs | Extends RunService step result contract with background/control-step metadata fields. |
| src/Sdk/DTPipelines/Pipelines/WaitStep.cs | Adds pipeline model for a “wait” step. |
| src/Sdk/DTPipelines/Pipelines/WaitAllStep.cs | Adds pipeline model for a “wait-all” step. |
| src/Sdk/DTPipelines/Pipelines/CancelStep.cs | Adds pipeline model for a “cancel” step. |
| src/Sdk/DTPipelines/Pipelines/StepConverter.cs | Enables JSON deserialization for new step types. |
| src/Sdk/DTPipelines/Pipelines/Step.cs | Registers new known step types and extends the StepType enum. |
| src/Sdk/DTPipelines/Pipelines/ActionStep.cs | Adds Background flag and ensures it’s cloned. |
| src/Runner.Worker/WaitStepRunner.cs | Adds runner step type for “wait” control step. |
| src/Runner.Worker/WaitAllStepRunner.cs | Adds runner step type for “wait-all” control step. |
| src/Runner.Worker/CancelStepRunner.cs | Adds runner step type for “cancel” control step. |
| src/Runner.Worker/StepsRunner.cs | Implements background execution, wait/wait-all/cancel handling, slot limiting, and GitHubContext isolation. |
| src/Runner.Worker/StepsContext.cs | Adds locking around step output/outcome/conclusion mutations. |
| src/Runner.Worker/JobExtension.cs | Wires new pipeline step types into job initialization and sets timeline variables for results metadata. |
| src/Runner.Worker/ExecutionContext.cs | Adds timeline-record variable setter and maps those variables into StepResult; adjusts template evaluator feature gating. |
| src/Runner.Worker/BackgroundStepContext.cs | Adds per-background-step tracking (task, CTS, result, external id). |
Copilot's findings
Comments suppressed due to low confidence (4)
src/Sdk/WebApi/WebApi/ResultsHttpClient.cs:568
File.AppendAllText("/tmp/bg-steps-debug.log", ...)is executed without a try/catch here. If the path is unavailable (e.g., Windows runner, locked filesystem, permissions), it will throw and can break step updates. Remove this statement or guard it behind safe, platform-appropriate diagnostics.
System.IO.File.AppendAllText("/tmp/bg-steps-debug.log", $"[BG-DEBUG] Result: name={step.Name}, isBackground={step.IsBackground}, stepType={step.StepType}\n");
return step;
src/Sdk/WebApi/WebApi/ResultsHttpClient.cs:644
- Serializing and logging full
StepsUpdateRequestJSON payloads to/tmpcan expose tokens/PII (steps metadata can contain user-controlled values) and adds unbounded I/O in a hot path. Please remove this debug logging or route it through existing trace logging with appropriate redaction and opt-in controls.
// DEBUG: Serialize and log the JSON payload
try
{
var json = Newtonsoft.Json.JsonConvert.SerializeObject(request, Newtonsoft.Json.Formatting.None);
System.IO.File.AppendAllText("/tmp/bg-steps-debug.log", $"[BG-DEBUG] JSON payload: {json}\n");
}
catch (Exception ex)
{
System.IO.File.AppendAllText("/tmp/bg-steps-debug.log", $"[BG-DEBUG] Serialize error: {ex.Message}\n");
}
src/Runner.Worker/StepsRunner.cs:605
- In
HandleWaitAsync, the localcompletedis assigned but never used. With TreatWarningsAsErrors enabled in this repo, this will fail the build. Remove the unused assignment (or use it if intended).
Trace.Info($"Waiting for {tasks.Count} background step(s)...");
var cancelTask = Task.Delay(Timeout.Infinite, cancellationToken);
var completed = await Task.WhenAny(Task.WhenAll(tasks), cancelTask);
if (cancellationToken.IsCancellationRequested)
{
src/Runner.Worker/JobExtension.cs:494
- This sets
cancel_step_idon the cancel step timeline record to the logical step id, but later logic expects to publish the background step's external/timeline id. IfStepsRunnercan't resolve the id, this logical value will be reported upstream. Consider deferring this variable until you can resolve the external id (or always overwrite/clear it during execution).
cancelRunner.ExecutionContext.SetTimelineRecordVariable("step_type", "cancel");
if (!string.IsNullOrEmpty(cancelRunner.CancelStepId))
{
cancelRunner.ExecutionContext.SetTimelineRecordVariable("cancel_step_id", cancelRunner.CancelStepId);
}
- Files reviewed: 18/18 changed files
- Comments generated: 14
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
ericsciple
reviewed
May 12, 2026
gordolara789-tech
approved these changes
May 13, 2026
gordolara789-tech
approved these changes
May 13, 2026
ericsciple
reviewed
May 14, 2026
ericsciple
reviewed
May 14, 2026
ericsciple
reviewed
May 14, 2026
ericsciple
reviewed
May 19, 2026
a3ba289 to
affa859
Compare
affa859 to
3825592
Compare
TingluoHuang
reviewed
Jun 3, 2026
TingluoHuang
reviewed
Jun 3, 2026
TingluoHuang
reviewed
Jun 3, 2026
TingluoHuang
reviewed
Jun 3, 2026
TingluoHuang
reviewed
Jun 3, 2026
TingluoHuang
reviewed
Jun 3, 2026
TingluoHuang
reviewed
Jun 3, 2026
3825592 to
09598e3
Compare
Thread safety: - Add CollectionLock to GlobalContext, wrap shared collections (JobTelemetry, DeprecatedNode20Actions, etc.) with locks SDK step models: - Add unified BackgroundStepControl pipeline type with StepType enum and StepConverter deserialization support Timeline & results plumbing: - Extend TimelineRecord with IsBackground, BackgroundControlType, BackgroundControlStepIds, and ParallelGroupId - Wire through ResultsHttpClient and JobServerQueue merge logic StepsContext: - Refactor to use CollectionLock for thread-safe outcome/conclusion/ output writes from concurrent background steps
09598e3 to
bc2513d
Compare
TingluoHuang
reviewed
Jun 3, 2026
| [DataMember(EmitDefaultValue = false)] | ||
| public bool IsBackground; | ||
| [DataMember(EmitDefaultValue = false)] | ||
| [JsonProperty("backgroundControlType")] |
Member
There was a problem hiding this comment.
do we need to provide [JsonProperty("backgroundControlType")]?
TingluoHuang
reviewed
Jun 3, 2026
| public string[] StepIds => Data?.StepIds; | ||
|
|
||
| [JsonIgnore] | ||
| public string ParallelGroupId => Data?.ParallelGroupId; |
Member
There was a problem hiding this comment.
should we move it to JobStep?
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 free
to 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.
Summary
Add infrastructure for background step support
This PR lays the groundwork for background steps by adding the necessary infrastructure without changing step execution behavior.
Thread safety
Adds
CollectionLocktoGlobalContextand wraps all concurrent access to shared collections (JobTelemetry,DeprecatedNode20Actions, etc.) with locks, preparing for multiple steps executing simultaneously.SDK step models
Introduces
WaitStep,WaitAllStep, andCancelSteppipeline types alongsideStepTypeenum extensions andStepConverterdeserialization support.Timeline & results plumbing
Extends
TimelineRecordwithIsBackground,StepType,WaitStepIds,CancelStepId, andParallelGroupIdfields. Wires them throughResultsHttpClientandJobServerQueuemerge logic.StepsContext
Refactors locking to use
CollectionLockfor thread-safe outcome/conclusion/output writes from concurrent background steps.Related to
https://github.com/github/actions-runtime/issues/5420
https://github.com/github/actions-runtime/issues/5421