diff --git a/src/Runner.Worker/Dap/JobExecutionView.cs b/src/Runner.Worker/Dap/JobExecutionView.cs index af7e7a01e..7e0367067 100644 --- a/src/Runner.Worker/Dap/JobExecutionView.cs +++ b/src/Runner.Worker/Dap/JobExecutionView.cs @@ -125,20 +125,25 @@ namespace GitHub.Runner.Worker.Dap } /// - /// Append a new entry. If is non-null, - /// registers the IStep -> line mapping for later lookup. If - /// is non-null, the entry is registered - /// as an unclaimed placeholder that a future - /// call can bind to a real - /// IStep (used by the predictive Post-step path). Re-renders the - /// YAML and updates the start-line table. + /// Append a new entry. Exactly one of + /// or may be non-null (or both may be + /// null for a static entry that needs no line lookup): + /// - non-null: registers the + /// IStep→line mapping immediately. Use when the real + /// is known at append time. + /// - non-null: registers an unclaimed + /// placeholder that a later binds to a + /// real . + /// Re-renders the YAML and updates the start-line table. /// /// 1-based line number of the newly-appended entry's - step: key. public int Append(JobExecutionViewEntry entry, IStep stepIdentity = null, string matchKey = null) { - if (entry == null) + ArgUtil.NotNull(entry, nameof(entry)); + if (stepIdentity != null && matchKey != null) { - throw new ArgumentNullException(nameof(entry)); + throw new ArgumentException( + "Append cannot register both a step identity and a placeholder match key on the same entry; pass at most one."); } lock (_lock) diff --git a/src/Runner.Worker/Dap/JobExecutionViewRenderer.cs b/src/Runner.Worker/Dap/JobExecutionViewRenderer.cs index 1c529fe05..2d3da89cb 100644 --- a/src/Runner.Worker/Dap/JobExecutionViewRenderer.cs +++ b/src/Runner.Worker/Dap/JobExecutionViewRenderer.cs @@ -366,10 +366,17 @@ namespace GitHub.Runner.Worker.Dap emitter.Emit(new StreamEnd()); string raw = sw.ToString(); + // Strip YAML document markers. Emitter elides these for most + // scalars but emits "--- " (with space) for some edge cases + // (e.g. empty strings). Defensively handle "---\n" too. if (raw.StartsWith("--- ", StringComparison.Ordinal)) { raw = raw.Substring(4); } + else if (raw.StartsWith("---\n", StringComparison.Ordinal)) + { + raw = raw.Substring(4); + } raw = raw.TrimEnd('\n'); const string DocEndMarker = "\n..."; if (raw.EndsWith(DocEndMarker, StringComparison.Ordinal)) diff --git a/src/Runner.Worker/Dap/StepEntryTranslator.cs b/src/Runner.Worker/Dap/StepEntryTranslator.cs index 4147c6655..9065c370e 100644 --- a/src/Runner.Worker/Dap/StepEntryTranslator.cs +++ b/src/Runner.Worker/Dap/StepEntryTranslator.cs @@ -16,12 +16,14 @@ namespace GitHub.Runner.Worker.Dap internal static class StepEntryTranslator { // Run-step internals carried on ActionStep.Inputs that are NOT - // user-authored `with:` entries. + // user-authored `with:` entries. The runner stores these under + // the keys defined in PipelineConstants.ScriptStepInputs, NOT + // their kebab-case workflow-YAML spellings. private static readonly HashSet RunStepInternalKeys = new(StringComparer.Ordinal) { - "script", - "shell", - "working-directory", + PipelineConstants.ScriptStepInputs.Script, + PipelineConstants.ScriptStepInputs.Shell, + PipelineConstants.ScriptStepInputs.WorkingDirectory, }; /// @@ -116,11 +118,11 @@ namespace GitHub.Runner.Worker.Dap var inputs = action.Inputs as MappingToken; if (inputs != null) { - if (TryGetMapValue(inputs, "script", out var scriptTok) && scriptTok != null) + if (TryGetMapValue(inputs, PipelineConstants.ScriptStepInputs.Script, out var scriptTok) && scriptTok != null) { run = scriptTok.ToString(); } - if (TryGetMapValue(inputs, "shell", out var shellTok) && shellTok != null) + if (TryGetMapValue(inputs, PipelineConstants.ScriptStepInputs.Shell, out var shellTok) && shellTok != null) { string shellText = shellTok.ToString(); if (!string.IsNullOrEmpty(shellText)) @@ -128,7 +130,7 @@ namespace GitHub.Runner.Worker.Dap shell = shellText; } } - if (TryGetMapValue(inputs, "working-directory", out var wdTok) && wdTok != null) + if (TryGetMapValue(inputs, PipelineConstants.ScriptStepInputs.WorkingDirectory, out var wdTok) && wdTok != null) { string wdText = wdTok.ToString(); if (!string.IsNullOrEmpty(wdText)) diff --git a/src/Runner.Worker/Dap/TemplateTokenYamlAdapter.cs b/src/Runner.Worker/Dap/TemplateTokenYamlAdapter.cs index 973ee7ee4..3436d9169 100644 --- a/src/Runner.Worker/Dap/TemplateTokenYamlAdapter.cs +++ b/src/Runner.Worker/Dap/TemplateTokenYamlAdapter.cs @@ -107,11 +107,20 @@ namespace GitHub.Runner.Worker.Dap adapter.WriteEnd(); string raw = sw.ToString(); - // Strip YAML document markers ("--- " prefix and "\n..." suffix). + // Strip YAML document markers. The Emitter most commonly elides + // these for our use (DocumentStart isImplicit=true), but emits + // them for some scalar edge cases (e.g. empty strings) and may + // emit them on their own line for collection roots under some + // settings. Strip both shapes defensively so callers never see + // a leaked marker leak into the embedded fragment. if (raw.StartsWith("--- ", StringComparison.Ordinal)) { raw = raw.Substring(4); } + else if (raw.StartsWith("---\n", StringComparison.Ordinal)) + { + raw = raw.Substring(4); + } const string DocEndMarker = "\n..."; if (raw.EndsWith(DocEndMarker + "\n", StringComparison.Ordinal)) { diff --git a/src/Test/L0/Worker/JobExecutionViewL0.cs b/src/Test/L0/Worker/JobExecutionViewL0.cs index 7bdebf4df..8fd29983a 100644 --- a/src/Test/L0/Worker/JobExecutionViewL0.cs +++ b/src/Test/L0/Worker/JobExecutionViewL0.cs @@ -422,5 +422,21 @@ namespace GitHub.Runner.Common.Tests.Worker Assert.Contains(line.Value, entryLines); } } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Append_RejectsBothStepIdentityAndMatchKey() + { + // Allowing both would orphan the IStep→line mapping the moment + // TryClaim overwrites _stepIdentities[index] for a different + // step, so the API rejects the combination at append time. + var view = new JobExecutionView("j"); + var entry = new JobExecutionViewEntry(JobExecutionPhase.Post, "Post X", uses: "actions/x@v1"); + Assert.Throws(() => + view.Append(entry, stepIdentity: NewStep("real"), matchKey: "k1")); + // State unchanged. + Assert.Equal(0, view.EntryCount); + } } } diff --git a/src/Test/L0/Worker/JobExecutionViewRendererL0.cs b/src/Test/L0/Worker/JobExecutionViewRendererL0.cs index 08f4f6fda..f7fe7af0b 100644 --- a/src/Test/L0/Worker/JobExecutionViewRendererL0.cs +++ b/src/Test/L0/Worker/JobExecutionViewRendererL0.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Linq; using GitHub.Runner.Worker.Dap; using Xunit; diff --git a/src/Test/L0/Worker/StepEntryTranslatorL0.cs b/src/Test/L0/Worker/StepEntryTranslatorL0.cs index 45edd3075..3f7c07c15 100644 --- a/src/Test/L0/Worker/StepEntryTranslatorL0.cs +++ b/src/Test/L0/Worker/StepEntryTranslatorL0.cs @@ -1,5 +1,4 @@ using System; -using System.Collections.Generic; using GitHub.DistributedTask.ObjectTemplating.Tokens; using GitHub.DistributedTask.Pipelines; using GitHub.Runner.Worker; @@ -244,13 +243,17 @@ namespace GitHub.Runner.Common.Tests.Worker [Trait("Category", "Worker")] public void Translate_RunStep_ExtractsShellAndWorkingDirectory() { + // The runner stores run-step inputs under the keys defined in + // PipelineConstants.ScriptStepInputs (camelCase), NOT their + // kebab-case workflow-YAML spellings — see + // ActionManifestManagerWrapper:244. var action = new ActionStep { Reference = new ScriptReference(), Inputs = Map( - ("script", Str("npm test")), - ("shell", Str("bash")), - ("working-directory", Str("./api"))), + (PipelineConstants.ScriptStepInputs.Script, Str("npm test")), + (PipelineConstants.ScriptStepInputs.Shell, Str("bash")), + (PipelineConstants.ScriptStepInputs.WorkingDirectory, Str("./api"))), }; var mock = NewActionRunnerMock(ActionRunStage.Main, "Run", new ScriptReference(), action); @@ -276,9 +279,9 @@ namespace GitHub.Runner.Common.Tests.Worker Reference = reference, Inputs = Map( ("mode", Str("ci")), - ("script", Str("leak")), - ("shell", Str("leak")), - ("working-directory", Str("leak"))), + (PipelineConstants.ScriptStepInputs.Script, Str("leak")), + (PipelineConstants.ScriptStepInputs.Shell, Str("leak")), + (PipelineConstants.ScriptStepInputs.WorkingDirectory, Str("leak"))), }; var mock = NewActionRunnerMock(ActionRunStage.Main, "Run", reference, action); @@ -288,9 +291,9 @@ namespace GitHub.Runner.Common.Tests.Worker Assert.NotNull(entry.WithYaml); Assert.Contains("mode: ci", entry.WithYaml); Assert.DoesNotContain("leak", entry.WithYaml); - Assert.DoesNotContain("script", entry.WithYaml); - Assert.DoesNotContain("shell", entry.WithYaml); - Assert.DoesNotContain("working-directory", entry.WithYaml); + Assert.DoesNotContain(PipelineConstants.ScriptStepInputs.Script, entry.WithYaml); + Assert.DoesNotContain(PipelineConstants.ScriptStepInputs.Shell, entry.WithYaml); + Assert.DoesNotContain(PipelineConstants.ScriptStepInputs.WorkingDirectory, entry.WithYaml); } [Fact]