1# Porting notes for package action
2
3`Parse` reads an `action.yml` the way **the C# runner** does when it loads an
4action (actions/runner at `5f0b3ddcc6ee9fe9441e00d55e85b94ea4aad798`), with the
5vendored schema of `@actions/workflow-parser` 0.3.61 (MIT, GitHub,
6actions/languageservices). `Lint` reproduces what that npm package reports.
7Nothing here was recorded on github.com: phase A of the CI-2 plan has not
8produced recordings of action manifests yet. What is checked is the runner's
9own code, run by `gen/runner`, and the npm package's own code, run by
10`gen/strict.mjs`.
11
12## Which loader the runner runs
13
14`ActionManifestManagerWrapper.Load` (the only caller) runs
15`ActionManifestManagerLegacy` and returns its result. The newer
16`ActionManifestManager` (on `Sdk/WorkflowParser`, the code the npm package is
17a port of) runs next to it only when the feature flag
18`actions_runner_compare_workflow_parser` or the environment variable
19`ACTIONS_RUNNER_COMPARE_WORKFLOW_PARSER` is set, and its result is thrown away
20(`EvaluateAndCompare`). So `Parse` follows the **legacy** loader and the legacy
21converter `PipelineTemplateConverter.ConvertToSteps`. The two loaders agree on
22most files and differ on a few (below); the new one is not reproduced.
23
24## Checks
25
26- `testdata/runner.json`: the answer of the runner's loader on every file of
27  `testdata/cases` (338 files). `gen/runner/Driver.cs` is
28  `ActionManifestManagerLegacy.Load/ConvertRuns/ConvertInputs` with what needs
29  an `IExecutionContext` removed (tracing, the file table, expression values:
30  none of it changes loading), compiled into a copy of `src/Sdk` with
31  `gen/runner/build.sh`, which refuses a clone at another commit or with
32  changes. `TestAgainstRunner` compares the error list (text and order) and the
33  model (inputs and defaults, deprecations, every field of `runs`, every step's
34  id, condition, name, env, with, shell, working directory, reference, and the
35  outputs). The only thing not compared word for word is YamlDotNet's wording of
36  YAML syntax errors (`While parsing a node, ...`), where yaml.v3's wording is
37  used; only the shape is compared.
38- `testdata/uses.json`: the same driver on 68 `uses:` values; `ParseUses` is
39  compared on error text and on the reference the runner builds
40  (`TestParseUsesAgainstRunner`).
41- `testdata/strict.json`: `parseAction` + `convertActionTemplate` of the npm
42  package on every case; `Lint` is compared on the error list
43  (`TestLintAgainstReference`). Eleven files are excused, each with its reason
44  (`knownLintDifferences`).
45- `FuzzParse` (no panic, an action iff no error, at most ten errors, every step
46  has an id and a condition) and `FuzzParseUses`.
47
48Regenerate with `gen/regen.sh` (`RUNNER_CLONE=<clone>` for the runner part); the
49diff must be empty. The cases are: the 24 files of the runner's
50`src/Test/TestData` that are action manifests (`runner_*`), the 96 manifests
51inlined in the language service tests (`ls_*`: `action-parser.test.ts`,
52`validate-action.test.ts`), and 218 written for this package. The
53language service's own expectations (`toBeGreaterThan(0)`) are not ported: the
54cases are replayed through the reference instead, which gives the messages.
55
56## Model
57
58`Action` has `Name`, `Description`, `Author`, ordered `Inputs` (with
59`Description`, `Required`, `Default` as a token, `HasDefault`,
60`DeprecationMessage`), ordered `Outputs` (with `Value`), and `Runs`:
61`Kind` (`RunsNode`, `RunsDocker`, `RunsComposite`, `RunsPlugin`), `Using`
62(lower case), node `Main/Pre/Post`, docker `Image/Entrypoint/PreEntrypoint/
63PostEntrypoint/Args/Env`, `PreIf/PostIf`, composite `Steps` (`[]*workflow.Step`),
64`Plugin`. Conditions default to `always()` for node and docker actions
65(`ActionManifestManagerLegacy.cs`, `preIfToken?.Value ?? "always()"`); they are
66kept as written, without `success() &&` (the schema's own description of
67`pre-if` says the same). A composite action's conditions are `always()`.
68
69Steps are `workflow.Step`, with two additions to that type:
70`EnvExpression` and `WithExpression` hold `env: ${{ fromJSON(...) }}` and
71`with: ${{ ... }}`, which the runner accepts on a step of an action and the
72workflow converter does not. `Step.If` is the runner's: `success()` when empty,
73`success() && (cond)` when the condition has no status function; the contexts
74of `if` are the schema's `step-if` plus `vars` and `needs`, which the runner's
75converter adds (`s_stepNamedValues`) and the schema lacks.
76
77## What the runner does that the other readers do not
78
79All verified by `TestAgainstRunner`.
80
81- **Schema.** The runner reads `action-root` (all keys but `name`,
82  `description`, `inputs`, `outputs`, `runs` are accepted, nothing is
83  required); the language service reads `action-root-strict`. `using` is any
84  non-empty string for the schema and compared with `OrdinalIgnoreCase` by the
85  loader: `Node20` and `DOCKER` run on GitHub, the strict schema refuses them.
86  `branding` is not read at all: `branding: {icon: nope}` loads.
87- **Errors are exceptions.** After the positioned errors, the exception that
88  ended the load is printed as `Exception.ToString()` (class name, `.NET`
89  message format, then a stack trace that is not reproduced): `using: node99`
90  is `System.ArgumentOutOfRangeException: Specified argument was out of the
91  range of valid values. (Parameter ''using: node99' is not supported, use
92  'docker', 'node12', 'node16', 'node20' or 'node24' instead.')`. `Error.Exception`
93  holds the class; `Error.Error()` formats it. At most ten errors are kept;
94  `Failed to load <path>` is the runner's final message (`LoadFailed`).
95- **Missing runs, using, main, image, steps** each have their text
96  (`Top level 'runs:' section is required for ...`, `Missing 'using' value. ...`,
97  `You are using a JavaScript Action but there is not an entry JavaScript file
98  provided in ...`, `... Container Action but an image is not provided in ...`,
99  `... composite action but there are no steps provided in ...`), after the
100  schema's own errors when it has some (`There's not enough info to determine
101  what you meant. Add one of these properties: ...`).
102- **YAML.** Anchors and aliases are refused (`Anchors are not currently
103  supported. Remove the anchor 'x'`); an explicit tag other than `!!bool`,
104  `!!float`, `!!int`, `!!null` is `Unexpected tag '...'` (the runner's string tag
105  is `tag:yaml.org,2002:string`, which no document spells, so `!!str` fails);
106  a tagged quoted scalar, an out-of-range tagged number and a plain `0x`/`0o`
107  integer over 32 bits (`Int32.TryParse`) have their messages; a second document
108  is `Expected stream end parse event`, an empty file `Expected document start
109  parse event`, a collection used as a key `Expected mapping end`. Such an error
110  stops the read where the runner meets it, so errors found earlier in the
111  document stay (`template.Context.RunnerYAML`).
112- **Keys, integers, case.** Mapping keys are resolved like values: `~` is null
113  (`Unexpected value ''`), `0x10` is the key `16`, `1.0` is `1`, `TRUE` is `true`,
114  so `0x10` and `16` are duplicates. `0x` and `0o` integers, plain or `!!int`,
115  are int32 with wraparound (`0xFFFFFFFF` is `-1`, `0x80000000` is
116  `-2147483648`; over 32 bits is an error, and `!!float` refuses them). Duplicate
117  keys, input names and step ids compare with .NET's OrdinalIgnoreCase
118  (`expr.OrdinalFold`): `é` and `É` are the same, `s` and `ſ` are not.
119- **Depth.** `TemplateMemory(maxDepth: 100)`: `Maximum object depth exceeded`
120  from the 100th nested collection, the root mapping being the first. Skipped
121  values (unknown keys) count (`template.Context.MaxDepth`).
122- **Numbers as strings.** A number read where a string is wanted shows as .NET's
123  `G15` (`1E+21`, `0.3` for `0.30000000000000004`, `-0`), not as JavaScript's
124  (`template.DotNetNumberString`).
125- **Steps** (legacy `ConvertToSteps`): the first token of the wrong type ends the
126  conversion (`Unexpected type 'StringToken' encountered while reading 'steps'.
127  ...`) instead of being recorded and skipped; an unknown key in a step is
128  `Error while reading 'steps item key'. Unexpected value 'k'`; the id check
129  (`ReferenceNameBuilder`) accepts ids that start with `__`, lets an id of 100
130  characters or more through, compares ids with OrdinalIgnoreCase, and words its
131  message `... IDs must start with a letter or '_' and must be less than 100
132  characters.` (the language service has `and and`); generated ids are
133  `__run`, `__owner_repo`, `__<image>`, `__self`, `__selfRepository`, made unique
134  with `_2`, `_3`, ... A step whose `uses` is malformed is recorded as an error
135  and **kept**, which makes the load fail later with
136  `System.NullReferenceException` if it has no id (the runner dereferences its
137  reference); a `$/x@ref` step is dropped instead.
138- **`uses`.** `Expected format {org}/{repo}[/path]@ref. Actual 'x'` and
139  `Self-repository references do not support an '@ref' suffix. Actual '$/x@v1'`.
140  Segments are split on `/` and `\` and empty ones dropped, so `/a/b@c` and
141  `a//b@c` are `a/b@c`; `docker://` is case-sensitive (`Docker://x` is an error)
142  although the image of a docker *action* is not (`ParseImage`).
143
144## Where the sources disagree
145
1461. **Runner (legacy) / runner (new) / language service**, on the same file:
147   - Step id text: legacy `... '_' and must be ...`; new loader adds
148     `IDs starting with '__' are reserved.` and refuses them; language service
149     has `and and` and the reserved check too. `Parse` follows legacy (what runs).
150   - `$/` steps: legacy generates `__selfRepository`; new generates the same;
151     language service `__self` (`createActionStepId`).
152   - Conditions of `runs.pre-if`, `post-if`: the runner reads them as plain
153     strings and never parses them when loading (an unknown context fails at run
154     time); the language service parses them with the contexts of `runs-if`
155     (`Unrecognized named-value: 'foo'`). `pre-if: ${{ always() }}` makes the
156     runner fail (`Unrecognized function: 'always'`, the schema type there is
157     `non-empty-string`, which allows no context), while the language service
158     accepts the schema and flags the markers in its own pass.
1592. **Schema vs runner's `action_yaml.json`.** Equal in meaning for every
160   definition `Parse` reads (`action-root` and below); the vendored file adds
161   `-strict` variants, `branding`, `using`, `runs-if` and descriptions, orders
162   the `runs` alternatives differently and writes `{"type": x}` where the
163   runner writes `x`. Checked by diffing the two after removing descriptions.
1643. **Schema vs plan/task text.** The task lists the composite step keys as
165   `uses, run, shell, with, env, name, id, if, working-directory,
166   continue-on-error`. The schema: a `run` step takes `name, id, if, run, shell,
167   env, continue-on-error, working-directory`; a `uses` step takes `name, id, if,
168   uses, with, env, continue-on-error`. `shell` and `working-directory` on a
169   `uses` step and `with` on a `run` step are refused; so is `timeout-minutes`,
170   although the runner's converter would read it.
1714. **`required` inputs: the plan vs the runner.** The CI-2 plan says "required
172   inputs warned like GitHub". The runner has no such warning for an action:
173   `ActionRunner.cs` warns `Unexpected input(s) 'x', valid inputs are ['a', 'b']`
174   (only for actions that have a manifest, `entryPoint` and `args` being valid
175   for docker actions) and `Input 'x' has been deprecated with message: ...`,
176   and `inputs.<id>.required` is not read by the loader nor checked later (a
177   `grep` of `Runner.Worker`, `Runner.Common`, `Sdk/WorkflowParser` and
178   `Sdk/DTPipelines` finds `required` checks only for reusable workflow inputs
179   and secrets, `WorkflowTemplateConverter.cs:2166,2223`). `Missing required
180   input` is the language service's editor message
181   (`validate-action-reference.ts`: `Missing required input \`x\`` / `Missing
182   required inputs: \`x\`, \`y\``), shown when no `with:` gives the input and it
183   has no default. `Input.Required` is parsed so that an editor can do the same;
184   the executor should not warn.
1855. **Language service TypeScript vs its own schema.** `convertRuns` falls back to
186   an empty composite when `using` or `main` is missing; the model here follows
187   the runner (an error instead).
1886. **`uses` messages.** Runner `{org}/{repo}[/path]@ref`; language service
189   `{owner}/{repo}[/path]@{ref}`, `'uses' value in action cannot be blank`, and
190   it rejects `/repo@v4` where the runner accepts `/a/b@c`. `ParseUses` is the
191   runner's.
192
193## Differences from the runner
194
195- **Stack traces** are not reproduced.
196- **YAML syntax errors** have yaml.v3's wording (`file.yml: did not find expected
197  key at line 5`); YamlDotNet's is `file.yml: (Line: 6, Col: 34, Idx: 122) -
198  (Line: 6, Col: 44, Idx: 132): While parsing a block mapping, ...`.
199- **Limits that the runner does not state**: a manifest over `template.MaxFileBytes`
200  (1 MiB) and a document that expands to more than `template.MaxNodes` nodes are
201  refused before reading. The runner reads the file with
202  `TemplateMemory(maxBytes: 10 MiB, maxEvents: 1 000 000)`; those two are not
203  reproduced.
204- **Not ported**: what happens after loading (`ActionManager`: choosing between
205  `action.yml`, `action.yaml` and `Dockerfile`, `Can't find 'action.yml',
206  'action.yaml' or 'Dockerfile'`, the `Dockerfile` image check `ParseImage`
207  reproduces, input defaults evaluated at run time, `Input '<n>' has been
208  deprecated with message: ...`, composite depth). `ParseImage` and
209  `Input.DeprecationMessage` carry what is needed.
210- **Expression errors** come from package `expr`, which follows the runner's
211  expression engine (`... Located at position N within expression: ...`). `Lint`
212  strips nothing: the language service's expression messages lack that suffix, so
213  `TestLintAgainstReference` strips it from ours before comparing.
214- **`Lint` ordering of schema errors** for files with several (`Required property
215  is missing` after `Unexpected value`) is the reference's and is replayed.
216- A mapping key that is a collection: the reference reads `undefined`
217  (`Unexpected value 'undefined'`); `Lint` reports `Unexpected value ''`
218  (`knownLintDifferences`). `Parse` is the runner's (`Expected mapping end`).
219- `Lint` keeps keys as strings, so it differs from the reference on keys that
220  are not strings: it misses a null key under a loose-key mapping (`inputs`,
221  `env`), which the reference and the runner reject, and the reference fails on
222  other non-string keys with `raw.indexOf is not a function`. Fixing the null
223  key in `Lint` would change how `template` reads workflow files, so it is
224  left alone.
225
226## To verify against recordings
227
228- [ ] What github.com prints for a manifest that fails to load (the runner's
229      messages are the ones above; whether the hosted service wraps or reorders
230      them is unknown).
231- [ ] That the hosted runners run the legacy loader (the flag
232      `actions_runner_compare_workflow_parser` may be set there; its result is
233      still discarded in this commit).
234- [ ] `using: node12`/`node16` after the migration to node20 (`HandlerFactory`),
235      which belongs to the executor.