Skip to content

Run only the CI a pull request's change can affect, and everything nightly - #882

Merged
robzolkos merged 3 commits into
mainfrom
ci/change-aware-workflows
Oct 12, 2026
Merged

robzolkos merged 3 commits into
mainfrom
ci/change-aware-workflows

Conversation

@robzolkos

@robzolkos robzolkos commented Oct 12, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #881 ("Build with Go 1.26.9 for eleven standard-library advisories"), which is stacked on #880 ("Bump golang.org/x/net to v0.60.0 for GO-2026-6617"). It's stacked only so its own Security check runs green; the change here is just .github/workflows/test.yml. When #880 merges, GitHub retargets this PR to main.

Problem

Every pull request runs the whole Go pipeline, whatever it changes. #878 ("Tell agents that rich-text fields support Mermaid diagrams") changed 29 lines of SKILL.md and one eval case, and still ran all 14 Test jobs. Measured on its first commit:

Wall clock Runner-minutes
Test workflow 6.0 min (16:26:23 → 16:32:20 UTC) 31.9
Slowest jobs Race shards 3.7–5.7 min, Tests 3.7, Integration 2.5, CLI Surface 2.3, Lint 1.6

The last 40 Test runs took 4.6–7.0 min, most of them 5.7–6.2. The race shards set the wall clock.

Change

A new Classify changes job sorts a pull request's files with dorny/paths-filter (already pinned in this workflow, predicate-quantifier: every):

The PR changes Runs Skips
anything outside the two lists below everything, as today nothing
only skills/**/*.md, skill-evals/** Tests, Integration Tests (embedded skill, drift fixtures, eval patterns) Lint, Security, CLI Surface Check, race shards
only root *.md, .github/*.md Tests' naming check (check-naming scans Markdown) every Go step and job

Pushes to main, workflow_dispatch and a new nightly schedule (05:23 UTC) are never narrowed. The schedule also gives govulncheck a daily run against the moving vulnerability database. govulncheck lives only in this workflow, which is how GO-2026-6617 sat unseen on main until someone pushed.

Concurrency: a new push to a pull request cancels that PR's run in flight. Pushes to main, the schedule and dispatches never cancel.

Caching is already there. setup-go v7 restores the module and build cache on every job (Cache hit for: setup-go-Linux-x64-ubuntu24-go-1.26.7-… in #878's Tests log). Nothing to add.

Required checks still report

main requires Tests, Lint, Security, Race Detection, Integration Tests, Nix flake builds and Secret Scanning.

  • No workflow-level paths. A filtered-out workflow never runs, and its required checks stay pending forever.
  • Job-level if. GitHub reports a job skipped by its condition as passing: "A job that is skipped will report its status as 'Success'. It will not prevent a pull request from merging, even if it is a required check." (docs)
  • Race Detection keeps always(). A failed shard still fails it. It skips only when go is false, which is when the shards skipped by design.
  • Fail-safe both ways. A path narrows the run only if it is listed, so an unlisted file runs everything, and so does any change to test.yml, including this PR. The gates read !cancelled() && needs.changes.outputs.X != 'false', so a failed or empty classifier runs every job. The classifier's own step rejects any answer other than true or false.
  • Tests never skips. On prose-only PRs it still runs the naming check, so the required check has something to answer for.

Proof

  • This PR changes test.yml, so its own run is the code path: everything runs.
  • A docs-only probe PR stacked on this branch is the narrowed path. Results are in the comment below.

Timings (estimated now, measured on the probe below)

PR kind Before After
Go/code change ~6 min, ~32 runner-min same, plus about 10 s for the classifier; superseded pushes get cancelled
Skill/eval only (like #878) ~6 min, ~32 runner-min ~4 min (Tests bounds it), ~8 runner-min
Prose only ~6 min, ~32 runner-min ~1 min, ~2 runner-min

Not done here (proposals)

  • Race sharding on PRs. Still the critical path for code PRs. More shards would cut wall time, but each one adds about 1 min of setup and compile.
  • CodeQL isn't required, so it could take a workflow-level paths-ignore for prose. It's left alone because code scanning flags PRs that have no analysis.

Summary by cubic

Makes the test workflow change-aware: a pull request now runs only the jobs its changed files can affect, and a nightly schedule (05:23 UTC) runs the full suite. Previously every PR ran all 14 Test jobs (~6 min, ~32 runner-minutes) even when it changed only prose or a skill's text.

A new Classify changes job sorts each PR's changed files and is fail-safe both ways: any unlisted file, any change to this workflow, or a classifier that fails or answers nothing runs everything. The gates are job-level ifs under !cancelled(), so GitHub reports skipped required checks as passing instead of leaving them pending, and Tests never skips entirely (it still runs the naming check on prose-only PRs). Skill/eval-only PRs skip Lint, Security, CLI Surface Check and the race shards; prose-only PRs run only the naming check.

Pushes to main, dispatches and the nightly schedule are never narrowed. The schedule also gives govulncheck a daily run against the moving vulnerability database; it lives only in this workflow, which is how GO-2026-6617 sat unseen on main until someone pushed.

Concurrency: A new push to a pull request cancels that PR's run in flight. Every non-PR run gets its own concurrency group so a newer pending run can't silently replace a queued nightly run.

Written for commit 33dda43. Summary will update on new commits.

View guided diff Turn on auto-fix

@robzolkos
robzolkos requested a review from a team as a code owner October 12, 2026 17:53
Copilot AI balanced review requested due to automatic review settings October 12, 2026 17:53
@github-actions github-actions Bot added the ci CI/CD workflows label Oct 12, 2026
@github-actions

Copy link
Copy Markdown

Sensitive Change Detection (shadow mode)

This PR modifies control-plane files:

  • .github/workflows/test.yml

Shadow mode — this check is informational only. When activated, changes to these paths will require approval from a maintainer.

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.

🟡 Changes recommended

The shared non-PR concurrency group can cancel pending main, scheduled, or dispatched runs.

1 open finding
What changed in this PR

Adds change-aware CI filtering for pull requests while preserving full scheduled and manually triggered checks.

Changes:

  • Classifies changes to selectively run Go, skill, and documentation checks.
  • Adds nightly execution and cancellation of superseded PR runs.
  • Keeps required checks reportable through job-level conditions.

[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.

File Description
.github/​workflows/​test.yml Adds scheduling, concurrency, change classification, and conditional CI execution.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/test.yml Resolved

@cubic-dev-ai cubic-dev-ai 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.

All reported issues were addressed across 1 file

You’re at about 92% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.

Reply with feedback, questions, or to request a fix.

View guided diff | Turn on auto-fix | Re-trigger cubic

Comment thread .github/workflows/test.yml Resolved
@robzolkos

Copy link
Copy Markdown
Collaborator Author

🤖 Measured. Each kind of change, from the Test workflow's jobs (wall clock is first job start to last job end):

Run Change Ran Required checks Wall Runner-min
#881 head (before this PR, same code) code 14 jobs all ran, pass 6.0 min 38.4
this PR, cc0869d test.yml, so a full run 15 jobs all ran, pass 6.1 min 36.0
probe #883 CONTRIBUTING.md only Classify, Tests (naming check only, 10 s), audit, Nix, preflight Tests pass; Lint, Security, Race Detection, Integration Tests skipped; Nix flake builds, Secret Scanning pass 0.6 min 1.3
probe #884 skills/basecamp-doctor/SKILL.md only + Tests, Integration Tests Tests, Integration Tests pass; Lint, Security, Race Detection skipped 4.2 min 7.4
dispatch on this branch non-PR event everything all ran, pass full —

Every required check name reported on every run: none stuck pending. The skipped ones are skipped check runs under the exact required names, from GitHub Actions. GitHub counts those as passing for a required check (docs: "It will not prevent a pull request from merging, even if it is a required check"). One limit on that proof: the probes targeted this branch, not main, so I couldn't watch the main ruleset evaluate them. That part rests on the documented behavior.

Runner-minutes for the code-change rows are inflated by the queueing while all these PRs ran at once. Wall clock is the comparable number.

Probes #883 and #884 are closed and their branches deleted.

Copilot AI previously approved these changes Oct 12, 2026

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.

🟢 Approved

The classifier, fallback behavior, required-check handling, and event-specific concurrency are consistent and validated by the probe runs.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

Base automatically changed from deps/go-toolchain-1.26.9 to deps/x-net-v0.60.0 October 12, 2026 18:54
Base automatically changed from deps/x-net-v0.60.0 to main October 12, 2026 19:00
…ghtly

Every pull request ran the whole Go pipeline, about 6 minutes and 32
runner-minutes, even when it changed only prose or a skill's text.
basecamp-cli#878 (29 lines of SKILL.md and an eval case) ran all
fourteen Test jobs.

A new Classify changes job sorts each pull request's files:

- Anything outside two short lists runs everything, as before.
- Skill and eval files only (skills/**/*.md, skill-evals/**) run Tests
  and Integration Tests, which read the embedded skill, its drift
  fixtures and the eval patterns. Lint, Security, CLI Surface Check and
  the race shards skip: none of them can see that change.
- Prose only (root *.md, .github/*.md) runs only Tests' naming check.

Fail-safe in both directions. A path narrows the run only if it is
listed, so an unlisted file and any change to this workflow run
everything. A classifier that fails or answers nothing narrows nothing:
the gates read != 'false' under !cancelled(), so an empty answer runs
the job.

Required checks keep reporting. The gates are job-level ifs, and GitHub
counts a job skipped by its condition as a passing check. A
workflow-level paths filter would leave Tests, Lint, Security, Race
Detection and Integration Tests pending forever. Race Detection keeps
always(), so a failed shard still fails it, and it skips only when the
shards skipped by design.

Pushes to main, a new nightly schedule and dispatches are never
narrowed. The schedule also gives govulncheck, which lives only in this
workflow, a daily run against the moving vulnerability database: the
GO-2026-6617 advisory sat unseen on main until someone pushed.

A new push to a pull request now cancels that pull request's run in
flight. Main, the schedule and dispatches never cancel.
A concurrency group keeps one running and one pending run, and a newer
pending run replaces the older one even with cancel-in-progress off. With
every main push, nightly run and dispatch in test-refs/heads/main, a push
could silently drop a queued nightly run. Pull requests keep one group per
PR, so a new push still supersedes the run in flight.
The job and step names already say what each one does. The reasoning
for why it's safe (fail-safe lists, job-level skips that report as
passing, Race Detection's always()) is in the pull request and the first
commit's message.
@robzolkos
robzolkos force-pushed the ci/change-aware-workflows branch from b3b2b5a to 33dda43 Compare October 12, 2026 19:04
@robzolkos
robzolkos requested a balanced review from Copilot October 12, 2026 19:21
Copilot AI dismissed their stale review, a newer Copilot review was requested October 12, 2026 19:21

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

It changes required-check execution and cancellation behavior in the repository’s central CI workflow.

0 open findings

🧠 Review effort: Balanced

@robzolkos
robzolkos merged commit 17c9f88 into main Oct 12, 2026
32 checks passed
@robzolkos
robzolkos deleted the ci/change-aware-workflows branch October 12, 2026 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci CI/CD workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants