Skip to content

Add parallel-steps rule: check wait/cancel references and steps allowed inside parallel#695

Open
devantler wants to merge 9 commits into
rhysd:mainfrom
devantler:parallel-steps-id-refs
Open

Add parallel-steps rule: check wait/cancel references and steps allowed inside parallel#695
devantler wants to merge 9 commits into
rhysd:mainfrom
devantler:parallel-steps-id-refs

Conversation

@devantler

@devantler devantler commented Jun 26, 2026

Copy link
Copy Markdown

🥞 Stacked on #694 — please review/merge after it. GitHub can't base a cross-fork PR on another fork's branch, so the base here is main: the diff includes #694's commits and narrows to just this rule once #694 merges.

Summary

Adds a parallel-steps lint rule for the new parallel steps feature, catching two classes of mistake the parser alone doesn't:

  1. wait/cancel references — a wait: <id> / cancel: <id> must refer to the ID of a preceding background: true step (analogous to how needs: refers to job IDs), so a typo like cancel: typo is flagged.
  2. Steps inside a parallel group — a parallel group may contain only run/uses steps; background, wait, wait-all, cancel, and a nested parallel are all rejected.

The first addresses the remaining review point on #694; the second extends the rule to the structural constraints GitHub enforces.

wait/cancel references

RuleParallelSteps walks each job's steps in document order (recursing into parallel: groups), tracking the IDs of background: true steps seen so far. A wait/cancel target that isn't among them is reported:

"serverr" is not the ID of a preceding background step. "wait" and "cancel" steps can only refer to an earlier step that has "background: true" [parallel-steps]

A single in-order pass checks three things at once: the target exists, is a background step, and precedes the reference. wait-all and expression-valued IDs (${{ }}) are skipped to avoid false positives.

Steps inside a parallel group

A parallel group only allows run/uses steps — verified against actions/languageservices' workflow-parser (shared by the backend and the VS Code extension), which rejects the rest at parse time. Each is reported:

- parallel:
    - run: echo build
      background: true   # "background" is not allowed for a step inside a "parallel" group …
    - wait: other        # "wait" step is not allowed inside a "parallel" group
    - wait-all:          # "wait-all" step is not allowed inside a "parallel" group
    - cancel: other      # "cancel" step is not allowed inside a "parallel" group
    - parallel:          # "parallel" step cannot be nested in another "parallel" step
        - run: echo nested

A step that's forbidden here is reported once — its wait/cancel reference isn't separately re-validated, so there's no cascading "unknown reference" error from the same step.

Tests / docs

  • New rule rule_parallel_steps.go, registered after RuleJobNeeds in linter.go.
  • testdata/err/parallel_step_refs.{yaml,out} — unknown / non-background / forward references, plus every disallowed step inside parallel.
  • testdata/ok/parallel_step_refs.yaml — dynamic background: ${{ … }} treated as a possible background step and expression-valued references skipped; testdata/ok/parallel_steps.yaml covers valid run + uses inside parallel.
  • docs/checks.md section + TOC entry; SARIF golden updated.
  • go test ./... green; gofmt / go vet / staticcheck clean.

🤖 Generated with Claude Code

devantler and others added 5 commits June 26, 2026 00:04
…arallel)

GitHub released parallel steps on 2026-06-25, introducing five new step keys:
`background`, `wait`, `wait-all`, `cancel`, and `parallel`. Until now actionlint
rejected workflows using them ("unexpected key ..." / "step must run ..."). This
adds parsing and AST support for them:

- `background: true` is parsed as a new optional `*Bool` field on a step (run or
  action step), mirroring `continue-on-error`.
- `wait`/`wait-all`, `cancel`, and `parallel` are modeled as new step execution
  kinds (`ExecWait`, `ExecCancel`, `ExecParallel`) so they no longer trip the
  "step must run ... run/uses" check. `wait` and `cancel` accept a single ID or a
  list of IDs; `parallel` parses its grouped steps recursively, and those nested
  steps are visited by every rule so they keep getting linted.

Expression/context checking for the new fields is intentionally deferred: the
generated context-availability table does not describe them yet, so checking now
would produce false positives. `name`/`if` on the new step kinds are still
checked as before.

Closes rhysd#693

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The GitHub workflow-syntax docs specify that `cancel` "targets a single
background step by its `id`" (unlike `wait`, which is a string or an array).
Parse `cancel` as a single string instead of a string-or-list so that
`cancel: [a, b]` is correctly reported as a type error, and fix the fixture
accordingly.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address review feedback on the parallel-steps PR:

- `wait-all` waits for all background steps and takes no arguments, so
  report an error when a value is given (e.g. `wait-all: foo`).
- `wait` and `wait-all` in the same step are contradictory (one targets
  specific background steps, the other waits for all); flag it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`wait` and `cancel` steps refer to the IDs of background steps, much like
`needs:` refers to job IDs. This adds a rule that flags a `wait`/`cancel`
target that doesn't match the ID of a preceding `background: true` step,
catching typos such as `cancel: typo`. A single in-order walk (recursing
into `parallel:` groups) validates existence, that the target is a
background step, and that it precedes the reference. `wait-all` and
expression-valued IDs are skipped.

Addresses review feedback on the parallel-steps PR.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The visitor already recurses into 'parallel:' groups, visiting every step in
order, so the rule can accumulate background step IDs in VisitStep (resetting
per job in VisitJobPre) instead of re-walking the step tree itself. This
matches the RuleID pattern and removes the manual recursion. No behaviour
change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@devantler

devantler commented Jun 26, 2026

Copy link
Copy Markdown
Author

Still working on this a bit more. I want to be sure that we catch this error scenario:

on: push
jobs:
  test:
    runs-on: ubuntu-latest
    steps:
      - parallel:
        - parallel: # not allowed. Should fail with actionable feedback.
          - run: echo "not good"

Done ✅ The code lints for this scenario as well now.

devantler and others added 2 commits June 26, 2026 15:26
A `parallel` group is shorthand for backgrounding each of its steps with an
implicit wait at the end, so a `parallel` step cannot itself appear inside
another `parallel` group. Flag it in the parallel-steps rule.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds an ok fixture covering the defensive paths: a step whose `background` is
an expression is treated as a possible background step (so a `wait`/`cancel`
to it isn't flagged), and an expression-valued `wait`/`cancel` reference is
skipped because it can't be resolved statically.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@devantler

devantler commented Jun 26, 2026

Copy link
Copy Markdown
Author

Now I am investigating whether wait/cancel/background are allowed inside a parallel: group. If not, I want to add linting checks for that also.

EDIT: They are not. I am working on adding linting for this now.


Done ✅

devantler and others added 2 commits June 26, 2026 15:43
GitHub's workflow parser (actions/languageservices) only allows `run` and
`uses` steps inside a `parallel` group; `background`, `wait`, `wait-all`,
`cancel`, and nested `parallel` steps are rejected at parse time. Extend the
parallel-steps rule to flag each of them so actionlint matches the runtime's
validation instead of only catching nested `parallel`.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…scade

A `wait`/`cancel` step inside a `parallel` group is already reported as not
allowed there, but its target was still validated, producing a second,
redundant "unknown reference" error for the same step. Track the steps that
are direct children of a `parallel` group and skip the reference check for
them, so each such step yields a single, root-cause error — matching GitHub's
parser, which reports only the structural violation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@devantler devantler marked this pull request as ready for review June 26, 2026 13:55
@devantler devantler changed the title Add parallel-steps rule: check wait/cancel step references Add parallel-steps rule: check wait/cancel references and steps allowed inside parallel Jun 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant