Skip to content

ci: run only the lanes a PR affects + build Docker on main only - #49272

Merged
ethernet8023 merged 5 commits into
mainfrom
bb/ci-affected-only
Jun 23, 2026
Merged

ethernet8023 merged 5 commits into
mainfrom
bb/ci-affected-only

Conversation

@OutThisLife

@OutThisLife OutThisLife commented Jun 19, 2026 •

Copy link
Copy Markdown

What this does

Replaces the scattered per-workflow path-gating pattern with a single orchestrator (ci.yml) that runs the classifier once, then conditionally calls sub-workflows via workflow_call. A final all-checks-pass gate job aggregates all results so branch protection only needs to require one check.

Also moves the heaviest job (multi-arch Docker) off PRs entirely — it builds on push-to-main and release only.

Why

Every heavy workflow previously had no on.paths filter and ran on every PR: each carried a comment that a path-gated workflow leaves its required check pending forever when no matching file changes, which blocks merge. The price was that a docs-only PR still ran everything — the TypeScript matrix, the full Python suite, ruff/ty, the Docusaurus build, and the ~9-11 min multi-arch Docker build — none of which the change could touch.

The old approach ran detect-changes inside every workflow and gated individual steps with if: guards. That worked but wasted runners (every workflow spins up just to no-op) and the required-checks list in branch protection had to name every individual job.

The new approach:

  • Runs detect-changes once in the orchestrator.
  • Conditionally calls each sub-workflow via workflow_call — skipped workflows don't spin up runners at all.
  • A single all-checks-pass job (with if: always()) runs after everything, treating skipped as success and only failing on actual failure. Branch protection requires only this one check.

How it works

Orchestrator (ci.yml)

detect  →  tests        (python lane)
        →  lint          (python lane, receives event_name input)
        →  typecheck     (frontend lane)
        →  docs-site     (site lane)
        →  history-check (PR-only)
        →  contributor-check (python lane)
        →  uv-lockfile   (always)
        →  docker-lint   (docker_meta lane)
        →  supply-chain  (scan/deps/mcp_catalog lanes, receives booleans as inputs)
        →  osv-scanner   (always)
                                     ↓
                          all-checks-pass (if: always())

All sub-workflows run in parallel after detect finishes. Each sub-workflow keeps its own job definitions, matrices, and concurrency settings — they just no longer have their own push:/pull_request: triggers (everything flows through ci.yml).

Classifier (scripts/ci/classify_changes.py)

Maps the PR diff to 7 lanes, surfaced as outputs by the .github/actions/detect-changes composite action:

Lane Gates Triggers on
python tests, lint, contributor-check any non-(docs/frontend/docker) file
frontend typecheck matrix + desktop-build ui-tui/, web/, apps/, root package*.json
site docs-site-checks website/, skills/, optional-skills/
docker_meta docker-lint Dockerfile, docker/, .hadolint.yml
scan supply-chain critical-pattern scanner .py, .pth, setup.py, pyproject.toml
deps dependency upper-bounds check pyproject.toml
mcp_catalog MCP catalog security review optional-mcps/, hermes_cli/mcp_catalog.py

Race-condition fix

The detect-changes action uses the GitHub compare endpoint with pinned base/head SHAs from the event payload (repos/{repo}/compare/{base}...{head}) instead of the "current PR files" endpoint (pulls/N/files). The SHAs are frozen at trigger time, so the file list is deterministic even if the PR receives a new push between trigger and detect.

Docker

docker-publish.yml gates every build/smoke/integration step on github.event_name != pull_request. Pre-merge Dockerfile/lock sanity is still covered on PRs by docker-lint (hadolint/shellcheck, gated on the docker_meta lane) and uv-lockfile-check (always runs).

workflow_call event name passthrough

When a workflow is called via workflow_call, github.event_name is "workflow_call", not the event that triggered the caller. The orchestrator passes the real event name as a workflow_call input (event_name), and lint.yml + supply-chain-audit.yml use inputs.event_name instead of github.event_name in their conditions.

Other changes

  • build-windows-installer.yml removed — it was workflow_dispatch only, not a PR-gated check, and is built elsewhere.
  • retry/action.yml — new composite action for retrying flaky dependency installs (npm ci, uv sync) whose only failures are transient network flakes.
  • Supply-chain-audit — internal changes job and all *-gate jobs removed; path-gating now handled centrally by the orchestrator. The scan/deps/mcp_catalog booleans arrive as workflow_call inputs.

Safety contract — fail open, never closed

  • Empty diff or any .github/ change → run everything.
  • python is a denylist: skipped only when every changed file is provably prose, a frontend-only package, or docker metadata; an unrecognized path keeps it on.
  • skills/**/SKILL.md is python-relevant (the skill-doc tests read that tree), not pure docs.
  • Non-PR events (push to main, release) always run the full pipeline.

What stays separate

docker-publish.yml, deploy-site.yml, skills-index.yml, skills-index-freshness.yml, upload_to_pypi.yml — not PR-gated required checks, trigger on release/schedule/dispatch.

Branch protection change

Replace the long list of individual required checks with just all-checks-pass (from the CI workflow).

Validation

  • Classifier — 20 contract tests (invariants, not snapshots) in tests/ci/test_classify_changes.py.
  • All workflow YAMLs + composite actions parse.
  • Live PR test (ci: gate ci at job run-time #51389): orchestrator runs, sub-workflows called in parallel, lane-gated skips work (e.g. docker-lint skipped when no Dockerfile changes, lint-diff skipped on PRs).
  • Post-merge (the real proof): first docs-only PR → only fast guards run; first Python-only PR → TS/docker lanes skip.

@github-actions

github-actions Bot commented Jun 19, 2026 •

Copy link
Copy Markdown

🔎 Lint report: bb/ci-affected-only vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 11115 on HEAD, 11114 on base (🆕 +1)

🆕 New issues (1):

Rule Count
unresolved-import 1
First entries
tests/ci/test_classify_changes.py:13: [unresolved-import] unresolved-import: Cannot resolve imported module `pytest`

✅ Fixed issues: none

Unchanged: 5858 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change P3 Low — cosmetic, nice to have labels Jun 19, 2026
@OutThisLife
OutThisLife force-pushed the bb/ci-affected-only branch from 2626c9b to e6814bd Compare June 19, 2026 21:34
@OutThisLife OutThisLife changed the title ci: skip unaffected lanes on PRs (docs/TS/Python-only) ci: run only the lanes a PR affects + build Docker on main only Jun 19, 2026
@OutThisLife
OutThisLife force-pushed the bb/ci-affected-only branch from e6814bd to 9353b43 Compare June 19, 2026 21:46
Heavy PR checks run on every PR because the workflows deliberately avoid
`on.paths` filters — a path-gated workflow leaves its required check pending
forever when no matching file changes, blocking merge. So a docs-only PR
still spins up the TypeScript matrix, the full Python suite, and ruff/ty.

Keep every workflow triggering on every PR (checks always report) but gate
the expensive *steps* on what the PR touches. Skipping a step (not the job)
leaves the job green, so required checks never hang — the same idiom already
proven in contributor-check.yml.

A classifier (scripts/ci/classify_changes.py) maps the PR diff to three
lanes — python, frontend, site — surfaced as step outputs by a composite
action (.github/actions/detect-changes). Fail-open: an empty diff or any
.github/ change runs everything; python is a denylist (skipped only when
every file is provably prose or a frontend-only package); skills/**/SKILL.md
counts as python-relevant since the skill-doc tests read that tree. Non-PR
events always run the full pipeline.
The image build + smoke test + integration suite are the heaviest jobs in CI
(~9-11 min) and ran on every PR. Gate them to push-to-main and release: a
broken build surfaces on the main push, while the cheap pre-merge guards
(docker-lint hadolint/shellcheck, uv-lockfile-check) still run on PRs to
catch the common Dockerfile/lockfile breakage. Steps skip on PRs so the job
stays green; the dead PR-only arm64 cache-warm build is removed.
`npm ci` / `uv sync` / toolchain header fetches occasionally die on
transient network blips — e.g. node-pty's node-gyp fetching Node headers
(an undici assert) during the typecheck job's `npm ci`, which killed the job
before `tsc` ever ran. "Re-run and it goes green" is exactly what CI should
do itself.

- New reusable `.github/actions/retry` composite action wraps a command and
  retries on failure (3x / 10s, command passed via env so it can't inject).
  Applied to every PR-path network install: npm ci (typecheck, desktop
  build, docs site), uv sync (tests, e2e), uv tool install (lint),
  pip install (docs site).
- typecheck now runs `npm ci --ignore-scripts`: `tsc` needs only sources +
  type defs, so skipping install scripts drops node-pty's native rebuild
  (whose header fetch was the flake) and is faster. Validated locally — tsc
  passes for ui-tui, apps/shared, and apps/desktop with scripts skipped.
- ripgrep download uses `curl --retry`.

Docker (main-only) and the release/windows workflows are intentionally left
for a follow-up.
@ethernet8023
ethernet8023 force-pushed the bb/ci-affected-only branch from 51b7477 to 3c73a5c Compare June 23, 2026 15:50
@github-actions

Copy link
Copy Markdown

⚠️ MCP catalog security review required

This PR changes the bundled MCP catalog or MCP catalog installer code. MCP entries can define local commands that users later install into mcp_servers, so this needs explicit maintainer review before merge.

A maintainer should verify:

  • any new/changed optional-mcps/**/manifest.yaml command and args are expected,
  • stdio transports do not use shell+egress/exfiltration payloads,
  • git install refs are pinned and bootstrap commands are minimal,
  • requested env vars/secrets match the upstream MCP's documented needs.

After review, add the mcp-catalog-reviewed label and re-run this check.

1 similar comment
@github-actions

Copy link
Copy Markdown

⚠️ MCP catalog security review required

This PR changes the bundled MCP catalog or MCP catalog installer code. MCP entries can define local commands that users later install into mcp_servers, so this needs explicit maintainer review before merge.

A maintainer should verify:

  • any new/changed optional-mcps/**/manifest.yaml command and args are expected,
  • stdio transports do not use shell+egress/exfiltration payloads,
  • git install refs are pinned and bootstrap commands are minimal,
  • requested env vars/secrets match the upstream MCP's documented needs.

After review, add the mcp-catalog-reviewed label and re-run this check.

ci: centralize path-gating behind single orchestrator + all-checks-pass
gate

Replace the scattered per-workflow detect-changes pattern with a single
ci.yml orchestrator that runs the classifier once, then conditionally
calls sub-workflows via workflow_call based on lane outputs. A final
all-checks-pass job (if: always()) aggregates all results so branch
protection only needs to require one check.

Changes:
- New .github/workflows/ci.yml orchestrator (detect + conditional calls
  + all-checks-pass gate)
- Extend classify_changes.py with scan/deps/mcp_catalog lanes, absorbing
  supply-chain-audit's internal changes job
- Update detect-changes/action.yml to expose the new lane outputs
- Convert all 10 PR-gated sub-workflows to workflow_call-only triggers,
  removing their push/pull_request triggers and per-step detect-changes
  guards (gating now happens at the orchestrator level)
- lint.yml + supply-chain-audit.yml receive event_name as a
workflow_call
  input to replace github.event_name (which is "workflow_call" inside
  called workflows)
- supply-chain-audit.yml: remove internal changes job + *-gate jobs
  (orchestrator handles gating, booleans arrive as inputs)
- contributor-check.yml: remove internal filter step
- Update test_classify_changes.py for 6-lane output + new supply-chain
  test cases
@ethernet8023
ethernet8023 force-pushed the bb/ci-affected-only branch from 5479ed9 to 912e8a7 Compare June 23, 2026 16:14
@ethernet8023
ethernet8023 merged commit c820eb6 into main Jun 23, 2026
30 checks passed
@ethernet8023
ethernet8023 deleted the bb/ci-affected-only branch June 23, 2026 16:30
@ethernet8023 ethernet8023 mentioned this pull request Jun 24, 2026
15 of 18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mcp-catalog-reviewed P3 Low — cosmetic, nice to have type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants