Repository navigation
Detect missing step references in Actions/AW expressions - #71926
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Commit pushed:
|
|
✅ Ponytail Reviewer completed successfully! Ponytail ReviewerOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (528 new lines under 📄 Draft ADR committed: The draft records the decision to add a default-enabled, Warning-level
📋 What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. ❓ Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
|
There was a problem hiding this comment.
Review: no blocking findings
The step-reference rule is covered by focused regression tests and the compiled-vs-authored scope handling is consistent with the documented boundaries. I did not identify an actionable correctness, security, or performance issue in the changed lines. This was a static review; tests were not executed.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · ~3 AIC · ⌖ ~1 AIC · ⊞ 1.6K
Comment /review to run again
There was a problem hiding this comment.
🔵 Needs a closer look
Default-on linting affects compilation gates, and unresolved authored-scope correctness issues warrant human review.
3 open issues
What changed in this PR
Adds a default step-reference warning rule to gh-aw’s lint framework to detect references to missing steps within the same job.
Changes:
- Checks literal step references in expressions and bare
ifconditions. - Prefers compiled Actions graphs and defers ambiguous authored scopes.
- Adds regression coverage and documents rule boundaries.
| File | Description |
|---|---|
pkg/workflow/lint/step_reference.go |
Implements step-reference detection and scope handling. |
pkg/workflow/lint/step_reference_test.go |
Tests expression parsing, job isolation, and authored scopes. |
pkg/workflow/lint/rule_contract_test.go |
Adds the new rule’s shared contract cases. |
pkg/workflow/lint/lint.go |
Registers the rule by default. |
pkg/workflow/lint/lint_test.go |
Updates expected rule registration. |
docs/src/content/docs/reference/workflow-linters.md |
Documents supported references and analysis boundaries. |
Review effort: Max
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd and /codebase-design to review the new step-reference linting rule (new_feature per pr-triage). No blocking issues found.
📋 Key Themes & Highlights
Review scope
- High-impact files reviewed:
pkg/workflow/lint/step_reference.go(226 lines, core logic),pkg/workflow/lint/step_reference_test.go(294 lines, tests),pkg/workflow/lint/lint.go(registration), and docs.
Positive Highlights
- ✅
/tdd: Excellent test coverage — 20+ expression-shape cases (dot/bracket access, case-insensitivity, nested computed indexes, wildcards, quoted text, escaped quotes), plus dedicated tests for scope boundaries (imports, safe-jobs, safe-outputs, restore-memory, dynamic IDs), determinism (re-run equality), read-only input invariants (JSON marshal equality before/after), and context cancellation. - ✅
/codebase-design: The rule closely mirrors the existingneeds-consistencyrule's shape (shared condition-path and expression-scalar traversal helpers, same bare-if parsing fallback), keeping the module family consistent and navigable rather than inventing a new pattern. - ✅ Clear scope documentation in
workflow-linters.mdexplicitly calls out boundaries (external outputs not validated, no ordering checks). - ✅ Regression test directly encodes the motivating bug (#71898) with before/after assertions.
Minor observations (non-blocking)
stepReferenceConditionPathextends the existingneedsConditionPathhelper forsetup-steps/pre-steps; reasonable, accepted duplication given it reuses the shared helper directly.- No
Linefield is set on diagnostics here (unlikeobfuscation's.run/.with.scripthandling), but that is consistent withneeds-consistency's existing behavior for non-script fields.
All 599 lines of diff reviewed; full pkg/workflow/lint test suite passes (30s). No actionable defects identified. Approving.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · ~133 AIC · ⌖ ~15 AIC · ⊞ 10.2K
Comment /matt to run again
|
@copilot address the following outstanding work in one pass:
Push the necessary fixes, reply to each listed review thread and resolve it when addressed. Ignore feedback already answered or resolved. Use the pr-finisher skill and stop when only human review or CI remains; do not trigger CI. Sous-chef head: 02e82ac
|
…-linting-rule Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

Generated conclusion expressions referenced reporting steps absent from work-queue jobs. Add a default
step-referencewarning rule to detect these dangling references.steps.<id>and bracket accesses against IDs in the same job, including job outputs, interpolations, and bareifconditions.For example, this output warns when
report_incompleteis absent: