Skip to content

fix(ci): include root action.yml in pinact and pre-commit checks - #2622

Merged
ralphbean merged 2 commits into
mainfrom
fix/pinact-cover-root-action-yml
Jun 24, 2026
Merged

fix(ci): include root action.yml in pinact and pre-commit checks#2622
ralphbean merged 2 commits into
mainfrom
fix/pinact-cover-root-action-yml

Conversation

@ralphbean

Copy link
Copy Markdown
Member

Summary

  • Add action.yml to .pinact.yaml file patterns
  • Add action\.yml$ to the pinact pre-commit hook's file regex

Context

PR #2508 pinned actions across .github/workflows/ and .github/actions/ but missed the root action.yml because neither pinact nor the pre-commit hook was configured to scan it. PR #2621 fixes the unpinned refs themselves; this PR closes the gap in the tooling so future unpinned refs are caught automatically.

Test plan

  • pinact run --fix=false --no-api now flags unpinned refs in root action.yml
  • make lint passes

🤖 Generated with Claude Code

@ralphbean
ralphbean enabled auto-merge June 24, 2026 17:49
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Fix CI tooling to scan root action.yml with pinact and pre-commit
🐞 Bug fix ⚙️ Configuration changes 🕐 Less than 10 minutes

Grey Divider

Description

• Extend pinact config to include the repository root action.yml.
• Update the pre-commit pinact hook to match root action.yml.
• Prevent future unpinned GitHub Action refs from bypassing checks.
Diagram

graph TD
  Dev["Developer"] --> PC(["pre-commit"]) --> Pinact(["pinact"]) --> Targets["YAML targets (incl. action.yml)"]
  PinactCfg[".pinact.yaml"] --> Pinact
  PCCfg[".pre-commit-config.yaml"] --> PC
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Broaden pinact patterns (e.g., **/action.yml) and remove hook regex filtering
  • ➕ Automatically covers future nested action.yml files without additional config changes
  • ➕ Avoids a split-brain configuration where both pinact and pre-commit must be kept in sync
  • ➖ May scan unintended directories (vendored/test fixtures), increasing noise and runtime
  • ➖ Larger blast radius for what is currently a targeted gap fix
2. Make pre-commit run pinact unconditionally (no file regex), rely solely on .pinact.yaml
  • ➕ Single source of truth for what gets scanned
  • ➕ Less chance of missing files due to regex drift
  • ➖ Potentially slower pre-commit runs on large repos if pinact enumerates more files than necessary
  • ➖ May require more careful pinact file targeting to keep local runs fast

Recommendation: Current approach is a good targeted fix: it closes the known gap (root action.yml) while keeping scan scope narrow and predictable. If scan targets are expected to evolve frequently, consider consolidating targeting into .pinact.yaml and minimizing pre-commit-side file filtering to reduce future drift.

Files changed (2) +2 / -0

Other (2) +2 / -0
.pinact.yamlAdd root action.yml to pinact scan patterns +1/-0

Add root action.yml to pinact scan patterns

• Extends the pinact file pattern list to include the repository root action.yml so pinact can validate/pin refs there as well as in workflow and action directories.

.pinact.yaml

.pre-commit-config.yamlInclude root action.yml in pinact pre-commit file regex +1/-0

Include root action.yml in pinact pre-commit file regex

• Updates the pinact pre-commit hook file selector to match action.yml at the repository root, ensuring the hook triggers when it changes.

.pre-commit-config.yaml

@github-actions

github-actions Bot commented Jun 24, 2026

Copy link
Copy Markdown

Site preview

Preview: https://8f0f626d-site.fullsend-ai.workers.dev

Commit: 52f61ba774e6c95967abb6d14ce439818c9602a3

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 24, 2026

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 5:52 PM UTC · Completed 6:02 PM UTC
Commit: e4280c7 · View workflow run →

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📜 Skill insights (0)

Context used
✅ Compliance rules (platform): 51 rules

Grey Divider


Action required

1. Workflows .yaml not scanned 🐞 Bug ≡ Correctness
Description
.pinact.yaml only matches .github/workflows/*.yml, but the repo contains
.github/workflows/fullsend.yaml, so pinact will never scan that workflow and unpinned uses: refs
in .yaml workflows can bypass the SHA-pin gate. This is especially risky because CI runs
pre-commit run --all-files, which will run pinact, but pinact itself still ignores .yaml
workflows due to the configured files: patterns.
Code

.pinact.yaml[R6-7]

  - pattern: ".github/workflows/*.yml"
  - pattern: "internal/scaffold/fullsend-repo/.github/workflows/*.yml"
Relevance

⭐⭐⭐ High

Team previously accepted that pinact files: overrides defaults; coverage gaps fixed via pattern
expansion in PR #2508.

PR-#2508
PR-#390

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The configured pinact file patterns only include *.yml, yet the repo contains an actual workflow
with a .yaml extension; therefore pinact won’t scan it under the current configuration. This
matches the prior accepted bug pattern that pinact files: must include all intended paths because
it overrides defaults.

.pinact.yaml[5-9]
.github/workflows/fullsend.yaml[22-49]
.pre-commit-config.yaml[60-73]
PR-#2508

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`.pinact.yaml` scopes pinact to `*.yml` workflows only, but the repo contains at least one `.yaml` workflow. Because pinact v3+ treats `files:` as an override scope, `.yaml` workflows are skipped entirely, allowing unpinned `uses:` refs in those files to slip through.

## Issue Context
CI runs `make lint-all` which executes `pre-commit run --all-files`. The pinact hook runs, but pinact only scans files matched by `.pinact.yaml`’s `files:` list.

## Fix Focus Areas
- .pinact.yaml[5-9]
 - Add an additional pattern for `.github/workflows/*.yaml` (and consider the same for the internal scaffold workflows for consistency), or replace with a glob that matches both extensions if supported (e.g., two explicit patterns).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. New scan breaks lint-all 🐞 Bug ☼ Reliability
Description
By adding root action.yml to pinact’s scan scope, CI (make lint-all) will now fail because
action.yml currently contains tag-based uses: references (e.g., actions/setup-go@v6) that
pinact flags as unpinned. This change introduces a new failing lint gate unless those references are
SHA-pinned (or explicitly ignored).
Code

.pinact.yaml[R8-9]

  - pattern: ".github/actions/*/action.yml"
+  - pattern: "action.yml"
Relevance

⭐⭐⭐ High

Repo enforces SHA-pinning via pinact hook; historically they accept fixes that prevent
lint/pre-commit failures from unpinned uses:.

PR-#2509
PR-#2508

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR explicitly adds action.yml to pinact’s files: scope, and CI runs lint-all which
executes pre-commit (including the pinact hook). The current action.yml contains multiple unpinned
uses: entries, so enabling scanning of that file will cause pinact to report failures.

.pinact.yaml[5-9]
action.yml[230-235]
action.yml[281-284]
action.yml[424-427]
Makefile[76-81]
.github/workflows/lint.yml[13-46]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR expands pinact coverage to include the repository root `action.yml`. That file currently contains unpinned `uses: owner/repo@vX` references, which will cause `pinact run --fix=false` (invoked by pre-commit in CI) to fail.

## Issue Context
CI runs `make lint-all` → `pre-commit run --all-files`, and the pinact hook executes `pinact run --fix=false --no-api`.

## Fix Focus Areas
- action.yml[230-235]
- action.yml[281-284]
- action.yml[424-427]
 - Replace tag-based refs with SHA-pinned refs (optionally retaining the tag in a trailing comment, consistent with other workflows in the repo).
- .pinact.yaml[5-9]
 - Keep `action.yml` in scope once the file is compliant.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread .pinact.yaml
Comment on lines 6 to 7
- pattern: ".github/workflows/*.yml"
- pattern: "internal/scaffold/fullsend-repo/.github/workflows/*.yml"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Workflows .yaml not scanned 🐞 Bug ≡ Correctness

.pinact.yaml only matches .github/workflows/*.yml, but the repo contains
.github/workflows/fullsend.yaml, so pinact will never scan that workflow and unpinned uses: refs
in .yaml workflows can bypass the SHA-pin gate. This is especially risky because CI runs
pre-commit run --all-files, which will run pinact, but pinact itself still ignores .yaml
workflows due to the configured files: patterns.
Agent Prompt
## Issue description
`.pinact.yaml` scopes pinact to `*.yml` workflows only, but the repo contains at least one `.yaml` workflow. Because pinact v3+ treats `files:` as an override scope, `.yaml` workflows are skipped entirely, allowing unpinned `uses:` refs in those files to slip through.

## Issue Context
CI runs `make lint-all` which executes `pre-commit run --all-files`. The pinact hook runs, but pinact only scans files matched by `.pinact.yaml`’s `files:` list.

## Fix Focus Areas
- .pinact.yaml[5-9]
  - Add an additional pattern for `.github/workflows/*.yaml` (and consider the same for the internal scaffold workflows for consistency), or replace with a glob that matches both extensions if supported (e.g., two explicit patterns).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread .pinact.yaml
Comment on lines 8 to +9
- pattern: ".github/actions/*/action.yml"
- pattern: "action.yml"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. New scan breaks lint-all 🐞 Bug ☼ Reliability

By adding root action.yml to pinact’s scan scope, CI (make lint-all) will now fail because
action.yml currently contains tag-based uses: references (e.g., actions/setup-go@v6) that
pinact flags as unpinned. This change introduces a new failing lint gate unless those references are
SHA-pinned (or explicitly ignored).
Agent Prompt
## Issue description
The PR expands pinact coverage to include the repository root `action.yml`. That file currently contains unpinned `uses: owner/repo@vX` references, which will cause `pinact run --fix=false` (invoked by pre-commit in CI) to fail.

## Issue Context
CI runs `make lint-all` → `pre-commit run --all-files`, and the pinact hook executes `pinact run --fix=false --no-api`.

## Fix Focus Areas
- action.yml[230-235]
- action.yml[281-284]
- action.yml[424-427]
  - Replace tag-based refs with SHA-pinned refs (optionally retaining the tag in a trailing comment, consistent with other workflows in the repo).
- .pinact.yaml[5-9]
  - Keep `action.yml` in scope once the file is compliant.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review

Findings

High

Previous run

Review

Findings

High


Labels: PR modifies CI tooling configuration (pinact and pre-commit hooks) to fix a coverage gap.

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

@fullsend-ai-review fullsend-ai-review Bot added component/ci CI pipelines and checks type/bug Confirmed defect in existing behavior labels Jun 24, 2026

@waynesun09 waynesun09 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Patterns are correctly scoped — pinact glob matches only root action.yml, pre-commit regex is properly anchored by ^(...). No over-matching or under-matching.

The pinact config and pre-commit hook only covered .github/workflows/,
.github/actions/, and the scaffold workflow directory. The root
action.yml was never scanned, which allowed unpinned tag refs to slip
through in PR #2508 (fixed by #2621).

Add action.yml to both .pinact.yaml file patterns and the pre-commit
hook's file regex so unpinned refs are caught automatically.

Assisted-by: Claude claude-opus-4-6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
The repo contains .github/workflows/fullsend.yaml which was not matched
by the *.yml-only patterns. Add *.yaml patterns for both the root and
scaffold workflow directories so pinact scans all workflow files
regardless of extension.

Assisted-by: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Ralph Bean <rbean@redhat.com>
@ralphbean
ralphbean force-pushed the fix/pinact-cover-root-action-yml branch from e4280c7 to 52f61ba Compare June 24, 2026 20:53
@ralphbean

Copy link
Copy Markdown
Member Author

Rebased onto main and pushed 52f61ba.

CI fix: The unpinned uses: refs in action.yml were from branch divergence — main already has them SHA-pinned from #2621. Rebase resolves it.

.yaml coverage: qodo flagged that .github/workflows/fullsend.yaml wasn't matched by the *.yml-only pinact patterns. Added *.yaml patterns for both workflow directories in 52f61ba. (The pre-commit files: regex was already a directory prefix match so the hook triggered correctly, but pinact itself would have skipped the file.)

Protected path: The fullsend-ai-review bot flagged .pre-commit-config.yaml as a protected path without a linked issue. This is a 2-line config fix closing a gap identified in #2621 and #2508 — Wayne already approved.

@fullsend-ai-review

fullsend-ai-review Bot commented Jun 24, 2026

Copy link
Copy Markdown

🤖 Review · ❌ Terminated · Started 8:56 PM UTC · Ended 9:05 PM UTC
Commit: ff8ec85 · View workflow run →

@ralphbean
ralphbean added this pull request to the merge queue Jun 24, 2026
@codecov

codecov Bot commented Jun 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@fullsend-ai-review fullsend-ai-review Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

See the review comment for full details.

@fullsend-ai-review

Copy link
Copy Markdown

🤖 Finished Review · ✅ Success · Started 8:56 PM UTC · Completed 9:05 PM UTC
Commit: 52f61ba · View workflow run →

Merged via the queue into main with commit 2886c4a Jun 24, 2026
12 checks passed
@ralphbean
ralphbean deleted the fix/pinact-cover-root-action-yml branch June 24, 2026 21:06
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jun 24, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 9:10 PM UTC · Completed 9:16 PM UTC
Commit: 52f61ba · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #2622 — fix(ci): include root action.yml in pinact and pre-commit checks

Timeline: Human-authored PR by ralphbean, created 17:49 UTC, merged 21:06 UTC (~3h17m). 4 lines added across 2 files (.pinact.yaml and .pre-commit-config.yaml).

What happened

  1. 17:52–18:02 — Review agent ran, issued CHANGES_REQUESTED for a [protected-path] finding on .pre-commit-config.yaml. Its own remediation said: "have a maintainer with CODEOWNERS authority approve the PR directly."
  2. 17:54 — qodo-code-review bot provided useful inline feedback (.yaml extension gap, potential lint-all breakage).
  3. 18:26 — Human reviewer (waynesun09) approved: "LGTM. Patterns are correctly scoped."
  4. 20:53 — Author rebased onto main (incorporating fix(ci): pin third-party actions in root action.yml to commit SHAs #2621 fixes) and addressed qodo feedback.
  5. 21:05 — Review agent re-ran and issued CHANGES_REQUESTED again with the identical protected-path finding, despite a human maintainer having already approved — satisfying the agent's own remediation criteria.
  6. 21:06 — PR merged.

Assessment

The main friction point was the review agent re-issuing CHANGES_REQUESTED after a human maintainer had already approved, which is the exact remediation the agent itself suggested. This created unnecessary noise on a small, well-scoped CI fix.

Existing coverage

All improvement opportunities identified are already tracked by open issues:

  • #1583 — Review agent should recognize human-resolved protected-path findings and stop re-flagging (direct match)
  • #1922 — Review bot should not override human approval with stale findings (direct match)
  • #1551 — Downgrade protected-path severity for human-authored PRs
  • #1500 — Review agent should not re-request changes for unchanged findings
  • #1068 — Should not file CHANGES_REQUESTED for governance-only findings the fix agent cannot resolve

No new proposals filed — the improvements are well-covered. Resolving #1583 and #1922 would directly eliminate the friction observed on this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/ci CI pipelines and checks type/bug Confirmed defect in existing behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants