ci: dynamic branch protection via ci-complete gate

[spomichter]

Problem

Branch protection on dev requires all CI jobs to pass. But MD-only PRs skip CI jobs via paths-ignore, leaving them in 'skipped' state — which branch protection treats as not passing, blocking the merge.

Solution

Add a ci-complete aggregator job with if: always(). Branch protection only needs to require ci-complete:

• ✅ All jobs pass → ci-complete passes
• ⏭️ Jobs skipped (MD-only PR) → ci-complete passes
• ❌ Any job fails → ci-complete fails

Also removes paths-ignore from the pull_request trigger so the workflow always runs (individual jobs still use their own path filters if any).

Branch Protection Change Required

After merging, update dev branch protection:

• Remove individual job requirements (run-tests, run-heavy-tests, etc.)
• Add only ci-complete as required status check

[greptile-apps]

Greptile Summary

This PR solves the problem of markdown-only PRs being blocked by branch protection on dev. Previously, paths-ignore: '**.md' caused CI jobs to be skipped, and branch protection treated skipped jobs as not passing.

The fix adds a ci-complete aggregator job that runs with if: always() and checks whether any upstream test job failed or was cancelled. Branch protection can then require only this single gate job instead of each individual CI job.

• Removes paths-ignore from the pull_request trigger so the workflow always fires (the existing check-changes path filter job still gates actual work)
• Adds ci-complete job that depends on all 6 test/lint jobs and passes if none failed or were cancelled
• The needs list correctly covers all test jobs: run-tests, run-heavy-tests, run-lcm-tests, run-integration-tests, run-ros-tests, run-mypy
• Requires a post-merge branch protection settings update (documented in the PR description)

Confidence Score: 4/5

• This PR is safe to merge — it adds a CI aggregator job with correct failure-detection logic and a well-understood pattern.
• The change is small and focused. The ci-complete job logic correctly uses contains(needs.*.result, 'failure') and contains(needs.*.result, 'cancelled') to catch failures. The needs list covers all test/lint jobs. The only concern is a minor style issue with expression injection in the run: block (safe in this case but not idiomatic). Score is 4 rather than 5 because the post-merge branch protection update is a manual step that, if forgotten, means the old individual job requirements still apply alongside this new gate.
• No files require special attention — the single changed file is straightforward.

Important Files Changed

Filename	Overview
.github/workflows/docker.yml	Adds a ci-complete aggregator job and removes paths-ignore from pull_request trigger. Logic is sound: the gate correctly checks for failure/cancelled across all test jobs. Minor style concern with expression injection in run: block.

Flowchart

flowchart TD
    PR["pull_request event"] --> CC["check-changes"]
    CC --> ROS["ros (build)"]
    CC --> PY["python (build)"]
    ROS --> RP["ros-python (build)"]
    PY --> DEV["dev (build)"]
    RP --> RDEV["ros-dev (build)"]
    
    DEV --> RT["run-tests"]
    DEV --> RHT["run-heavy-tests"]
    DEV --> RLT["run-lcm-tests"]
    DEV --> RIT["run-integration-tests"]
    RDEV --> RRT["run-ros-tests"]
    RDEV --> RM["run-mypy"]
    
    RT --> CI["ci-complete"]
    RHT --> CI
    RLT --> CI
    RIT --> CI
    RRT --> CI
    RM --> CI
    
    CI -->|"all pass/skip"| PASS["✅ Branch protection passes"]
    CI -->|"any fail/cancel"| FAIL["❌ Branch protection fails"]

    style CI fill:#f9f,stroke:#333,stroke-width:2px
    style PASS fill:#9f9,stroke:#333
    style FAIL fill:#f99,stroke:#333

Loading

_{Last reviewed commit: dec251a}

[greptile-apps]

_{1 file reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Use if: condition instead of shell expression injection

The ${{ }} expression inside a run: block is evaluated and string-interpolated into the shell script before execution. While the output here is a safe boolean string ("true" / "false"), this pattern is flagged by GitHub's security scanner as an expression injection risk. The idiomatic approach is to move the logic into the if: condition of the step:

Suggested change

	- name: CI gate
	run: |
	if [[ "${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }}" == "true" ]]; then
	echo "❌ One or more CI jobs failed or were cancelled"
	exit 1
	fi
	echo "✅ All CI checks passed or were intentionally skipped"
	- name: CI gate
	if: ${{ contains(needs.*.result, 'failure') || contains(needs.*.result, 'cancelled') }}
	run: |
	echo "❌ One or more CI jobs failed or were cancelled"
	exit 1
	- name: CI passed
	run: echo "✅ All CI checks passed or were intentionally skipped"

This avoids injecting expressions into the shell context entirely.