amtronics/platform-actions

Watch

Fork 0

chore: The checker does not validate templates/, only actions #43

New issue

Closed

opened 2026-09-04 13:10:04 +00:00 by ahmad · 1 comment

ahmad commented

2026-09-04 13:10:04 +00:00

Owner

Context

.forgejo/scripts/check-actions.py walks */action.yml and validates each composite action:
the fields it needs, and bash -n over every run: body with ${{ }} expressions replaced.
templates/release.yml is not an action, so none of it is checked — not the YAML, not the
shell.

That is the wrong file to leave unvalidated. An action is used through a moving v3 tag, so
a mistake in one is caught the first time any pipeline runs it. A template is copied: a
broken run: body is duplicated into an application repository, possibly several, before
anything executes it, and each copy then has to be fixed separately. The blast radius is
larger and the feedback is later.

Found while adding a docker login step to the template (#37, PR #41). Its shell was correct,
but only because it was checked by hand with the checker's own technique — 7 run steps,
0 invalid. Nothing in CI would have objected to a missing fi.

Scope

In: the existing checker also validating the YAML and run: bodies of files under
templates/.
Out: executing the template; validating the placeholder values; anything about the actions
themselves, which are already covered.

Acceptance criteria

Given a template with a syntactically invalid run: body, when the checker runs, then it fails and names the file, the job and the step.
Given a template that is not valid YAML, when the checker runs, then it fails and names the file.
Given the templates as they stand today, when the checker runs, then it passes.
Given the change, when the checker's own test suite runs, then it covers a failing template and a passing one.

Notes

The workflow shape differs from an action's: jobs.<id>.steps[] rather than
runs.steps[], and a template legitimately contains <PLACEHOLDER> text that is not valid
anything. The expression substitution the checker already applies handles ${{ }}, but a bare
<OWNER>/<APP> inside a run: body would need the same treatment or an explicit exemption —
worth checking whether any current template has one before choosing which.

Keep it in the same checker rather than a second script. Two scripts that both nearly validate
workflows is how the copies in this repository drifted in the first place.

### Context `.forgejo/scripts/check-actions.py` walks `*/action.yml` and validates each composite action: the fields it needs, and `bash -n` over every `run:` body with `${{ }}` expressions replaced. `templates/release.yml` is not an action, so none of it is checked — not the YAML, not the shell. That is the wrong file to leave unvalidated. An action is used through a moving `v3` tag, so a mistake in one is caught the first time any pipeline runs it. A template is **copied**: a broken `run:` body is duplicated into an application repository, possibly several, before anything executes it, and each copy then has to be fixed separately. The blast radius is larger and the feedback is later. Found while adding a `docker login` step to the template (#37, PR #41). Its shell was correct, but only because it was checked by hand with the checker's own technique — 7 run steps, 0 invalid. Nothing in CI would have objected to a missing `fi`. ### Scope - In: the existing checker also validating the YAML and `run:` bodies of files under `templates/`. - Out: executing the template; validating the placeholder values; anything about the actions themselves, which are already covered. ### Acceptance criteria - [ ] Given a template with a syntactically invalid `run:` body, when the checker runs, then it fails and names the file, the job and the step. - [ ] Given a template that is not valid YAML, when the checker runs, then it fails and names the file. - [ ] Given the templates as they stand today, when the checker runs, then it passes. - [ ] Given the change, when the checker's own test suite runs, then it covers a failing template and a passing one. ### Notes The workflow shape differs from an action's: `jobs.<id>.steps[]` rather than `runs.steps[]`, and a template legitimately contains `<PLACEHOLDER>` text that is not valid anything. The expression substitution the checker already applies handles `${{ }}`, but a bare `<OWNER>/<APP>` inside a `run:` body would need the same treatment or an explicit exemption — worth checking whether any current template has one before choosing which. Keep it in the same checker rather than a second script. Two scripts that both nearly validate workflows is how the copies in this repository drifted in the first place.

ahmad added the

labels

2026-09-04 13:10:04 +00:00

ahmad referenced this issue

2026-09-04 13:34:31 +00:00

Log in to Docker Hub before image pulls in reusable build workflows #37

ahmad self-assigned this

2026-09-05 09:32:52 +00:00

ahmad added

state/in-progress

and removed

state/ready

labels

2026-09-05 09:32:53 +00:00

ahmad commented

2026-09-05 09:33:47 +00:00

Author

Owner

Taking this. Two files: check-actions.py gains template checking, and test-check-actions.py gains the cases for it.

A workflow is not an action, so the existing walk does not transfer. An action's bodies live at runs.steps[]; a workflow's at jobs.<id>.steps[]. The expression substitution and bash -n are identical, so those are reused rather than reimplemented — what is new is the traversal and the reporting, which must name the job as well as the step, since a template has several jobs and "step 3" alone would not locate it.

The placeholder question this issue raised, answered by measuring. I worried a bare <OWNER>/<APP> inside a run: body would need substituting or exempting. There are none: templates/release.yml has 7 run bodies and every placeholder in the file sits in uses:, with: or env: values, which are not shell. So no exemption today — but the checker should fail loudly rather than mysteriously if one appears, and bash -n on <OWNER> reports a syntax error naming the line, which is the right behaviour.

One detail the test fixtures force. main() exits 1 with "No action.yml found" when the tree has none, so a temp tree containing only a template would fail for the wrong reason and a case asserting on the exit code would pass while testing nothing. Each template fixture gets a valid action.yml alongside it, so the exit code reflects the template alone.

Verification. The four criteria as cases in the committed suite — that is what the fourth asks for, and unlike the last three changes here it will run again. Each case written so it fails against the unfixed checker first: a suite that only ever ran against the fixed version would record four passes and prove nothing, which is the defect this repository keeps producing.

Taking this. Two files: `check-actions.py` gains template checking, and `test-check-actions.py` gains the cases for it. **A workflow is not an action, so the existing walk does not transfer.** An action's bodies live at `runs.steps[]`; a workflow's at `jobs.<id>.steps[]`. The expression substitution and `bash -n` are identical, so those are reused rather than reimplemented — what is new is the traversal and the reporting, which must name the **job** as well as the step, since a template has several jobs and "step 3" alone would not locate it. **The placeholder question this issue raised, answered by measuring.** I worried a bare `<OWNER>/<APP>` inside a `run:` body would need substituting or exempting. There are none: `templates/release.yml` has 7 run bodies and every placeholder in the file sits in `uses:`, `with:` or `env:` values, which are not shell. So no exemption today — but the checker should fail loudly rather than mysteriously if one appears, and `bash -n` on `<OWNER>` reports a syntax error naming the line, which is the right behaviour. **One detail the test fixtures force.** `main()` exits 1 with "No action.yml found" when the tree has none, so a temp tree containing only a template would fail for the wrong reason and a case asserting on the exit code would pass while testing nothing. Each template fixture gets a valid `action.yml` alongside it, so the exit code reflects the template alone. **Verification.** The four criteria as cases in the committed suite — that is what the fourth asks for, and unlike the last three changes here it will run again. Each case written so it **fails against the unfixed checker first**: a suite that only ever ran against the fixed version would record four passes and prove nothing, which is the defect this repository keeps producing.

ahmad referenced this issue from a commit

2026-09-05 09:35:07 +00:00

chore: Check the templates, not only the actions

ahmad referenced this issue from a pull request that will close it,

2026-09-05 09:35:29 +00:00

chore: Check the templates, not only the actions #49

ahmad added

state/in-review

and removed

state/in-progress

labels

2026-09-05 09:35:45 +00:00