ci: build pull requests, and only the devices they reach by widgetii · Pull Request #120 · OpenIPC/builder

widgetii · 2026-08-16T19:43:20Z

This repo has no pull_request trigger at all. A change lands unbuilt, and the nightly finds out the next morning — or a user does. Turning PR CI on meant 107 device builds per push, which is why it stayed off.

Almost every change here is one device: 639 of the file touches in the last 200 commits sit under a single devices/<dir>/, and the median commit reaches 2 devices. Narrowing first is what makes PR CI affordable at all.

Ported from OpenIPC/firmware#2273 and #2275.

The mapping is read off the tree, the way builder.sh reads it

builder.sh:121 locates a device by its defconfig and copies that whole devices/<dir>/ tree over the firmware clone:

ITEM=$(find devices -name ${DEVICE}_defconfig | cut -d/ -f1,2)
cp -afv ${BUILDER_DIR}/${ITEM}/* ${FIRMWARE_DIR}

So the devices a file affects are exactly the devices whose defconfig shares its directory. That distinction is load-bearing — devices/common/ backs 18 targets and devices/apfpv/ backs 2, so treating a directory as one device would skip 17 real builds. A defconfig still names one target even inside a shared directory.

changed path	devices built
`devices/t31_lite_wyze-v3b/…/customizer.sh`	1
`devices/apfpv/general/overlay/etc/udhcpd.conf`	2
`devices/common/general/overlay/etc/inittab`	18
`devices/common/…/configs/gk7205v200_fpv_defconfig`	1
`package/…`, `builder.sh`	107
`.github/workflows/master.yml`	15 (smoke)
`README.md`, `repack.sh`, `package.sh`, manifest workflow	0

repack.sh and package.sh are developer/end-user tools — no workflow invokes them; only builder.sh is on the CI path.

The device list is derived, not written down

Adding a camera is the work here: 8 of the 10 other PRs open right now add a device, and 14% of recent commits do. A written-down matrix would mean every one of those PRs also edits ci-matrix.py — and a change to that file cannot narrow, because it is what does the narrowing. Registering one camera would have cost a full 107-device build to prove one device: the most common change getting the worst outcome.

So the matrix is every defconfig under devices/, minus a NOT_BUILT opt-out holding the 7 that are in the tree but out of CI. Adding a device is creating its directory, nothing else — and it builds exactly that device. It no longer needs registering in master.yml either.

This is the opposite call to firmware's ci-matrix.py, on purpose: there the matrix is curated (29 defconfigs deliberately never built, boards are not the product), so an explicit list is the honest description. The derived list here is identical to the 107 master.yml carried — nothing lost, nothing gained.

The nightly is NOT narrowed

builder.sh re-clones OpenIPC/firmware at HEAD every run, so what a nightly builds is decided mostly outside this repo. master.yml already carries the scar of an earlier gate that skipped on this repo's HEAD and left upstream fixes invisible to users. Every event except pull_request gets the whole matrix, and --self-test asserts that for schedule, workflow_dispatch and push.

A release-clobbering bug this would have introduced

The three upload steps were gated only on env.NORFW || env.NANDFW. Adding a pull_request trigger without touching them would have had every PR build overwrite the published nightly and latest assets that users flash. They and the Telegram post are now guarded on the event.

Smoke tier

master.yml gets a 15-device smoke set rather than all 107 — it decides how a build runs, not what it produces. --self-test enforces the cover (every vendor, architecture, toolchain tuple and variant, plus both shared device directories) rather than the list.

It earned itself immediately: the first run rejected my hand-picked set for missing arm-openipc-linux-gnueabi — which exactly one target in the tree has (hi3536dv100_fpv) — and musleabihf.

Also

CI Gate umbrella check. A dynamic matrix cannot be named in branch protection, and nothing reported a verdict on a change before because nothing built one.
Concurrency group so a second push to a PR cancels the first. It has already proved itself on this PR: pushing the derived-list commit cancelled the in-flight 107-device run instead of leaving both to finish.

Numbers

Replaying the last 200 commits:

	device-builds
if every PR built everything	20 330
with the selector	5 930 (70.8% less)

Zero coverage leaks. 43 of the 49 that still widen do so on paths that no longer exist — devices/ssc338q_apfpv/ before it became devices/apfpv/, the old top-level common/, a deleted builder-local.sh. Restricted to commits whose paths survive: 78.2%.

Note on this PR's own run

It touches .github/scripts/ci-matrix.py, which is deliberately excluded from the smoke tier — it decides the matrix, so it cannot be trusted to decide a smaller one for itself. So it runs all 107, which is also the regression test you want for turning PR CI on. Adding a device will not do this.

🤖 Generated with Claude Code

This repo had no pull_request trigger at all. A change landed unbuilt, and the nightly found out the next morning -- or a user did. Turning PR CI on meant 107 device builds per push, which is why it stayed off. Almost every change here is one device: 639 of the file touches in the last 200 commits sit under a single devices/<dir>/, and the median commit reaches 2 devices. So narrowing first is what makes PR CI affordable at all. .github/scripts/ci-matrix.py maps the paths a PR touches to the devices that build them, and master.yml takes its matrix from it. The mapping is not written down, it is read off the tree the same way builder.sh reads it. builder.sh line 121 locates a device by its defconfig and copies that WHOLE devices/<dir>/ tree over the firmware clone, so the devices a file affects are exactly the devices whose defconfig shares its directory. That distinction is load-bearing: devices/common/ backs 18 targets and devices/apfpv/ backs 2, and treating a directory as one device would skip 17 real builds. A defconfig still names one target even inside a shared directory. THE NIGHTLY IS NOT NARROWED. builder.sh re-clones OpenIPC/firmware at HEAD on every run, so what a nightly builds is decided mostly outside this repo; master.yml already carries the scar of an earlier gate that skipped on this repo's HEAD and left upstream fixes invisible to users. Every event except pull_request gets the whole matrix, and --self-test asserts that for schedule, workflow_dispatch and push. Nothing on a pull_request may write a release. The three upload steps were gated only on an image existing, so with a pull_request trigger added they would have clobbered the published nightly and latest assets that users flash. They and the Telegram post are now guarded on the event. master.yml and check-adjacent plumbing get a 15-device smoke set rather than all 107: they decide how a build runs, not what it produces. --self-test enforces the cover -- every vendor, architecture, toolchain tuple and variant, plus both shared device directories -- rather than the list, so the set can be re-picked while the property holds. It already earned itself: the first run rejected my hand-picked set for missing arm-openipc-linux-gnueabi, which exactly one target in the tree has, and musleabihf. Also adds the CI Gate umbrella check, since a dynamic matrix cannot be named in branch protection and nothing reported a verdict before; and freezes the 7 defconfigs that exist but are in no matrix, so a device falling out of CI is a test failure rather than a silent skip. Replaying the last 200 commits: 20330 device-builds if every PR built everything, 5930 with the selector, 70.8% less, no coverage leaks. 43 of the 49 that still widen do so on paths that no longer exist (devices/ssc338q_apfpv/ before it became devices/apfpv/, the old top-level common/, a deleted builder-local.sh); restricted to commits whose paths survive, it is 78.2%. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

qodo-free-for-open-source-projects · 2026-08-16T19:44:32Z

PR Summary by Qodo

ci: enable PR builds with path-based device matrix narrowing

✨ Enhancement ⚙️ Configuration changes 🕐 40+ Minutes

AI Description

• Add pull_request CI that builds only device targets affected by the PR’s changed paths.
• Introduce a self-tested selector script with a smoke tier for CI-only workflow changes.
• Prevent PR runs from publishing/overwriting nightly/latest release assets; add an umbrella CI Gate
 check.

Diagram

graph TD
  pr{{"GitHub event"}} --> pre["Preflight job"] --> build["Firmware build job"] --> gate["CI Gate"]
  pr --> sel["Select devices job"] --> selector["ci-matrix.py selector"] --> matrix["Matrix JSON"] --> build
  build -->|"non-PR only"| publish["Release + Telegram"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Use git diff (base..head) instead of PR files API

➕ Avoids GitHub API pagination/limits and token/auth failure modes
➕ Works offline for local/test runs and reduces network coupling
➖ Requires reliable base SHA in all event contexts; can be tricky with merge commits
➖ Still needs rename-handling parity and careful path normalization

2. Use actions-based path filtering (e.g., dorny/paths-filter) + mapping file

➕ Less custom code to maintain; common, well-understood GitHub Actions pattern
➕ Can keep mapping declarative for reviewers
➖ Hard to express the “defconfig directory implies affected targets” rule without substantial mapping complexity
➖ Still needs safety-first widening semantics and shared-directory handling (common/apfpv)

3. Per-device reusable workflow with `paths:` triggers

➕ Can reduce selector complexity by letting GitHub handle dispatching
➕ Clear ownership boundaries per device/workflow
➖ Doesn’t work well with required checks (filtered workflows never run → pending forever)
➖ Explodes workflow count and maintenance overhead; still needs a stable umbrella check

Recommendation: The PR’s approach (central selector job producing a dynamic matrix, with conservative widening and a self-test) is the best fit for correctness and branch-protection friendliness. If future API brittleness becomes a problem (auth outages or file-list limits), consider switching the changed-files source to git diff while keeping the same classification rules and self-test guarantees.

Files changed (2) +671 / -133

Other (2) +671 / -133

ci-matrix.pyAdd self-tested PR-to-device matrix selector +564/-0

Add self-tested PR-to-device matrix selector

• Introduces a Python selector that maps changed file paths to impacted device targets by reading defconfig locations from the repo tree (mirroring builder.sh directory-copy semantics). Provides a smoke target set for CI-plumbing-only changes, a ci:full label override, draft-PR skipping, and a comprehensive --self-test to prevent silent coverage regressions.

.github/scripts/ci-matrix.py

master.ymlEnable PR CI with dynamic matrix, safety gates, and umbrella status +107/-133

Enable PR CI with dynamic matrix, safety gates, and umbrella status

• Adds pull_request triggering, PR-only concurrency cancellation, and a new select job that runs ci-matrix.py to compute the build matrix. Replaces the static 107-entry matrix with the selector output, disables release/Telegram publishing on PR events to prevent clobbering nightly/latest assets, and adds a CI Gate job to provide a single branch-protection-friendly verdict.

.github/workflows/master.yml

qodo-free-for-open-source-projects · 2026-08-16T19:46:55Z

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~Output contract mismatch~~ ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

ci-matrix.py claims it “write[s] $GITHUB_OUTPUT” in normal mode, but it actually prints key=value
lines to stdout and relies on the caller to redirect into $GITHUB_OUTPUT. This mismatch can easily
lead to a future workflow invoking the script without redirection and silently not setting step
outputs.

Code

.github/scripts/ci-matrix.py[R36-38]

+Usage:
+    ci-matrix.py               # read GitHub Actions env, write $GITHUB_OUTPUT
+    ci-matrix.py --stdin       # read a file list on stdin, print the decision

Evidence
The docstring promises direct $GITHUB_OUTPUT writing, but the code prints outputs to stdout and
the workflow explicitly redirects stdout into ${GITHUB_OUTPUT}, proving the contract mismatch.
.github/scripts/ci-matrix.py[36-39]
.github/scripts/ci-matrix.py[551-560]
.github/workflows/master.yml[48-56]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`.github/scripts/ci-matrix.py` documents that its default mode writes directly to `$GITHUB_OUTPUT`, but the implementation prints outputs to stdout and depends on the workflow step to redirect. This is a footgun for future reuse.
## Issue Context
- The script’s docstring says default usage writes `$GITHUB_OUTPUT`.
- The implementation prints `matrix=...`, `needs-build=...`, `reason=...` to stdout.
- The workflow currently compensates by redirecting stdout into `${GITHUB_OUTPUT}`.
## Fix Focus Areas
Choose one:
1) Update the docstring to accurately state: “prints GitHub output-format lines to stdout; redirect to `$GITHUB_OUTPUT` in Actions”.
2) Or make the script honor `$GITHUB_OUTPUT` when set (append key/value records to that file), and optionally keep stdout printing for `--stdin`.
- .github/scripts/ci-matrix.py[36-40]
- .github/scripts/ci-matrix.py[551-560]
- .github/workflows/master.yml[48-56]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Tip of the day

💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Adding a camera IS the work in this repo: 8 of the 10 PRs open right now add a device, and 14% of recent commits do. With the list written down in ci-matrix.py, every one of those PRs also had to edit that file -- and a change to that file cannot narrow, because it is what does the narrowing. So registering one camera cost a full 107-device build to prove one device. The most common change got the worst outcome. The matrix is now every defconfig under devices/, minus a NOT_BUILT opt-out that keeps the 7 already in the tree but out of CI. Adding a device is creating its directory, nothing else, and it builds exactly that device. This is the opposite call to OpenIPC/firmware's ci-matrix.py, on purpose. There the matrix is curated -- 29 defconfigs are deliberately never built and boards are not the product -- so an explicit list is the honest description and an opt-out list would be noise. Here devices are the product and the list is just "all of them". The derived matrix is identical to the 107 master.yml carried: nothing lost, nothing gained. --self-test now checks the opt-outs still name real defconfigs, so a rename cannot leave a device silently building under its new name while its old name sits in NOT_BUILT. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The docstring said the default mode writes $GITHUB_OUTPUT; it printed key=value lines to stdout and left the workflow to redirect them there. Raised in review on #120. Fixing the prose would have made the file honest, but the redirect itself is the weaker half. Under `>> ${GITHUB_OUTPUT}` every print() in this file is one keystroke away from corrupting the step outputs, and a crash between the first line and the last leaves a half-written file that Actions still reads. That the diagnostics go to stderr is what keeps it safe today -- a rule that has to hold forever, rather than a property of the code. So the script appends the three lines itself when $GITHUB_OUTPUT is set, and prints them when it is not, so a local run still shows its decision. --stdin never touches the file: it is the mode for reading with your eyes. master.yml drops the redirect, though leaving one in place would have been harmless -- with the file written directly, stdout is empty. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Step 6 of "Adding a new device" still says to add `- <device>` to the matrix in .github/workflows/master.yml, and calls that matrix "the *only* build registry". Neither has been true since #120 moved the matrix into .github/scripts/ci-matrix.py, where it is derived from the tree rather than listed. master.yml now has no device list at all. This is not a hypothetical staleness. Two of the pull requests open right now do exactly what step 6 says: #111 (Tapo TC70 v3) and #108 (KC110) each append a line to a block that no longer exists, so both are merge conflicts rather than the one-line changes they were written as. Their authors did the documented thing. So step 6 is now the opposite instruction -- there is nothing to add, and adding it is the mistake. It also names NOT_BUILT, since that is the one list a contributor could reasonably go looking for, and says to leave it alone. Two adjacent claims went stale with the same change and are fixed here: the master.yml bullet described a cron-and-dispatch workflow with no pull_request trigger, and nothing in the file mentioned ci-matrix.py at all -- including that --self-test runs in the Select devices job and will fail a PR that puts the tree and the selector out of step. Step 8 no longer says "locally" as though that were the only way a device gets built.

) Step 6 of "Adding a new device" still said to add `- <device>` to the matrix in .github/workflows/master.yml, and called that matrix "the *only* build registry". Neither has been true since #120 moved the matrix into .github/scripts/ci-matrix.py, where it is derived from the tree rather than listed. master.yml has no device list at all. Not a hypothetical staleness: #111 (Tapo TC70 v3) and #108 (KC110) each append a line to a block that no longer exists, so both are merge conflicts rather than the one-line changes they were written as. Their authors did the documented thing. So step 6 is now the opposite instruction -- there is nothing to add, and adding it is the mistake. It also names NOT_BUILT, since that is the one list a contributor could reasonably go looking for, and says to leave it alone. Two adjacent claims went stale with the same change: the master.yml bullet described a cron-and-dispatch workflow with no pull_request trigger, and nothing mentioned ci-matrix.py at all -- including that --self-test runs in the Select devices job and will fail a PR that puts the tree and the selector out of step. A second correction, caught by Qodo in review: "opening the PR is what gets the device built" holds only for a non-draft PR. classify() returns needs_build=False for a draft and self_test() asserts it; master.yml lists ready_for_review in its triggers precisely because a PR opened as a draft would otherwise never build once marked ready. Left as written, a contributor could open a draft, see no build, and read that as the device being fine. The ci:full label is documented for the same reason -- it is the other contributor-facing lever over the matrix.

Both devices have had a complete devices/<dir>/ in this tree for a long time and had never once been built. #120 froze that state rather than changing it -- it put the 7 defconfigs that were in no master.yml matrix into NOT_BUILT so that a device falling out of CI would be a test failure instead of a silent skip. Freezing was right for a change whose job was to not alter what gets built; it was never meant to be permanent for devices whose only problem is that nobody added the matrix line. Two contributors did try to add that line. #111 (Tapo TC70 v3, July) and #108 (KC110, June) both append to the device list in master.yml, which is where the registry lived when they were written. It does not live there any more -- the matrix is read off the tree -- so both PRs are merge conflicts now, and the devices they asked for are exactly the two entries removed here. This is their CI half, landed the way the selector wants it. #108 also carries a BR2_OPENIPC_MAJESTIC="ultimate" line and a vendor libsns_ov2735.so for the no-video bug; those are unaffected and stay with that PR. The two self-test cases that used t31_lite_tp-link-tapo-tc70-v3 as their example of an unbuilt device move to t31_lite_xiaomi-mjsxj05hl, which is still in NOT_BUILT. The overlay case also switches to a path that exists, since the device it now names has no etc/inittab. Neither device had ever been through a build, so this PR was where we found out. That was affordable exactly once: a change to ci-matrix.py cannot narrow -- it is what does the narrowing -- so this ran the full matrix and proved both in the same run. Both are green: Firmware (hi3518ev200_ultimate_tplink-kasa-kc110) SUCCESS Firmware (t31_lite_tp-link-tapo-tc70-v3) SUCCESS

widgetii requested review from cronyx, flyrouter and viktorxda as code owners August 16, 2026 19:43

qodo-free-for-open-source-projects Bot reviewed Aug 16, 2026

View reviewed changes

Comment thread .github/scripts/ci-matrix.py

widgetii and others added 2 commits August 16, 2026 22:48

widgetii merged commit edc0a0f into master Aug 17, 2026
110 checks passed

widgetii deleted the ci-narrow-matrix branch August 17, 2026 04:05

openipc-ai mentioned this pull request Sep 11, 2026

Add moes wcm-p52 #80

Closed

Navigation Menu

Uh oh!

ci: build pull requests, and only the devices they reach - #120

ci: build pull requests, and only the devices they reach#120
widgetii merged 3 commits into
masterfrom
ci-narrow-matrix

widgetii commented Aug 16, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 16, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 16, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

widgetii commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The mapping is read off the tree, the way builder.sh reads it

The device list is derived, not written down

The nightly is NOT narrowed

A release-clobbering bug this would have introduced

Smoke tier

Also

Numbers

Note on this PR's own run

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 16, 2026

PR Summary by Qodo

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

widgetii commented Aug 16, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Aug 16, 2026 •

edited

Loading