diff --git a/src/Runner.Worker/Dap/JobExecutionView.cs b/src/Runner.Worker/Dap/JobExecutionView.cs index db7af0b75..af7e7a01e 100644 --- a/src/Runner.Worker/Dap/JobExecutionView.cs +++ b/src/Runner.Worker/Dap/JobExecutionView.cs @@ -10,9 +10,21 @@ namespace GitHub.Runner.Worker.Dap /// 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. + /// Each can register the entry in one of three modes: + /// - With a non-null stepIdentity: registers the IStep→line mapping + /// immediately. Used for entries whose real is already + /// known at append time. + /// - With a non-null matchKey: registers an unclaimed placeholder + /// that a later binds to a real . + /// Used for entries whose is materialized later. A + /// placeholder that is never claimed simply stays in the view and is never + /// paused on — the IStep→line mapping is only populated on claim. + /// - With neither: a static entry that needs no line lookup. + /// + /// and never remove or reorder + /// existing entries. does not re-render. The IStep→line + /// mapping is rebuilt on every render, so lookups stay accurate even if a later + /// Append happens to shift previously-emitted entries. /// internal sealed class JobExecutionView { @@ -193,46 +205,6 @@ namespace GitHub.Runner.Worker.Dap } } - /// - /// 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. diff --git a/src/Runner.Worker/Dap/JobExecutionViewRenderer.cs b/src/Runner.Worker/Dap/JobExecutionViewRenderer.cs index d01743b84..1c529fe05 100644 --- a/src/Runner.Worker/Dap/JobExecutionViewRenderer.cs +++ b/src/Runner.Worker/Dap/JobExecutionViewRenderer.cs @@ -88,15 +88,6 @@ namespace GitHub.Runner.Worker.Dap public string WithYaml { get; } public string Shell { get; } public string WorkingDirectory { get; } - - /// - /// Set when the corresponding step was skipped (e.g. predicted Post - /// placeholder for a Main step that never executed because its - /// if: evaluated false). Rendered as an inline YAML comment - /// on the entry's - step: line so subsequent entry line - /// numbers stay stable. - /// - public bool IsSkipped { get; internal set; } } /// @@ -213,11 +204,6 @@ namespace GitHub.Runner.Worker.Dap startLines[i] = newlinesEmitted + 1; sb.Append(" - step: ").Append(FormatScalar(entry.DisplayName)); - if (entry.IsSkipped) - { - // Inline comment — keeps following entry line numbers stable. - sb.Append(" # (skipped — main step did not execute)"); - } sb.Append('\n'); newlinesEmitted++; diff --git a/src/Test/L0/Worker/JobExecutionViewL0.cs b/src/Test/L0/Worker/JobExecutionViewL0.cs index 51d5232e3..7bdebf4df 100644 --- a/src/Test/L0/Worker/JobExecutionViewL0.cs +++ b/src/Test/L0/Worker/JobExecutionViewL0.cs @@ -422,46 +422,5 @@ namespace GitHub.Runner.Common.Tests.Worker 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); - } } } diff --git a/src/Test/L0/Worker/JobExecutionViewRendererL0.cs b/src/Test/L0/Worker/JobExecutionViewRendererL0.cs index 608c71e3d..08f4f6fda 100644 --- a/src/Test/L0/Worker/JobExecutionViewRendererL0.cs +++ b/src/Test/L0/Worker/JobExecutionViewRendererL0.cs @@ -582,38 +582,6 @@ namespace GitHub.Runner.Common.Tests.Worker sourceLine: 0)); } - [Fact] - [Trait("Level", "L0")] - [Trait("Category", "Worker")] - public void Render_EmitsSkippedAnnotationForMarkedEntry() - { - var entry = new JobExecutionViewEntry(JobExecutionPhase.Post, "Post X", uses: "actions/x@v1"); - entry.IsSkipped = true; - - var result = JobExecutionViewRenderer.Render("j", new[] { entry }); - - // Annotation is inline on the `- step:` line so subsequent - // entry line numbers stay stable. - Assert.Contains("- step: Post X # (skipped — main step did not execute)\n", result.Yaml); - } - - [Fact] - [Trait("Level", "L0")] - [Trait("Category", "Worker")] - public void Render_SkippedAnnotation_DoesNotShiftSubsequentLines() - { - var skipped = new JobExecutionViewEntry(JobExecutionPhase.Post, "Post A", uses: "actions/a@v1"); - var following = new JobExecutionViewEntry(JobExecutionPhase.Post, "Post B", uses: "actions/b@v1"); - - var unmarked = JobExecutionViewRenderer.Render("j", new[] { skipped, following }); - skipped.IsSkipped = true; - var marked = JobExecutionViewRenderer.Render("j", new[] { skipped, following }); - - // Following entry's start line must not move when the prior - // entry gets an inline skipped annotation. - Assert.Equal(unmarked.EntryStartLines[1], marked.EntryStartLines[1]); - } - [Fact] [Trait("Level", "L0")] [Trait("Category", "Worker")]