Skip to content

fix(ci): skip native-host-build when no native-relevant paths changed - #2116

Merged
carlos-alm merged 3 commits into
mainfrom
fix/ci-scope-native-matrix
Jul 17, 2026
Merged

fix(ci): skip native-host-build when no native-relevant paths changed#2116
carlos-alm merged 3 commits into
mainfrom
fix/ci-scope-native-matrix

Conversation

@carlos-alm

Copy link
Copy Markdown
Contributor

Summary

  • `native-host-build` (3-OS matrix: ubuntu/macos/windows) ran an unconditional Rust build + `cargo test` on every push to main and every PR, regardless of whether the diff touched anything native. `test`, `parity`, and `pre-publish-benchmark` all depend on its artifacts.
  • Added a `detect-changes` job that diffs the PR base (or the previous push SHA) for `crates/**`, `Cargo.toml`, `Cargo.lock`, and the native addon loader (`src/infrastructure/native.ts`)
  • `native-host-build` now skips entirely (all 3 OSes) when none of those changed — fail-open: if the detector itself fails or is inconclusive, the build still runs, so a broken detector can't silently hide a real Rust regression
  • `test`, `parity`, and `pre-publish-benchmark` keep running on all 3 OSes regardless — they just fall back to the last-published native binary (via the normal `npm install` optional-dependency resolution) instead of a freshly PR-built one when `native-host-build` was skipped. This preserves full cross-platform test/parity coverage for every PR; only the redundant Rust rebuild+`cargo test` is cut for non-native changes.

Why

Part of the broader CI/CD cost-reduction effort (#2108). This is the biggest single lever in that effort — most day-to-day PRs in this repo don't touch `crates/**`, so they were paying for a 6-leg macOS/Windows/Linux native rebuild (plus `cargo test` on each) for no reason. 880 CI runs in the last 30 days were all paying this cost unconditionally.

Closes #2111

Test plan

  • YAML parses, `actionlint` shows no new findings (pre-existing shellcheck notes only, unrelated lines)
  • A pure-TS/docs PR should show `native-host-build` skipped and `test`/`parity`/`pre-publish-benchmark` still green on all 3 OSes
  • A PR touching `crates/**` should show `native-host-build` still running on all 3 OSes as before
  • Push to main (no native changes) should show the same skip behavior

native-host-build, test, parity, and pre-publish-benchmark all ran a full
3-OS Rust build (via native-host-build) unconditionally on every push/PR,
even for pure-TS/docs changes that can't affect the native addon.

Add a detect-changes job that diffs against the PR base (or previous push
SHA) for crates/**, Cargo.toml/lock, and the native addon loader. When none
of those changed, native-host-build is skipped entirely (fail-open: any
detection failure still builds, so a broken detector can't hide a real
Rust regression). test/parity/pre-publish-benchmark keep running on all 3
OSes regardless — they just fall back to the last-published native binary
via npm install instead of a freshly PR-built one, preserving full
cross-platform test/parity coverage while cutting the redundant rebuild.
@greptile-apps

greptile-apps Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a detect-changes job that diffs the repository against the last semantic-version release tag and gates native-host-build (3-OS Rust matrix) on the result, with a documented fail-open design so a broken detector can never silently mask a Rust regression.

  • detect-changes: runs git diff $LAST_TAG HEAD and emits native_changed=true/false; fails open (no tag found, script error) so native-host-build always runs in ambiguous cases.
  • native-host-build: skips all three matrix legs only when detect-changes succeeded AND reported no native-relevant paths changed; artifact download/install steps in downstream jobs are individually gated on native-host-build.result == 'success'.
  • test / parity / pre-publish-benchmark: updated with always() + != 'failure' / != 'cancelled' guards so they keep running against the published npm binary even when the native build was legitimately skipped.

Confidence Score: 5/5

Safe to merge — the fail-open design ensures correctness in all edge cases, and no downstream job can silently use a stale binary when native code has changed.

The logic is sound and the control flow is well-reasoned: detect-changes can only suppress native-host-build when it positively confirms no native paths changed since the last release tag, and every other outcome keeps the build running. The one limitation — that the skip only fires when no unreleased native commits exist — is intentional, documented, and correct from a safety standpoint.

Only .github/workflows/ci.yml changed. The detect-changes script block (lines 76-90) is the section worth re-reading — specifically the tag-based diff logic and the comment explaining why a PR-base diff was rejected.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Adds detect-changes job that diffs against the last release tag to gate native-host-build; all three matrix legs are skipped together when no native paths changed. Downstream jobs use always() + result guards to keep running against the published binary. Logic is correct and fail-open, but the tag-based diff means the skip only triggers when no native commits exist between the last release and HEAD — a condition the code's own comments say doesn't hold today.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([push / pull_request]) --> DC[detect-changes\ngit diff LAST_TAG HEAD]
    DC -->|script fails / no usable tag| NHB_RUN
    DC -->|native_changed=true| NHB_RUN[native-host-build\n3-OS matrix]
    DC -->|native_changed=false| NHB_SKIP[native-host-build\nSKIPPED]

    NHB_RUN -->|success| DL[Download + install\nPR-built .node artifact]
    NHB_SKIP -->|skipped| PUB[Use published npm\nbinary as fallback]

    DL --> TEST[test / parity / pre-publish-benchmark\nrun on all 3 OSes]
    PUB --> TEST

    NHB_RUN -->|failure| SKIP_DOWN[test / parity / benchmark\nSKIPPED via failure guard]

    TEST --> CP[ci-pipeline\nfail on any failure/cancelled]
    SKIP_DOWN --> CP
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A([push / pull_request]) --> DC[detect-changes\ngit diff LAST_TAG HEAD]
    DC -->|script fails / no usable tag| NHB_RUN
    DC -->|native_changed=true| NHB_RUN[native-host-build\n3-OS matrix]
    DC -->|native_changed=false| NHB_SKIP[native-host-build\nSKIPPED]

    NHB_RUN -->|success| DL[Download + install\nPR-built .node artifact]
    NHB_SKIP -->|skipped| PUB[Use published npm\nbinary as fallback]

    DL --> TEST[test / parity / pre-publish-benchmark\nrun on all 3 OSes]
    PUB --> TEST

    NHB_RUN -->|failure| SKIP_DOWN[test / parity / benchmark\nSKIPPED via failure guard]

    TEST --> CP[ci-pipeline\nfail on any failure/cancelled]
    SKIP_DOWN --> CP
Loading

Reviews (2): Last reviewed commit: "fix(ci): diff native-relevant changes ag..." | Re-trigger Greptile

native-host-build was skipping based on a diff against the PR base SHA
(or previous push SHA), which only answers whether *this* change
touched native code. It says nothing about whether the published npm
binary that test/parity/pre-publish-benchmark fall back to has already
drifted from HEAD. That binary is pinned to the last workflow_dispatch
release and only gets rebuilt on demand — native-relevant commits
routinely land on main between releases (62 since v3.15.0 at time of
writing) without republishing.

Diff against the last release tag instead, so native-host-build
correctly reruns whenever the fallback binary would be stale, not just
when the current PR/push touched crates/.
@carlos-alm

Copy link
Copy Markdown
Contributor Author

Thanks for the review — confirming the fail-open design and the tag-based diff are intentional, as noted. CI on 485dd63 now shows native-host-build correctly running on all 3 OSes (previously it was wrongly skipped, which was the root cause of the Test/Engine-parity failures on the prior push — the fallback binary had drifted 62 native commits past the pinned v3.15.0 release). Watching Test/Engine-parity/pre-publish-benchmark now that a fresh binary is available.

@carlos-alm
carlos-alm merged commit 46d7058 into main Jul 17, 2026
12 checks passed
@carlos-alm
carlos-alm deleted the fix/ci-scope-native-matrix branch July 17, 2026 06:14
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cost: CI's 3-OS matrix runs unconditionally regardless of whether native code changed

1 participant