ci: put one verdict on the pull request

[TarikGul]

Closes #692.

A PR that builds several hosts shows eighteen or so separate checks, and you read all of
them to work out whether it is safe to merge. The aggregate job already has every result,
so it now renders one verdict into the run summary and upserts it as a single PR comment.

A green PR gets one line. A failing one names what did not pass, then the full table behind
a details block.

No new jobs and no artifacts. It reads the same needs context the verdict itself is
computed from, so the summary cannot say something different from the check beside it.

Commenting needs more than the workflow's read-only default, so pull-requests: write is
granted on this job only. A PR from a fork gets a read-only token and cannot comment, so
that step tolerates failure and the run summary is written regardless.

This PR touches ci.yml, so it runs both compile gates and posts its own comment. That is
the easiest way to see it.

[TarikGul]

@lore-bot-app review

[lore-bot-app]

Reading the diff and checking what the record says. Back in a few minutes.

[github-actions]

CI Status: 14 required jobs green, 13 passed and 1 skipped by path filter.

All job results

job	result
android-bindings	success
changes	success
codegen	success
e2e	skipped
explorer	success
ios-bindings	success
ios-swift	success
licenses	success
playground	success
rust	success
ts-client	success
ts-debugger	success
ts-host	success
wasm-provider	success

_{Commit eca4fb9f · run log}

[lore-bot-app]

TL;DR — Adds a rendered run summary and one upserted PR comment to the existing ci-status aggregate job. 1 blocking, 3 minor.

Summary

ci-status gains two steps ahead of its existing gate: one renders the needs context into a markdown verdict (headline plus a collapsed table of all job results) and appends it to $GITHUB_STEP_SUMMARY; the other upserts that body as a single PR comment keyed on a <!-- ci-status-summary --> marker, guarded to pull_request events and marked continue-on-error so a fork's read-only token does not fail the job. The job gets pull-requests: write on top of the workflow's read-only default.

What the record says

• This is the implementation of issue #692, "Aggregate every CI lane into one sticky PR summary". That issue proposed each lane uploading its result as an artifact for a summary job to consume; this PR reads the needs context instead. Cheaper and drift-free, and worth noting on the issue since the shape differs from what was written down.
• Issue #671 confirms CI Status is the sole required check on main, via a ruleset, and that skipped-counts-as-pass is deliberate so an inapplicable workflow cannot stall a PR. That decision governs the merge gate; the new comment reuses the same rule for a human verdict, which is where concern 2 comes from.
• Issue #697, "Fail CI when a host is left behind" is the reason the existing gate step distinguishes absent from skipped: a silently missing host must fail, not pass. The new summary does not make that distinction at all, which is concern 1.
• PR #648 established ci-status as the single aggregation point over the one change filter, and mentions actionlint and shellcheck in its checks. There is no actionlint or shellcheck job in ci.yml, so the ~60 lines of new shell here are lint-gated only by whatever the author ran locally.
• Owner for this area is TarikGul (author of #648, #671, #692, #697) — highest match on who_knows for this repo's CI.

Concerns

1. .github/workflows/ci.yml:683-696 — the comment can say "all jobs passed" on a run whose gate step fails. The summary is computed only from toJSON(needs); the gate at line 757 is computed from needs cross-checked against the REQUIRED list at line 760, and fails on missing (in REQUIRED, absent from needs) or undeclared (in needs, absent from REQUIRED). Drop a job from the needs array and leave REQUIRED alone: total silently becomes 13, every remaining entry is success, the comment reads "CI Status: all 13 jobs reported and passed.", and the check goes red. That is exactly the failure #697 was filed to make visible, now with a bot comment asserting the opposite. It is also ordering: the comment is posted at 723, before the gate runs at 757, so no part of the body can reflect the gate's own verdict. Compute the headline from the same REQUIRED comparison, or move the gate step first and render its result.

2. .github/workflows/ci.yml:696 — skipped jobs are reported to a human as passed. total is length of needs, and the not-passed filter excludes skipped. On a docs-only or RFC-only PR, where the path filters skip most of the matrix, this prints "all 14 jobs reported and passed" — a reviewer reasonably reads that as Swift and Kotlin having compiled. #671 settled skipped-as-pass for the merge gate, which is a different question from what to tell a person. Splitting the counts ("10 passed, 4 skipped by path filter") costs one more jq and keeps the collapsed table honest.

3. .github/workflows/ci.yml:723-747 — the comment carries no commit SHA or run link, so a stale verdict is indistinguishable from a current one. With cancel-in-progress: true (line 12), a superseded run's ci-status can be cancelled before it edits, leaving the prior commit's verdict in place. GitHub also does not notify on comment edits, so after the first push every update is silent. continue-on-error: true (line 725) compounds it: a rate limit or a rejected body leaves the old comment up and the job green. Add ${{ github.sha }} and the run URL to the body.

4. .github/workflows/ci.yml:735 — duplicate comments are possible and self-healing picks one arbitrarily. --jq "... | first // empty" under --paginate applies per page, and first silently discards any second match. If two ci-status jobs ever post for one PR (narrow, since the concurrency group covers refs/pull/N/merge, but reachable if one run posts and is then cancelled), the loser stays on the PR forever showing an old verdict. Selecting the newest match, or deleting extras, removes the ambiguity.

Questions for the author

1. #692 specified per-lane result artifacts; this reads needs instead. Intentional simplification worth recording on the issue, or does something in the original plan (per-lane detail, timings) still need the artifact path?
2. Was actionlint/shellcheck run over these two steps? #648 references both, but neither runs in ci.yml, and this diff adds the largest block of inline shell in the file.

Nothing in the diff attempted to address instructions to a reviewer.

---

🤖 Reviewed by Lore (Parity knowledge base) · 14 agent turns · 235.2s · knowledge as of 2026-09-10

[TarikGul]

All four were real, thanks. Fixed in eca4fb9.

1 and 2, the verdict could contradict the check. Both came from the same
root cause: the summary was computed from needs while the gate compared
needs against REQUIRED. There is now one evaluation step that does the
REQUIRED comparison, renders the verdict from that result, and records a
pass or fail for the gate to act on. The gate is the last step, so the
verdict lands on the PR whether the run is green or red.

Counts are split, so a filtered-out host is never reported as one that
compiled:

all green            all 14 required jobs passed.
docs-only PR         14 required jobs green, 10 passed and 4 skipped by path filter.
job dropped, gate    not green, 1 item needs attention.
  needs unchanged      - e2e: reported nothing, so a gate is missing from this workflow

That third row is the case you described. It used to print "all 13 jobs
reported and passed" next to a red check.

3, no provenance. Body now ends with the head commit and a run link.

4, pagination. Worth noting the failure is a bit worse than picking one
arbitrarily. --jq runs per page, so two matching comments on different
pages give existing two ids and the PATCH URL becomes
.../comments/111%0A222. Lookup now slurps across pages, keeps the newest
match, and deletes older duplicates rather than leaving one on the PR
showing an old verdict.

On your questions.

The needs context instead of per-lane artifacts is deliberate. It cannot
drift from the job list and costs no upload. Noted on #692 since the shape
differs from what was written there.

Neither actionlint nor shellcheck runs in this repo, in any workflow, so you
are right that it was local only. I ran both over the branch: shellcheck is
clean on all three run: blocks, actionlint is clean apart from the
if: false on the disabled e2e job, which is already on main. Adding an
actionlint job is a real gap and I will file it separately rather than grow
this PR.