chore: Validate the actions on every pull request #27

ahmad · 2026-09-03T04:35:49Z

ahmad commented

2026-09-03 04:35:49 +00:00

Issue

Closes #25

Problem

This repository had no pull-request pipeline. .forgejo/workflows/ held only release.yml, and main required an approval and no status checks — so a change to a composite action reached every consuming repository having been read by a reviewer and executed by nothing. The v3 tag moves on merge, so "every consuming repository" means the whole fleet on its next run. It is the most shared repository here and was the least checked.

Solution

A validate job that does what can honestly be done without a host:

parses every action.yml, and checks the fields a composite action needs (name, description, runs, runs.using: composite);
runs bash -n over each run: body, with ${{ }} expressions replaced by a quoted placeholder — a body is not shell until the runner substitutes those, and quoting matters because an expression appears where a word is expected;
lints the README;
posts its own output tail as a comment when it fails, since no job logs are served by this instance's API.

.forgejo/scripts/check-actions.py holds the logic so it runs locally too.

Review notes

I tested that it fails, which is the only claim worth making about a checker. Four cases, exit status captured from the script rather than from a pipeline:

tree	exit	first line reported
clean	0	—
a `run:` body missing its `done`	1	`step 0 has a shell syntax error: bash: line 47: …`
`action.yml` that does not parse	1	`does not parse as YAML: mapping values are not allowed…`
`description` removed	1	`missing required field 'description'`

Getting there took two corrections worth naming. My first "syntax error" case was if [ -z "$X" ; then — which is valid shell ([ is a command, the missing ] is a runtime failure), so it proved nothing; the real case is an unterminated for. And the first version of the script re-parsed a file outside its try to count steps, so a genuinely broken action.yml produced a Python traceback instead of the message it had already prepared. Both are fixed, and the second is why check_action now returns the count.

All six actions and their fifteen run: steps pass on main today, so the check is green because the repository is, not because it is lenient.

The README's table separators were normalised (|---| to | --- |, eight rows) because the lint this now runs flagged 46 violations of the fleet's own config. Table pipes only; no prose changed.

Registering PR / validate (pull_request) as a required check needs it to have run at least once, so I do that after this is green and read the protection back.

Risks and trade-offs

bash -n catches syntax, not semantics: a valid-but-wrong command still passes. The alternative — executing the actions — needs a host and credentials a pull-request pipeline should not hold.
npx --yes markdownlint-cli2 fetches from npm on each run rather than pinning a lockfile, because this repository has no package manifest and adding one for a single linter is more moving parts than it saves. The version is pinned in the command.

### Issue Closes #25 ### Problem This repository had no pull-request pipeline. `.forgejo/workflows/` held only `release.yml`, and `main` required an approval and **no** status checks — so a change to a composite action reached every consuming repository having been read by a reviewer and executed by nothing. The `v3` tag moves on merge, so "every consuming repository" means the whole fleet on its next run. It is the most shared repository here and was the least checked. ### Solution A `validate` job that does what can honestly be done without a host: - parses every `action.yml`, and checks the fields a composite action needs (`name`, `description`, `runs`, `runs.using: composite`); - runs `bash -n` over each `run:` body, with `${{ }}` expressions replaced by a quoted placeholder — a body is not shell until the runner substitutes those, and quoting matters because an expression appears where a word is expected; - lints the README; - posts its own output tail as a comment when it fails, since no job logs are served by this instance's API. `.forgejo/scripts/check-actions.py` holds the logic so it runs locally too. ### Review notes **I tested that it fails, which is the only claim worth making about a checker.** Four cases, exit status captured from the script rather than from a pipeline: | tree | exit | first line reported | | --- | --- | --- | | clean | 0 | — | | a `run:` body missing its `done` | 1 | `step 0 has a shell syntax error: bash: line 47: …` | | `action.yml` that does not parse | 1 | `does not parse as YAML: mapping values are not allowed…` | | `description` removed | 1 | `missing required field 'description'` | Getting there took two corrections worth naming. My first "syntax error" case was `if [ -z "$X" ; then` — which is *valid* shell (`[` is a command, the missing `]` is a runtime failure), so it proved nothing; the real case is an unterminated `for`. And the first version of the script re-parsed a file outside its `try` to count steps, so a genuinely broken `action.yml` produced a Python traceback instead of the message it had already prepared. Both are fixed, and the second is why `check_action` now returns the count. All six actions and their fifteen `run:` steps pass on `main` today, so the check is green because the repository is, not because it is lenient. **The README's table separators** were normalised (`|---|` to `| --- |`, eight rows) because the lint this now runs flagged 46 violations of the fleet's own config. Table pipes only; no prose changed. Registering `PR / validate (pull_request)` as a required check needs it to have run at least once, so I do that after this is green and read the protection back. ### Risks and trade-offs - `bash -n` catches syntax, not semantics: a valid-but-wrong command still passes. The alternative — executing the actions — needs a host and credentials a pull-request pipeline should not hold. - `npx --yes markdownlint-cli2` fetches from npm on each run rather than pinning a lockfile, because this repository has no package manifest and adding one for a single linter is more moving parts than it saves. The version is pinned in the command.

ahmad added 1 commit

2026-09-03 04:35:50 +00:00

chore: Validate the actions on every pull request

PR / validate (pull_request) Successful in 5m7s

Details

0d3bce8264

This repository had no pull-request pipeline: main required an approval and
no status checks, so a change to a shared action reached every consuming
repository having been read by a reviewer and executed by nothing. The v3 tag
moves on merge, so that is the whole fleet on the next run.

The new job parses each action.yml, checks the fields a composite action
needs, and runs bash -n over every run body with the expressions replaced by
a placeholder. It cannot execute the actions, which needs credentials this
pipeline should not hold.

The README's table separators are normalised so the lint it now runs passes.

ahmad commented

2026-09-03 04:42:14 +00:00

Green on 0d3bce8 — PR / validate passes on its first run, which is also the first time anything in this repository has been checked by a pipeline.

Self-verification against the acceptance criteria.

1. Every action.yml is parsed and a syntax error fails the check. Six actions parsed. Verified negatively by replacing one with name: x / bad indent: [ — exit 1, does not parse as YAML: mapping values are not allowed….

2. Each composite run: body is checked for shell syntax errors. Fifteen bodies across the six actions, each through bash -n with ${{ }} replaced by a quoted placeholder. Verified negatively with an unterminated for loop — exit 1, naming the action, the step index and bash's own message.

3. The README is linted as it is in the application repositories. Same markdownlint-cli2 and the same fleet config. It needed eight table separator rows normalised to pass; that is in the diff, pipes only.

4. A violation blocks the merge. PR / validate (pull_request) is now a required status check on main — set and read back rather than assumed: status checks enabled, that context listed, one approval still required.

5. Branch protection lists it. Same read-back.

Two mistakes I made getting here, since they say something about the check's value. My first negative test used if [ -z "$X" ; then, which is valid shell — [ is a command and the missing ] is a runtime failure — so it "passed" and proved nothing; the real case is an unterminated block. And the first version of the script re-parsed a file outside its try to count steps, so a genuinely broken action.yml produced a Python traceback instead of the message it had already composed. Both fixed, and the second is why check_action returns the count rather than the caller recomputing it.

What this cannot do is execute the actions — that needs a host, a registry and credentials a pull-request pipeline should not hold, and the consuming repositories' own runs are where that happens. It catches the class of defect that would otherwise reach every repository in the fleet on the next v3 move, unexecuted.

Green on `0d3bce8` — `PR / validate` passes on its first run, which is also the first time anything in this repository has been checked by a pipeline. Self-verification against the acceptance criteria. **1. Every `action.yml` is parsed and a syntax error fails the check.** Six actions parsed. Verified negatively by replacing one with `name: x` / ` bad indent: [` — exit 1, `does not parse as YAML: mapping values are not allowed…`. **2. Each composite `run:` body is checked for shell syntax errors.** Fifteen bodies across the six actions, each through `bash -n` with `${{ }}` replaced by a quoted placeholder. Verified negatively with an unterminated `for` loop — exit 1, naming the action, the step index and bash's own message. **3. The README is linted as it is in the application repositories.** Same `markdownlint-cli2` and the same fleet config. It needed eight table separator rows normalised to pass; that is in the diff, pipes only. **4. A violation blocks the merge.** `PR / validate (pull_request)` is now a required status check on `main` — set and read back rather than assumed: status checks enabled, that context listed, one approval still required. **5. Branch protection lists it.** Same read-back. **Two mistakes I made getting here, since they say something about the check's value.** My first negative test used `if [ -z "$X" ; then`, which is valid shell — `[` is a command and the missing `]` is a runtime failure — so it "passed" and proved nothing; the real case is an unterminated block. And the first version of the script re-parsed a file outside its `try` to count steps, so a genuinely broken `action.yml` produced a Python traceback instead of the message it had already composed. Both fixed, and the second is why `check_action` returns the count rather than the caller recomputing it. What this cannot do is execute the actions — that needs a host, a registry and credentials a pull-request pipeline should not hold, and the consuming repositories' own runs are where that happens. It catches the class of defect that would otherwise reach every repository in the fleet on the next `v3` move, unexecuted.

ahmad changed title from ~~WIP: chore: Validate the actions on every pull request~~ to chore: Validate the actions on every pull request

2026-09-03 04:42:15 +00:00

ahmad_bot requested changes

2026-09-03 05:02:36 +00:00

Dismissed

ahmad_bot left a comment

This is the right shape and the reasoning in the description is sound — testing that a checker fails is the only claim worth making about one, and the two corrections you named (the [ case that is valid shell, and the re-parse outside the try) are the ones that matter.

Two findings, both in check-actions.py, and I am requesting changes on the first because it means AC1 is not met as written. Both fixes are a couple of lines.

Verified by running the script against fixture trees, exit status taken from the script:

fixture	exit	what happened
`runs:` empty (null)	1	`AttributeError: 'NoneType' object has no attribute 'get'`
`runs: composite` (a string, not a mapping)	1	`AttributeError: 'str' object has no attribute 'get'`
broken action at `group/inner/action.yml`	0	not checked at all — only `act/action.yml` was listed

I also re-ran the clean tree and an expression containing braces (fromJSON('{...}')); both pass, so the placeholder substitution is not producing false positives.

Everything else checks out: PR / validate is green on this head, the workflow's set -o pipefail is present on all three teed steps so a failing command inside a pipe cannot report success, and the README lint config matches what the application repositories use. Registering the required check after the first green run, then reading the protection back, is the right order — AC5 is the one criterion that cannot be met inside this PR.

I could not verify that the failure-report step can actually post: it needs a real red run and a token with comment scope on this instance. The || echo means a failure to post cannot turn a green job red, which is the important half.

This is the right shape and the reasoning in the description is sound — testing that a checker *fails* is the only claim worth making about one, and the two corrections you named (the `[` case that is valid shell, and the re-parse outside the `try`) are the ones that matter. Two findings, both in `check-actions.py`, and I am requesting changes on the first because it means AC1 is not met as written. Both fixes are a couple of lines. **Verified by running the script against fixture trees**, exit status taken from the script: | fixture | exit | what happened | |---|---|---| | `runs:` empty (null) | 1 | `AttributeError: 'NoneType' object has no attribute 'get'` | | `runs: composite` (a string, not a mapping) | 1 | `AttributeError: 'str' object has no attribute 'get'` | | broken action at `group/inner/action.yml` | **0** | not checked at all — only `act/action.yml` was listed | I also re-ran the clean tree and an expression containing braces (`fromJSON('{...}')`); both pass, so the placeholder substitution is not producing false positives. Everything else checks out: `PR / validate` is green on this head, the workflow's `set -o pipefail` is present on all three teed steps so a failing command inside a pipe cannot report success, and the README lint config matches what the application repositories use. Registering the required check after the first green run, then reading the protection back, is the right order — AC5 is the one criterion that cannot be met inside this PR. I could not verify that the failure-report step can actually post: it needs a real red run and a token with comment scope on this instance. The `|| echo` means a failure to post cannot turn a green job red, which is the important half.

.forgejo/scripts/check-actions.py Outdated

					
				@ -0,0 +27,4 @@

				def shell_bodies(action: dict) -> list[tuple[int, str]]:

				    """Every `run:` body in a composite action, with its step index."""

				    steps = action.get("runs", {}).get("steps", []) or []

A runs: that is not a mapping crashes here instead of being reported — and simply guarding it would let the file pass silently.

action.get("runs", {}) returns the default only when the key is absent. A present-but-empty runs: gives None, and runs: composite gives a string; both reach .get("steps"):

runs:            -> AttributeError: 'NoneType' object has no attribute 'get'
runs: composite  -> AttributeError: 'str' object has no attribute 'get'

Both exit 1, so the merge is still blocked — this is not a hole, it is the same class of defect the docstring above check_action says was already fixed once for the YAML-parse path: the checker raises where it had a diagnosis ready.

The part worth care: the obvious fix makes it worse. I patched shell_bodies to return [] for a non-mapping runs and re-ran the string fixture — it reported act/action.yml: ok (0 run step(s) checked) and exited 0. The using check at line 56 is itself guarded by isinstance(runs, dict), so nothing else flags it, and a malformed action would pass clean.

Fix both together — report the shape, then skip:

    runs = action.get("runs")
    if "runs" in action and not isinstance(runs, dict):
        problems.append(f"{path}: runs is {type(runs).__name__}, expected a mapping")
    elif isinstance(runs, dict) and runs.get("using") != "composite":
        problems.append(f"{path}: runs.using is {runs.get('using')!r}, expected 'composite'")

and have shell_bodies take runs rather than the whole action, returning [] when it is not a mapping.

**A `runs:` that is not a mapping crashes here instead of being reported — and simply guarding it would let the file pass silently.** `action.get("runs", {})` returns the default only when the key is *absent*. A present-but-empty `runs:` gives `None`, and `runs: composite` gives a string; both reach `.get("steps")`: ``` runs: -> AttributeError: 'NoneType' object has no attribute 'get' runs: composite -> AttributeError: 'str' object has no attribute 'get' ``` Both exit 1, so the merge is still blocked — this is not a hole, it is the same class of defect the docstring above `check_action` says was already fixed once for the YAML-parse path: the checker raises where it had a diagnosis ready. The part worth care: the obvious fix makes it worse. I patched `shell_bodies` to return `[]` for a non-mapping `runs` and re-ran the string fixture — it reported `act/action.yml: ok (0 run step(s) checked)` and **exited 0**. The `using` check at line 56 is itself guarded by `isinstance(runs, dict)`, so nothing else flags it, and a malformed action would pass clean. Fix both together — report the shape, then skip: ```python runs = action.get("runs") if "runs" in action and not isinstance(runs, dict): problems.append(f"{path}: runs is {type(runs).__name__}, expected a mapping") elif isinstance(runs, dict) and runs.get("using") != "composite": problems.append(f"{path}: runs.using is {runs.get('using')!r}, expected 'composite'") ``` and have `shell_bodies` take `runs` rather than the whole action, returning `[]` when it is not a mapping.

.forgejo/scripts/check-actions.py Outdated

					
				@ -0,0 +72,4 @@

				def main() -> int:

				    actions = sorted(ROOT.glob("*/action.yml"))

*/action.yml is one level deep, so a nested action is silently skipped — and AC1 says every one.

Path.glob("*/action.yml") matches exactly one path segment. I put a deliberately broken action (an unterminated for) at group/inner/action.yml alongside a valid act/action.yml: the script printed only act/action.yml: ok (1 run step(s) checked) and exited 0.

That is the failure mode this whole pull request exists to close. A file that is never read cannot fail the check, and the result is indistinguishable from a clean run — the same "read by a reviewer and executed by nothing" that the description opens with, one level down. It costs nothing today because every action here is flat, and it costs everything the first time someone groups one.

Fix:

actions = sorted(ROOT.rglob("action.yml"))

rglob also makes the "No action.yml found — this script is in the wrong place" guard mean what it says.

**`*/action.yml` is one level deep, so a nested action is silently skipped — and AC1 says every one.** `Path.glob("*/action.yml")` matches exactly one path segment. I put a deliberately broken action (an unterminated `for`) at `group/inner/action.yml` alongside a valid `act/action.yml`: the script printed only `act/action.yml: ok (1 run step(s) checked)` and **exited 0**. That is the failure mode this whole pull request exists to close. A file that is never read cannot fail the check, and the result is indistinguishable from a clean run — the same "read by a reviewer and executed by nothing" that the description opens with, one level down. It costs nothing today because every action here is flat, and it costs everything the first time someone groups one. Fix: ```python actions = sorted(ROOT.rglob("action.yml")) ``` `rglob` also makes the "No action.yml found — this script is in the wrong place" guard mean what it says.

ahmad changed title from ~~chore: Validate the actions on every pull request~~ to WIP: chore: Validate the actions on every pull request

2026-09-03 05:32:56 +00:00

ahmad added 1 commit

2026-09-03 05:33:30 +00:00

fix: Read every action and report a malformed runs block

PR / validate (pull_request) Successful in 1m42s

Details

35d73f3200

The glob matched one directory level, so an action grouped in a
subdirectory was never read — and a file that is never read cannot fail,
which is indistinguishable from a clean run. That is the failure this
pipeline exists to close, one level down.

A runs block that is empty or written as a string also crashed the script
instead of being reported. Guarding it alone would be worse: the file would
pass as "0 run steps checked", so the shape is now reported and the steps
skipped.

ahmad commented

2026-09-03 05:33:48 +00:00

Both fixed in 35d73f3, and both findings were right — the second one especially, because the obvious fix would have been worse than the crash.

Finding 1 — rglob instead of glob. You are right that this is the same failure the pull request exists to close, one level down: a file that is never read cannot fail, and the run looks clean. Fixed, and the guard message now means what it says.

Finding 2 — report the shape, then skip. I took your version rather than the guard I would have reached for. shell_bodies now takes runs instead of the whole action, so a malformed one is the caller's problem to report, and the docstring says why an empty return without a report would be worse than the AttributeError it replaces: 0 run steps checked reads as success.

Re-ran the fixtures, exit status from the script:

fixture	exit	reported
broken action at `group/inner/action.yml`	1	`step 0 has a shell syntax error: …`
`runs:` empty	1	`runs is NoneType, expected a mapping`
`runs: composite` (string)	1	`runs is str, expected a mapping`
`runs.using: node20`	1	`runs.using is 'node20', expected 'composite'`
valid fixture restored	0	—

The third row is the one that would have passed silently under the naive guard. The fourth confirms the using check still fires now that the branches are exclusive.

On the repository itself: all six actions and their fifteen run: bodies still pass, and the README still lints clean.

Ready again immediately rather than waiting for your next look — leaving a pull request in WIP: while waiting for a re-review is what stalled #24.

Both fixed in `35d73f3`, and both findings were right — the second one especially, because the obvious fix would have been worse than the crash. **Finding 1 — `rglob` instead of `glob`.** You are right that this is the same failure the pull request exists to close, one level down: a file that is never read cannot fail, and the run looks clean. Fixed, and the guard message now means what it says. **Finding 2 — report the shape, then skip.** I took your version rather than the guard I would have reached for. `shell_bodies` now takes `runs` instead of the whole action, so a malformed one is the caller's problem to report, and the docstring says why an empty return without a report would be worse than the `AttributeError` it replaces: `0 run steps checked` reads as success. Re-ran the fixtures, exit status from the script: | fixture | exit | reported | | --- | --- | --- | | broken action at `group/inner/action.yml` | 1 | `step 0 has a shell syntax error: …` | | `runs:` empty | 1 | `runs is NoneType, expected a mapping` | | `runs: composite` (string) | 1 | `runs is str, expected a mapping` | | `runs.using: node20` | 1 | `runs.using is 'node20', expected 'composite'` | | valid fixture restored | 0 | — | The third row is the one that would have passed silently under the naive guard. The fourth confirms the `using` check still fires now that the branches are exclusive. On the repository itself: all six actions and their fifteen `run:` bodies still pass, and the README still lints clean. Ready again immediately rather than waiting for your next look — leaving a pull request in `WIP:` while waiting for a re-review is what stalled #24.

ahmad commented

2026-09-03 05:35:42 +00:00

Green on the new head — the required check passed on the very commit that fixed it, which is the pipeline doing its job on its own change.

ahmad changed title from ~~WIP: chore: Validate the actions on every pull request~~ to chore: Validate the actions on every pull request

2026-09-03 05:35:44 +00:00

ahmad_bot approved these changes

2026-09-03 06:01:30 +00:00

Dismissed

ahmad_bot left a comment

Approving. Re-reviewed the delta only (0d3bce8 → 35d73f3); both findings are fixed, and I re-ran the same fixtures against the new script rather than reading it:

fixture	before	now
broken action at `group/inner/action.yml`	exit 0, never read	exit 1, `group/inner/action.yml: 1 problem(s) (1 run step(s) checked)`
`runs:` empty	exit 1 via `AttributeError`	exit 1, `runs is NoneType, expected a mapping`
`runs: composite`	exit 1 via `AttributeError`	exit 1, `runs is str, expected a mapping`
clean tree	exit 0	exit 0

You also went further than I asked, guarding steps as a list and each step as a mapping, and the shell_bodies docstring now states the reason the naive fix was wrong — which is the part I would have wanted written down.

Checked that rglob does not change what runs today: the tree at this head holds exactly six action.yml files, all at depth one, so glob and rglob select the same set. Nothing new is being linted into the run, and PR / validate is green on this head. The actions check also runs before the npx step, so there is no cache directory for rglob to wander into.

One thing left, and it is yours to weigh rather than mine to block on. The new guards stop the crash but do not report the shape one level down: steps: nope and steps: holding bare strings both print ok (0 run step(s) checked) and exit 0.

strsteps   exit=0 :: act/action.yml: ok (0 run step(s) checked)
liststeps  exit=0 :: act/action.yml: ok (0 run step(s) checked)

That is the argument your own docstring makes about runs: — "returning an empty list here for such a file, without the caller saying so, would let it pass as '0 run steps checked', which is worse than the crash it replaces" — applied to steps. It is weaker than the two findings I blocked on: a steps: of the wrong shape fails when the runner loads the action, loudly and on the first consumer run, rather than slipping through as a plausible-looking script. Two lines in check_action would close it if you think it is worth the symmetry.

AC1–AC4 are met. AC5 — the check registered as required on main and the protection read back — still cannot be done inside the pull request, and after the first green run on main is the right moment. Approval does not merge.

Approving. Re-reviewed the delta only (`0d3bce8` → `35d73f3`); both findings are fixed, and I re-ran the same fixtures against the new script rather than reading it: | fixture | before | now | |---|---|---| | broken action at `group/inner/action.yml` | exit **0**, never read | exit 1, `group/inner/action.yml: 1 problem(s) (1 run step(s) checked)` | | `runs:` empty | exit 1 via `AttributeError` | exit 1, `runs is NoneType, expected a mapping` | | `runs: composite` | exit 1 via `AttributeError` | exit 1, `runs is str, expected a mapping` | | clean tree | exit 0 | exit 0 | You also went further than I asked, guarding `steps` as a list and each step as a mapping, and the `shell_bodies` docstring now states the reason the naive fix was wrong — which is the part I would have wanted written down. Checked that `rglob` does not change what runs today: the tree at this head holds exactly six `action.yml` files, all at depth one, so `glob` and `rglob` select the same set. Nothing new is being linted into the run, and `PR / validate` is green on this head. The actions check also runs before the `npx` step, so there is no cache directory for `rglob` to wander into. **One thing left, and it is yours to weigh rather than mine to block on.** The new guards stop the crash but do not report the shape one level down: `steps: nope` and `steps:` holding bare strings both print `ok (0 run step(s) checked)` and exit 0. ``` strsteps exit=0 :: act/action.yml: ok (0 run step(s) checked) liststeps exit=0 :: act/action.yml: ok (0 run step(s) checked) ``` That is the argument your own docstring makes about `runs:` — "returning an empty list here for such a file, without the caller saying so, would let it pass as '0 run steps checked', which is worse than the crash it replaces" — applied to `steps`. It is weaker than the two findings I blocked on: a `steps:` of the wrong shape fails when the runner loads the action, loudly and on the first consumer run, rather than slipping through as a plausible-looking script. Two lines in `check_action` would close it if you think it is worth the symmetry. AC1–AC4 are met. AC5 — the check registered as required on `main` and the protection read back — still cannot be done inside the pull request, and after the first green run on `main` is the right moment. Approval does not merge.

ahmad changed title from ~~chore: Validate the actions on every pull request~~ to WIP: chore: Validate the actions on every pull request

2026-09-03 06:02:58 +00:00

ahmad commented

2026-09-03 06:03:27 +00:00

Taken rather than merged past, in 0151529.

You framed it as mine to weigh, and the weighing is short: my own docstring says an empty return without a report is worse than the crash it replaced, and steps: nope printing ok (0 run step(s) checked) is exactly that. Merging with the approval in hand and the hole documented in the file would have left the checker contradicting its own stated reasoning.

steps that is not a list, and any entry that is not a mapping, are now reported:

fixture	exit	reported
valid action	0	—
`steps: nope`	1	`runs.steps is str, expected a list`
`steps:` with a bare string entry	1	`step 0 is str, expected a mapping`
`steps: []`	0	— (an action with no steps is odd but not malformed)

The last row is deliberate: an empty list is a real, parseable shape, and I would rather not invent a rule the issue did not ask for.

Thank you for checking that rglob selects the same six files as glob at this head, and that the actions check runs before npx so there is no cache directory to wander into — both were worth confirming and neither was in my own verification.

Ready again.

Taken rather than merged past, in `0151529`. You framed it as mine to weigh, and the weighing is short: my own docstring says an empty return without a report is worse than the crash it replaced, and `steps: nope` printing `ok (0 run step(s) checked)` is exactly that. Merging with the approval in hand and the hole documented in the file would have left the checker contradicting its own stated reasoning. `steps` that is not a list, and any entry that is not a mapping, are now reported: | fixture | exit | reported | | --- | --- | --- | | valid action | 0 | — | | `steps: nope` | 1 | `runs.steps is str, expected a list` | | `steps:` with a bare string entry | 1 | `step 0 is str, expected a mapping` | | `steps: []` | 0 | — (an action with no steps is odd but not malformed) | The last row is deliberate: an empty list is a real, parseable shape, and I would rather not invent a rule the issue did not ask for. Thank you for checking that `rglob` selects the same six files as `glob` at this head, and that the actions check runs before `npx` so there is no cache directory to wander into — both were worth confirming and neither was in my own verification. Ready again.

ahmad added 1 commit

2026-09-03 06:03:28 +00:00

fix: Report a malformed steps block instead of counting zero

PR / validate (pull_request) Successful in 1m31s

Details

0151529c20

A steps: that is not a list, or an entry that is not a mapping, yielded no
bodies and printed "0 run steps checked" — which reads as a clean action
rather than a malformed one. It is the same argument the runs guard already
makes, one level down, and leaving it would have contradicted the docstring
that explains why silence is worse than a crash.

ahmad dismissed ahmad_bot's review

2026-09-03 06:03:29 +00:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

ahmad changed title from ~~WIP: chore: Validate the actions on every pull request~~ to chore: Validate the actions on every pull request

2026-09-03 06:06:31 +00:00

ahmad_bot approved these changes

2026-09-03 06:31:14 +00:00

ahmad_bot left a comment

Approving. Delta only (35d73f3 → 0151529) — the steps: residue I left open is closed.

Re-ran the fixtures against the new script:

fixture	before	now
`steps: nope`	`ok (0 run step(s) checked)`, exit 0	exit 1, `runs.steps is str, expected a list`
`steps:` holding a bare string	`ok (0 run step(s) checked)`, exit 0	exit 1, `step 0 is str, expected a mapping`
a `uses:` step beside a `run:` step	—	exit 0, `ok (1 run step(s) checked)` — no false positive from the new per-step check
clean / broken shell	exit 0 / exit 1	unchanged

PR / validate green on this head.

One observation and explicitly not a request: runs: {using: composite} with no steps: key at all still passes as ok (0 run step(s) checked), since the new guard starts at steps is not None. It is the same family, but it fails when the runner loads the action rather than reaching a consumer looking checked, and I have already taken two rounds on this theme — I would rather name it than open a third. Your call whether it is worth a line.

AC1–AC4 met. AC5 still belongs after the first green run on main: register PR / validate (pull_request) as required and read the protection back. Approval does not merge.

Approving. Delta only (`35d73f3` → `0151529`) — the `steps:` residue I left open is closed. Re-ran the fixtures against the new script: | fixture | before | now | |---|---|---| | `steps: nope` | `ok (0 run step(s) checked)`, exit 0 | exit 1, `runs.steps is str, expected a list` | | `steps:` holding a bare string | `ok (0 run step(s) checked)`, exit 0 | exit 1, `step 0 is str, expected a mapping` | | a `uses:` step beside a `run:` step | — | exit 0, `ok (1 run step(s) checked)` — no false positive from the new per-step check | | clean / broken shell | exit 0 / exit 1 | unchanged | `PR / validate` green on this head. One observation and explicitly not a request: `runs: {using: composite}` with no `steps:` key at all still passes as `ok (0 run step(s) checked)`, since the new guard starts at `steps is not None`. It is the same family, but it fails when the runner loads the action rather than reaching a consumer looking checked, and I have already taken two rounds on this theme — I would rather name it than open a third. Your call whether it is worth a line. AC1–AC4 met. AC5 still belongs after the first green run on `main`: register `PR / validate (pull_request)` as required and read the protection back. Approval does not merge.

ahmad merged commit 4109e1bbc4 into main

2026-09-03 06:32:43 +00:00

ahmad referenced this pull request from a commit

2026-09-03 06:32:44 +00:00

chore: Validate the actions on every pull request (#27)

ahmad deleted branch feature/25-pr-pipeline

2026-09-03 06:32:46 +00:00

ahmad referenced this pull request

2026-09-03 06:34:16 +00:00

A composite action with no steps still passes the checker #28

Sign in to join this conversation.

No reviewers