Reduce Smoke CI REST fan-out by scoping pull_request triggers#44934
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis. No test files were added or modified in this PR. Test Quality Sentinel skipped. |
|
✅ PR Code Quality Reviewer completed the code quality review. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #44934 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100). |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
There was a problem hiding this comment.
Pull request overview
Scopes Smoke CI pull-request runs to relevant paths, reducing unnecessary REST usage.
Changes:
- Adds matching PR and push path filters.
- Recompiles Smoke CI.
- Refreshes PR-trigger metadata across generated workflows.
Show a summary per file
| File | Description |
|---|---|
eslint-factory/src/rules/no-throw-plain-object.ts |
Formatting-only change. |
.github/workflows/visual-regression-checker.lock.yml |
Refreshes PR metadata. |
.github/workflows/test-quality-sentinel.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-update-cross-repo-pr.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-test-tools.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-temporary-id.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-project.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-pi.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-opencode.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-multi-pr.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-gemini.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-crush.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-create-cross-repo-pr.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-copilot-arm.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-codex.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-claude.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-ci.md |
Adds PR path filtering. |
.github/workflows/smoke-ci.lock.yml |
Compiles the new filter. |
.github/workflows/smoke-call-workflow.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-antigravity.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-agent-scoped-approved.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-agent-public-none.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-agent-public-approved.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-agent-all-none.lock.yml |
Refreshes PR metadata. |
.github/workflows/smoke-agent-all-merged.lock.yml |
Refreshes PR metadata. |
.github/workflows/refiner.lock.yml |
Refreshes PR metadata. |
.github/workflows/pr-description-caveman.lock.yml |
Refreshes PR metadata. |
.github/workflows/pr-code-quality-reviewer.lock.yml |
Refreshes PR metadata. |
.github/workflows/mattpocock-skills-reviewer.lock.yml |
Refreshes PR metadata. |
.github/workflows/impeccable-skills-reviewer.lock.yml |
Refreshes PR metadata. |
.github/workflows/firewall-escape.lock.yml |
Refreshes PR metadata. |
.github/workflows/design-decision-gate.lock.yml |
Refreshes PR metadata. |
.github/workflows/dependabot-repair.lock.yml |
Refreshes PR metadata. |
.github/workflows/changeset.lock.yml |
Refreshes PR metadata. |
.github/workflows/ai-moderator.lock.yml |
Refreshes PR metadata. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 35/35 changed files
- Comments generated: 1
- Review effort level: Medium
| schedule: daily | ||
| pull_request: | ||
| types: [opened, synchronize, reopened] | ||
| paths: ['cmd/**', 'pkg/**', '*.go', 'go.mod', 'actions/setup/js/**'] |
There was a problem hiding this comment.
The path-scoped pull_request trigger in smoke-ci.md is correct and well-aligned with the existing push paths. The schedule: daily trigger is unaffected (GitHub ignores path filters for schedule events). All other lock file changes are a mechanical recompile artifact (has_pull_request:true added to manifests). The eslint formatting change is purely cosmetic. No blocking issues.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 26.2 AIC · ⌖ 4.94 AIC · ⊞ 4.8K
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design — change is clean, purposeful, and correctly scoped.
📋 Key Themes & Highlights
Summary
This PR adds a paths filter to the pull_request trigger of Smoke CI, mirroring the already-present push filter. The result: the workflow skips on unrelated PRs (docs-only changes, etc.), reducing unnecessary REST fan-out without affecting any test logic.
Positive Highlights
- ✅ Path filter exactly matches the existing
pushfilter — no drift between the two triggers - ✅ Source (
smoke-ci.md) and compiled (smoke-ci.lock.yml) kept in sync viamake recompile - ✅ All other lock file changes are purely generated metadata updates — no manual edits
- ✅ PR description clearly explains motivation and behavioral impact
One thing to be aware of
Changes to .github/workflows/smoke-ci.md itself (without touching Go/JS code) will no longer trigger Smoke CI. This is intentional for a smoke workflow that validates code correctness, but worth noting for contributors who update the workflow definition.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 36.2 AIC · ⌖ 4.38 AIC · ⊞ 6.6K
Comment /matt to run again
There was a problem hiding this comment.
Two concerns worth resolving before merge
The paths-filter optimization is sound for its stated goal, but two issues need attention:
-
Workflow self-validation gap —
.github/workflows/smoke-ci.mdand.github/workflows/smoke-ci.lock.ymlare not in the newpathsfilter. A PR that changes only the smoke-CI workflow itself will skip the run. See inline comment. -
has_pull_requestmanifest nondeterminism — All 33 unrelated lock files gained"has_pull_request":truein their manifests during this recompile. If the compiler emits a different value when run outside a PR, every lock file in the repo will diverge on the nextmake recompile, causing spurious diffs and undermining manifest integrity checks. See inline comment.
🔎 Code quality review by PR Code Quality Reviewer · 100.7 AIC · ⌖ 4.73 AIC · ⊞ 5.4K
Comment /review to run again
Comments that could not be inline-anchored
.github/workflows/smoke-ci.md:13
Smoke CI won't self-validate on PRs that only touch the workflow source: .github/workflows/smoke-ci.md and .github/workflows/smoke-ci.lock.yml are absent from the new paths filter, so a PR that modifies the smoke-CI workflow itself will skip the run that would catch regressions in the workflow.
<details>
<summary>💡 Suggested fix</summary>
Add the workflow files to the paths list so that changes to the workflow are validated by the workflow:
paths:
- 'cmd/**'
- 'pkg/**'…
</details>
<details><summary>.github/workflows/smoke-ci.lock.yml:180</summary>
**\`has_pull_request:true\` was injected into all 33 unrelated lock manifests during recompilation in PR context**: if this flag is context-dependent, future \`make recompile\` runs on the default branch will produce different manifests and generate spurious diffs across all lock files.
<details>
<summary>💡 Details</summary>
Every `*.lock.yml` file that was previously missing `has_pull_request` now has `"has_pull_request":true` appended to its `gh-aw-manifest` comment. If `make recompile` pr…
</details>
<details><summary>.github/workflows/smoke-ci.md:13</summary>
**`actions/setup/sh/**`, `actions/setup/src/**`, and `actions/setup/action.yml` are excluded from the paths filter**: the filter covers only `actions/setup/js/**`, but the setup action also contains shell scripts in `sh/`, compiled sources in `src/`, and `action.yml`. A PR touching any of those files will silently skip Smoke CI.
<details>
<summary>💡 Suggested fix</summary>
Widen the setup action glob:
```yaml
paths:
- cmd/**
- pkg/**
- *.go
- go.mod
- actions/setup/** # covers j…
</details>
<details><summary>.github/workflows/smoke-ci.md:13</summary>
**`go.sum` is absent from the paths filter**: a PR that only updates `go.sum` (e.g. a dependency version bump via `go get`) will not trigger Smoke CI, even though the resulting binary is different.
<details>
<summary>💡 Suggested fix</summary>
Add `go.sum` alongside `go.mod`:
```yaml
paths:
- cmd/**
- pkg/**
- *.go
- go.mod
- go.sum
- actions/setup/**Note this gap exists in both the push and pull_request triggers (the push filter has the same go.sum omission).
</de…
Smoke CI is the single largest workflow consumer of GitHub REST calls, with usage driven more by run frequency than per-run cost. This change reduces unnecessary fan-out by aligning PR trigger scope with the workflow’s actual code surface.
Trigger scope
pull_request.pathstoSmoke CIpushpath filter so the workflow only runs for changes that can affect the smoke path:cmd/**pkg/***.gogo.modactions/setup/js/**Generated workflow
smoke-ci.lock.ymlto reflect the trigger change in the executable workflowBehavioral impact