From 7ce591912686fd07ac22742ac2beaa8ea8404682 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Fri, 11 Jul 2025 13:29:47 +0000 Subject: [PATCH 01/23] node 24 initial --- docs/checks/nodejs.md | 4 +- src/Misc/externals.sh | 11 +++ src/Misc/layoutbin/update.sh.template | 11 ++- src/Runner.Common/Util/NodeUtil.cs | 2 +- src/Runner.Worker/ActionManifestManager.cs | 7 +- src/Test/L0/Worker/ActionManagerL0.cs | 71 +++++++++++++++++++ src/Test/L0/Worker/ActionManifestManagerL0.cs | 45 +++++++++++- src/Test/L0/Worker/HandlerFactoryL0.cs | 1 + src/Test/L0/Worker/Handlers/NodeHandlerL0.cs | 35 +++++++++ src/Test/L0/Worker/StepHostL0.cs | 54 ++++++++++++++ src/Test/TestData/node24action.yml | 20 ++++++ 11 files changed, 251 insertions(+), 10 deletions(-) create mode 100644 src/Test/L0/Worker/Handlers/NodeHandlerL0.cs create mode 100644 src/Test/TestData/node24action.yml diff --git a/docs/checks/nodejs.md b/docs/checks/nodejs.md index 7308dcadc50..62b2958cd95 100644 --- a/docs/checks/nodejs.md +++ b/docs/checks/nodejs.md @@ -4,9 +4,9 @@ Make sure the built-in node.js has access to GitHub.com or GitHub Enterprise Server. -The runner carries its own copy of node.js executable under `/externals/node20/`. +The runner carries its own copies of node.js executables under `/externals/node20/` and `/externals/node24/`. -All javascript base Actions will get executed by the built-in `node` at `/externals/node20/`. +All javascript base Actions will get executed by the built-in `node` at either `/externals/node20/` or `/externals/node24/` depending on the version specified in the action's metadata. > Not the `node` from `$PATH` diff --git a/src/Misc/externals.sh b/src/Misc/externals.sh index c6c893a0be6..4d6b043da0a 100755 --- a/src/Misc/externals.sh +++ b/src/Misc/externals.sh @@ -7,6 +7,7 @@ NODE_ALPINE_URL=https://github.com/actions/alpine_nodejs/releases/download # When you update Node versions you must also create a new release of alpine_nodejs at that updated version. # Follow the instructions here: https://github.com/actions/alpine_nodejs?tab=readme-ov-file#getting-started NODE20_VERSION="20.19.3" +NODE24_VERSION="24.4.0" get_abs_path() { # exploits the fact that pwd will print abs path when no args @@ -139,6 +140,8 @@ function acquireExternalTool() { if [[ "$PACKAGERUNTIME" == "win-x64" || "$PACKAGERUNTIME" == "win-x86" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/$PACKAGERUNTIME/node.exe" node20/bin acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/$PACKAGERUNTIME/node.lib" node20/bin + acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/$PACKAGERUNTIME/node.exe" node24/bin + acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/$PACKAGERUNTIME/node.lib" node24/bin if [[ "$PRECACHE" != "" ]]; then acquireExternalTool "https://github.com/microsoft/vswhere/releases/download/2.6.7/vswhere.exe" vswhere fi @@ -149,6 +152,8 @@ if [[ "$PACKAGERUNTIME" == "win-arm64" ]]; then # todo: replace these with official release when available acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/$PACKAGERUNTIME/node.exe" node20/bin acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/$PACKAGERUNTIME/node.lib" node20/bin + acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/$PACKAGERUNTIME/node.exe" node24/bin + acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/$PACKAGERUNTIME/node.lib" node24/bin if [[ "$PRECACHE" != "" ]]; then acquireExternalTool "https://github.com/microsoft/vswhere/releases/download/2.6.7/vswhere.exe" vswhere fi @@ -157,23 +162,29 @@ fi # Download the external tools only for OSX. if [[ "$PACKAGERUNTIME" == "osx-x64" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-darwin-x64.tar.gz" node20 fix_nested_dir + acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-darwin-x64.tar.gz" node24 fix_nested_dir fi if [[ "$PACKAGERUNTIME" == "osx-arm64" ]]; then # node.js v12 doesn't support macOS on arm64. acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-darwin-arm64.tar.gz" node20 fix_nested_dir + acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-darwin-arm64.tar.gz" node24 fix_nested_dir fi # Download the external tools for Linux PACKAGERUNTIMEs. if [[ "$PACKAGERUNTIME" == "linux-x64" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-x64.tar.gz" node20 fix_nested_dir acquireExternalTool "$NODE_ALPINE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-alpine-x64.tar.gz" node20_alpine + acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-linux-x64.tar.gz" node24 fix_nested_dir + acquireExternalTool "$NODE_ALPINE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-alpine-x64.tar.gz" node24_alpine fi if [[ "$PACKAGERUNTIME" == "linux-arm64" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-arm64.tar.gz" node20 fix_nested_dir + acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-linux-arm64.tar.gz" node24 fix_nested_dir fi if [[ "$PACKAGERUNTIME" == "linux-arm" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-armv7l.tar.gz" node20 fix_nested_dir + acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-linux-armv7l.tar.gz" node24 fix_nested_dir fi diff --git a/src/Misc/layoutbin/update.sh.template b/src/Misc/layoutbin/update.sh.template index 832385edd5f..476ae11e57d 100755 --- a/src/Misc/layoutbin/update.sh.template +++ b/src/Misc/layoutbin/update.sh.template @@ -135,12 +135,17 @@ if [[ "$currentplatform" == 'darwin' && restartinteractiverunner -eq 0 ]]; then then # inspect the open file handles to find the node process # we can't actually inspect the process using ps because it uses relative paths and doesn't follow symlinks - nodever="node20" + # Try finding node24 first, then fallback to earlier versions if needed + nodever="node24" path=$(lsof -a -g "$procgroup" -F n | grep $nodever/bin/node | grep externals | tail -1 | cut -c2-) - if [[ $? -ne 0 || -z "$path" ]] # Fallback if RunnerService.js was started with node16 + if [[ $? -ne 0 || -z "$path" ]] # Fallback if RunnerService.js was started with node20 then - nodever="node16" + nodever="node20" path=$(lsof -a -g "$procgroup" -F n | grep $nodever/bin/node | grep externals | tail -1 | cut -c2-) + if [[ $? -ne 0 || -z "$path" ]] # Fallback if RunnerService.js was started with node16 + then + nodever="node16" + path=$(lsof -a -g "$procgroup" -F n | grep $nodever/bin/node | grep externals | tail -1 | cut -c2-) if [[ $? -ne 0 || -z "$path" ]] # Fallback if RunnerService.js was started with node12 then nodever="node12" diff --git a/src/Runner.Common/Util/NodeUtil.cs b/src/Runner.Common/Util/NodeUtil.cs index 1a9252cde0f..170a3e58fba 100644 --- a/src/Runner.Common/Util/NodeUtil.cs +++ b/src/Runner.Common/Util/NodeUtil.cs @@ -6,7 +6,7 @@ namespace GitHub.Runner.Common.Util public static class NodeUtil { private const string _defaultNodeVersion = "node20"; - public static readonly ReadOnlyCollection BuiltInNodeVersions = new(new[] { "node20" }); + public static readonly ReadOnlyCollection BuiltInNodeVersions = new(new[] { "node20", "node24" }); public static string GetInternalNodeVersion() { var forcedInternalNodeVersion = Environment.GetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion); diff --git a/src/Runner.Worker/ActionManifestManager.cs b/src/Runner.Worker/ActionManifestManager.cs index 351c2427daf..c731b3d5d5c 100644 --- a/src/Runner.Worker/ActionManifestManager.cs +++ b/src/Runner.Worker/ActionManifestManager.cs @@ -450,7 +450,8 @@ private ActionExecutionData ConvertRuns( } else if (string.Equals(usingToken.Value, "node12", StringComparison.OrdinalIgnoreCase) || string.Equals(usingToken.Value, "node16", StringComparison.OrdinalIgnoreCase) || - string.Equals(usingToken.Value, "node20", StringComparison.OrdinalIgnoreCase)) + string.Equals(usingToken.Value, "node20", StringComparison.OrdinalIgnoreCase) || + string.Equals(usingToken.Value, "node24", StringComparison.OrdinalIgnoreCase)) { if (string.IsNullOrEmpty(mainToken?.Value)) { @@ -490,7 +491,7 @@ private ActionExecutionData ConvertRuns( } else { - throw new ArgumentOutOfRangeException($"'using: {usingToken.Value}' is not supported, use 'docker', 'node12', 'node16' or 'node20' instead."); + throw new ArgumentOutOfRangeException($"'using: {usingToken.Value}' is not supported, use 'docker', 'node12', 'node16', 'node20' or 'node24' instead."); } } else if (pluginToken != null) @@ -501,7 +502,7 @@ private ActionExecutionData ConvertRuns( }; } - throw new NotSupportedException("Missing 'using' value. 'using' requires 'composite', 'docker', 'node12', 'node16' or 'node20'."); + throw new NotSupportedException("Missing 'using' value. 'using' requires 'composite', 'docker', 'node12', 'node16', 'node20' or 'node24'."); } private void ConvertInputs( diff --git a/src/Test/L0/Worker/ActionManagerL0.cs b/src/Test/L0/Worker/ActionManagerL0.cs index 50d5b99d106..8ba4051c4ce 100644 --- a/src/Test/L0/Worker/ActionManagerL0.cs +++ b/src/Test/L0/Worker/ActionManagerL0.cs @@ -1659,6 +1659,77 @@ public void LoadsNode20ActionDefinition() Teardown(); } } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void LoadsNode24ActionDefinition() + { + try + { + // Arrange. + Setup(); + const string Content = @" +# Container action +name: 'Hello World' +description: 'Greet the world and record the time' +author: 'GitHub' +inputs: + greeting: # id of input + description: 'The greeting we choose - will print ""{greeting}, World!"" on stdout' + required: true + default: 'Hello' + entryPoint: # id of input + description: 'optional docker entrypoint overwrite.' + required: false +outputs: + time: # id of output + description: 'The time we did the greeting' +icon: 'hello.svg' # vector art to display in the GitHub Marketplace +color: 'green' # optional, decorates the entry in the GitHub Marketplace +runs: + using: 'node24' + main: 'task.js' +"; + Pipelines.ActionStep instance; + string directory; + CreateAction(yamlContent: Content, instance: out instance, directory: out directory); + + // Act. + Definition definition = _actionManager.LoadAction(_ec.Object, instance); + + // Assert. + Assert.NotNull(definition); + Assert.Equal(directory, definition.Directory); + Assert.NotNull(definition.Data); + Assert.NotNull(definition.Data.Inputs); // inputs + Dictionary inputDefaults = new(StringComparer.OrdinalIgnoreCase); + foreach (var input in definition.Data.Inputs) + { + var name = input.Key.AssertString("key").Value; + var value = input.Value.AssertScalar("value").ToString(); + + _hc.GetTrace().Info($"Default: {name} = {value}"); + inputDefaults[name] = value; + } + + Assert.Equal(2, inputDefaults.Count); + Assert.True(inputDefaults.ContainsKey("greeting")); + Assert.Equal("Hello", inputDefaults["greeting"]); + Assert.True(string.IsNullOrEmpty(inputDefaults["entryPoint"])); + Assert.NotNull(definition.Data.Execution); // execution + + Assert.NotNull(definition.Data.Execution as NodeJSActionExecutionData); + Assert.Equal("task.js", (definition.Data.Execution as NodeJSActionExecutionData).Script); + Assert.Equal("node24", (definition.Data.Execution as NodeJSActionExecutionData).NodeVersion); + } + finally + { + Teardown(); + } + } + + [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] diff --git a/src/Test/L0/Worker/ActionManifestManagerL0.cs b/src/Test/L0/Worker/ActionManifestManagerL0.cs index 91f604c0645..dae75c8f600 100644 --- a/src/Test/L0/Worker/ActionManifestManagerL0.cs +++ b/src/Test/L0/Worker/ActionManifestManagerL0.cs @@ -502,6 +502,49 @@ public void Load_Node20Action() } } + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Load_Node24Action() + { + try + { + //Arrange + Setup(); + + var actionManifest = new ActionManifestManager(); + actionManifest.Initialize(_hc); + + //Act + var result = actionManifest.Load(_ec.Object, Path.Combine(TestUtil.GetTestDataPath(), "node24action.yml")); + + //Assert + Assert.Equal("Hello World", result.Name); + Assert.Equal("Greet the world and record the time", result.Description); + Assert.Equal(2, result.Inputs.Count); + Assert.Equal("greeting", result.Inputs[0].Key.AssertString("key").Value); + Assert.Equal("Hello", result.Inputs[0].Value.AssertString("value").Value); + Assert.Equal("entryPoint", result.Inputs[1].Key.AssertString("key").Value); + Assert.Equal("", result.Inputs[1].Value.AssertString("value").Value); + Assert.Equal(1, result.Deprecated.Count); + + Assert.True(result.Deprecated.ContainsKey("greeting")); + result.Deprecated.TryGetValue("greeting", out string value); + Assert.Equal("This property has been deprecated", value); + + Assert.Equal(ActionExecutionType.NodeJS, result.Execution.ExecutionType); + + var nodeAction = result.Execution as NodeJSActionExecutionData; + + Assert.Equal("main.js", nodeAction.Script); + Assert.Equal("node24", nodeAction.NodeVersion); + } + finally + { + Teardown(); + } + } + [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] @@ -758,7 +801,7 @@ public void Load_CompositeActionNoUsing() //Assert var err = Assert.Throws(() => actionManifest.Load(_ec.Object, action_path)); Assert.Contains($"Failed to load {action_path}", err.Message); - _ec.Verify(x => x.AddIssue(It.Is(s => s.Message.Contains("Missing 'using' value. 'using' requires 'composite', 'docker', 'node12', 'node16' or 'node20'.")), It.IsAny()), Times.Once); + _ec.Verify(x => x.AddIssue(It.Is(s => s.Message.Contains("Missing 'using' value. 'using' requires 'composite', 'docker', 'node12', 'node16', 'node20' or 'node24'.")), It.IsAny()), Times.Once); } finally { diff --git a/src/Test/L0/Worker/HandlerFactoryL0.cs b/src/Test/L0/Worker/HandlerFactoryL0.cs index 5f88d5211d0..48ae4898f09 100644 --- a/src/Test/L0/Worker/HandlerFactoryL0.cs +++ b/src/Test/L0/Worker/HandlerFactoryL0.cs @@ -33,6 +33,7 @@ private TestHostContext CreateTestContext([CallerMemberName] string testName = " [InlineData("node12", "node20")] [InlineData("node16", "node20")] [InlineData("node20", "node20")] + [InlineData("node24", "node24")] public void IsNodeVersionUpgraded(string inputVersion, string expectedVersion) { using (TestHostContext hc = CreateTestContext()) diff --git a/src/Test/L0/Worker/Handlers/NodeHandlerL0.cs b/src/Test/L0/Worker/Handlers/NodeHandlerL0.cs new file mode 100644 index 00000000000..78c4053f174 --- /dev/null +++ b/src/Test/L0/Worker/Handlers/NodeHandlerL0.cs @@ -0,0 +1,35 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Runtime.CompilerServices; +using System.Threading; +using System.Threading.Tasks; +using GitHub.DistributedTask.WebApi; +using GitHub.Runner.Sdk; +using GitHub.Runner.Worker; +using GitHub.Runner.Worker.Handlers; +using Moq; +using Xunit; + +namespace GitHub.Runner.Common.Tests.Worker.Handlers +{ + public sealed class NodeHandlerL0 + { + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void NodeJSActionExecutionDataSupportsNode24() + { + // Create NodeJSActionExecutionData with node24 + var nodeJSData = new NodeJSActionExecutionData + { + NodeVersion = "node24", + Script = "test.js" + }; + + // Act & Assert + Assert.Equal("node24", nodeJSData.NodeVersion); + Assert.Equal(ActionExecutionType.NodeJS, nodeJSData.ExecutionType); + } + } +} diff --git a/src/Test/L0/Worker/StepHostL0.cs b/src/Test/L0/Worker/StepHostL0.cs index 47a5d3344da..bac7d41d90a 100644 --- a/src/Test/L0/Worker/StepHostL0.cs +++ b/src/Test/L0/Worker/StepHostL0.cs @@ -162,6 +162,60 @@ public async Task DetermineNode20RuntimeVersionInUnknowContainerAsync() Assert.Equal("node20", nodeVersion); } } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async Task DetermineNode24RuntimeVersionInAlpineContainerAsync() + { + using (TestHostContext hc = CreateTestContext()) + { + // Arrange. + var sh = new ContainerStepHost(); + sh.Initialize(hc); + sh.Container = new ContainerInfo() { ContainerId = "1234abcd" }; + + _dc.Setup(d => d.DockerExec(_ec.Object, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())) + .Callback((IExecutionContext ec, string id, string options, string command, List output) => + { + output.Add("alpine"); + }) + .ReturnsAsync(0); + + // Act. + var nodeVersion = await sh.DetermineNodeRuntimeVersion(_ec.Object, "node24"); + + // Assert. + Assert.Equal("node24_alpine", nodeVersion); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async Task DetermineNode24RuntimeVersionInUnknownContainerAsync() + { + using (TestHostContext hc = CreateTestContext()) + { + // Arrange. + var sh = new ContainerStepHost(); + sh.Initialize(hc); + sh.Container = new ContainerInfo() { ContainerId = "1234abcd" }; + + _dc.Setup(d => d.DockerExec(_ec.Object, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())) + .Callback((IExecutionContext ec, string id, string options, string command, List output) => + { + output.Add("github"); + }) + .ReturnsAsync(0); + + // Act. + var nodeVersion = await sh.DetermineNodeRuntimeVersion(_ec.Object, "node24"); + + // Assert. + Assert.Equal("node24", nodeVersion); + } + } #endif } } diff --git a/src/Test/TestData/node24action.yml b/src/Test/TestData/node24action.yml new file mode 100644 index 00000000000..653e558a0c0 --- /dev/null +++ b/src/Test/TestData/node24action.yml @@ -0,0 +1,20 @@ +name: 'Hello World' +description: 'Greet the world and record the time' +author: 'Test Corporation' +inputs: + greeting: # id of input + description: 'The greeting we choose - will print ""{greeting}, World!"" on stdout' + required: true + default: 'Hello' + deprecationMessage: 'This property has been deprecated' + entryPoint: # id of input + description: 'optional docker entrypoint overwrite.' + required: false +outputs: + time: # id of output + description: 'The time we did the greeting' +icon: 'hello.svg' # vector art to display in the GitHub Marketplace +color: 'green' # optional, decorates the entry in the GitHub Marketplace +runs: + using: 'node24' + main: 'main.js' \ No newline at end of file From d43638b527f231842f81bdfc2c88a52c3dcc4b88 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Fri, 11 Jul 2025 13:39:33 +0000 Subject: [PATCH 02/23] apline update --- src/Misc/externals.sh | 74 ++++++++++++++++++++++++++++++------------- 1 file changed, 52 insertions(+), 22 deletions(-) diff --git a/src/Misc/externals.sh b/src/Misc/externals.sh index 4d6b043da0a..fe7b53214b1 100755 --- a/src/Misc/externals.sh +++ b/src/Misc/externals.sh @@ -4,7 +4,11 @@ PRECACHE=$2 NODE_URL=https://nodejs.org/dist NODE_ALPINE_URL=https://github.com/actions/alpine_nodejs/releases/download -# When you update Node versions you must also create a new release of alpine_nodejs at that updated version. + acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-x64.tar.gz" node20 fix_nested_dir + acquireExternalTool "$NODE_ALPINE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-alpine-x64.tar.gz" node20_alpine + acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-linux-x64.tar.gz" node24 fix_nested_dir + acquireExternalTool "$NODE_ALPINE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-alpine-x64.tar.gz" node24_alpine "" optional +finew release of alpine_nodejs at that updated version. # Follow the instructions here: https://github.com/actions/alpine_nodejs?tab=readme-ov-file#getting-started NODE20_VERSION="20.19.3" NODE24_VERSION="24.4.0" @@ -34,6 +38,7 @@ function acquireExternalTool() { local download_source=$1 # E.g. https://github.com/microsoft/vswhere/releases/download/2.6.7/vswhere.exe local target_dir="$LAYOUT_DIR/externals/$2" # E.g. $LAYOUT_DIR/externals/vswhere local fix_nested_dir=$3 # Flag that indicates whether to move nested contents up one directory. + local optional_tool=$4 # Flag that indicates if this tool is optional and we should continue on error # Extract the portion of the URL after the protocol. E.g. github.com/microsoft/vswhere/releases/download/2.6.7/vswhere.exe local relative_url="${download_source#*://}" @@ -68,33 +73,58 @@ function acquireExternalTool() { # --retry 3 Retries transient errors 3 times (timeouts, 5xx) if [[ "$(printf '%s\n' "7.71.0" "$CURL_VERSION" | sort -V | head -n1)" != "7.71.0" ]]; then # Curl version is less than or equal to 7.71.0, skipping retry-all-errors flag - curl -fSL --retry 3 -o "$partial_target" "$download_source" 2>"${download_target}_download.log" || checkRC 'curl' + curl -fSL --retry 3 -o "$partial_target" "$download_source" 2>"${download_target}_download.log" + local curl_exit_code=$? + if [ $curl_exit_code -ne 0 ]; then + if [ "$optional_tool" == "optional" ]; then + echo "Failed: curl failed with return code $curl_exit_code. This tool is optional, continuing." + else + checkRC 'curl' + fi + fi else # Curl version is greater than 7.71.0, running curl with --retry-all-errors flag - curl -fSL --retry 3 --retry-all-errors -o "$partial_target" "$download_source" 2>"${download_target}_download.log" || checkRC 'curl' + curl -fSL --retry 3 --retry-all-errors -o "$partial_target" "$download_source" 2>"${download_target}_download.log" + local curl_exit_code=$? + if [ $curl_exit_code -ne 0 ]; then + if [ "$optional_tool" == "optional" ]; then + echo "Failed: curl failed with return code $curl_exit_code. This tool is optional, continuing." + else + checkRC 'curl' + fi + fi fi - # Move the partial file to the download target. - mv "$partial_target" "$download_target" || checkRC 'mv' - - # Extract to current directory - # Ensure we can extract those files - # We might use them during dev.sh - if [[ "$download_basename" == *.zip ]]; then - # Extract the zip. - echo "Testing zip" - unzip "$download_target" -d "$download_dir" > /dev/null - local rc=$? - if [[ $rc -ne 0 && $rc -ne 1 ]]; then - failed "unzip failed with return code $rc" + # Only move the partial file to the download target if curl succeeded + if [ $curl_exit_code -eq 0 ]; then + # Move the partial file to the download target. + mv "$partial_target" "$download_target" || checkRC 'mv' + + # Extract to current directory + # Ensure we can extract those files + # We might use them during dev.sh + if [[ "$download_basename" == *.zip ]]; then + # Extract the zip. + echo "Testing zip" + unzip "$download_target" -d "$download_dir" > /dev/null + local rc=$? + if [[ $rc -ne 0 && $rc -ne 1 ]]; then + failed "unzip failed with return code $rc" + fi + elif [[ "$download_basename" == *.tar.gz ]]; then + # Extract the tar gz. + echo "Testing tar gz" + tar xzf "$download_target" -C "$download_dir" > /dev/null || checkRC 'tar' fi - elif [[ "$download_basename" == *.tar.gz ]]; then - # Extract the tar gz. - echo "Testing tar gz" - tar xzf "$download_target" -C "$download_dir" > /dev/null || checkRC 'tar' fi fi else + # Check if the download exists + if [ ! -f "$download_target" ] && [ "$optional_tool" == "optional" ]; then + echo "Download for $download_source not found. This tool is optional, continuing." + return 0 + fi + # Extract to layout. mkdir -p "$target_dir" || checkRC 'mkdir' local nested_dir="" @@ -174,9 +204,9 @@ fi # Download the external tools for Linux PACKAGERUNTIMEs. if [[ "$PACKAGERUNTIME" == "linux-x64" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-x64.tar.gz" node20 fix_nested_dir - acquireExternalTool "$NODE_ALPINE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-alpine-x64.tar.gz" node20_alpine + acquireExternalTool "$NODE_ALPINE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-alpine-x64.tar.gz" node20_alpine optional acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-linux-x64.tar.gz" node24 fix_nested_dir - acquireExternalTool "$NODE_ALPINE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-alpine-x64.tar.gz" node24_alpine + acquireExternalTool "$NODE_ALPINE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-alpine-x64.tar.gz" node24_alpine optional fi if [[ "$PACKAGERUNTIME" == "linux-arm64" ]]; then From e3619caa991708ba38207311ce5072af857dfd28 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Fri, 11 Jul 2025 13:45:43 +0000 Subject: [PATCH 03/23] =?UTF-8?q?remove=20apline=E2=89=A0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Misc/externals.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/Misc/externals.sh b/src/Misc/externals.sh index fe7b53214b1..737596ea43a 100755 --- a/src/Misc/externals.sh +++ b/src/Misc/externals.sh @@ -204,9 +204,8 @@ fi # Download the external tools for Linux PACKAGERUNTIMEs. if [[ "$PACKAGERUNTIME" == "linux-x64" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-x64.tar.gz" node20 fix_nested_dir - acquireExternalTool "$NODE_ALPINE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-alpine-x64.tar.gz" node20_alpine optional + acquireExternalTool "$NODE_ALPINE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-alpine-x64.tar.gz" node20_alpine acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-linux-x64.tar.gz" node24 fix_nested_dir - acquireExternalTool "$NODE_ALPINE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-alpine-x64.tar.gz" node24_alpine optional fi if [[ "$PACKAGERUNTIME" == "linux-arm64" ]]; then From 0af68765099ded06f79135afc10fa7716385c4b9 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Fri, 11 Jul 2025 13:49:46 +0000 Subject: [PATCH 04/23] remove line --- src/Misc/externals.sh | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Misc/externals.sh b/src/Misc/externals.sh index 737596ea43a..ab47d223b6c 100755 --- a/src/Misc/externals.sh +++ b/src/Misc/externals.sh @@ -8,7 +8,6 @@ NODE_ALPINE_URL=https://github.com/actions/alpine_nodejs/releases/download acquireExternalTool "$NODE_ALPINE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-alpine-x64.tar.gz" node20_alpine acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-linux-x64.tar.gz" node24 fix_nested_dir acquireExternalTool "$NODE_ALPINE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-alpine-x64.tar.gz" node24_alpine "" optional -finew release of alpine_nodejs at that updated version. # Follow the instructions here: https://github.com/actions/alpine_nodejs?tab=readme-ov-file#getting-started NODE20_VERSION="20.19.3" NODE24_VERSION="24.4.0" From f83aab0f5e5beb2b709d0425884189701621a4d5 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Fri, 11 Jul 2025 13:53:54 +0000 Subject: [PATCH 05/23] remove these calls --- src/Misc/externals.sh | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Misc/externals.sh b/src/Misc/externals.sh index ab47d223b6c..fa2a36c0044 100755 --- a/src/Misc/externals.sh +++ b/src/Misc/externals.sh @@ -4,10 +4,6 @@ PRECACHE=$2 NODE_URL=https://nodejs.org/dist NODE_ALPINE_URL=https://github.com/actions/alpine_nodejs/releases/download - acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-x64.tar.gz" node20 fix_nested_dir - acquireExternalTool "$NODE_ALPINE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-alpine-x64.tar.gz" node20_alpine - acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-linux-x64.tar.gz" node24 fix_nested_dir - acquireExternalTool "$NODE_ALPINE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-alpine-x64.tar.gz" node24_alpine "" optional # Follow the instructions here: https://github.com/actions/alpine_nodejs?tab=readme-ov-file#getting-started NODE20_VERSION="20.19.3" NODE24_VERSION="24.4.0" From b15643db4bf61efddf73a4a6f0dee23ed6a24382 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Fri, 11 Jul 2025 14:23:49 +0000 Subject: [PATCH 06/23] update since linux arm doesn't have 32 bit --- src/Misc/externals.sh | 70 +++++++++++++++---------------------------- 1 file changed, 24 insertions(+), 46 deletions(-) diff --git a/src/Misc/externals.sh b/src/Misc/externals.sh index fa2a36c0044..aec8913f6fd 100755 --- a/src/Misc/externals.sh +++ b/src/Misc/externals.sh @@ -33,7 +33,6 @@ function acquireExternalTool() { local download_source=$1 # E.g. https://github.com/microsoft/vswhere/releases/download/2.6.7/vswhere.exe local target_dir="$LAYOUT_DIR/externals/$2" # E.g. $LAYOUT_DIR/externals/vswhere local fix_nested_dir=$3 # Flag that indicates whether to move nested contents up one directory. - local optional_tool=$4 # Flag that indicates if this tool is optional and we should continue on error # Extract the portion of the URL after the protocol. E.g. github.com/microsoft/vswhere/releases/download/2.6.7/vswhere.exe local relative_url="${download_source#*://}" @@ -68,58 +67,33 @@ function acquireExternalTool() { # --retry 3 Retries transient errors 3 times (timeouts, 5xx) if [[ "$(printf '%s\n' "7.71.0" "$CURL_VERSION" | sort -V | head -n1)" != "7.71.0" ]]; then # Curl version is less than or equal to 7.71.0, skipping retry-all-errors flag - curl -fSL --retry 3 -o "$partial_target" "$download_source" 2>"${download_target}_download.log" - local curl_exit_code=$? - if [ $curl_exit_code -ne 0 ]; then - if [ "$optional_tool" == "optional" ]; then - echo "Failed: curl failed with return code $curl_exit_code. This tool is optional, continuing." - else - checkRC 'curl' - fi - fi + curl -fSL --retry 3 -o "$partial_target" "$download_source" 2>"${download_target}_download.log" || checkRC 'curl' else # Curl version is greater than 7.71.0, running curl with --retry-all-errors flag - curl -fSL --retry 3 --retry-all-errors -o "$partial_target" "$download_source" 2>"${download_target}_download.log" - local curl_exit_code=$? - if [ $curl_exit_code -ne 0 ]; then - if [ "$optional_tool" == "optional" ]; then - echo "Failed: curl failed with return code $curl_exit_code. This tool is optional, continuing." - else - checkRC 'curl' - fi - fi + curl -fSL --retry 3 --retry-all-errors -o "$partial_target" "$download_source" 2>"${download_target}_download.log" || checkRC 'curl' fi - # Only move the partial file to the download target if curl succeeded - if [ $curl_exit_code -eq 0 ]; then - # Move the partial file to the download target. - mv "$partial_target" "$download_target" || checkRC 'mv' - - # Extract to current directory - # Ensure we can extract those files - # We might use them during dev.sh - if [[ "$download_basename" == *.zip ]]; then - # Extract the zip. - echo "Testing zip" - unzip "$download_target" -d "$download_dir" > /dev/null - local rc=$? - if [[ $rc -ne 0 && $rc -ne 1 ]]; then - failed "unzip failed with return code $rc" - fi - elif [[ "$download_basename" == *.tar.gz ]]; then - # Extract the tar gz. - echo "Testing tar gz" - tar xzf "$download_target" -C "$download_dir" > /dev/null || checkRC 'tar' + # Move the partial file to the download target. + mv "$partial_target" "$download_target" || checkRC 'mv' + + # Extract to current directory + # Ensure we can extract those files + # We might use them during dev.sh + if [[ "$download_basename" == *.zip ]]; then + # Extract the zip. + echo "Testing zip" + unzip "$download_target" -d "$download_dir" > /dev/null + local rc=$? + if [[ $rc -ne 0 && $rc -ne 1 ]]; then + failed "unzip failed with return code $rc" fi + elif [[ "$download_basename" == *.tar.gz ]]; then + # Extract the tar gz. + echo "Testing tar gz" + tar xzf "$download_target" -C "$download_dir" > /dev/null || checkRC 'tar' fi fi else - # Check if the download exists - if [ ! -f "$download_target" ] && [ "$optional_tool" == "optional" ]; then - echo "Download for $download_source not found. This tool is optional, continuing." - return 0 - fi - # Extract to layout. mkdir -p "$target_dir" || checkRC 'mkdir' local nested_dir="" @@ -201,6 +175,8 @@ if [[ "$PACKAGERUNTIME" == "linux-x64" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-x64.tar.gz" node20 fix_nested_dir acquireExternalTool "$NODE_ALPINE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-alpine-x64.tar.gz" node20_alpine acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-linux-x64.tar.gz" node24 fix_nested_dir + # Use v24.3.0 for alpine as it's the latest available in actions/alpine_nodejs + acquireExternalTool "$NODE_ALPINE_URL/v24.3.0/node-v24.3.0-alpine-x64.tar.gz" node24_alpine fi if [[ "$PACKAGERUNTIME" == "linux-arm64" ]]; then @@ -210,5 +186,7 @@ fi if [[ "$PACKAGERUNTIME" == "linux-arm" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-armv7l.tar.gz" node20 fix_nested_dir - acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-linux-armv7l.tar.gz" node24 fix_nested_dir + # Node.js 24 doesn't provide official armv7l builds + # Using Node 20 as a fallback for Node 24 on linux-arm + ln -sf ../node20 "$LAYOUT_DIR/externals/node24" fi From a34f7563332ff133fc1244f8419b9e9db7f888b3 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Fri, 11 Jul 2025 14:30:00 +0000 Subject: [PATCH 07/23] mkdir if directory doesn't exist --- src/Misc/externals.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Misc/externals.sh b/src/Misc/externals.sh index aec8913f6fd..05cc16607be 100755 --- a/src/Misc/externals.sh +++ b/src/Misc/externals.sh @@ -188,5 +188,6 @@ if [[ "$PACKAGERUNTIME" == "linux-arm" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-armv7l.tar.gz" node20 fix_nested_dir # Node.js 24 doesn't provide official armv7l builds # Using Node 20 as a fallback for Node 24 on linux-arm + mkdir -p "$LAYOUT_DIR/externals/node24" || checkRC 'mkdir' ln -sf ../node20 "$LAYOUT_DIR/externals/node24" fi From 922a35bdb7c34b0b1048ef176a0e1d30280df5a9 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Fri, 11 Jul 2025 14:36:26 +0000 Subject: [PATCH 08/23] add comment --- src/Misc/externals.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Misc/externals.sh b/src/Misc/externals.sh index 05cc16607be..47c2e5d7946 100755 --- a/src/Misc/externals.sh +++ b/src/Misc/externals.sh @@ -4,6 +4,7 @@ PRECACHE=$2 NODE_URL=https://nodejs.org/dist NODE_ALPINE_URL=https://github.com/actions/alpine_nodejs/releases/download +# When you update Node versions you must also create a new release of alpine_nodejs at that updated version. # Follow the instructions here: https://github.com/actions/alpine_nodejs?tab=readme-ov-file#getting-started NODE20_VERSION="20.19.3" NODE24_VERSION="24.4.0" From ea5b865b1b338b219a5bf7effbcd4d78f3d65b9d Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Fri, 11 Jul 2025 19:30:16 +0000 Subject: [PATCH 09/23] Update to use the latest version of alpine that was just published and also don't use a fallback version for linux arm 32 bit --- src/Misc/externals.sh | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/Misc/externals.sh b/src/Misc/externals.sh index 47c2e5d7946..7f8baf4c8b1 100755 --- a/src/Misc/externals.sh +++ b/src/Misc/externals.sh @@ -176,8 +176,7 @@ if [[ "$PACKAGERUNTIME" == "linux-x64" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-x64.tar.gz" node20 fix_nested_dir acquireExternalTool "$NODE_ALPINE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-alpine-x64.tar.gz" node20_alpine acquireExternalTool "$NODE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-linux-x64.tar.gz" node24 fix_nested_dir - # Use v24.3.0 for alpine as it's the latest available in actions/alpine_nodejs - acquireExternalTool "$NODE_ALPINE_URL/v24.3.0/node-v24.3.0-alpine-x64.tar.gz" node24_alpine + acquireExternalTool "$NODE_ALPINE_URL/v${NODE24_VERSION}/node-v${NODE24_VERSION}-alpine-x64.tar.gz" node24_alpine fi if [[ "$PACKAGERUNTIME" == "linux-arm64" ]]; then @@ -187,8 +186,4 @@ fi if [[ "$PACKAGERUNTIME" == "linux-arm" ]]; then acquireExternalTool "$NODE_URL/v${NODE20_VERSION}/node-v${NODE20_VERSION}-linux-armv7l.tar.gz" node20 fix_nested_dir - # Node.js 24 doesn't provide official armv7l builds - # Using Node 20 as a fallback for Node 24 on linux-arm - mkdir -p "$LAYOUT_DIR/externals/node24" || checkRC 'mkdir' - ln -sf ../node20 "$LAYOUT_DIR/externals/node24" fi From 2418b747d84d2f737d4d86ac06c44c616b957710 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Tue, 15 Jul 2025 12:42:05 +0000 Subject: [PATCH 10/23] Feature flag node 24 --- src/Runner.Common/Constants.cs | 1 + src/Runner.Common/Util/NodeUtil.cs | 15 ++++ src/Runner.Worker/FeatureManager.cs | 18 ++++ src/Runner.Worker/Handlers/HandlerFactory.cs | 15 +++- src/Runner.Worker/Handlers/StepHost.cs | 21 +++++ src/Test/L0/Worker/FeatureManagerL0.cs | 58 +++++++++++++ src/Test/L0/Worker/HandlerFactoryL0.cs | 91 +++++++++++++++++++- src/Test/L0/Worker/StepHostL0.cs | 6 ++ 8 files changed, 222 insertions(+), 3 deletions(-) create mode 100644 src/Test/L0/Worker/FeatureManagerL0.cs diff --git a/src/Runner.Common/Constants.cs b/src/Runner.Common/Constants.cs index 03d01b6288d..edf8a9b519a 100644 --- a/src/Runner.Common/Constants.cs +++ b/src/Runner.Common/Constants.cs @@ -169,6 +169,7 @@ public static class Features public static readonly string AllowRunnerContainerHooks = "DistributedTask.AllowRunnerContainerHooks"; public static readonly string AddCheckRunIdToJobContext = "actions_add_check_run_id_to_job_context"; public static readonly string DisplayHelpfulActionsDownloadErrors = "actions_display_helpful_actions_download_errors"; + public static readonly string UseNode24 = "runner_usenode24"; } public static readonly string InternalTelemetryIssueDataKey = "_internal_telemetry"; diff --git a/src/Runner.Common/Util/NodeUtil.cs b/src/Runner.Common/Util/NodeUtil.cs index 170a3e58fba..8c4343dd599 100644 --- a/src/Runner.Common/Util/NodeUtil.cs +++ b/src/Runner.Common/Util/NodeUtil.cs @@ -6,6 +6,7 @@ namespace GitHub.Runner.Common.Util public static class NodeUtil { private const string _defaultNodeVersion = "node20"; + private const string _node24Version = "node24"; public static readonly ReadOnlyCollection BuiltInNodeVersions = new(new[] { "node20", "node24" }); public static string GetInternalNodeVersion() { @@ -16,6 +17,20 @@ public static string GetInternalNodeVersion() { return forcedInternalNodeVersion; } + + var useNode24FlagValue = Environment.GetEnvironmentVariable(Constants.Runner.Features.UseNode24) ?? + Environment.GetEnvironmentVariable("RUNNER_USENODE24") ?? + Environment.GetEnvironmentVariable("runner_usenode24"); + + var useNode24 = !string.IsNullOrEmpty(useNode24FlagValue) && + (string.Equals(useNode24FlagValue, "true", StringComparison.OrdinalIgnoreCase) || + string.Equals(useNode24FlagValue, "1", StringComparison.OrdinalIgnoreCase)); + + if (useNode24) + { + return _node24Version; + } + return _defaultNodeVersion; } } diff --git a/src/Runner.Worker/FeatureManager.cs b/src/Runner.Worker/FeatureManager.cs index 98f49e8fd5f..99ba2d30709 100644 --- a/src/Runner.Worker/FeatureManager.cs +++ b/src/Runner.Worker/FeatureManager.cs @@ -11,5 +11,23 @@ public static bool IsContainerHooksEnabled(Variables variables) var isContainerHooksPathSet = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable(Constants.Hooks.ContainerHooksPath)); return isContainerHookFeatureFlagSet && isContainerHooksPathSet; } + + public static bool IsNode24Enabled(Variables variables) + { + bool isEnabled = variables?.GetBoolean(Constants.Runner.Features.UseNode24) ?? false; + + if (!isEnabled) + { + var envValue = Environment.GetEnvironmentVariable(Constants.Runner.Features.UseNode24) ?? + Environment.GetEnvironmentVariable("RUNNER_USENODE24") ?? + Environment.GetEnvironmentVariable("runner_usenode24"); + + isEnabled = !string.IsNullOrEmpty(envValue) && + (string.Equals(envValue, "true", StringComparison.OrdinalIgnoreCase) || + string.Equals(envValue, "1", StringComparison.OrdinalIgnoreCase)); + } + + return isEnabled; + } } } diff --git a/src/Runner.Worker/Handlers/HandlerFactory.cs b/src/Runner.Worker/Handlers/HandlerFactory.cs index 5ea9361cda7..0283eb298b5 100644 --- a/src/Runner.Worker/Handlers/HandlerFactory.cs +++ b/src/Runner.Worker/Handlers/HandlerFactory.cs @@ -57,11 +57,22 @@ public IHandler Create( handler = HostContext.CreateService(); var nodeData = data as NodeJSActionExecutionData; + if (string.Equals(nodeData.NodeVersion, "node24", StringComparison.InvariantCultureIgnoreCase)) + { + var useNode24 = FeatureManager.IsNode24Enabled(executionContext.Global.Variables); + if (!useNode24) + { + executionContext.Warning($"Action requested Node 24 runtime, but feature flag '{Constants.Runner.Features.UseNode24}' is not enabled. You can enable it with the RUNNER_USENODE24 environment variable. Using Node 20 instead."); + nodeData.NodeVersion = "node20"; + } + } // With node12 EoL in 04/2022 and node16 EoL in 09/23, we want to execute all JS actions using node20 - if (string.Equals(nodeData.NodeVersion, "node12", StringComparison.InvariantCultureIgnoreCase) || + // unless the Node 24 feature flag is enabled + else if (string.Equals(nodeData.NodeVersion, "node12", StringComparison.InvariantCultureIgnoreCase) || string.Equals(nodeData.NodeVersion, "node16", StringComparison.InvariantCultureIgnoreCase)) { - nodeData.NodeVersion = "node20"; + var useNode24 = FeatureManager.IsNode24Enabled(executionContext.Global.Variables); + nodeData.NodeVersion = useNode24 ? "node24" : "node20"; } (handler as INodeScriptActionHandler).Data = nodeData; diff --git a/src/Runner.Worker/Handlers/StepHost.cs b/src/Runner.Worker/Handlers/StepHost.cs index 1270dd90e6f..20693f09956 100644 --- a/src/Runner.Worker/Handlers/StepHost.cs +++ b/src/Runner.Worker/Handlers/StepHost.cs @@ -60,6 +60,16 @@ public string ResolvePathForStepHost(IExecutionContext executionContext, string public Task DetermineNodeRuntimeVersion(IExecutionContext executionContext, string preferredVersion) { + if (string.Equals(preferredVersion, "node24", StringComparison.OrdinalIgnoreCase)) + { + var useNode24 = FeatureManager.IsNode24Enabled(executionContext.Global.Variables); + if (!useNode24) + { + executionContext.Warning($"Action requested Node 24 runtime, but feature flag '{Constants.Runner.Features.UseNode24}' is not enabled. You can enable it with the RUNNER_USENODE24 environment variable. Using Node 20 instead."); + return Task.FromResult("node20"); + } + } + return Task.FromResult(preferredVersion); } @@ -137,6 +147,17 @@ public string ResolvePathForStepHost(IExecutionContext executionContext, string public async Task DetermineNodeRuntimeVersion(IExecutionContext executionContext, string preferredVersion) { + // Enforce feature flag check for node24 requests + if (string.Equals(preferredVersion, "node24", StringComparison.OrdinalIgnoreCase)) + { + var useNode24 = FeatureManager.IsNode24Enabled(executionContext.Global.Variables); + if (!useNode24) + { + executionContext.Warning($"Action requested Node 24 runtime, but feature flag '{Constants.Runner.Features.UseNode24}' is not enabled. You can enable it with the RUNNER_USENODE24 environment variable. Using Node 20 instead."); + preferredVersion = "node20"; + } + } + // Optimistically use the default string nodeExternal = preferredVersion; diff --git a/src/Test/L0/Worker/FeatureManagerL0.cs b/src/Test/L0/Worker/FeatureManagerL0.cs new file mode 100644 index 00000000000..55700e6cea9 --- /dev/null +++ b/src/Test/L0/Worker/FeatureManagerL0.cs @@ -0,0 +1,58 @@ +using System.Collections.Generic; +using System.Runtime.CompilerServices; +using GitHub.DistributedTask.WebApi; +using GitHub.Runner.Worker; +using Xunit; + +namespace GitHub.Runner.Common.Tests.Worker +{ + public sealed class FeatureManagerL0 + { + private TestHostContext CreateTestContext([CallerMemberName] string testName = "") + { + var hostContext = new TestHostContext(this, testName); + return hostContext; + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void IsNode24EnabledWhenFeatureFlagSet() + { + using (TestHostContext hc = CreateTestContext()) + { + // Arrange + var variables = new Dictionary + { + { Constants.Runner.Features.UseNode24, new VariableValue("true") } + }; + Variables serverVariables = new(hc, variables); + + // Act + bool isNode24Enabled = FeatureManager.IsNode24Enabled(serverVariables); + + // Assert + Assert.True(isNode24Enabled); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void IsNode24DisabledWhenFeatureFlagNotSet() + { + using (TestHostContext hc = CreateTestContext()) + { + // Arrange + var variables = new Dictionary(); + Variables serverVariables = new(hc, variables); + + // Act + bool isNode24Enabled = FeatureManager.IsNode24Enabled(serverVariables); + + // Assert + Assert.False(isNode24Enabled); + } + } + } +} diff --git a/src/Test/L0/Worker/HandlerFactoryL0.cs b/src/Test/L0/Worker/HandlerFactoryL0.cs index 48ae4898f09..1cd99518655 100644 --- a/src/Test/L0/Worker/HandlerFactoryL0.cs +++ b/src/Test/L0/Worker/HandlerFactoryL0.cs @@ -33,7 +33,7 @@ private TestHostContext CreateTestContext([CallerMemberName] string testName = " [InlineData("node12", "node20")] [InlineData("node16", "node20")] [InlineData("node20", "node20")] - [InlineData("node24", "node24")] + [InlineData("node24", "node20")] public void IsNodeVersionUpgraded(string inputVersion, string expectedVersion) { using (TestHostContext hc = CreateTestContext()) @@ -73,5 +73,94 @@ public void IsNodeVersionUpgraded(string inputVersion, string expectedVersion) Assert.Equal(expectedVersion, handler.Data.NodeVersion); } } + + [Theory] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + [InlineData("node12", "node24")] + [InlineData("node16", "node24")] + [InlineData("node24", "node24")] + public void NodeVersionWithFeatureFlagEnabled(string inputVersion, string expectedVersion) + { + using (TestHostContext hc = CreateTestContext()) + { + // Arrange. + var hf = new HandlerFactory(); + hf.Initialize(hc); + + // Server Feature Flag - enable Node 24 + var variables = new Dictionary + { + { Constants.Runner.Features.UseNode24, new VariableValue("true") } + }; + Variables serverVariables = new(hc, variables); + + // Workflow opt-out + var workflowVariables = new Dictionary(); + + _ec.Setup(x => x.Global).Returns(new GlobalContext() + { + Variables = serverVariables, + EnvironmentVariables = workflowVariables + }); + + // Act. + var data = new NodeJSActionExecutionData(); + data.NodeVersion = inputVersion; + var handler = hf.Create( + _ec.Object, + new ScriptReference(), + new Mock().Object, + data, + new Dictionary(), + new Dictionary(), + new Variables(hc, new Dictionary()), "", new List() + ) as INodeScriptActionHandler; + + // Assert. + Assert.Equal(expectedVersion, handler.Data.NodeVersion); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Node24ExplicitlyRequested_DowngradedWhenFeatureFlagOff() + { + using (TestHostContext hc = CreateTestContext()) + { + // Arrange. + var hf = new HandlerFactory(); + hf.Initialize(hc); + + // Server Feature Flag - feature flag NOT set + var variables = new Dictionary(); + Variables serverVariables = new(hc, variables); + + _ec.Setup(x => x.Global).Returns(new GlobalContext() + { + Variables = serverVariables, + EnvironmentVariables = new Dictionary() + }); + + // Act - Node 24 explicitly requested in action.yml + var data = new NodeJSActionExecutionData(); + data.NodeVersion = "node24"; + var handler = hf.Create( + _ec.Object, + new ScriptReference(), + new Mock().Object, + data, + new Dictionary(), + new Dictionary(), + new Variables(hc, new Dictionary()), + "", + new List() + ) as INodeScriptActionHandler; + + // Assert - should be downgraded to Node 20 + Assert.Equal("node20", handler.Data.NodeVersion); + } + } } } diff --git a/src/Test/L0/Worker/StepHostL0.cs b/src/Test/L0/Worker/StepHostL0.cs index bac7d41d90a..9148cbe09ab 100644 --- a/src/Test/L0/Worker/StepHostL0.cs +++ b/src/Test/L0/Worker/StepHostL0.cs @@ -175,6 +175,9 @@ public async Task DetermineNode24RuntimeVersionInAlpineContainerAsync() sh.Initialize(hc); sh.Container = new ContainerInfo() { ContainerId = "1234abcd" }; + // Enable Node 24 feature flag + _ec.Object.Global.Variables.Set(Constants.Runner.Features.UseNode24, "true"); + _dc.Setup(d => d.DockerExec(_ec.Object, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())) .Callback((IExecutionContext ec, string id, string options, string command, List output) => { @@ -202,6 +205,9 @@ public async Task DetermineNode24RuntimeVersionInUnknownContainerAsync() sh.Initialize(hc); sh.Container = new ContainerInfo() { ContainerId = "1234abcd" }; + // Enable Node 24 feature flag + _ec.Object.Global.Variables.Set(Constants.Runner.Features.UseNode24, "true"); + _dc.Setup(d => d.DockerExec(_ec.Object, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())) .Callback((IExecutionContext ec, string id, string options, string command, List output) => { From 6a21e10fbc3f2cc8ae14d3e959abc71c3f5c4efd Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Tue, 15 Jul 2025 13:34:05 +0000 Subject: [PATCH 11/23] add tests for env variables --- src/Test/L0/Worker/HandlerFactoryL0.cs | 129 +++++++++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/src/Test/L0/Worker/HandlerFactoryL0.cs b/src/Test/L0/Worker/HandlerFactoryL0.cs index 1cd99518655..6b6165fc68e 100644 --- a/src/Test/L0/Worker/HandlerFactoryL0.cs +++ b/src/Test/L0/Worker/HandlerFactoryL0.cs @@ -122,6 +122,135 @@ public void NodeVersionWithFeatureFlagEnabled(string inputVersion, string expect } } + [Theory] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + [InlineData("runner_usenode24", "true")] + [InlineData("RUNNER_USENODE24", "true")] + [InlineData("runner_usenode24", "1")] + [InlineData("RUNNER_USENODE24", "1")] + public void NodeVersionWithEnvironmentVariableEnabled(string envVarName, string envVarValue) + { + try + { + // Set the environment variable for testing + Environment.SetEnvironmentVariable(envVarName, envVarValue); + + using (TestHostContext hc = CreateTestContext()) + { + // Arrange. + var hf = new HandlerFactory(); + hf.Initialize(hc); + + // Server Feature Flag - NOT set in variables + var variables = new Dictionary(); + Variables serverVariables = new(hc, variables); + + // Workflow variables + var workflowVariables = new Dictionary(); + + _ec.Setup(x => x.Global).Returns(new GlobalContext() + { + Variables = serverVariables, + EnvironmentVariables = workflowVariables + }); + + // Act - Test with each Node version + var testVersions = new[] { "node12", "node16", "node24" }; + foreach (var inputVersion in testVersions) + { + var data = new NodeJSActionExecutionData(); + data.NodeVersion = inputVersion; + var handler = hf.Create( + _ec.Object, + new ScriptReference(), + new Mock().Object, + data, + new Dictionary(), + new Dictionary(), + new Variables(hc, new Dictionary()), + "", + new List() + ) as INodeScriptActionHandler; + + // Node 24 should be used for all versions when the feature flag is enabled via env var + string expectedVersion = "node24"; + + // Assert - should use Node 24 + Assert.Equal(expectedVersion, handler.Data.NodeVersion); + } + } + } + finally + { + // Clean up the environment variable after the test + Environment.SetEnvironmentVariable(envVarName, null); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void NodeVersionWithFeatureFlagNameEnvironmentVariableEnabled() + { + try + { + // Use the actual feature flag name from constants + string envVarName = Constants.Runner.Features.UseNode24; + Environment.SetEnvironmentVariable(envVarName, "true"); + + using (TestHostContext hc = CreateTestContext()) + { + // Arrange. + var hf = new HandlerFactory(); + hf.Initialize(hc); + + // Server Feature Flag - NOT set in variables + var variables = new Dictionary(); + Variables serverVariables = new(hc, variables); + + // Workflow variables + var workflowVariables = new Dictionary(); + + _ec.Setup(x => x.Global).Returns(new GlobalContext() + { + Variables = serverVariables, + EnvironmentVariables = workflowVariables + }); + + // Act - Test with each Node version + var testVersions = new[] { "node12", "node16", "node24" }; + foreach (var inputVersion in testVersions) + { + var data = new NodeJSActionExecutionData(); + data.NodeVersion = inputVersion; + var handler = hf.Create( + _ec.Object, + new ScriptReference(), + new Mock().Object, + data, + new Dictionary(), + new Dictionary(), + new Variables(hc, new Dictionary()), + "", + new List() + ) as INodeScriptActionHandler; + + // Node 24 should be used for all versions when the feature flag is enabled via env var + string expectedVersion = "node24"; + + // Assert - should use Node 24 + Assert.Equal(expectedVersion, handler.Data.NodeVersion); + } + } + } + finally + { + // Clean up the environment variable after the test + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); + } + } + [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] From 64b51eb4acce0e46ec665bf265ae48a58d09fc58 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Tue, 15 Jul 2025 13:46:43 +0000 Subject: [PATCH 12/23] test update --- src/Test/L0/Worker/HandlerFactoryL0.cs | 98 ++++++++++++++------------ 1 file changed, 52 insertions(+), 46 deletions(-) diff --git a/src/Test/L0/Worker/HandlerFactoryL0.cs b/src/Test/L0/Worker/HandlerFactoryL0.cs index 6b6165fc68e..5b689810b9a 100644 --- a/src/Test/L0/Worker/HandlerFactoryL0.cs +++ b/src/Test/L0/Worker/HandlerFactoryL0.cs @@ -155,30 +155,33 @@ public void NodeVersionWithEnvironmentVariableEnabled(string envVarName, string EnvironmentVariables = workflowVariables }); - // Act - Test with each Node version - var testVersions = new[] { "node12", "node16", "node24" }; - foreach (var inputVersion in testVersions) - { - var data = new NodeJSActionExecutionData(); - data.NodeVersion = inputVersion; - var handler = hf.Create( - _ec.Object, - new ScriptReference(), - new Mock().Object, - data, - new Dictionary(), - new Dictionary(), - new Variables(hc, new Dictionary()), - "", - new List() - ) as INodeScriptActionHandler; + // Act - Test with one Node version to verify the behavior + var inputVersion = "node16"; // Use node16 as a representative case + + // Make sure we have a handler instance to use + var nodeHandler = new Mock(); + nodeHandler.SetupAllProperties(); + hc.EnqueueInstance(nodeHandler.Object); + + var data = new NodeJSActionExecutionData(); + data.NodeVersion = inputVersion; + var handler = hf.Create( + _ec.Object, + new ScriptReference(), + new Mock().Object, + data, + new Dictionary(), + new Dictionary(), + new Variables(hc, new Dictionary()), + "", + new List() + ) as INodeScriptActionHandler; - // Node 24 should be used for all versions when the feature flag is enabled via env var - string expectedVersion = "node24"; - - // Assert - should use Node 24 - Assert.Equal(expectedVersion, handler.Data.NodeVersion); - } + // Node 24 should be used when the feature flag is enabled via env var + string expectedVersion = "node24"; + + // Assert - should use Node 24 + Assert.Equal(expectedVersion, handler.Data.NodeVersion); } } finally @@ -218,30 +221,33 @@ public void NodeVersionWithFeatureFlagNameEnvironmentVariableEnabled() EnvironmentVariables = workflowVariables }); - // Act - Test with each Node version - var testVersions = new[] { "node12", "node16", "node24" }; - foreach (var inputVersion in testVersions) - { - var data = new NodeJSActionExecutionData(); - data.NodeVersion = inputVersion; - var handler = hf.Create( - _ec.Object, - new ScriptReference(), - new Mock().Object, - data, - new Dictionary(), - new Dictionary(), - new Variables(hc, new Dictionary()), - "", - new List() - ) as INodeScriptActionHandler; + // Act - Test with one Node version to verify the behavior + var inputVersion = "node16"; // Use node16 as a representative case + + // Make sure we have a handler instance to use + var nodeHandler = new Mock(); + nodeHandler.SetupAllProperties(); + hc.EnqueueInstance(nodeHandler.Object); + + var data = new NodeJSActionExecutionData(); + data.NodeVersion = inputVersion; + var handler = hf.Create( + _ec.Object, + new ScriptReference(), + new Mock().Object, + data, + new Dictionary(), + new Dictionary(), + new Variables(hc, new Dictionary()), + "", + new List() + ) as INodeScriptActionHandler; - // Node 24 should be used for all versions when the feature flag is enabled via env var - string expectedVersion = "node24"; - - // Assert - should use Node 24 - Assert.Equal(expectedVersion, handler.Data.NodeVersion); - } + // Node 24 should be used when the feature flag is enabled via env var + string expectedVersion = "node24"; + + // Assert - should use Node 24 + Assert.Equal(expectedVersion, handler.Data.NodeVersion); } } finally From 0e7cd9e2bbbd9d15335cf6af235e6585a98d8455 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Wed, 16 Jul 2025 12:38:27 +0000 Subject: [PATCH 13/23] Refactor Node version handling to remove feature flag checks for Node 24 actions and update related tests. Add tests for internal node version feature flagging. --- src/Runner.Worker/Handlers/HandlerFactory.cs | 15 +- src/Runner.Worker/Handlers/StepHost.cs | 21 --- src/Test/L0/Util/NodeUtilL0.cs | 170 +++++++++++++++++++ src/Test/L0/Worker/HandlerFactoryL0.cs | 21 ++- 4 files changed, 182 insertions(+), 45 deletions(-) create mode 100644 src/Test/L0/Util/NodeUtilL0.cs diff --git a/src/Runner.Worker/Handlers/HandlerFactory.cs b/src/Runner.Worker/Handlers/HandlerFactory.cs index 0283eb298b5..5ea9361cda7 100644 --- a/src/Runner.Worker/Handlers/HandlerFactory.cs +++ b/src/Runner.Worker/Handlers/HandlerFactory.cs @@ -57,22 +57,11 @@ public IHandler Create( handler = HostContext.CreateService(); var nodeData = data as NodeJSActionExecutionData; - if (string.Equals(nodeData.NodeVersion, "node24", StringComparison.InvariantCultureIgnoreCase)) - { - var useNode24 = FeatureManager.IsNode24Enabled(executionContext.Global.Variables); - if (!useNode24) - { - executionContext.Warning($"Action requested Node 24 runtime, but feature flag '{Constants.Runner.Features.UseNode24}' is not enabled. You can enable it with the RUNNER_USENODE24 environment variable. Using Node 20 instead."); - nodeData.NodeVersion = "node20"; - } - } // With node12 EoL in 04/2022 and node16 EoL in 09/23, we want to execute all JS actions using node20 - // unless the Node 24 feature flag is enabled - else if (string.Equals(nodeData.NodeVersion, "node12", StringComparison.InvariantCultureIgnoreCase) || + if (string.Equals(nodeData.NodeVersion, "node12", StringComparison.InvariantCultureIgnoreCase) || string.Equals(nodeData.NodeVersion, "node16", StringComparison.InvariantCultureIgnoreCase)) { - var useNode24 = FeatureManager.IsNode24Enabled(executionContext.Global.Variables); - nodeData.NodeVersion = useNode24 ? "node24" : "node20"; + nodeData.NodeVersion = "node20"; } (handler as INodeScriptActionHandler).Data = nodeData; diff --git a/src/Runner.Worker/Handlers/StepHost.cs b/src/Runner.Worker/Handlers/StepHost.cs index 20693f09956..1270dd90e6f 100644 --- a/src/Runner.Worker/Handlers/StepHost.cs +++ b/src/Runner.Worker/Handlers/StepHost.cs @@ -60,16 +60,6 @@ public string ResolvePathForStepHost(IExecutionContext executionContext, string public Task DetermineNodeRuntimeVersion(IExecutionContext executionContext, string preferredVersion) { - if (string.Equals(preferredVersion, "node24", StringComparison.OrdinalIgnoreCase)) - { - var useNode24 = FeatureManager.IsNode24Enabled(executionContext.Global.Variables); - if (!useNode24) - { - executionContext.Warning($"Action requested Node 24 runtime, but feature flag '{Constants.Runner.Features.UseNode24}' is not enabled. You can enable it with the RUNNER_USENODE24 environment variable. Using Node 20 instead."); - return Task.FromResult("node20"); - } - } - return Task.FromResult(preferredVersion); } @@ -147,17 +137,6 @@ public string ResolvePathForStepHost(IExecutionContext executionContext, string public async Task DetermineNodeRuntimeVersion(IExecutionContext executionContext, string preferredVersion) { - // Enforce feature flag check for node24 requests - if (string.Equals(preferredVersion, "node24", StringComparison.OrdinalIgnoreCase)) - { - var useNode24 = FeatureManager.IsNode24Enabled(executionContext.Global.Variables); - if (!useNode24) - { - executionContext.Warning($"Action requested Node 24 runtime, but feature flag '{Constants.Runner.Features.UseNode24}' is not enabled. You can enable it with the RUNNER_USENODE24 environment variable. Using Node 20 instead."); - preferredVersion = "node20"; - } - } - // Optimistically use the default string nodeExternal = preferredVersion; diff --git a/src/Test/L0/Util/NodeUtilL0.cs b/src/Test/L0/Util/NodeUtilL0.cs new file mode 100644 index 00000000000..0daedb9d4a4 --- /dev/null +++ b/src/Test/L0/Util/NodeUtilL0.cs @@ -0,0 +1,170 @@ +using System; +using System.Collections.Generic; +using Xunit; +using GitHub.Runner.Common.Util; + +namespace GitHub.Runner.Common.Tests.Util +{ + public class NodeUtilL0 + { + [Fact] + public void GetInternalNodeVersion_DefaultsToNode20() + { + // Arrange + Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); + Environment.SetEnvironmentVariable("RUNNER_USENODE24", null); + Environment.SetEnvironmentVariable("runner_usenode24", null); + + // Act + var nodeVersion = NodeUtil.GetInternalNodeVersion(); + + // Assert + Assert.Equal("node20", nodeVersion); + } + + [Theory] + [InlineData("node20")] + [InlineData("node24")] + public void GetInternalNodeVersion_ReturnsCorrectForcedInternalNodeVersion(string forcedVersion) + { + // Arrange + Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, forcedVersion); + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); + Environment.SetEnvironmentVariable("RUNNER_USENODE24", null); + Environment.SetEnvironmentVariable("runner_usenode24", null); + + // Act + var nodeVersion = NodeUtil.GetInternalNodeVersion(); + + // Assert + Assert.Equal(forcedVersion, nodeVersion); + + // Cleanup + Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); + } + + [Fact] + public void GetInternalNodeVersion_IgnoresInvalidForcedInternalNodeVersion() + { + // Arrange + Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, "invalidVersion"); + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); + Environment.SetEnvironmentVariable("RUNNER_USENODE24", null); + Environment.SetEnvironmentVariable("runner_usenode24", null); + + // Act + var nodeVersion = NodeUtil.GetInternalNodeVersion(); + + // Assert + Assert.Equal("node20", nodeVersion); + + // Cleanup + Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); + } + + [Theory] + [InlineData("true")] + [InlineData("TRUE")] + [InlineData("True")] + [InlineData("1")] + public void GetInternalNodeVersion_UsesNode24WhenFeatureFlagEnabled(string flagValue) + { + // Arrange + Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, flagValue); + + // Act + var nodeVersion = NodeUtil.GetInternalNodeVersion(); + + // Assert + Assert.Equal("node24", nodeVersion); + + // Cleanup + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); + } + + [Theory] + [InlineData("true")] + [InlineData("TRUE")] + [InlineData("True")] + [InlineData("1")] + public void GetInternalNodeVersion_UsesNode24WhenRunnerUseNode24Enabled(string flagValue) + { + // Arrange + Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); + Environment.SetEnvironmentVariable("RUNNER_USENODE24", flagValue); + + // Act + var nodeVersion = NodeUtil.GetInternalNodeVersion(); + + // Assert + Assert.Equal("node24", nodeVersion); + + // Cleanup + Environment.SetEnvironmentVariable("RUNNER_USENODE24", null); + } + + [Theory] + [InlineData("true")] + [InlineData("TRUE")] + [InlineData("True")] + [InlineData("1")] + public void GetInternalNodeVersion_UsesNode24WhenrunnerUseNode24Enabled(string flagValue) + { + // Arrange + Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); + Environment.SetEnvironmentVariable("RUNNER_USENODE24", null); + Environment.SetEnvironmentVariable("runner_usenode24", flagValue); + + // Act + var nodeVersion = NodeUtil.GetInternalNodeVersion(); + + // Assert + Assert.Equal("node24", nodeVersion); + + // Cleanup + Environment.SetEnvironmentVariable("runner_usenode24", null); + } + + [Theory] + [InlineData("false")] + [InlineData("0")] + [InlineData("")] + public void GetInternalNodeVersion_UsesDefaultWhenFeatureFlagDisabled(string flagValue) + { + // Arrange + Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, flagValue); + + // Act + var nodeVersion = NodeUtil.GetInternalNodeVersion(); + + // Assert + Assert.Equal("node20", nodeVersion); + + // Cleanup + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); + } + + [Fact] + public void GetInternalNodeVersion_ForcedVersionTakesPrecedenceOverFeatureFlag() + { + // Arrange + Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, "node20"); + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, "true"); + + // Act + var nodeVersion = NodeUtil.GetInternalNodeVersion(); + + // Assert + Assert.Equal("node20", nodeVersion); + + // Cleanup + Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); + Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); + } + } +} \ No newline at end of file diff --git a/src/Test/L0/Worker/HandlerFactoryL0.cs b/src/Test/L0/Worker/HandlerFactoryL0.cs index 5b689810b9a..1e87fc2bed7 100644 --- a/src/Test/L0/Worker/HandlerFactoryL0.cs +++ b/src/Test/L0/Worker/HandlerFactoryL0.cs @@ -33,7 +33,7 @@ private TestHostContext CreateTestContext([CallerMemberName] string testName = " [InlineData("node12", "node20")] [InlineData("node16", "node20")] [InlineData("node20", "node20")] - [InlineData("node24", "node20")] + [InlineData("node24", "node24")] public void IsNodeVersionUpgraded(string inputVersion, string expectedVersion) { using (TestHostContext hc = CreateTestContext()) @@ -77,8 +77,8 @@ public void IsNodeVersionUpgraded(string inputVersion, string expectedVersion) [Theory] [Trait("Level", "L0")] [Trait("Category", "Worker")] - [InlineData("node12", "node24")] - [InlineData("node16", "node24")] + [InlineData("node12", "node20")] + [InlineData("node16", "node20")] [InlineData("node24", "node24")] public void NodeVersionWithFeatureFlagEnabled(string inputVersion, string expectedVersion) { @@ -177,8 +177,7 @@ public void NodeVersionWithEnvironmentVariableEnabled(string envVarName, string new List() ) as INodeScriptActionHandler; - // Node 24 should be used when the feature flag is enabled via env var - string expectedVersion = "node24"; + string expectedVersion = "node20"; // Assert - should use Node 24 Assert.Equal(expectedVersion, handler.Data.NodeVersion); @@ -243,10 +242,9 @@ public void NodeVersionWithFeatureFlagNameEnvironmentVariableEnabled() new List() ) as INodeScriptActionHandler; - // Node 24 should be used when the feature flag is enabled via env var - string expectedVersion = "node24"; + string expectedVersion = "node20"; - // Assert - should use Node 24 + // Assert - should use Node 24 since the env variable doesn't affect this Assert.Equal(expectedVersion, handler.Data.NodeVersion); } } @@ -260,11 +258,12 @@ public void NodeVersionWithFeatureFlagNameEnvironmentVariableEnabled() [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] - public void Node24ExplicitlyRequested_DowngradedWhenFeatureFlagOff() + public void Node24ExplicitlyRequested_DoNotDowngradeWhenFeatureFlagOff() { using (TestHostContext hc = CreateTestContext()) { // Arrange. + var hf = new HandlerFactory(); hf.Initialize(hc); @@ -293,8 +292,8 @@ public void Node24ExplicitlyRequested_DowngradedWhenFeatureFlagOff() new List() ) as INodeScriptActionHandler; - // Assert - should be downgraded to Node 20 - Assert.Equal("node20", handler.Data.NodeVersion); + // Assert - should be still node24 because the feature flag is just for internal node versions + Assert.Equal("node24", handler.Data.NodeVersion); } } } From f06af3351dc3b5661fbe24d6b34e4feef7618b69 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Wed, 16 Jul 2025 18:25:23 +0000 Subject: [PATCH 14/23] Remove ff and internal node usage. Warning for linux 32bit arm since node24. --- src/Runner.Common/Constants.cs | 1 - src/Runner.Common/Util/NodeUtil.cs | 17 +- src/Runner.Worker/FeatureManager.cs | 19 +- src/Runner.Worker/Handlers/StepHost.cs | 50 ++++- src/Test/L0/Util/NodeUtilL0.cs | 170 ----------------- src/Test/L0/Worker/FeatureManagerL0.cs | 58 ------ src/Test/L0/Worker/HandlerFactoryL0.cs | 194 +------------------- src/Test/L0/Worker/StepHostL0.cs | 6 - src/Test/L0/Worker/StepHostNodeVersionL0.cs | 62 +++++++ 9 files changed, 117 insertions(+), 460 deletions(-) delete mode 100644 src/Test/L0/Util/NodeUtilL0.cs delete mode 100644 src/Test/L0/Worker/FeatureManagerL0.cs create mode 100644 src/Test/L0/Worker/StepHostNodeVersionL0.cs diff --git a/src/Runner.Common/Constants.cs b/src/Runner.Common/Constants.cs index edf8a9b519a..03d01b6288d 100644 --- a/src/Runner.Common/Constants.cs +++ b/src/Runner.Common/Constants.cs @@ -169,7 +169,6 @@ public static class Features public static readonly string AllowRunnerContainerHooks = "DistributedTask.AllowRunnerContainerHooks"; public static readonly string AddCheckRunIdToJobContext = "actions_add_check_run_id_to_job_context"; public static readonly string DisplayHelpfulActionsDownloadErrors = "actions_display_helpful_actions_download_errors"; - public static readonly string UseNode24 = "runner_usenode24"; } public static readonly string InternalTelemetryIssueDataKey = "_internal_telemetry"; diff --git a/src/Runner.Common/Util/NodeUtil.cs b/src/Runner.Common/Util/NodeUtil.cs index 8c4343dd599..1a9252cde0f 100644 --- a/src/Runner.Common/Util/NodeUtil.cs +++ b/src/Runner.Common/Util/NodeUtil.cs @@ -6,8 +6,7 @@ namespace GitHub.Runner.Common.Util public static class NodeUtil { private const string _defaultNodeVersion = "node20"; - private const string _node24Version = "node24"; - public static readonly ReadOnlyCollection BuiltInNodeVersions = new(new[] { "node20", "node24" }); + public static readonly ReadOnlyCollection BuiltInNodeVersions = new(new[] { "node20" }); public static string GetInternalNodeVersion() { var forcedInternalNodeVersion = Environment.GetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion); @@ -17,20 +16,6 @@ public static string GetInternalNodeVersion() { return forcedInternalNodeVersion; } - - var useNode24FlagValue = Environment.GetEnvironmentVariable(Constants.Runner.Features.UseNode24) ?? - Environment.GetEnvironmentVariable("RUNNER_USENODE24") ?? - Environment.GetEnvironmentVariable("runner_usenode24"); - - var useNode24 = !string.IsNullOrEmpty(useNode24FlagValue) && - (string.Equals(useNode24FlagValue, "true", StringComparison.OrdinalIgnoreCase) || - string.Equals(useNode24FlagValue, "1", StringComparison.OrdinalIgnoreCase)); - - if (useNode24) - { - return _node24Version; - } - return _defaultNodeVersion; } } diff --git a/src/Runner.Worker/FeatureManager.cs b/src/Runner.Worker/FeatureManager.cs index 99ba2d30709..e783b592858 100644 --- a/src/Runner.Worker/FeatureManager.cs +++ b/src/Runner.Worker/FeatureManager.cs @@ -1,5 +1,6 @@ using System; using GitHub.Runner.Common; +using GitHub.Runner.Sdk; namespace GitHub.Runner.Worker { @@ -11,23 +12,5 @@ public static bool IsContainerHooksEnabled(Variables variables) var isContainerHooksPathSet = !string.IsNullOrEmpty(Environment.GetEnvironmentVariable(Constants.Hooks.ContainerHooksPath)); return isContainerHookFeatureFlagSet && isContainerHooksPathSet; } - - public static bool IsNode24Enabled(Variables variables) - { - bool isEnabled = variables?.GetBoolean(Constants.Runner.Features.UseNode24) ?? false; - - if (!isEnabled) - { - var envValue = Environment.GetEnvironmentVariable(Constants.Runner.Features.UseNode24) ?? - Environment.GetEnvironmentVariable("RUNNER_USENODE24") ?? - Environment.GetEnvironmentVariable("runner_usenode24"); - - isEnabled = !string.IsNullOrEmpty(envValue) && - (string.Equals(envValue, "true", StringComparison.OrdinalIgnoreCase) || - string.Equals(envValue, "1", StringComparison.OrdinalIgnoreCase)); - } - - return isEnabled; - } } } diff --git a/src/Runner.Worker/Handlers/StepHost.cs b/src/Runner.Worker/Handlers/StepHost.cs index 1270dd90e6f..f2782b2c8cd 100644 --- a/src/Runner.Worker/Handlers/StepHost.cs +++ b/src/Runner.Worker/Handlers/StepHost.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using GitHub.DistributedTask.Pipelines.ContextData; using System.Text; using System.Threading; using System.Threading.Tasks; @@ -9,11 +8,41 @@ using GitHub.Runner.Sdk; using System.Linq; using GitHub.Runner.Worker.Container.ContainerHooks; -using System.IO; using System.Threading.Channels; +using System.Runtime.InteropServices; namespace GitHub.Runner.Worker.Handlers { + /// + /// Helper class for node version compatibility checks + /// + public static class NodeCompatibilityChecker + { + /// + /// Checks if Node24 is requested but running on ARM32 Linux, and falls back to Node20 with a warning. + /// + /// The execution context for logging + /// The preferred Node version + /// The adjusted Node version + public static string CheckNodeVersionForArm32(IExecutionContext executionContext, string preferredVersion) + { + if (preferredVersion != null && preferredVersion.StartsWith("node24", StringComparison.OrdinalIgnoreCase)) + { + bool isArm32 = RuntimeInformation.ProcessArchitecture == Architecture.Arm || + Environment.GetEnvironmentVariable("PROCESSOR_ARCHITECTURE")?.Contains("ARM") == true; + bool isLinux = RuntimeInformation.IsOSPlatform(OSPlatform.Linux); + + if (isArm32 && isLinux) + { + executionContext.Warning($"Node 24 is not supported on Linux ARM32 platforms. Falling back to Node 20."); + return "node20"; + } + } + + return preferredVersion; + } + } + public interface IStepHost : IRunnerService { event EventHandler OutputDataReceived; @@ -48,6 +77,8 @@ public interface IDefaultStepHost : IStepHost { } + + public sealed class DefaultStepHost : RunnerService, IDefaultStepHost { public event EventHandler OutputDataReceived; @@ -58,9 +89,12 @@ public string ResolvePathForStepHost(IExecutionContext executionContext, string return path; } + public Task DetermineNodeRuntimeVersion(IExecutionContext executionContext, string preferredVersion) { - return Task.FromResult(preferredVersion); + // Check if Node24 is requested but we're on ARM32 Linux + string adjustedVersion = NodeCompatibilityChecker.CheckNodeVersionForArm32(executionContext, preferredVersion); + return Task.FromResult(adjustedVersion); } public async Task ExecuteAsync(IExecutionContext context, @@ -137,7 +171,9 @@ public string ResolvePathForStepHost(IExecutionContext executionContext, string public async Task DetermineNodeRuntimeVersion(IExecutionContext executionContext, string preferredVersion) { - // Optimistically use the default + // If Node24 is requested but we're on ARM32, fall back to Node20 with a warning + preferredVersion = NodeCompatibilityChecker.CheckNodeVersionForArm32(executionContext, preferredVersion); + string nodeExternal = preferredVersion; if (FeatureManager.IsContainerHooksEnabled(executionContext.Global.Variables)) @@ -264,7 +300,12 @@ await containerHookManager.RunScriptStepAsync(context, private string CheckPlatformForAlpineContainer(IExecutionContext executionContext, string preferredVersion) { + // Handle ARM32 architecture specifically for node24 + preferredVersion = NodeCompatibilityChecker.CheckNodeVersionForArm32(executionContext, preferredVersion); + string nodeExternal = preferredVersion; + + // Check for Alpine container compatibility if (!Constants.Runner.PlatformArchitecture.Equals(Constants.Architecture.X64)) { var os = Constants.Runner.Platform.ToString(); @@ -272,6 +313,7 @@ private string CheckPlatformForAlpineContainer(IExecutionContext executionContex var msg = $"JavaScript Actions in Alpine containers are only supported on x64 Linux runners. Detected {os} {arch}"; throw new NotSupportedException(msg); } + nodeExternal = $"{preferredVersion}_alpine"; executionContext.Debug($"Container distribution is alpine. Running JavaScript Action with external tool: {nodeExternal}"); return nodeExternal; diff --git a/src/Test/L0/Util/NodeUtilL0.cs b/src/Test/L0/Util/NodeUtilL0.cs deleted file mode 100644 index 0daedb9d4a4..00000000000 --- a/src/Test/L0/Util/NodeUtilL0.cs +++ /dev/null @@ -1,170 +0,0 @@ -using System; -using System.Collections.Generic; -using Xunit; -using GitHub.Runner.Common.Util; - -namespace GitHub.Runner.Common.Tests.Util -{ - public class NodeUtilL0 - { - [Fact] - public void GetInternalNodeVersion_DefaultsToNode20() - { - // Arrange - Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); - Environment.SetEnvironmentVariable("RUNNER_USENODE24", null); - Environment.SetEnvironmentVariable("runner_usenode24", null); - - // Act - var nodeVersion = NodeUtil.GetInternalNodeVersion(); - - // Assert - Assert.Equal("node20", nodeVersion); - } - - [Theory] - [InlineData("node20")] - [InlineData("node24")] - public void GetInternalNodeVersion_ReturnsCorrectForcedInternalNodeVersion(string forcedVersion) - { - // Arrange - Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, forcedVersion); - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); - Environment.SetEnvironmentVariable("RUNNER_USENODE24", null); - Environment.SetEnvironmentVariable("runner_usenode24", null); - - // Act - var nodeVersion = NodeUtil.GetInternalNodeVersion(); - - // Assert - Assert.Equal(forcedVersion, nodeVersion); - - // Cleanup - Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); - } - - [Fact] - public void GetInternalNodeVersion_IgnoresInvalidForcedInternalNodeVersion() - { - // Arrange - Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, "invalidVersion"); - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); - Environment.SetEnvironmentVariable("RUNNER_USENODE24", null); - Environment.SetEnvironmentVariable("runner_usenode24", null); - - // Act - var nodeVersion = NodeUtil.GetInternalNodeVersion(); - - // Assert - Assert.Equal("node20", nodeVersion); - - // Cleanup - Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); - } - - [Theory] - [InlineData("true")] - [InlineData("TRUE")] - [InlineData("True")] - [InlineData("1")] - public void GetInternalNodeVersion_UsesNode24WhenFeatureFlagEnabled(string flagValue) - { - // Arrange - Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, flagValue); - - // Act - var nodeVersion = NodeUtil.GetInternalNodeVersion(); - - // Assert - Assert.Equal("node24", nodeVersion); - - // Cleanup - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); - } - - [Theory] - [InlineData("true")] - [InlineData("TRUE")] - [InlineData("True")] - [InlineData("1")] - public void GetInternalNodeVersion_UsesNode24WhenRunnerUseNode24Enabled(string flagValue) - { - // Arrange - Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); - Environment.SetEnvironmentVariable("RUNNER_USENODE24", flagValue); - - // Act - var nodeVersion = NodeUtil.GetInternalNodeVersion(); - - // Assert - Assert.Equal("node24", nodeVersion); - - // Cleanup - Environment.SetEnvironmentVariable("RUNNER_USENODE24", null); - } - - [Theory] - [InlineData("true")] - [InlineData("TRUE")] - [InlineData("True")] - [InlineData("1")] - public void GetInternalNodeVersion_UsesNode24WhenrunnerUseNode24Enabled(string flagValue) - { - // Arrange - Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); - Environment.SetEnvironmentVariable("RUNNER_USENODE24", null); - Environment.SetEnvironmentVariable("runner_usenode24", flagValue); - - // Act - var nodeVersion = NodeUtil.GetInternalNodeVersion(); - - // Assert - Assert.Equal("node24", nodeVersion); - - // Cleanup - Environment.SetEnvironmentVariable("runner_usenode24", null); - } - - [Theory] - [InlineData("false")] - [InlineData("0")] - [InlineData("")] - public void GetInternalNodeVersion_UsesDefaultWhenFeatureFlagDisabled(string flagValue) - { - // Arrange - Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, flagValue); - - // Act - var nodeVersion = NodeUtil.GetInternalNodeVersion(); - - // Assert - Assert.Equal("node20", nodeVersion); - - // Cleanup - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); - } - - [Fact] - public void GetInternalNodeVersion_ForcedVersionTakesPrecedenceOverFeatureFlag() - { - // Arrange - Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, "node20"); - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, "true"); - - // Act - var nodeVersion = NodeUtil.GetInternalNodeVersion(); - - // Assert - Assert.Equal("node20", nodeVersion); - - // Cleanup - Environment.SetEnvironmentVariable(Constants.Variables.Agent.ForcedInternalNodeVersion, null); - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); - } - } -} \ No newline at end of file diff --git a/src/Test/L0/Worker/FeatureManagerL0.cs b/src/Test/L0/Worker/FeatureManagerL0.cs deleted file mode 100644 index 55700e6cea9..00000000000 --- a/src/Test/L0/Worker/FeatureManagerL0.cs +++ /dev/null @@ -1,58 +0,0 @@ -using System.Collections.Generic; -using System.Runtime.CompilerServices; -using GitHub.DistributedTask.WebApi; -using GitHub.Runner.Worker; -using Xunit; - -namespace GitHub.Runner.Common.Tests.Worker -{ - public sealed class FeatureManagerL0 - { - private TestHostContext CreateTestContext([CallerMemberName] string testName = "") - { - var hostContext = new TestHostContext(this, testName); - return hostContext; - } - - [Fact] - [Trait("Level", "L0")] - [Trait("Category", "Worker")] - public void IsNode24EnabledWhenFeatureFlagSet() - { - using (TestHostContext hc = CreateTestContext()) - { - // Arrange - var variables = new Dictionary - { - { Constants.Runner.Features.UseNode24, new VariableValue("true") } - }; - Variables serverVariables = new(hc, variables); - - // Act - bool isNode24Enabled = FeatureManager.IsNode24Enabled(serverVariables); - - // Assert - Assert.True(isNode24Enabled); - } - } - - [Fact] - [Trait("Level", "L0")] - [Trait("Category", "Worker")] - public void IsNode24DisabledWhenFeatureFlagNotSet() - { - using (TestHostContext hc = CreateTestContext()) - { - // Arrange - var variables = new Dictionary(); - Variables serverVariables = new(hc, variables); - - // Act - bool isNode24Enabled = FeatureManager.IsNode24Enabled(serverVariables); - - // Assert - Assert.False(isNode24Enabled); - } - } - } -} diff --git a/src/Test/L0/Worker/HandlerFactoryL0.cs b/src/Test/L0/Worker/HandlerFactoryL0.cs index 1e87fc2bed7..37981e46aa9 100644 --- a/src/Test/L0/Worker/HandlerFactoryL0.cs +++ b/src/Test/L0/Worker/HandlerFactoryL0.cs @@ -42,7 +42,7 @@ public void IsNodeVersionUpgraded(string inputVersion, string expectedVersion) var hf = new HandlerFactory(); hf.Initialize(hc); - // Server Feature Flag + // Setup variables var variables = new Dictionary(); Variables serverVariables = new(hc, variables); @@ -73,201 +73,21 @@ public void IsNodeVersionUpgraded(string inputVersion, string expectedVersion) Assert.Equal(expectedVersion, handler.Data.NodeVersion); } } - - [Theory] - [Trait("Level", "L0")] - [Trait("Category", "Worker")] - [InlineData("node12", "node20")] - [InlineData("node16", "node20")] - [InlineData("node24", "node24")] - public void NodeVersionWithFeatureFlagEnabled(string inputVersion, string expectedVersion) - { - using (TestHostContext hc = CreateTestContext()) - { - // Arrange. - var hf = new HandlerFactory(); - hf.Initialize(hc); - - // Server Feature Flag - enable Node 24 - var variables = new Dictionary - { - { Constants.Runner.Features.UseNode24, new VariableValue("true") } - }; - Variables serverVariables = new(hc, variables); - - // Workflow opt-out - var workflowVariables = new Dictionary(); - _ec.Setup(x => x.Global).Returns(new GlobalContext() - { - Variables = serverVariables, - EnvironmentVariables = workflowVariables - }); - - // Act. - var data = new NodeJSActionExecutionData(); - data.NodeVersion = inputVersion; - var handler = hf.Create( - _ec.Object, - new ScriptReference(), - new Mock().Object, - data, - new Dictionary(), - new Dictionary(), - new Variables(hc, new Dictionary()), "", new List() - ) as INodeScriptActionHandler; + - // Assert. - Assert.Equal(expectedVersion, handler.Data.NodeVersion); - } - } - - [Theory] - [Trait("Level", "L0")] - [Trait("Category", "Worker")] - [InlineData("runner_usenode24", "true")] - [InlineData("RUNNER_USENODE24", "true")] - [InlineData("runner_usenode24", "1")] - [InlineData("RUNNER_USENODE24", "1")] - public void NodeVersionWithEnvironmentVariableEnabled(string envVarName, string envVarValue) - { - try - { - // Set the environment variable for testing - Environment.SetEnvironmentVariable(envVarName, envVarValue); - - using (TestHostContext hc = CreateTestContext()) - { - // Arrange. - var hf = new HandlerFactory(); - hf.Initialize(hc); - - // Server Feature Flag - NOT set in variables - var variables = new Dictionary(); - Variables serverVariables = new(hc, variables); - - // Workflow variables - var workflowVariables = new Dictionary(); - - _ec.Setup(x => x.Global).Returns(new GlobalContext() - { - Variables = serverVariables, - EnvironmentVariables = workflowVariables - }); - - // Act - Test with one Node version to verify the behavior - var inputVersion = "node16"; // Use node16 as a representative case - - // Make sure we have a handler instance to use - var nodeHandler = new Mock(); - nodeHandler.SetupAllProperties(); - hc.EnqueueInstance(nodeHandler.Object); - - var data = new NodeJSActionExecutionData(); - data.NodeVersion = inputVersion; - var handler = hf.Create( - _ec.Object, - new ScriptReference(), - new Mock().Object, - data, - new Dictionary(), - new Dictionary(), - new Variables(hc, new Dictionary()), - "", - new List() - ) as INodeScriptActionHandler; - - string expectedVersion = "node20"; - - // Assert - should use Node 24 - Assert.Equal(expectedVersion, handler.Data.NodeVersion); - } - } - finally - { - // Clean up the environment variable after the test - Environment.SetEnvironmentVariable(envVarName, null); - } - } - [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] - public void NodeVersionWithFeatureFlagNameEnvironmentVariableEnabled() - { - try - { - // Use the actual feature flag name from constants - string envVarName = Constants.Runner.Features.UseNode24; - Environment.SetEnvironmentVariable(envVarName, "true"); - - using (TestHostContext hc = CreateTestContext()) - { - // Arrange. - var hf = new HandlerFactory(); - hf.Initialize(hc); - - // Server Feature Flag - NOT set in variables - var variables = new Dictionary(); - Variables serverVariables = new(hc, variables); - - // Workflow variables - var workflowVariables = new Dictionary(); - - _ec.Setup(x => x.Global).Returns(new GlobalContext() - { - Variables = serverVariables, - EnvironmentVariables = workflowVariables - }); - - // Act - Test with one Node version to verify the behavior - var inputVersion = "node16"; // Use node16 as a representative case - - // Make sure we have a handler instance to use - var nodeHandler = new Mock(); - nodeHandler.SetupAllProperties(); - hc.EnqueueInstance(nodeHandler.Object); - - var data = new NodeJSActionExecutionData(); - data.NodeVersion = inputVersion; - var handler = hf.Create( - _ec.Object, - new ScriptReference(), - new Mock().Object, - data, - new Dictionary(), - new Dictionary(), - new Variables(hc, new Dictionary()), - "", - new List() - ) as INodeScriptActionHandler; - - string expectedVersion = "node20"; - - // Assert - should use Node 24 since the env variable doesn't affect this - Assert.Equal(expectedVersion, handler.Data.NodeVersion); - } - } - finally - { - // Clean up the environment variable after the test - Environment.SetEnvironmentVariable(Constants.Runner.Features.UseNode24, null); - } - } - - [Fact] - [Trait("Level", "L0")] - [Trait("Category", "Worker")] - public void Node24ExplicitlyRequested_DoNotDowngradeWhenFeatureFlagOff() + public void Node24ExplicitlyRequested_HonoredByDefault() { using (TestHostContext hc = CreateTestContext()) { // Arrange. - var hf = new HandlerFactory(); hf.Initialize(hc); - // Server Feature Flag - feature flag NOT set + // Basic variables setup var variables = new Dictionary(); Variables serverVariables = new(hc, variables); @@ -287,12 +107,12 @@ public void Node24ExplicitlyRequested_DoNotDowngradeWhenFeatureFlagOff() data, new Dictionary(), new Dictionary(), - new Variables(hc, new Dictionary()), - "", + new Variables(hc, new Dictionary()), + "", new List() ) as INodeScriptActionHandler; - // Assert - should be still node24 because the feature flag is just for internal node versions + // Assert - should be node24 as requested Assert.Equal("node24", handler.Data.NodeVersion); } } diff --git a/src/Test/L0/Worker/StepHostL0.cs b/src/Test/L0/Worker/StepHostL0.cs index 9148cbe09ab..bac7d41d90a 100644 --- a/src/Test/L0/Worker/StepHostL0.cs +++ b/src/Test/L0/Worker/StepHostL0.cs @@ -175,9 +175,6 @@ public async Task DetermineNode24RuntimeVersionInAlpineContainerAsync() sh.Initialize(hc); sh.Container = new ContainerInfo() { ContainerId = "1234abcd" }; - // Enable Node 24 feature flag - _ec.Object.Global.Variables.Set(Constants.Runner.Features.UseNode24, "true"); - _dc.Setup(d => d.DockerExec(_ec.Object, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())) .Callback((IExecutionContext ec, string id, string options, string command, List output) => { @@ -205,9 +202,6 @@ public async Task DetermineNode24RuntimeVersionInUnknownContainerAsync() sh.Initialize(hc); sh.Container = new ContainerInfo() { ContainerId = "1234abcd" }; - // Enable Node 24 feature flag - _ec.Object.Global.Variables.Set(Constants.Runner.Features.UseNode24, "true"); - _dc.Setup(d => d.DockerExec(_ec.Object, It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())) .Callback((IExecutionContext ec, string id, string options, string command, List output) => { diff --git a/src/Test/L0/Worker/StepHostNodeVersionL0.cs b/src/Test/L0/Worker/StepHostNodeVersionL0.cs new file mode 100644 index 00000000000..82fb1bf1746 --- /dev/null +++ b/src/Test/L0/Worker/StepHostNodeVersionL0.cs @@ -0,0 +1,62 @@ +using GitHub.Runner.Common.Tests; +using GitHub.Runner.Worker; +using GitHub.Runner.Worker.Container; +using GitHub.Runner.Worker.Handlers; +using Moq; +using System; +using System.Collections.Generic; +using System.Runtime.InteropServices; +using System.Threading.Tasks; +using Xunit; + +namespace GitHub.Runner.Common.Tests.Worker +{ + public sealed class StepHostNodeVersionL0 + { + private Mock _ec; + private DefaultStepHost _defaultStepHost; + + public StepHostNodeVersionL0() + { + _ec = new Mock(); + _defaultStepHost = new DefaultStepHost(); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CheckNodeVersionForArm32_Node24OnArm32Linux() + { + // Test the NodeCompatibilityChecker directly + string preferredVersion = "node24"; + string result = NodeCompatibilityChecker.CheckNodeVersionForArm32(_ec.Object, preferredVersion); + + // Verify we called Warning if on ARM32 + if (RuntimeInformation.ProcessArchitecture == Architecture.Arm && + RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + { + _ec.Verify(x => x.Warning(It.IsAny()), Times.Once); + Assert.Equal("node20", result); + } + else + { + // On non-ARM32 platforms, should pass through the version unmodified + _ec.Verify(x => x.Warning(It.IsAny()), Times.Never); + Assert.Equal("node24", result); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void CheckNodeVersionForArm32_PassThroughNonNode24Versions() + { + string preferredVersion = "node20"; + string result = NodeCompatibilityChecker.CheckNodeVersionForArm32(_ec.Object, preferredVersion); + + // Should never warn or modify the version for non-node24 inputs + _ec.Verify(x => x.Warning(It.IsAny()), Times.Never); + Assert.Equal("node20", result); + } + } +} From 219a6966a27fdff8f3378c38f8543b52bdbbbc8a Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Wed, 16 Jul 2025 19:27:02 +0000 Subject: [PATCH 15/23] remove warning checks --- src/Runner.Common/Util/PlatformUtil.cs | 0 src/Test/L0/Worker/StepHostNodeVersionL0.cs | 15 ++++++++------- 2 files changed, 8 insertions(+), 7 deletions(-) create mode 100644 src/Runner.Common/Util/PlatformUtil.cs diff --git a/src/Runner.Common/Util/PlatformUtil.cs b/src/Runner.Common/Util/PlatformUtil.cs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/Test/L0/Worker/StepHostNodeVersionL0.cs b/src/Test/L0/Worker/StepHostNodeVersionL0.cs index 82fb1bf1746..cb1cb59881e 100644 --- a/src/Test/L0/Worker/StepHostNodeVersionL0.cs +++ b/src/Test/L0/Worker/StepHostNodeVersionL0.cs @@ -31,17 +31,19 @@ public void CheckNodeVersionForArm32_Node24OnArm32Linux() string preferredVersion = "node24"; string result = NodeCompatibilityChecker.CheckNodeVersionForArm32(_ec.Object, preferredVersion); - // Verify we called Warning if on ARM32 - if (RuntimeInformation.ProcessArchitecture == Architecture.Arm && - RuntimeInformation.IsOSPlatform(OSPlatform.Linux)) + // On ARM32 Linux, we should fall back to node20 + bool isArm32 = RuntimeInformation.ProcessArchitecture == Architecture.Arm || + Environment.GetEnvironmentVariable("PROCESSOR_ARCHITECTURE")?.Contains("ARM") == true; + bool isLinux = RuntimeInformation.IsOSPlatform(OSPlatform.Linux); + + if (isArm32 && isLinux) { - _ec.Verify(x => x.Warning(It.IsAny()), Times.Once); + // Should downgrade to node20 on ARM32 Linux Assert.Equal("node20", result); } else { // On non-ARM32 platforms, should pass through the version unmodified - _ec.Verify(x => x.Warning(It.IsAny()), Times.Never); Assert.Equal("node24", result); } } @@ -54,8 +56,7 @@ public void CheckNodeVersionForArm32_PassThroughNonNode24Versions() string preferredVersion = "node20"; string result = NodeCompatibilityChecker.CheckNodeVersionForArm32(_ec.Object, preferredVersion); - // Should never warn or modify the version for non-node24 inputs - _ec.Verify(x => x.Warning(It.IsAny()), Times.Never); + // Should never modify the version for non-node24 inputs Assert.Equal("node20", result); } } From 7a5e49a74508ff704c42675a5a67da4db6944370 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Wed, 16 Jul 2025 19:32:45 +0000 Subject: [PATCH 16/23] Rename method (linuxarm32) --- src/Runner.Worker/Handlers/StepHost.cs | 8 ++++---- src/Test/L0/Worker/StepHostNodeVersionL0.cs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Runner.Worker/Handlers/StepHost.cs b/src/Runner.Worker/Handlers/StepHost.cs index f2782b2c8cd..99555016fa7 100644 --- a/src/Runner.Worker/Handlers/StepHost.cs +++ b/src/Runner.Worker/Handlers/StepHost.cs @@ -24,7 +24,7 @@ public static class NodeCompatibilityChecker /// The execution context for logging /// The preferred Node version /// The adjusted Node version - public static string CheckNodeVersionForArm32(IExecutionContext executionContext, string preferredVersion) + public static string CheckNodeVersionForLinusArm32(IExecutionContext executionContext, string preferredVersion) { if (preferredVersion != null && preferredVersion.StartsWith("node24", StringComparison.OrdinalIgnoreCase)) { @@ -93,7 +93,7 @@ public string ResolvePathForStepHost(IExecutionContext executionContext, string public Task DetermineNodeRuntimeVersion(IExecutionContext executionContext, string preferredVersion) { // Check if Node24 is requested but we're on ARM32 Linux - string adjustedVersion = NodeCompatibilityChecker.CheckNodeVersionForArm32(executionContext, preferredVersion); + string adjustedVersion = NodeCompatibilityChecker.CheckNodeVersionForLinusArm32(executionContext, preferredVersion); return Task.FromResult(adjustedVersion); } @@ -172,7 +172,7 @@ public string ResolvePathForStepHost(IExecutionContext executionContext, string public async Task DetermineNodeRuntimeVersion(IExecutionContext executionContext, string preferredVersion) { // If Node24 is requested but we're on ARM32, fall back to Node20 with a warning - preferredVersion = NodeCompatibilityChecker.CheckNodeVersionForArm32(executionContext, preferredVersion); + preferredVersion = NodeCompatibilityChecker.CheckNodeVersionForLinusArm32(executionContext, preferredVersion); string nodeExternal = preferredVersion; @@ -301,7 +301,7 @@ await containerHookManager.RunScriptStepAsync(context, private string CheckPlatformForAlpineContainer(IExecutionContext executionContext, string preferredVersion) { // Handle ARM32 architecture specifically for node24 - preferredVersion = NodeCompatibilityChecker.CheckNodeVersionForArm32(executionContext, preferredVersion); + preferredVersion = NodeCompatibilityChecker.CheckNodeVersionForLinusArm32(executionContext, preferredVersion); string nodeExternal = preferredVersion; diff --git a/src/Test/L0/Worker/StepHostNodeVersionL0.cs b/src/Test/L0/Worker/StepHostNodeVersionL0.cs index cb1cb59881e..e1d47f891ea 100644 --- a/src/Test/L0/Worker/StepHostNodeVersionL0.cs +++ b/src/Test/L0/Worker/StepHostNodeVersionL0.cs @@ -29,7 +29,7 @@ public void CheckNodeVersionForArm32_Node24OnArm32Linux() { // Test the NodeCompatibilityChecker directly string preferredVersion = "node24"; - string result = NodeCompatibilityChecker.CheckNodeVersionForArm32(_ec.Object, preferredVersion); + string result = NodeCompatibilityChecker.CheckNodeVersionForLinusArm32(_ec.Object, preferredVersion); // On ARM32 Linux, we should fall back to node20 bool isArm32 = RuntimeInformation.ProcessArchitecture == Architecture.Arm || @@ -54,7 +54,7 @@ public void CheckNodeVersionForArm32_Node24OnArm32Linux() public void CheckNodeVersionForArm32_PassThroughNonNode24Versions() { string preferredVersion = "node20"; - string result = NodeCompatibilityChecker.CheckNodeVersionForArm32(_ec.Object, preferredVersion); + string result = NodeCompatibilityChecker.CheckNodeVersionForLinusArm32(_ec.Object, preferredVersion); // Should never modify the version for non-node24 inputs Assert.Equal("node20", result); From e46d4d0f2c8645b48cc501776bfc3e853ee051d8 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Wed, 16 Jul 2025 19:46:28 +0000 Subject: [PATCH 17/23] remove file --- src/Runner.Common/Util/PlatformUtil.cs | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 src/Runner.Common/Util/PlatformUtil.cs diff --git a/src/Runner.Common/Util/PlatformUtil.cs b/src/Runner.Common/Util/PlatformUtil.cs deleted file mode 100644 index e69de29bb2d..00000000000 From c4dfeba134bcef95f03f9c8e4a911bb2e0e7efa3 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Wed, 16 Jul 2025 23:40:21 +0000 Subject: [PATCH 18/23] Refactor Node version compatibility checks to NodeUtil and update tests --- src/Runner.Common/Util/NodeUtil.cs | 18 ++++++ src/Runner.Worker/Handlers/StepHost.cs | 64 ++++++-------------- src/Test/L0/Worker/NodeCompatibilityTests.cs | 0 src/Test/L0/Worker/StepHostNodeVersionL0.cs | 16 +++-- 4 files changed, 48 insertions(+), 50 deletions(-) create mode 100644 src/Test/L0/Worker/NodeCompatibilityTests.cs diff --git a/src/Runner.Common/Util/NodeUtil.cs b/src/Runner.Common/Util/NodeUtil.cs index 1a9252cde0f..7b9d9f3d849 100644 --- a/src/Runner.Common/Util/NodeUtil.cs +++ b/src/Runner.Common/Util/NodeUtil.cs @@ -18,5 +18,23 @@ public static string GetInternalNodeVersion() } return _defaultNodeVersion; } + + /// + /// Checks if Node24 is requested but running on ARM32 Linux, and determines if fallback is needed. + /// + /// The preferred Node version + /// A tuple containing the adjusted node version and an optional warning message + public static (string nodeVersion, string warningMessage) CheckNodeVersionForLinuxArm32(string preferredVersion) + { + if (!string.IsNullOrEmpty(preferredVersion) && preferredVersion.StartsWith("node24", StringComparison.OrdinalIgnoreCase)) + { + if (Constants.Runner.PlatformArchitecture.Equals(Constants.Architecture.Arm) && Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux)) + { + return ("node20", "Node 24 is not supported on Linux ARM32 platforms. Falling back to Node 20."); + } + } + + return (preferredVersion, null); + } } } diff --git a/src/Runner.Worker/Handlers/StepHost.cs b/src/Runner.Worker/Handlers/StepHost.cs index 99555016fa7..71f00e57d0b 100644 --- a/src/Runner.Worker/Handlers/StepHost.cs +++ b/src/Runner.Worker/Handlers/StepHost.cs @@ -13,36 +13,6 @@ namespace GitHub.Runner.Worker.Handlers { - /// - /// Helper class for node version compatibility checks - /// - public static class NodeCompatibilityChecker - { - /// - /// Checks if Node24 is requested but running on ARM32 Linux, and falls back to Node20 with a warning. - /// - /// The execution context for logging - /// The preferred Node version - /// The adjusted Node version - public static string CheckNodeVersionForLinusArm32(IExecutionContext executionContext, string preferredVersion) - { - if (preferredVersion != null && preferredVersion.StartsWith("node24", StringComparison.OrdinalIgnoreCase)) - { - bool isArm32 = RuntimeInformation.ProcessArchitecture == Architecture.Arm || - Environment.GetEnvironmentVariable("PROCESSOR_ARCHITECTURE")?.Contains("ARM") == true; - bool isLinux = RuntimeInformation.IsOSPlatform(OSPlatform.Linux); - - if (isArm32 && isLinux) - { - executionContext.Warning($"Node 24 is not supported on Linux ARM32 platforms. Falling back to Node 20."); - return "node20"; - } - } - - return preferredVersion; - } - } - public interface IStepHost : IRunnerService { event EventHandler OutputDataReceived; @@ -78,7 +48,6 @@ public interface IDefaultStepHost : IStepHost } - public sealed class DefaultStepHost : RunnerService, IDefaultStepHost { public event EventHandler OutputDataReceived; @@ -89,12 +58,16 @@ public string ResolvePathForStepHost(IExecutionContext executionContext, string return path; } - public Task DetermineNodeRuntimeVersion(IExecutionContext executionContext, string preferredVersion) { - // Check if Node24 is requested but we're on ARM32 Linux - string adjustedVersion = NodeCompatibilityChecker.CheckNodeVersionForLinusArm32(executionContext, preferredVersion); - return Task.FromResult(adjustedVersion); + // Use NodeUtil to check if Node24 is requested but we're on ARM32 Linux + var (nodeVersion, warningMessage) = Common.Util.NodeUtil.CheckNodeVersionForLinuxArm32(preferredVersion); + if (!string.IsNullOrEmpty(warningMessage)) + { + executionContext.Warning(warningMessage); + } + + return Task.FromResult(nodeVersion); } public async Task ExecuteAsync(IExecutionContext context, @@ -171,10 +144,12 @@ public string ResolvePathForStepHost(IExecutionContext executionContext, string public async Task DetermineNodeRuntimeVersion(IExecutionContext executionContext, string preferredVersion) { - // If Node24 is requested but we're on ARM32, fall back to Node20 with a warning - preferredVersion = NodeCompatibilityChecker.CheckNodeVersionForLinusArm32(executionContext, preferredVersion); - - string nodeExternal = preferredVersion; + // Use NodeUtil to check if Node24 is requested but we're on ARM32 Linux + var (nodeExternal, warningMessage) = Common.Util.NodeUtil.CheckNodeVersionForLinuxArm32(preferredVersion); + if (!string.IsNullOrEmpty(warningMessage)) + { + executionContext.Warning(warningMessage); + } if (FeatureManager.IsContainerHooksEnabled(executionContext.Global.Variables)) { @@ -300,10 +275,12 @@ await containerHookManager.RunScriptStepAsync(context, private string CheckPlatformForAlpineContainer(IExecutionContext executionContext, string preferredVersion) { - // Handle ARM32 architecture specifically for node24 - preferredVersion = NodeCompatibilityChecker.CheckNodeVersionForLinusArm32(executionContext, preferredVersion); - - string nodeExternal = preferredVersion; + // Use NodeUtil to check if Node24 is requested but we're on ARM32 Linux + var (nodeExternal, warningMessage) = Common.Util.NodeUtil.CheckNodeVersionForLinuxArm32(preferredVersion); + if (!string.IsNullOrEmpty(warningMessage)) + { + executionContext.Warning(warningMessage); + } // Check for Alpine container compatibility if (!Constants.Runner.PlatformArchitecture.Equals(Constants.Architecture.X64)) @@ -313,7 +290,6 @@ private string CheckPlatformForAlpineContainer(IExecutionContext executionContex var msg = $"JavaScript Actions in Alpine containers are only supported on x64 Linux runners. Detected {os} {arch}"; throw new NotSupportedException(msg); } - nodeExternal = $"{preferredVersion}_alpine"; executionContext.Debug($"Container distribution is alpine. Running JavaScript Action with external tool: {nodeExternal}"); return nodeExternal; diff --git a/src/Test/L0/Worker/NodeCompatibilityTests.cs b/src/Test/L0/Worker/NodeCompatibilityTests.cs new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/Test/L0/Worker/StepHostNodeVersionL0.cs b/src/Test/L0/Worker/StepHostNodeVersionL0.cs index e1d47f891ea..76a64071dfa 100644 --- a/src/Test/L0/Worker/StepHostNodeVersionL0.cs +++ b/src/Test/L0/Worker/StepHostNodeVersionL0.cs @@ -27,9 +27,9 @@ public StepHostNodeVersionL0() [Trait("Category", "Worker")] public void CheckNodeVersionForArm32_Node24OnArm32Linux() { - // Test the NodeCompatibilityChecker directly + // Test via NodeUtil directly string preferredVersion = "node24"; - string result = NodeCompatibilityChecker.CheckNodeVersionForLinusArm32(_ec.Object, preferredVersion); + var (nodeVersion, warningMessage) = GitHub.Runner.Common.Util.NodeUtil.CheckNodeVersionForLinuxArm32(preferredVersion); // On ARM32 Linux, we should fall back to node20 bool isArm32 = RuntimeInformation.ProcessArchitecture == Architecture.Arm || @@ -39,12 +39,15 @@ public void CheckNodeVersionForArm32_Node24OnArm32Linux() if (isArm32 && isLinux) { // Should downgrade to node20 on ARM32 Linux - Assert.Equal("node20", result); + Assert.Equal("node20", nodeVersion); + Assert.NotNull(warningMessage); + Assert.Contains("Node 24 is not supported on Linux ARM32 platforms", warningMessage); } else { // On non-ARM32 platforms, should pass through the version unmodified - Assert.Equal("node24", result); + Assert.Equal("node24", nodeVersion); + Assert.Null(warningMessage); } } @@ -54,10 +57,11 @@ public void CheckNodeVersionForArm32_Node24OnArm32Linux() public void CheckNodeVersionForArm32_PassThroughNonNode24Versions() { string preferredVersion = "node20"; - string result = NodeCompatibilityChecker.CheckNodeVersionForLinusArm32(_ec.Object, preferredVersion); + var (nodeVersion, warningMessage) = GitHub.Runner.Common.Util.NodeUtil.CheckNodeVersionForLinuxArm32(preferredVersion); // Should never modify the version for non-node24 inputs - Assert.Equal("node20", result); + Assert.Equal("node20", nodeVersion); + Assert.Null(warningMessage); } } } From 47c19eb2eacfef1a68b821abb2ee435c406a2413 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Thu, 17 Jul 2025 00:23:07 +0000 Subject: [PATCH 19/23] remove this using that is not used --- src/Runner.Worker/FeatureManager.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Runner.Worker/FeatureManager.cs b/src/Runner.Worker/FeatureManager.cs index e783b592858..98f49e8fd5f 100644 --- a/src/Runner.Worker/FeatureManager.cs +++ b/src/Runner.Worker/FeatureManager.cs @@ -1,6 +1,5 @@ using System; using GitHub.Runner.Common; -using GitHub.Runner.Sdk; namespace GitHub.Runner.Worker { From 4a15294872a143f9c6462f1f50a1efd6d6c3b63a Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Thu, 17 Jul 2025 00:25:02 +0000 Subject: [PATCH 20/23] remove file --- src/Test/L0/Worker/NodeCompatibilityTests.cs | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 src/Test/L0/Worker/NodeCompatibilityTests.cs diff --git a/src/Test/L0/Worker/NodeCompatibilityTests.cs b/src/Test/L0/Worker/NodeCompatibilityTests.cs deleted file mode 100644 index e69de29bb2d..00000000000 From c27093016699cdbe95d217107ffd04f26fd0188b Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Thu, 17 Jul 2025 00:25:39 +0000 Subject: [PATCH 21/23] remove empty lines --- src/Runner.Worker/Handlers/StepHost.cs | 1 - src/Test/L0/Worker/ActionManagerL0.cs | 1 - 2 files changed, 2 deletions(-) diff --git a/src/Runner.Worker/Handlers/StepHost.cs b/src/Runner.Worker/Handlers/StepHost.cs index 71f00e57d0b..6d0af2c1a17 100644 --- a/src/Runner.Worker/Handlers/StepHost.cs +++ b/src/Runner.Worker/Handlers/StepHost.cs @@ -47,7 +47,6 @@ public interface IDefaultStepHost : IStepHost { } - public sealed class DefaultStepHost : RunnerService, IDefaultStepHost { public event EventHandler OutputDataReceived; diff --git a/src/Test/L0/Worker/ActionManagerL0.cs b/src/Test/L0/Worker/ActionManagerL0.cs index 8ba4051c4ce..328c5b5f61b 100644 --- a/src/Test/L0/Worker/ActionManagerL0.cs +++ b/src/Test/L0/Worker/ActionManagerL0.cs @@ -1729,7 +1729,6 @@ public void LoadsNode24ActionDefinition() } } - [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")] From 57855c467ed4e6b0b7d12ee3620d2b58a905dc8f Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Thu, 17 Jul 2025 00:36:18 +0000 Subject: [PATCH 22/23] equals instead of starts with and remove imports that are not needed --- src/Runner.Common/Util/NodeUtil.cs | 2 +- src/Runner.Worker/Handlers/StepHost.cs | 1 - src/Test/L0/Worker/StepHostNodeVersionL0.cs | 10 +++------- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/Runner.Common/Util/NodeUtil.cs b/src/Runner.Common/Util/NodeUtil.cs index 7b9d9f3d849..854d25ce49f 100644 --- a/src/Runner.Common/Util/NodeUtil.cs +++ b/src/Runner.Common/Util/NodeUtil.cs @@ -26,7 +26,7 @@ public static string GetInternalNodeVersion() /// A tuple containing the adjusted node version and an optional warning message public static (string nodeVersion, string warningMessage) CheckNodeVersionForLinuxArm32(string preferredVersion) { - if (!string.IsNullOrEmpty(preferredVersion) && preferredVersion.StartsWith("node24", StringComparison.OrdinalIgnoreCase)) + if (string.Equals(preferredVersion, "node24", StringComparison.OrdinalIgnoreCase)) { if (Constants.Runner.PlatformArchitecture.Equals(Constants.Architecture.Arm) && Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux)) { diff --git a/src/Runner.Worker/Handlers/StepHost.cs b/src/Runner.Worker/Handlers/StepHost.cs index 6d0af2c1a17..211009658e4 100644 --- a/src/Runner.Worker/Handlers/StepHost.cs +++ b/src/Runner.Worker/Handlers/StepHost.cs @@ -9,7 +9,6 @@ using System.Linq; using GitHub.Runner.Worker.Container.ContainerHooks; using System.Threading.Channels; -using System.Runtime.InteropServices; namespace GitHub.Runner.Worker.Handlers { diff --git a/src/Test/L0/Worker/StepHostNodeVersionL0.cs b/src/Test/L0/Worker/StepHostNodeVersionL0.cs index 76a64071dfa..6ba8c9fa4e7 100644 --- a/src/Test/L0/Worker/StepHostNodeVersionL0.cs +++ b/src/Test/L0/Worker/StepHostNodeVersionL0.cs @@ -1,12 +1,8 @@ -using GitHub.Runner.Common.Tests; -using GitHub.Runner.Worker; -using GitHub.Runner.Worker.Container; +using GitHub.Runner.Worker; using GitHub.Runner.Worker.Handlers; using Moq; using System; -using System.Collections.Generic; using System.Runtime.InteropServices; -using System.Threading.Tasks; using Xunit; namespace GitHub.Runner.Common.Tests.Worker @@ -29,7 +25,7 @@ public void CheckNodeVersionForArm32_Node24OnArm32Linux() { // Test via NodeUtil directly string preferredVersion = "node24"; - var (nodeVersion, warningMessage) = GitHub.Runner.Common.Util.NodeUtil.CheckNodeVersionForLinuxArm32(preferredVersion); + var (nodeVersion, warningMessage) = Common.Util.NodeUtil.CheckNodeVersionForLinuxArm32(preferredVersion); // On ARM32 Linux, we should fall back to node20 bool isArm32 = RuntimeInformation.ProcessArchitecture == Architecture.Arm || @@ -57,7 +53,7 @@ public void CheckNodeVersionForArm32_Node24OnArm32Linux() public void CheckNodeVersionForArm32_PassThroughNonNode24Versions() { string preferredVersion = "node20"; - var (nodeVersion, warningMessage) = GitHub.Runner.Common.Util.NodeUtil.CheckNodeVersionForLinuxArm32(preferredVersion); + var (nodeVersion, warningMessage) = Common.Util.NodeUtil.CheckNodeVersionForLinuxArm32(preferredVersion); // Should never modify the version for non-node24 inputs Assert.Equal("node20", nodeVersion); From d5ae14d7978b4afddb19ca1f603a91a970e8eb5c Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Thu, 17 Jul 2025 00:56:22 +0000 Subject: [PATCH 23/23] Remove if statement nesting --- src/Runner.Common/Util/NodeUtil.cs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/Runner.Common/Util/NodeUtil.cs b/src/Runner.Common/Util/NodeUtil.cs index 854d25ce49f..65a3278fb02 100644 --- a/src/Runner.Common/Util/NodeUtil.cs +++ b/src/Runner.Common/Util/NodeUtil.cs @@ -26,12 +26,11 @@ public static string GetInternalNodeVersion() /// A tuple containing the adjusted node version and an optional warning message public static (string nodeVersion, string warningMessage) CheckNodeVersionForLinuxArm32(string preferredVersion) { - if (string.Equals(preferredVersion, "node24", StringComparison.OrdinalIgnoreCase)) + if (string.Equals(preferredVersion, "node24", StringComparison.OrdinalIgnoreCase) && + Constants.Runner.PlatformArchitecture.Equals(Constants.Architecture.Arm) && + Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux)) { - if (Constants.Runner.PlatformArchitecture.Equals(Constants.Architecture.Arm) && Constants.Runner.Platform.Equals(Constants.OSPlatform.Linux)) - { - return ("node20", "Node 24 is not supported on Linux ARM32 platforms. Falling back to Node 20."); - } + return ("node20", "Node 24 is not supported on Linux ARM32 platforms. Falling back to Node 20."); } return (preferredVersion, null);