From d6b52ac966ca00a90dadecde8fd4c59b867e82e7 Mon Sep 17 00:00:00 2001 From: Francesco Renzi Date: Tue, 12 May 2026 13:10:22 -0700 Subject: [PATCH] Add JobExecutionView state container Thread-safe append-only container of JobExecutionViewEntry. Supports predictive Post-step placeholders via TryClaim(matchKey, step) and TryMarkSkipped(matchKey) so the rendered view can reflect Main steps that are skipped by their if: condition. Provides line lookup by IStep for DAP stack-trace frame construction. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/Runner.Worker/Dap/JobExecutionView.cs | 299 ++++++++++++++ src/Test/L0/Worker/JobExecutionViewL0.cs | 467 ++++++++++++++++++++++ 2 files changed, 766 insertions(+) create mode 100644 src/Runner.Worker/Dap/JobExecutionView.cs create mode 100644 src/Test/L0/Worker/JobExecutionViewL0.cs diff --git a/src/Runner.Worker/Dap/JobExecutionView.cs b/src/Runner.Worker/Dap/JobExecutionView.cs new file mode 100644 index 000000000..db7af0b75 --- /dev/null +++ b/src/Runner.Worker/Dap/JobExecutionView.cs @@ -0,0 +1,299 @@ +using System; +using System.Collections.Generic; +using GitHub.Runner.Sdk; + +namespace GitHub.Runner.Worker.Dap +{ + /// + /// Stateful, append-only container that wraps + /// for runtime use. Maintains a mutable list of entries, caches the rendered YAML, + /// and provides O(1) lookup from identity to the current line + /// in the rendered YAML where that step's - step: key appears. + /// + /// Append-only growth model: post-steps are discovered lazily during execution + /// and appended. Setup/pre/main entry line numbers are stable across appends — + /// only the synthetic Cleanup boundary (which is not tracked here) shifts. + /// + internal sealed class JobExecutionView + { + private readonly object _lock = new(); + private readonly string _jobId; + private readonly List _entries = new(); + private readonly List _stepIdentities = new(); + private readonly Dictionary _lineByStep = + new(ReferenceEqualityComparer.Instance); + // Map matchKey -> entry index for placeholders awaiting a future + // TryClaim. Removed when claimed. + private readonly Dictionary _unclaimedByKey = + new(StringComparer.Ordinal); + private string _yaml; + private IReadOnlyList _entryStartLines = Array.Empty(); + + public JobExecutionView(string jobId) + { + if (string.IsNullOrWhiteSpace(jobId)) + { + throw new ArgumentException("jobId must not be null or whitespace.", nameof(jobId)); + } + + _jobId = jobId; + Render(); + } + + public string JobId + { + get { return _jobId; } + } + + /// + /// Currently rendered YAML. Always reflects all entries appended so far, + /// plus the synthetic Setup header and Cleanup footer emitted by the renderer. + /// + public string Yaml + { + get + { + lock (_lock) + { + return _yaml; + } + } + } + + /// Number of entries (excludes synthetic Setup/Cleanup boundaries). + public int EntryCount + { + get + { + lock (_lock) + { + return _entries.Count; + } + } + } + + /// + /// 1-based line where entry 's - step: key + /// currently appears in . + /// + public int GetLine(int entryIndex) + { + lock (_lock) + { + if (entryIndex < 0 || entryIndex >= _entries.Count) + { + throw new ArgumentOutOfRangeException(nameof(entryIndex)); + } + + return _entryStartLines[entryIndex]; + } + } + + /// + /// 1-based line for the entry whose reference identity + /// matches . Returns null if + /// is null or has not been registered. + /// + public int? TryGetLineForStep(IStep step) + { + if (step == null) + { + return null; + } + + lock (_lock) + { + if (_lineByStep.TryGetValue(step, out var line)) + { + return line; + } + + return null; + } + } + + /// + /// 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. + /// + /// 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) + { + throw new ArgumentNullException(nameof(entry)); + } + + lock (_lock) + { + if (stepIdentity != null && _lineByStep.ContainsKey(stepIdentity)) + { + throw new InvalidOperationException("step already registered in execution view"); + } + if (matchKey != null && _unclaimedByKey.ContainsKey(matchKey)) + { + throw new InvalidOperationException($"matchKey already registered: {matchKey}"); + } + + _entries.Add(entry); + _stepIdentities.Add(stepIdentity); + Render(); + + int index = _entries.Count - 1; + if (matchKey != null) + { + _unclaimedByKey[matchKey] = index; + } + return _entryStartLines[index]; + } + } + + /// + /// Bind a previously-appended placeholder entry (registered via + /// with + /// a non-null matchKey) to a real . + /// Returns the 1-based line of the now-claimed entry on success. + /// Returns null when no unclaimed placeholder exists for + /// , OR when + /// is already registered for a different entry (defensive). + /// Does not re-render: claim only updates the IStep -> line index. + /// + public int? TryClaim(string matchKey, IStep stepIdentity) + { + if (matchKey == null) + { + throw new ArgumentNullException(nameof(matchKey)); + } + if (stepIdentity == null) + { + throw new ArgumentNullException(nameof(stepIdentity)); + } + + lock (_lock) + { + if (!_unclaimedByKey.TryGetValue(matchKey, out int index)) + { + return null; + } + if (_lineByStep.ContainsKey(stepIdentity)) + { + // Bail rather than double-register the step. + return null; + } + + _unclaimedByKey.Remove(matchKey); + _stepIdentities[index] = stepIdentity; + _lineByStep[stepIdentity] = _entryStartLines[index]; + return _entryStartLines[index]; + } + } + + /// + /// Mark a previously-appended unclaimed placeholder as skipped. Used + /// when the predicting Main step never runs (skipped by if:), + /// so its predicted Post-step placeholder should not appear as a + /// step that will execute. Re-renders the view (inline comment only + /// — subsequent entry line numbers stay stable). + /// + /// + /// true if a matching unclaimed placeholder was marked; false when + /// no placeholder exists for , or the + /// placeholder has already been claimed (claim wins). + /// + public bool TryMarkSkipped(string matchKey) + { + ArgUtil.NotNull(matchKey, nameof(matchKey)); + + lock (_lock) + { + if (!_unclaimedByKey.TryGetValue(matchKey, out int index)) + { + return false; + } + // Defensive: only mark if it's still an unclaimed placeholder. + if (_stepIdentities[index] != null) + { + return false; + } + + if (_entries[index].IsSkipped) + { + // Idempotent — already marked. + return true; + } + _entries[index].IsSkipped = true; + _unclaimedByKey.Remove(matchKey); + Render(); + return true; + } + } + + /// + /// Bulk-append for the initial population. Equivalent to calling + /// once per pair, but renders only once at the end. + /// State is left unchanged if any input is invalid. + /// + public void AppendRange(IEnumerable<(JobExecutionViewEntry entry, IStep stepIdentity)> items) + { + ArgUtil.NotNull(items, nameof(items)); + + // Materialize first so we don't enumerate twice. + var materialized = new List<(JobExecutionViewEntry entry, IStep stepIdentity)>(items); + for (int i = 0; i < materialized.Count; i++) + { + if (materialized[i].entry == null) + { + throw new ArgumentException($"items[{i}].entry is null.", nameof(items)); + } + } + + lock (_lock) + { + // Validate no duplicates within the input or with existing identities, + // before mutating state. + var seen = new HashSet(ReferenceEqualityComparer.Instance); + foreach (var (_, stepIdentity) in materialized) + { + if (stepIdentity == null) + { + continue; + } + if (_lineByStep.ContainsKey(stepIdentity) || !seen.Add(stepIdentity)) + { + throw new InvalidOperationException("step already registered in execution view"); + } + } + + foreach (var (entry, stepIdentity) in materialized) + { + _entries.Add(entry); + _stepIdentities.Add(stepIdentity); + } + Render(); + } + } + + // Caller MUST hold _lock (constructor's call is safe — no concurrent access yet). + private void Render() + { + var result = JobExecutionViewRenderer.Render(_jobId, _entries.AsReadOnly()); + _yaml = result.Yaml; + _entryStartLines = result.EntryStartLines; + + _lineByStep.Clear(); + for (int i = 0; i < _stepIdentities.Count; i++) + { + var step = _stepIdentities[i]; + if (step != null) + { + _lineByStep[step] = _entryStartLines[i]; + } + } + } + } +} diff --git a/src/Test/L0/Worker/JobExecutionViewL0.cs b/src/Test/L0/Worker/JobExecutionViewL0.cs new file mode 100644 index 000000000..51d5232e3 --- /dev/null +++ b/src/Test/L0/Worker/JobExecutionViewL0.cs @@ -0,0 +1,467 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using GitHub.Runner.Worker; +using GitHub.Runner.Worker.Dap; +using Moq; +using Xunit; + +namespace GitHub.Runner.Common.Tests.Worker +{ + public sealed class JobExecutionViewL0 + { + private static JobExecutionViewEntry MainEntry(string name) + { + return new JobExecutionViewEntry(JobExecutionPhase.Main, name, run: name); + } + + private static IStep NewStep(string displayName = "step") + { + var mock = new Mock(); + mock.Setup(s => s.DisplayName).Returns(displayName); + return mock.Object; + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Constructor_RendersEmptyView() + { + var view = new JobExecutionView("my-job"); + + Assert.Equal(0, view.EntryCount); + Assert.Contains("# Job: my-job", view.Yaml); + Assert.Contains("- step: Setup job", view.Yaml); + Assert.Contains("- step: Complete job", view.Yaml); + + // Only the two synthetic boundaries appear. + int stepCount = view.Yaml.Split("- step: ").Length - 1; + Assert.Equal(2, stepCount); + } + + [Theory] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void Constructor_ThrowsOnInvalidJobId(string jobId) + { + Assert.Throws(() => new JobExecutionView(jobId)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Append_IncrementsEntryCount() + { + var view = new JobExecutionView("j"); + + int line0 = view.Append(MainEntry("a")); + int line1 = view.Append(MainEntry("b")); + int line2 = view.Append(MainEntry("c")); + + Assert.Equal(3, view.EntryCount); + Assert.True(line0 < line1); + Assert.True(line1 < line2); + Assert.Equal(line0, view.GetLine(0)); + Assert.Equal(line1, view.GetLine(1)); + Assert.Equal(line2, view.GetLine(2)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Append_PreservesPriorEntryLines() + { + var view = new JobExecutionView("j"); + + int l0 = view.Append(MainEntry("a")); + int l1 = view.Append(MainEntry("b")); + int l2 = view.Append(MainEntry("c")); + + view.Append(MainEntry("d")); + Assert.Equal(l0, view.GetLine(0)); + Assert.Equal(l1, view.GetLine(1)); + Assert.Equal(l2, view.GetLine(2)); + + view.Append(MainEntry("e")); + Assert.Equal(l0, view.GetLine(0)); + Assert.Equal(l1, view.GetLine(1)); + Assert.Equal(l2, view.GetLine(2)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Append_RegistersStepIdentity() + { + var view = new JobExecutionView("j"); + var step = NewStep(); + + int line = view.Append(MainEntry("a"), step); + + Assert.Equal(line, view.GetLine(0)); + Assert.Equal(line, view.TryGetLineForStep(step)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Append_NullStepIdentity_StillAppends() + { + var view = new JobExecutionView("j"); + + view.Append(MainEntry("a"), stepIdentity: null); + + Assert.Equal(1, view.EntryCount); + Assert.Null(view.TryGetLineForStep(null)); + Assert.Contains("- step: a", view.Yaml); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Append_DuplicateStepIdentity_Throws() + { + var view = new JobExecutionView("j"); + var step = NewStep(); + + view.Append(MainEntry("a"), step); + Assert.Throws(() => view.Append(MainEntry("b"), step)); + + // State preserved: only the first entry is present. + Assert.Equal(1, view.EntryCount); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Append_NullEntry_Throws() + { + var view = new JobExecutionView("j"); + Assert.Throws(() => view.Append(null)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void AppendRange_AppendsAllAndRendersOnce() + { + var view = new JobExecutionView("j"); + var steps = Enumerable.Range(0, 5).Select(i => NewStep("s" + i)).ToList(); + var items = steps + .Select((s, i) => (entry: MainEntry("e" + i), stepIdentity: s)) + .ToList(); + + view.AppendRange(items); + + Assert.Equal(5, view.EntryCount); + for (int i = 0; i < 5; i++) + { + int line = view.GetLine(i); + Assert.Equal(line, view.TryGetLineForStep(steps[i])); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void AppendRange_RejectsDuplicateInInput() + { + var view = new JobExecutionView("j"); + var dup = NewStep(); + var items = new List<(JobExecutionViewEntry, IStep)> + { + (MainEntry("a"), dup), + (MainEntry("b"), dup), + }; + + Assert.Throws(() => view.AppendRange(items)); + Assert.Equal(0, view.EntryCount); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void AppendRange_RejectsOverlapWithExisting() + { + var view = new JobExecutionView("j"); + var step = NewStep(); + view.Append(MainEntry("a"), step); + + var items = new List<(JobExecutionViewEntry, IStep)> + { + (MainEntry("b"), step), + }; + + Assert.Throws(() => view.AppendRange(items)); + Assert.Equal(1, view.EntryCount); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void AppendRange_NullItems_Throws() + { + var view = new JobExecutionView("j"); + Assert.Throws(() => view.AppendRange(null)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void TryGetLineForStep_NullStep_ReturnsNull() + { + var view = new JobExecutionView("j"); + Assert.Null(view.TryGetLineForStep(null)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void TryGetLineForStep_UnknownStep_ReturnsNull() + { + var view = new JobExecutionView("j"); + var step = NewStep(); + Assert.Null(view.TryGetLineForStep(step)); + } + + [Theory] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + [InlineData(-1)] + [InlineData(2)] + public void GetLine_OutOfRange_Throws(int index) + { + var view = new JobExecutionView("j"); + view.Append(MainEntry("a")); + view.Append(MainEntry("b")); + + Assert.Throws(() => view.GetLine(index)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Yaml_UpdatesAfterAppend() + { + var view = new JobExecutionView("j"); + view.Append(MainEntry("first")); + string before = view.Yaml; + Assert.Contains("- step: first", before); + + view.Append(MainEntry("second")); + string after = view.Yaml; + + Assert.Contains("- step: first", after); + Assert.Contains("- step: second", after); + Assert.NotEqual(before, after); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Yaml_AlwaysEndsWithCleanupBoundary() + { + var view = new JobExecutionView("j"); + Assert.EndsWith("cleanup:\n - step: Complete job\n", view.Yaml); + + view.Append(MainEntry("a")); + Assert.EndsWith("cleanup:\n - step: Complete job\n", view.Yaml); + + view.Append(MainEntry("b")); + view.Append(MainEntry("c")); + Assert.EndsWith("cleanup:\n - step: Complete job\n", view.Yaml); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Append_WithMatchKey_TracksUnclaimed() + { + var view = new JobExecutionView("j"); + + int line = view.Append(MainEntry("placeholder"), stepIdentity: null, matchKey: "k1"); + + var step = NewStep("real"); + int? claimed = view.TryClaim("k1", step); + Assert.Equal(line, claimed); + Assert.Equal(line, view.TryGetLineForStep(step)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void TryClaim_UnknownKey_ReturnsNull() + { + var view = new JobExecutionView("j"); + view.Append(MainEntry("a"), stepIdentity: null, matchKey: "k1"); + + Assert.Null(view.TryClaim("nope", NewStep())); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void TryClaim_AlreadyClaimed_ReturnsNull() + { + var view = new JobExecutionView("j"); + view.Append(MainEntry("a"), stepIdentity: null, matchKey: "k1"); + + var first = NewStep("first"); + Assert.NotNull(view.TryClaim("k1", first)); + + var second = NewStep("second"); + Assert.Null(view.TryClaim("k1", second)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void TryClaim_StepAlreadyRegistered_ReturnsNull() + { + var view = new JobExecutionView("j"); + var step = NewStep(); + // Step is registered for the first entry. + view.Append(MainEntry("a"), step); + // A placeholder is registered for the second entry. + view.Append(MainEntry("b"), stepIdentity: null, matchKey: "k1"); + + // Trying to claim the placeholder with the already-registered + // step must return null (defensive — would otherwise double-bind). + Assert.Null(view.TryClaim("k1", step)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Append_DuplicateMatchKey_Throws() + { + var view = new JobExecutionView("j"); + view.Append(MainEntry("a"), stepIdentity: null, matchKey: "k1"); + + Assert.Throws( + () => view.Append(MainEntry("b"), stepIdentity: null, matchKey: "k1")); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void Append_MatchKeyNull_BehavesLikeOldOverload() + { + var view = new JobExecutionView("j"); + var step = NewStep(); + + int line = view.Append(MainEntry("a"), step); + + Assert.Equal(line, view.GetLine(0)); + Assert.Equal(line, view.TryGetLineForStep(step)); + // TryClaim with any key must return null since no matchKey was registered. + Assert.Null(view.TryClaim("anything", NewStep())); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void TryClaim_AfterClaim_TryGetLineForStepResolves() + { + var view = new JobExecutionView("j"); + int line = view.Append(MainEntry("placeholder"), stepIdentity: null, matchKey: "k1"); + + var step = NewStep(); + Assert.Equal(line, view.TryClaim("k1", step)); + Assert.Equal(line, view.TryGetLineForStep(step)); + + // And a later Append doesn't lose the claim (Render rebuilds + // the IStep -> line map from the persisted identities). + view.Append(MainEntry("b")); + Assert.Equal(line, view.TryGetLineForStep(step)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void TryClaim_NullArgs_Throws() + { + var view = new JobExecutionView("j"); + Assert.Throws(() => view.TryClaim(null, NewStep())); + Assert.Throws(() => view.TryClaim("k", null)); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async Task ConcurrentAppends_DontCorruptState() + { + var view = new JobExecutionView("j"); + const int N = 50; + var steps = Enumerable.Range(0, N).Select(i => NewStep("s" + i)).ToList(); + var returnedLines = new ConcurrentBag(); + + var tasks = Enumerable.Range(0, N).Select(i => Task.Run(() => + { + int line = view.Append(MainEntry("e" + i), steps[i]); + returnedLines.Add(line); + })).ToArray(); + + await Task.WhenAll(tasks); + + Assert.Equal(N, view.EntryCount); + Assert.Equal(N, returnedLines.Distinct().Count()); + + // Every step identity resolves to some line in [0, N). + var entryLines = Enumerable.Range(0, N).Select(view.GetLine).ToHashSet(); + Assert.Equal(N, entryLines.Count); + foreach (var step in steps) + { + int? line = view.TryGetLineForStep(step); + Assert.NotNull(line); + Assert.Contains(line.Value, entryLines); + } + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void TryMarkSkipped_MarksUnclaimedPlaceholder() + { + var view = new JobExecutionView("j"); + var postEntry = new JobExecutionViewEntry(JobExecutionPhase.Post, "Post X", uses: "actions/x@v1"); + view.Append(postEntry, stepIdentity: null, matchKey: "k1"); + + Assert.True(view.TryMarkSkipped("k1")); + Assert.True(postEntry.IsSkipped); + Assert.Contains("(skipped — main step did not execute)", view.Yaml); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void TryMarkSkipped_ReturnsFalseForUnknownKey() + { + var view = new JobExecutionView("j"); + Assert.False(view.TryMarkSkipped("nope")); + Assert.DoesNotContain("(skipped", view.Yaml); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public void TryMarkSkipped_ReturnsFalseForClaimedPlaceholder() + { + var view = new JobExecutionView("j"); + var postEntry = new JobExecutionViewEntry(JobExecutionPhase.Post, "Post X", uses: "actions/x@v1"); + view.Append(postEntry, stepIdentity: null, matchKey: "k1"); + + var step = NewStep("real-post"); + Assert.NotNull(view.TryClaim("k1", step)); + + // Already claimed — must not mark as skipped. + Assert.False(view.TryMarkSkipped("k1")); + Assert.False(postEntry.IsSkipped); + } + } +}