DAP: add JobExecutionView model + renderer#4417
Open
rentziass wants to merge 8 commits into
Open
Conversation
Pure renderer that emits a phase-keyed YAML view (setup/pre/main/ post/cleanup) of a job's runner execution plan. Tracks per-entry start lines so the debugger can map IStep instances back to source locations. Includes a skipped-step annotation for predicted Post placeholders whose Main step never executes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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>
IObjectWriter that bridges the runner's TemplateWriter to YamlDotNet
so TemplateToken trees can be serialized with ${{ }} expressions
preserved. Lets the execution view show step parameters (with:, env:,
if:, etc.) exactly as authored in the workflow file, before any
expression evaluation.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Translates IStep instances (ActionRunner, JobExtensionRunner, etc.) into JobExecutionViewEntry. Filters auto-generated step IDs so only explicit IDs surface in the view, and emits step parameters via the TemplateTokenYamlAdapter to preserve pre-evaluation expressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The workflow parser tokenizes a mixed scalar like
`${{ runner.os }}-primes` as a single BasicExpressionToken whose
internal expression is `format('{0}-primes', runner.os)`. The
adapter was driving TemplateWriter, which calls `ToString()` and
surfaces the parser's normalized rewrite verbatim.
Replace TemplateWriter.Write with a local walker that mirrors it
except for BasicExpressionToken — those go through
`ToDisplayString()`, reversing the format(...) rewrite back to the
authored form. All other token kinds delegate to the same writer
calls TemplateWriter would make.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`StringWriter` defaults to `Environment.NewLine`, which is CRLF on Windows. YamlDotNet's `Emitter` calls `WriteLine` internally, so the emitted YAML mixes CRLF (from the emitter) with the explicit `\n` we append in the renderer skeleton. That breaks the document-end stripping in `FormatScalar` and `TemplateTokenYamlAdapter.Serialize` and corrupts the rendered view on Windows. Set `sw.NewLine = "\n"` in both emitter entry points so the output is always LF, regardless of host. Add regression tests asserting no `\r` appears in the rendered view or in adapter output. The tests are noop guards on Linux (where `Environment.NewLine` is already \n) but catch any future regression on the Windows CI matrix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces the core model + rendering pipeline for a DAP “job execution view”: translating runner steps into a pure-data representation and rendering that representation as stable, line-addressable YAML (intended to mirror how the runner will execute the job, not the original workflow file).
Changes:
- Added
JobExecutionViewEntry/JobExecutionPhaseplus a deterministic YAML renderer that tracks each entry’s- step:line. - Added a stateful
JobExecutionViewcontainer to append entries during execution and resolveIStepidentity → YAML line in O(1). - Added
StepEntryTranslatorandTemplateTokenYamlAdapterto preserve author-facing expressions when emitting YAML fragments, plus L0 coverage.
Show a summary per file
| File | Description |
|---|---|
| src/Runner.Worker/Dap/TemplateTokenYamlAdapter.cs | Adapter to serialize TemplateToken back to YAML while preserving expression display forms. |
| src/Runner.Worker/Dap/StepEntryTranslator.cs | Translates IStep/IActionRunner into JobExecutionViewEntry data for rendering. |
| src/Runner.Worker/Dap/JobExecutionViewRenderer.cs | Pure renderer producing execution-view YAML and per-entry start line mapping. |
| src/Runner.Worker/Dap/JobExecutionView.cs | Stateful append-only view with cached YAML and step→line lookups, plus placeholder claiming/skipping. |
| src/Test/L0/Worker/TemplateTokenYamlAdapterL0.cs | Unit tests for YAML serialization of TemplateToken values and expressions. |
| src/Test/L0/Worker/StepEntryTranslatorL0.cs | Unit tests for translating runner steps into view entries. |
| src/Test/L0/Worker/JobExecutionViewRendererL0.cs | Unit tests for rendering format, field ordering, quoting, and line number stability. |
| src/Test/L0/Worker/JobExecutionViewL0.cs | Unit tests for append/claim/skip behavior and concurrency safety in the stateful view. |
Copilot's findings
Comments suppressed due to low confidence (4)
src/Test/L0/Worker/JobExecutionViewRendererL0.cs:475
iStepis computed but never used, which triggers CS0219 and fails the build when warnings are treated as errors. Remove the unused local (or actually use it as part of the ordering assertions).
int iStep = y.IndexOf(" - step: ", StringComparison.Ordinal) >= 0
? y.IndexOf("- step:", StringComparison.Ordinal) : y.IndexOf("- step:", StringComparison.Ordinal);
int iId = y.IndexOf(" id:", StringComparison.Ordinal);
src/Runner.Worker/Dap/StepEntryTranslator.cs:138
- This lookup uses
"working-directory", but ActionStep.Inputs keys for script steps areworkingDirectory(seePipelineConstants.ScriptStepInputs.WorkingDirectoryand ScriptHandler). As-is,workingDirectorywill never populate in the view. Update the key(s) used here accordingly.
}
}
if (TryGetMapValue(inputs, "working-directory", out var wdTok) && wdTok != null)
{
string wdText = wdTok.ToString();
if (!string.IsNullOrEmpty(wdText))
{
workingDirectory = wdText;
}
}
src/Test/L0/Worker/StepEntryTranslatorL0.cs:254
- These tests use the run-step input key
working-directory, but the runner’s ActionStep.Inputs usesworkingDirectory(seePipelineConstants.ScriptStepInputs.WorkingDirectory). Once the translator is fixed to read the real key, update the test to match (and optionally add coverage that both spellings are accepted if you support backward compatibility).
{
Reference = new ScriptReference(),
Inputs = Map(
("script", Str("npm test")),
("shell", Str("bash")),
("working-directory", Str("./api"))),
};
src/Test/L0/Worker/StepEntryTranslatorL0.cs:283
- The translator’s run-step internal key filtering should match the actual input keys (
script,shell,workingDirectory). This test currently usesworking-directory, which doesn’t align withPipelineConstants.ScriptStepInputs.WorkingDirectoryand could mask bugs in the filter logic. Update the test data/expectations to use the real key (or assert both are filtered if you intend to support both spellings).
var action = new ActionStep
{
Reference = reference,
Inputs = Map(
("mode", Str("ci")),
("script", Str("leak")),
("shell", Str("leak")),
("working-directory", Str("leak"))),
};
var mock = NewActionRunnerMock(ActionRunStage.Main, "Run", reference, action);
- Files reviewed: 8/8 changed files
- Comments generated: 5
Marking placeholders as skipped was a special-case treatment for predicted Post placeholders whose Main step never executed. In practice this is no different from any step that doesn't run: the debugger simply never pauses on it, and the IStep→line mapping is only populated when a placeholder is claimed. Drop `IsSkipped` from `JobExecutionViewEntry`, the inline `# (skipped — ...)` rendering, and `JobExecutionView.TryMarkSkipped`. Placeholders that are never claimed simply stay in the view as informational entries — the IStep→line dictionary excludes them, so the debugger never tries to pause on them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove unused `using System.Linq;` from JobExecutionViewRendererL0
and `using System.Collections.Generic;` from StepEntryTranslatorL0.
- StepEntryTranslator: read run-step inputs through
`PipelineConstants.ScriptStepInputs.*` (`script`, `shell`,
`workingDirectory`). The runner stores these keys in their
constants-defined spelling — see ActionManifestManagerWrapper:244 —
not their kebab-case workflow-YAML form, so the previous
`working-directory` lookup never matched in practice. Tests
updated to use the same constants.
- TemplateTokenYamlAdapter / JobExecutionViewRenderer.FormatScalar:
defensively also strip a `---\n` prefix on top of the existing
`--- ` prefix. Not observed in any input I exercised, but cheap
insurance against an Emitter quirk on some YAML configurations.
- JobExecutionView.Append: reject passing both `stepIdentity` and
`matchKey`. The combination would orphan the original step's
line mapping the moment TryClaim overwrites `_stepIdentities`
for a different step. Add an L0 test covering the precondition.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is part 1 of 2 of the work to serve the job "source" during debugging.
In this PR is all the code to build that representation: this is intended to be a 1:1 representation of how the runner sees the job it's executing rendered as YAML, it's not the workflow file the job comes from; this allows us to consider pre and post actions steps as "lines" we can pause/add breakpoints (coming next) and show things exactly as the runner interprets them. We use YamlDotNet to render scalar values but we "hand-write" the source skeleton because we need to keep track of each step's line to be able to refer to it later (and to write comments but that's secondary at the moment).
Example YAML served as source