fix: ci: make the CI Summary gate fail when a job it needs did not succeed

[srpatcha]

Summary

CI Summary (the ci-summary job in .github/workflows/ci.yml) is an aggregate gate that cannot fail. It declares needs: [lint, test, coverage, validate-platforms, simulator-smoke, docs, security, build] and if: always(), and its only step echoes ${{ needs.*.result }} into $GITHUB_STEP_SUMMARY. Nothing has ever evaluated those results, so the job reports success no matter what happened above it.

This adds a Gate step that fails when any needed job did not succeed. The existing Summary step is unchanged.

The finding this fixes

On all five open Dependabot PRs (#21, #22, #23, #24, #25), lint failed, six downstream jobs were skipped behind needs: lint, and CI Summary reported pass — in 3-5 seconds. Since gh api repos/embeddedos-org/EoSim/branches/master/protection returns required_status_checks: null, and all five PRs report mergeStateStatus: UNSTABLE, reviewDecision: APPROVED, mergeable: MERGEABLE, the only aggregate signal on those PRs is a green tick that means nothing.

Surfaced by the scheduled architecture review of EoSim#21 (and repeated on #22-#25). It is a fifth instance of the §28.2 "Vacuous evidence" proposal of 2026-09-02, whose fourth bullet reads: "An aggregating gate job must fail on any non-success among its dependencies, and must not print a summary asserting more than it checked."

Why skipped is treated as a failure

Because in this workflow it can only mean an upstream job failed or was cancelled. I verified that none of the eight needed jobs carries a job-level if: — parsed .github/workflows/ci.yml and checked each one — so there is no path by which a needed job is legitimately skipped. If a conditional job is added later, this step needs revisiting; the inline comment says so.

Results are passed through env: rather than interpolated directly into run:, which is the pattern that produced VULN-1652 in codecov/codecov-action. needs.*.result is a fixed enum and not attacker-controlled, so this is hygiene rather than a live issue.

What I ran to verify this

Extracted the Gate step's run: block from the modified YAML and executed it against four result sets, including the real one from PR #21:

# real state on EoSim#21
RESULTS="lint=failure test=skipped coverage=skipped validate-platforms=skipped
          simulator-smoke=skipped docs=skipped security=success build=skipped"
-> ::error:: for lint(failure) and six skipped jobs; exit 1     # was: pass

all success                       -> "All jobs succeeded."      exit 0
lint ok, test=failure, build=skipped -> ::error:: x2            exit 1
lint ok, test=cancelled           -> ::error:: x1               exit 1

Also verified with a YAML parse that the file still loads, that ci-summary still declares all eight dependencies, that the folded RESULTS scalar renders as one space-separated line of job=result tokens, and that the pre-existing Summary step is untouched.

What I did not verify

• The workflow was not run. No GitHub Actions execution took place. actionlint is not available on this host; verification is a Python YAML parse plus local execution of the extracted shell block, not a real workflow run.
• I did not verify the step's behaviour under a job-level cancelled of ci-summary itself, or under a re-run of failed jobs only.

Expect this PR's own CI Summary to be red — that is the fix working

master is currently red on lint, from pre-existing ruff violations in eosim/ unrelated to this change (I001, F401, UP031, N806 across eosim/analysis, eosim/api, eosim/artifacts, eosim/cli, eosim/plugins, eosim/tests). Before this change that produced a green CI Summary; after it, CI Summary will correctly go red. Nothing here breaks the pipeline — it stops the pipeline from lying about being green.

Suggested order of operations

1. Land a ruff check --fix / ruff format pass on master so lint goes green and the rest of the pipeline can run at all.
2. Land this.
3. Only then add CI Summary to branch protection as a required check. Requiring it before step 2 would block everything; requiring it before this PR would require a check that cannot fail.

Separately, and not addressed here: .github/workflows/ci.yml:226 runs pip-audit --strict 2>/dev/null || true, so the Security Scan check also reports success unconditionally, and safety is installed at :224 and never invoked. Same class of defect, left for a focused change.

---

Opened by the scheduled EmbeddedOS architecture review. Advisory automation: it never approves, requests changes, merges or force-pushes. Push back here if any of this is wrong — a wrong fix is a bug worth reporting.

---

Opened by the scheduled autoreview pipeline (model claude-opus-5), branched from origin/master. No human has reviewed this yet. Close it freely if the fix is wrong - a bad automated PR is a bug worth reporting.

Fixes #30

[srpatcha]

Review — EoSim#26 "fix: ci: make the CI Summary gate fail when a job it needs did not succeed"

head: 78234b7 author: srpatcha ci: fail (CI Summary, Lint & Format, Quick Checks)

Role boundary first. This branch is autofix/ci-summary-gate-cannot-fail, opened by the
same automated review process writing this report. .ai/reviewer.md is explicit: "If you
implemented it, you do not approve it." This review therefore carries no merge verdict —
post-review.sh cannot approve or request changes — and the findings below should be read
as a checklist for a human maintainer, not as clearance. Discount them accordingly.

Verdict: The gate is correct and the workflow is fully covered, and its own red
CI Summary is the proof. Two things to settle before merge: the job list is duplicated by
hand and can drift, and merging this reddens every EoSim PR on day one because lint is
already broken on master for unrelated reasons.

Coverage is exact. ci-summary at ci.yml:266 declares
needs: [lint, test, coverage, validate-platforms, simulator-smoke, docs, security, build],
and the RESULTS block names those same eight, no more and no fewer. None of the eight
carries an if:, so the comment's claim that skipped can only follow an upstream failure
or cancellation is correct as the workflow stands. Passing results through env: rather
than interpolating into run: is the right construction. The Gate step is appended after
the Summary step, so the results table is still written to $GITHUB_STEP_SUMMARY before
the job exits non-zero.

The defect being fixed is real and was live on this very run: Lint & Format failed, six
jobs skipped, and CI Summary would have reported success. It now reports
job 'lint' concluded 'failure' and exits 1.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	.github/workflows/ci.yml:292–300 (added)	The gate's job list is a second, hand-maintained copy of needs:. Today the two match exactly; nothing keeps them matching. A job added to needs: and forgotten in RESULTS is silently ungated — the same class of defect this PR exists to fix, reintroduced one edit later. The loop has a second face of this: if RESULTS were ever empty the body never runs, failed stays 0, and the step prints "All jobs succeeded" having checked nothing.	Derive the list instead of restating it: env: NEEDS: ${{ toJSON(needs) }}, then jq -r 'to_entries[]' over it, failing on any .value.result != "success", and failing first if to_entries | length is 0. One source of truth, and the vacuous-pass case is closed at the same time.
2	Medium	— (sequencing)	Merging this turns CI Summary red on every open EoSim PR immediately. ruff check eosim/ fails on master for reasons no PR introduced — UP031 in eosim/integrations/ecosystem.py:675, eos_runner.py:35,42, openfoam.py:95, eosim/tests/runner.py:45,50, eosim/tests/scenarios.py:43,54,58; I001 in eosim/integrations/ns3.py:4 and eosim/plugins/loader.py:4; UP015 in eosim/integrations/verilator.py:20 (job 100924878779). No open EoSim PR fixes this — checked #15, #16, #21–#25. That is the gate working as designed, not a defect in it, but shipping it alone converts a false green into a blanket red that no contributor can clear.	Land the ruff fix first, or land both together. Note that most of these are UP031 percent-format rewrites, which ruff classifies as an unsafe autofix in eosim/ source — that is a reviewed change to production code, not a mechanical one, so it wants its own PR and its own test run rather than being folded in here.
3	Low	.github/workflows/ci.yml:285 (added comment)	The comment says the summary step "only prints" and that nothing "ever looked at needs.*.result". The first half is right; the second is not quite — the Summary step interpolates needs.<job>.result into the markdown table at ci.yml:275–282. It reads them and acts on none of them, which is the actual and more pointed bug: the job knew every result and reported success anyway.	Reword to "printed every needs.*.result into the summary table and branched on none of them".

Architecture conformance

Conforms, and closes a gap the design already records.

• §21. EoSim is Tier 1 — Foundation. A CI-only change inside .github/workflows/;
  no source, no dependency, no import or manifest entry, so §5.1 is not engaged.
• §17. EoSim is the simulation and CI adoption primitive — "CI tests sharing the same
  application artifacts used on real hardware." A summary check that reports green over a
  red lint and six skipped jobs makes that CI contract unfalsifiable.
• §28 (Status, Evidence and Claims Policy). The design defines what evidence each
  status requires and never requires the automation producing it to be capable of failing.
  A green CI Summary over eight unchecked results is precisely a claim without evidence.
• .ai/autoreview/proposals/2026-09.md, 2026-09-02, "The evidence policy is silent on
  checks that verify nothing", proposed §28.2 Vacuous evidence, whose fourth bullet reads:
  "An aggregating gate job must fail on any non-success among its dependencies, and must not
  print a summary asserting more than it checked." This PR is that bullet, implemented. Its
  trigger list already includes eBoot#81, whose sanity-gate had the identical bug — so
  this is the second instance of one pattern, which is an argument for the policy rather
  than for another proposal. No new proposal appended for this PR.

Its migration note also predicted this PR's finding 2 — "Expect an initial wave of newly-red
pipelines … which is the point of the change and should not be read as a regression caused
by it." That is the right frame for the EoSim lint breakage; the sequencing recommendation
stands regardless, because contributors should not be the ones absorbing the wave.

Proposed changes

1. Replace the hand-listed RESULTS with toJSON(needs) plus an empty-set guard
   (finding 1). Smallest form:

   - name: Gate
     env:
       NEEDS: ${{ toJSON(needs) }}
     run: |
       count=$(echo "$NEEDS" | jq 'to_entries | length')
       [ "$count" -gt 0 ] || { echo "::error::gate saw no needs"; exit 1; }
       bad=$(echo "$NEEDS" | jq -r 'to_entries[] | select(.value.result != "success") | "\(.key)=\(.value.result)"')
       [ -z "$bad" ] || { echo "$bad" | while read -r j; do echo "::error::job '${j%%=*}' concluded '${j#*=}'"; done; exit 1; }
       echo "All jobs succeeded."

   This needs jq, which is preinstalled on ubuntu-latest.
2. Fix the comment per finding 3.
3. Open the ruff cleanup as a separate PR and merge it first (finding 2).
4. Take this out of draft only once 3 is in flight, so the gate does not land on a red trunk.

Not checked

• The gate was not executed against a passing run. Every result observed here is a
  failing one, so the success path — all eight success, step exits 0 — is unproven. The
  logic is three lines and reads correctly, but that is inference, not a run.
• The EoSim clone is dirty (179 files) and was skipped by the sync step, so nothing was run
  locally; the workflow was read from origin/master plus this diff, which appends only.
• Behaviour under concurrency: cancel-in-progress: true (ci.yml:10–12) was not observed.
  A cancelled run's jobs conclude cancelled, the gate fails, and CI Summary shows red
  rather than cancelled. Superseded runs are replaced by the new run's checks for the same
  ref, so this should not strand a red check, but that was reasoned about, not seen.
• Whether CI Summary is a required check on EoSim's branch protection was not verified,
  and it decides how much finding 2 actually blocks.
• No fix PR was opened. Finding 2's remedy touches production string formatting across ten
  files in eosim/ and cannot be verified from here without a clean checkout — outside the
  "small and provable" limit the review brief sets for autofixes.

---

Automated architecture review of 78234b7f143b — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.