Skip to content

Detect missing step references in Actions/AW expressions - #71926

Merged
pelikhan merged 4 commits into
mainfrom
copilot/add-action-aw-linting-rule
Oct 25, 2026
Merged

pelikhan merged 4 commits into
mainfrom
copilot/add-action-aw-linting-rule

Conversation

Copilot AI commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Generated conclusion expressions referenced reporting steps absent from work-queue jobs. Add a default step-reference warning rule to detect these dangling references.

  • Detection: Validate literal steps.<id> and bracket accesses against IDs in the same job, including job outputs, interpolations, and bare if conditions.
  • Scope: Prefer compiled Actions graphs; defer ambiguous authored scopes involving imports, generated steps, or dynamic IDs.
  • Coverage: Add regression cases for the conclusion expressions, setup/pre-step producers, and expression parsing; document rule boundaries.

For example, this output warns when report_incomplete is absent:

outputs:
  incomplete_count: ${{ steps.report_incomplete.outputs.incomplete_count }}

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI changed the title Add Actions/AW lint rule for missing step references Detect missing step references in Actions/AW expressions Oct 25, 2026
Copilot AI requested a review from pelikhan October 25, 2026 16:53
@pelikhan
pelikhan marked this pull request as ready for review October 25, 2026 16:55
Copilot AI max review requested due to automatic review settings October 25, 2026 16:55
@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

Commit pushed: 02e82ac

🏗️ ADR gate enforced by Design Decision Gate 🏗️

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

✅ Ponytail Reviewer completed successfully!

Ponytail Reviewer

Outcome: reported_no_action

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.

Generated from Ponytail Reviewer · gpt56 · ~13 AIC · ⌖ ~6 AIC · ⊞ 14.8K · ◷

Generated by Ponytail Reviewer for #71926

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

✅ Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel

Outcome: reported_no_action

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.

Generated from Test Quality Sentinel · haiku45 · ~6 AIC · ⌖ ~10 AIC · ◇ ~3 AIC · ⊞ 9.2K · ◷

🧪 Test quality analysis by Test Quality Sentinel

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

✅ PR Code Quality Reviewer completed the code quality review.

🔎 Code quality review by PR Code Quality Reviewer

@github-actions

Copy link
Copy Markdown
Contributor

🏗️ Design Decision Gate — ADR Required

This PR makes significant changes to core business logic (528 new lines under pkg/) but does not have a linked Architecture Decision Record (ADR).

📄 Draft ADR committed: docs/adr/71926-add-step-reference-lint-rule.md — review and complete it before merging.

The draft records the decision to add a default-enabled, Warning-level step-reference lint rule that validates literal steps.<id> accesses against same-job step IDs. It treats the compiled Actions graph as authoritative and defers ambiguous AW scopes (imports, compiler-owned jobs, restore-memory, dynamic IDs).

🔒 This PR cannot merge until an ADR is linked in the PR body.

📋 What to do next
  1. Review the draft ADR committed to your branch. It was generated from the PR diff.
  2. Complete the missing sections: confirm deciders, refine the rationale, and check the alternatives (actionlint-only, stricter output/ordering validation, generator-only fix).
  3. Commit the finalized ADR to docs/adr/ on your branch.
  4. Reference the ADR in this PR body by adding a line such as:

    ADR: ADR-71926: Add a Default step-reference Lint Rule

Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision.

❓ Why ADRs Matter

"AI made me procrastinate on key design decisions. Because refactoring was cheap, I could always say 'I'll deal with this later.' Deferring decisions corroded my ability to think clearly."

ADRs create a searchable, permanent record of why the codebase looks the way it does.

📋 Michael Nygard ADR Format Reference

An ADR must contain these four sections to be considered complete:

  • Context: What is the problem? What forces are at play?
  • Decision: What did you decide? Why?
  • Alternatives Considered: What else could have been done?
  • Consequences: What are the trade-offs (positive and negative)?

All ADRs are stored in docs/adr/ as Markdown files numbered by PR number.

🏗️ ADR gate enforced by Design Decision Gate 🏗️ · pi · opus55 · ~27 AIC · ⌖ ~41 AIC · ⊞ 1.9K · ◷
Comment /review to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 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 if conditions.
  • 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

Comment thread pkg/workflow/lint/step_reference.go
Comment thread pkg/workflow/lint/step_reference.go
Comment thread pkg/workflow/lint/step_reference.go

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 existing needs-consistency rule'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.md explicitly 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)

  • stepReferenceConditionPath extends the existing needsConditionPath helper for setup-steps/pre-steps; reasonable, accepted duplication given it reuses the shared helper directly.
  • No Line field is set on diagnostics here (unlike obfuscation's .run/.with.script handling), but that is consistent with needs-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

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot address the following outstanding work in one pass:

  1. Update this branch with the latest main using make merge-main, resolving any conflicts and preserving the intended changes.
  2. Review (pkg/workflow/lint/step_reference.go:69): An authored job named push_repo_memory or update_cache_memory is a normal custom job when the corresponding generated job is absent, but this predicate skips it and hides dangling-step diagnostics. pkg/constants/job_constants.go:114–116 explicitly says source-backed checks must not exempt maintenance names, and pkg/workflow/lint/needs_consistency.go:178 uses KnownBuiltInJobNames. Use that narrower set for authored scopes. - Detect missing step references in Actions/AW expressions #71926 (comment)
  3. Review (pkg/workflow/lint/step_reference.go:70): configureCustomJobSteps injects ghes-host-config whenever a non-reusable custom job supplies steps, setup-steps, or pre-steps (pkg/workflow/compiler_custom_job_execution.go:129–135). AW-only linting therefore incorrectly warns for valid references such as steps['ghes-host-config'].outcome. Include that producer in the authored custom-job index and add a regression case, while keeping the compiled graph authoritative. - Detect missing step references in Actions/AW expressions #71926 (comment)
  4. Review (pkg/workflow/lint/step_reference.go:132): AW-only linting checks safe-job descriptions and inputs.* against the safe job's IDs, although these fields are evaluated in the agent job. For example, an input default ${{ steps.prepare.outputs.value }} backed by pre-agent-steps incorrectly warns. Skip those metadata fields here and let the compiled graph check their actual job scope. - Detect missing step references in Actions/AW expressions #71926 (comment)

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
Sous-chef work: 181b02f719e9a2a698defd6359e6aeda34839b7b070cc62397c00a85d38fb659 1985318f0585d6a951e3e2842be08098464268edaa436a27388afc38bdae1b2b 283a81e5396040fb896bb3ee64d437c528dfc3337b89acec47b63d48826e8b16
Sous-chef state: ac4a5a6e52b0286c7e64b832316274ff59ffa668417bf6544f225263a3c96667

Generated by 👨‍🍳 PR Sous Chef · pi · haiku45 · ~7 AIC · ⌖ ~7 AIC · ⊞ 1.1K · ◷
Comment /souschef to run again

…-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>
@pelikhan
pelikhan merged commit a5ffb31 into main Oct 25, 2026
38 checks passed
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.

4 participants