Skip to content

chore: Scope CI workflow path filters to skip Rust build on doc-only PRs #265

Description

@inureyes

Problem / Background

.github/workflows/ci.yml has no paths or paths-ignore filter on either of its triggers (push to main/develop, pull_request to main). Every pull request therefore runs the full Rust toolchain, even when the diff contains no Rust at all.

Both jobs in that workflow are pure Rust and both run on ubuntu-latest:

  • ci (display name "CI"): cargo fmt --check, cargo clippy -- -D warnings, then cargo test --lib --verbose and cargo test --tests --verbose -- --skip integration_test.
  • msrv (display name "MSRV (cargo check on declared rust-version)"): reads rust-version from Cargo.toml (currently 1.96), installs that toolchain, and runs cargo check --workspace --locked.

Neither job inspects anything outside the Rust workspace, so on a documentation-only or workflow-YAML-only PR they compile the entire tree to prove nothing.

Concrete case that motivated this: PR #264 (merged as 6fdc598) changed only .github/workflows/release.yml, two new composite actions under .github/actions/, and ARCHITECTURE.md. Zero Rust source changes. Both CI jobs still ran a full build; the ci job was still pending roughly 10 minutes after the PR was opened, and the PR was merged without waiting for it. That is the failure mode worth fixing: a check that cannot say anything useful about the diff still gates attention, and in practice gets ignored, which erodes the signal for PRs where it does matter.

Proposed Solution

Scope the CI workflow so it does not run when the diff cannot possibly affect the Rust build.

Prefer paths-ignore (blocklist) over paths (allowlist). An allowlist fails unsafe: the day someone adds a new crate under crates/, a build.rs, or another workspace member that nobody remembered to add to the list, CI silently stops running on it and nobody notices until something breaks on main. A blocklist fails safe: anything unrecognized still runs the full build, and the worst case is a wasted run rather than an unvalidated merge. This repository is already a multi-crate workspace (. plus crates/bssh-russh-sftp), so new-path drift is a realistic scenario, not a hypothetical.

Starting candidates for paths-ignore:

  • **/*.md
  • docs/**
  • LICENSE
  • NOTICE
  • .github/workflows/release.yml
  • .github/workflows/update_homebrew_formula.yml
  • .github/actions/**

The repository also has .github/workflows/debian_build.yml and .github/workflows/launchpad_ppa.yml, which are packaging workflows with their own triggers. Editing them does not change any Rust source that cargo fmt, cargo clippy, or cargo test would evaluate, so they are reasonable additions to the list. Settle the final list during implementation by reading the current tree rather than copying this one verbatim.

.github/workflows/ci.yml itself must NOT be ignored. A change to the CI definition has to validate itself; otherwise a broken edit to the workflow merges green because the workflow declined to run on its own change.

Changes to Rust sources, Cargo.toml, Cargo.lock, benches/, tests/, crates/, and anything else not on the ignore list continue to run both jobs unchanged.

Technical Considerations

Required status checks are the decision point

If either CI job is ever made a required status check under branch protection, a workflow-level paths-ignore skip means the check never reports at all. GitHub does not treat "not run" as "passed" for a required check, so such a PR sits un-mergeable forever. This is the standard GitHub Actions required-check pitfall and it is the main reason to think before adding paths-ignore.

Right now this is not a blocker: main has no branch protection at all (gh api repos/lablup/bssh/branches/main/protection returns 404 "Branch not protected"), so neither ci nor msrv is a required check today. A plain paths-ignore works and is the simplest change.

Whoever implements this should pick one of these and record the choice:

  1. Workflow-level paths-ignore. Simplest, matches the repository's current unprotected state. If branch protection with required checks is added later, this must be revisited before the protection rule goes live.
  2. Always-reporting gate job. Keep paths-ignore off the workflow, add a lightweight job that always runs and whose name is the one registered as the required check, and have it depend on the conditional Rust jobs. The gate reports success when the heavy jobs are skipped, so the required check is always satisfied.
  3. Path detection with conditional steps. Use a cheap first job (for example dorny/paths-filter) to classify the diff, then gate the expensive steps on its output with if:. The jobs always report, so required checks keep working, at the cost of a few seconds of runner time per PR.

Option 1 is acceptable for this repository as it stands. If it is chosen, the issue's resolution should state plainly that adding branch protection later requires migrating to option 2 or 3.

Out of scope

.github/workflows/release.yml is a separate concern with its own triggers and is not modified by this change, beyond possibly appearing in the CI workflow's ignore list.

Acceptance Criteria

  • A documentation-only PR (for example touching only README.md or docs/**) does not start the ci or msrv jobs. Implemented in chore: skip CI Rust build on non-Rust-only changes #267, not yet observed live. Verified statically by replaying the ignore globs over all 428 tracked files; confirming it needs a separate doc-only PR. Left unchecked deliberately.
  • A release-YAML-only PR (touching only .github/workflows/release.yml and/or .github/actions/**) does not start the ci or msrv jobs. Implemented in chore: skip CI Rust build on non-Rust-only changes #267, not yet observed live, same reason as above. Left unchecked deliberately.
  • A PR touching any Rust source, Cargo.toml, Cargo.lock, or .github/workflows/ci.yml still runs both ci and msrv. Confirmed live on chore: skip CI Rust build on non-Rust-only changes #267, whose only changed file is ci.yml: both jobs were triggered by the pull_request event and passed. Rust sources, Cargo.toml, and Cargo.lock match no ignore pattern, verified by glob replay.
  • The chosen approach is compatible with main gaining branch protection later, or the issue records explicitly which approach was chosen and exactly what would need to change if required status checks are introduced. Option 1 chosen, recorded with the required migration in the comment below, in the chore: skip CI Rust build on non-Rust-only changes #267 PR body, and in a comment block at the top of ci.yml.
  • Behavior verified on a real pull request after implementation, not only by reading the YAML. Record the PR number and which jobs ran or were skipped. Half satisfied. PR chore: skip CI Rust build on non-Rust-only changes #267 (only ci.yml changed) confirms the positive half: CI ran and passed in 7m8s, MSRV ran and passed in 28s. The negative half, a doc-only PR skipping both jobs, is not verified; it needs a separate throwaway PR. Left unchecked deliberately.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions