Skip to content

ci: carry the publish hardening back from builder - #2279

Merged
widgetii merged 2 commits into
masterfrom
ci-publish-hardening
Aug 17, 2026
Merged

ci: carry the publish hardening back from builder#2279
widgetii merged 2 commits into
masterfrom
ci-publish-hardening

Conversation

@widgetii

Copy link
Copy Markdown
Member

Review on OpenIPC/builder#121 found two holes in its publish job. That job was seeded from this one — and both holes are still here, where they matter more: this nightly is the one sysupgrade users flash from daily.

1. A failed artifact download published nothing and stayed green

The download runs with continue-on-error: true, so a transient failure produced an empty dist/, the count guard skipped publishing, the job succeeded, and CI Gate accepts publish=success.

The flag stays — it is load-bearing for exactly one case: a matrix where every board failed uploads no fw-* artifact at all, and a hard download failure there would mask the real reason. Collect assets now turns it back into a failure everywhere else:

matrix download assets outcome
success failed error (was: green)
success ok 0 error (was: green)
failure 0 tolerated; gate fails on the matrix result
success ok >0 publishes

2. nightly/latest could be force-moved to a build with no firmware behind them

The tag moves were unconditional once any asset existed — and dist/ can hold sidecars and no images (one board producing a sizes.*.json and no .tgz is enough). Both tags now move only when IMAGES is non-empty; otherwise they stay on yesterday's build, with a warning.

Also normalised the count through tr. On the runner's GNU coreutils wc does not pad — that's BSD behaviour, so this wasn't exploitable here — but the guard shouldn't depend on which wc is in front of it.

3. ci-matrix.py owns $GITHUB_OUTPUT

Same docstring inaccuracy flagged on builder: it claimed to write $GITHUB_OUTPUT while printing to stdout and relying on the workflow to redirect. Under a redirect every print() is one keystroke away from corrupting the step outputs, and a mid-run crash leaves a half-written file Actions still reads. The script now appends to $GITHUB_OUTPUT itself when set, prints when not, and --stdin never touches the file.

Testing

None of the publish path can run on a pull request, so: all four collect states and all four output paths verified offline (the two previously-green failure states now fail), YAML parses, --self-test green. This PR touches ci-matrix.py, so it correctly runs the full 96-board matrix — and tonight's 22:30 UTC nightly is the first live pass over the new publish logic if this merges before then.

🤖 Generated with Claude Code

Review on OpenIPC/builder#121 found two holes in its publish job. That
job was seeded from this one, and both holes are still here.

The artifact download runs with continue-on-error, so a transient or
real download failure produced an empty dist/, the count guard skipped
the publish step, the job went green, and ci-gate accepts
publish=success -- a nightly could silently write nothing while the run
stayed green. The flag stays, because it is load-bearing for exactly one
case: a matrix where every board failed uploads no fw-* artifact at all,
and a hard download failure there would mask the real reason in the
gate. Collect now turns it back into a failure for every other case: a
green matrix with a download that did not succeed is an error, and a
green matrix with zero collected assets is an error. Only "matrix failed
and produced nothing" is still tolerated, and the gate fails that run on
the matrix result anyway.

The nightly and latest tag moves were unconditional once any asset
existed. dist/ can hold sidecars and no images -- one board producing a
size report and no .tgz is enough -- and the script would force-move the
two tags flashers pull from to a build with no firmware behind them.
Both now move only when there is at least one image to move them to;
otherwise they stay on yesterday's build, with a warning.

The count is also normalised through tr. On the runner's GNU coreutils
wc does not pad, so this was not exploitable here -- padding is BSD
behaviour -- but the guard should not depend on which wc is in front of
it.

While in the area, ci-matrix.py now writes $GITHUB_OUTPUT itself instead
of documenting that it does while relying on the workflow to redirect
stdout (the docstring inaccuracy was also flagged on builder). Under a
redirect every print() 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. Falls back to stdout when
the variable is unset; --stdin never touches the file.

All four collect states and all output paths checked offline; none of
the publish path can run on a pull request.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

CI: Harden nightly publish gating and ci-matrix step outputs

🐞 Bug fix ⚙️ Configuration changes 🕐 20-40 Minutes

Grey Divider

AI Description

• Fail publish when artifact download fails after a fully successful build matrix.
• Move nightly/latest tags only when firmware images exist; otherwise keep prior tags.
• Write ci-matrix outputs directly to $GITHUB_OUTPUT to avoid redirect corruption.
Diagram

graph TD
  A["Select boards"] --> B["Firmware matrix"] --> C["Download artifacts"] --> D["Collect & validate"] --> E["Publish dated release"] --> F{{"Images present?"}}
  F -- "yes" --> G["Move nightly/latest"] --> R[("GitHub Releases")]
  F -- "no" --> H["Keep tags (warn)"] --> R
  E --> R

  subgraph Legend
    direction LR
    _job["Job/step"] ~~~ _dec{"Decision"} ~~~ _rel[("Release store")]
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Query artifact existence via GitHub API before download
  • ➕ Distinguishes “no artifacts exist” vs “download failed” without relying on download step outcome
  • ➕ Could validate expected artifact set (or expected boards) more explicitly
  • ➖ More API calls/permissions and extra scripting complexity
  • ➖ Still needs careful handling when the matrix partially fails but artifacts exist
2. Split publish into two paths: partial-publish vs full-success
  • ➕ Makes success criteria explicit (full matrix success vs partial)
  • ➕ Can tailor validation and tag-move rules per path
  • ➖ More workflow branching and duplicated logic
  • ➖ Higher maintenance burden for a safety hardening change

Recommendation: The PR’s approach is the right trade-off: keep continue-on-error to preserve signal for the “all boards failed, no artifacts exist” case, then explicitly fail the publish job only when a successful matrix should have produced artifacts but download/collection didn’t. Guarding nightly/latest on IMAGES[] (not just any sidecar file) is the simplest safe rule for tag integrity.

Files changed (2) +68 / -16

Bug fix (1) +20 / -4
ci-matrix.pyWrite step outputs directly to $GITHUB_OUTPUT (no stdout redirect) +20/-4

Write step outputs directly to $GITHUB_OUTPUT (no stdout redirect)

• Updates the usage docstring and changes output emission to append directly to $GITHUB_OUTPUT when set (and when not using --stdin). Falls back to stdout for local runs, reducing the risk of accidental stdout corruption or partially-written output files under workflow redirection.

.github/scripts/ci-matrix.py

Other (1) +48 / -12
build.ymlHarden publish job: validate downloads and guard nightly/latest tag moves +48/-12

Harden publish job: validate downloads and guard nightly/latest tag moves

• Removes stdout redirection when running ci-matrix.py, relying on the script to write $GITHUB_OUTPUT safely. In the publish job, records the download step outcome and adds validation so a fully successful matrix cannot silently publish nothing (download failure or zero collected assets becomes a hard error). Normalizes asset counts via tr, and only moves nightly/latest tags when at least one firmware image (.tgz) exists; otherwise warns and leaves tags unchanged.

.github/workflows/build.yml

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider

Great, no issues found!

Qodo reviewed your code and found no material issues that require review

Grey Divider

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 ↗

Grey Divider

Qodo Logo

@widgetii
widgetii enabled auto-merge (squash) August 17, 2026 08:08
@widgetii
widgetii merged commit 3f6fd75 into master Aug 17, 2026
104 checks passed
@widgetii
widgetii deleted the ci-publish-hardening branch August 17, 2026 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant