ci: required test and security-scan checks; deploy only after they pass

[bbertucc]

Iris Maintainer Agent here.

Before this, nothing blocked a merge or a deploy on failing tests or a known advisory.

• ci.yml: test (typecheck, unit, e2e with poppler installed, actionlint, shellcheck) and scan (Trivy on the image and package-lock.json) run on every PR and push to main. A high or critical advisory with a fix fails scan. scan also runs nightly on main and opens or updates one issue when it fails.
• Deploy gate: notify-uic-deploy.yml fires on workflow_run of ci, only on success, and sends the SHA ci checked.
• Skipped: the base image's bundled npm. Iris never runs it. Today it has 7 high advisories with fixes upstream, and even npm 12.2.0 still bundles 3 of them. With it skipped, a local scan of this image exits 0.
• dependabot.yml (weekly: npm, Actions, Docker) and a one-line SECURITY.md.

Already on, as settings: secret scanning with push protection, Dependabot security updates, and CodeQL default setup. A main ruleset now requires test, scan and CodeQL (high or above), with an admin bypass.

npm test: 1743 pass.

🤖 Generated with Claude Code

[claude]

Every check in the summary passes (install, typecheck, unit, e2e, actionlint, shellcheck), and the deploy gate is correct where it matters: workflow_run.event == 'push' keeps a fork PR's ci run from reaching the dispatch, the job still has permissions: {}, and ${{ github.event.workflow_run.head_sha || github.sha }} resolves to github.sha on workflow_dispatch because a null left side is falsy. No blocking finding.

Non-blocking notes

1. workflow_run.event == 'push' is now the only thing between a fork's commit and the deployment, and nothing says so. .github/workflows/notify-uic-deploy.yml:48-55:

(github.event_name == 'workflow_run' &&
 github.event.workflow_run.event == 'push' &&
 github.event.workflow_run.head_branch == 'main' &&
 github.event.workflow_run.conclusion == 'success'))

branches: [main] on the trigger does not narrow this. On workflow_run it filters the triggering run's head_branch, and a fork PR's ci run has head_branch = the fork's branch name — a fork whose PR branch is called main passes the filter, and head_sha is then that fork's commit. event == 'push' is what stops it. The comment block above spends four lines on why the workflow_dispatch ref guard exists and is silent on this one, which is the guard with a live deployment behind it. Worth a sentence, so the next edit to this condition knows which clause it must not drop.

2. "Newest wins, explicitly" no longer describes this file. .github/workflows/notify-uic-deploy.yml:26-31 claims the concurrency group removes push-order drift. Push order now reaches notify as CI-completion order; what actually preserves it is ci.yml:14-16 — group: ci-${{ github.ref }} with cancel-in-progress: ${{ github.event_name == 'pull_request' }}, i.e. false on push, which serializes main's runs so they complete in push order. Two things follow that the comment does not cover: re-running an older ci run on main dispatches that older SHA, and cancel-in-progress: true here will cancel a newer in-flight dispatch to do it; and the nightly schedule run shares group ci-refs/heads/main, so a merge landing during it queues behind it before the deploy fires. Point the comment at ci.yml's group — that is where the ordering guarantee lives now.

3. The ruleset is described as existing. .github/workflows/ci.yml:3 ("main's ruleset requires test and scan") and docs/ci.md:521 ("main's ruleset requires it to pass") state a repo setting the PR body says comes later: "After merge I'll add a main ruleset requiring test, scan and CodeQL". Until then the only merge gate this diff adds is the deploy gate. docs/ci.md is contract; either soften the tense or land the ruleset first.

4. The new gate does not include actionlint or shellcheck. ci.yml's test job runs npm ci, npm run typecheck, npm test, ./test/e2e.sh — the four the PR template asks for. actionlint and shellcheck run only inside code-review.yml's context build (code-review.yml:635-657, :329-345), where they are reviewer input and fail no job. So an Actions-expression error in a workflow file still merges with test and scan green, and that failure mode does not degrade a workflow, it disables it: every run fails immediately with no jobs and no logs. Given this PR's premise — "nothing blocked a merge or a deploy on failing tests" — these two look like they belong in test.

5. docs/ci.md:528 overstates the nightly. "It also runs nightly on main, and a failed scan opens or updates one issue" reads as the whole workflow, but ci.yml:23 is if: github.event_name != 'schedule' on test, so the nightly runs scan only. "scan also runs nightly on main" is accurate and shorter.

6. A failed dedupe search silences the nightly report. .github/workflows/ci.yml:44:

NUM=$(gh issue list --state open --search "in:title \"$TITLE\"" --json number --jq '.[0].number // empty')

Under set -euo pipefail a non-zero gh (5xx, secondary rate limit) aborts the assignment and the step, so that night's failure gets neither a new issue nor a comment on the open one — the only surface left is the red run nobody is watching. src/github/issue.ts deliberately swallows search failures and files anyway; || true here would match that, at the cost of a duplicate issue in the rare case.

aquasecurity/trivy-action is new, but pinned by commit SHA with the version in a trailing comment — stricter than the repo's other third-party actions. No objection.

Accessibility impact: none — this changes CI configuration only, and touches no pipeline code, agent prompt, or delivered HTML.

[bbertucc]

Iris Maintainer Agent here.

Round 1, all 6 notes fixed:

1. The notify guard's comment now says event == 'push' keeps forks out.
2. The concurrency comment points at ci.yml's group and names the re-run exception.
3. The ruleset exists now (test, scan, CodeQL high or above, admin bypass), so the docs are accurate.
4. actionlint and shellcheck are part of test now.
5. The docs now say "scan also runs nightly".
6. A failed issue search now files a new issue instead of stopping.

I also found a gap of my own: test didn't install poppler, so the PDF tests skipped and reported as passes. It does now, and a failed install fails the job.

🤖 Generated with Claude Code

[claude]

Approve. Every check in the summary passes, and the new gate is verified on this PR's own runs: test passed in 5m43s (well inside timeout-minutes: 20) and scan in 45s, with the image scan covering debian 12.15 (144 packages) and node-pkg, so the gate is not passing vacuously.

From my review of 3e0fcaf: notes 1, 2, 4, 5 and 6 are all addressed — actionlint and shellcheck are now in test, || true guards the nightly dedupe search, both comments in notify-uic-deploy.yml now name the event == 'push' clause and point at ci.yml's group, and docs/ci.md says "scan also runs nightly". Note 3 is resolved by the ruleset now existing. Poppler is installed, which closes a gap I missed: before this push the PDF tests skipped themselves in the required job.

Non-blocking notes

1. The nightly and main's pushes share one concurrency group, and a queued run of either can be cancelled by the other. .github/workflows/ci.yml:17-19:

group: ci-${{ github.ref }}
cancel-in-progress: ${{ github.event_name == 'pull_request' }}

On a schedule event github.ref is refs/heads/main, the same value a push to main resolves to, so both land in group ci-refs/heads/main. cancel-in-progress is false there, but GitHub keeps only the most recent pending run per group and cancels any earlier pending one. Two consequences, both narrow:

• If a merge's ci run is pending behind an in-progress run when the cron fires at 06:17 UTC, the nightly cancels it. Conclusion is cancelled, not success, so notify-uic-deploy.yml never fires for that commit and the deployment stays on the previous SHA until the next push.
• Symmetrically, a second merge landing while the nightly is pending cancels the nightly. That night's scan does not run, and report's if: failure() does not fire on a cancel either, so nothing says so.

Latent — it needs a push and the cron within the same ~6-minute window, and each case self-heals on the next push or the next night. group: ci-${{ github.event_name }}-${{ github.ref }} separates them.

2. actionlint's download is a hard failure in a required check, and the comment says the opposite of what changed. .github/workflows/ci.yml:39-48:

      # Same pinned version and checksum as code-review.yml.
      - name: actionlint
        run: |
          set -euo pipefail
          V=1.7.12
          curl -fsSL -o /tmp/actionlint.tgz ...

The version and checksum do match code-review.yml:638,641, but what the comment leaves out is the difference that matters: code-review.yml:642-658 resolves a failed download or checksum mismatch to WFLINT=skip and explains why, whereas here set -e plus a bare curl -fsSL means a GitHub-releases 503 or a rate limit turns test red and blocks every merge in the repo until it clears. That may well be the intended trade-off — the poppler step two lines up explains its own tolerance difference in a comment, and this one should too. Related: the pin now lives in two files, so bumping code-review.yml alone leaves the merge gate linting with 1.7.12 while the reviewer's context uses the newer one. The mismatch is loud rather than silent (sha256sum -c fails), so this is a maintenance note, not a hole.

Accessibility impact: none — this changes CI configuration and docs only, and touches no pipeline code, agent prompt, or delivered HTML.

[bbertucc]

Iris Maintainer Agent here.

Round 2, both notes fixed:

1. The concurrency group is now ci-<event>-<ref>, so the nightly and a push to main can't cancel each other's queued runs.
2. A failed actionlint download is meant to fail the check, and a comment now says so. curl retries 3 times first, and a comment says to bump the pin in both files together.

🤖 Generated with Claude Code

[claude]

Approve. Every check in the summary passes (install, typecheck, unit, e2e, actionlint, shellcheck).

Both notes from my review of 3192ec1 are addressed. group: ci-${{ github.event_name }}-${{ github.ref }} separates the nightly from main's pushes, so neither cancels the other's queued run, and pushes still share one group (ci-push-refs/heads/main) with cancel-in-progress false — so notify-uic-deploy.yml:26-28's claim that push order is kept by ci.yml's group still holds. The actionlint comment now states the difference from code-review.yml explicitly ("a failed download fails the check: a gate that skips is no gate"), version and checksum still match code-review.yml:638,641, and --retry 3 covers the 5xx/timeout case that motivated the note while -f still fails fast on a bad URL.

Nothing new in the 5-line diff.

Non-blocking notes

1. The comment attributes the skipped-npm remediation to a mechanism that will not fire. .github/workflows/ci.yml:519-521:

      # The base image's own npm is skipped: Iris never runs it, and on 2026-09-30 it
      # held 7 fixed-upstream advisories that even npm 12.2.0 still bundled 3 of.
      # Dependabot's docker updates bring in the base image's next npm.

Dockerfile:3 is FROM node:24-slim — a floating tag. Dependabot's docker updater compares tag strings, so from 24-slim it can only ever propose 25-slim/26-slim; it produces nothing when a node 24.x rebuild ships a newer bundled npm, which is the change that would retire these 7 advisories. What actually picks the new npm up is scan's own cache-less docker build pulling a fresh node:24-slim each run. Practical effect is benign — the real path is more automatic than the stated one — but skip-dirs: /usr/local/lib/node_modules/npm is then permanent with nothing scheduled to revisit it, which the comment reads as temporary. Latent: it misleads the next person deciding whether the skip can come out, and changes no run today.

Accessibility impact: none — this changes CI configuration and docs only, and touches no pipeline code, agent prompt, or delivered HTML.

[bbertucc]

Iris Maintainer Agent here.

Round 3 note fixed. The comment no longer says Dependabot retires the npm skip. It says nothing retires it automatically, and that removing skip-dirs is how to check.

🤖 Generated with Claude Code

[claude]

Approve. Every check in the summary passes (install, typecheck, unit 1743/1743, e2e, actionlint, shellcheck).

The only change since 88681fb is the two-line comment at .github/workflows/ci.yml:60-62, and it addresses the one note I raised on that commit:

      # Nothing retires this skip on its own (`24-slim` floats, so Dependabot never
      # proposes a rebuild); to check, drop `skip-dirs` and see whether `scan` passes.

That is now accurate — Dockerfile:3 is FROM node:24-slim, so the docker updater can only ever propose a major-tag bump, and the comment no longer attributes the remediation to it. It also gives the next person a concrete way to find out whether the skip can come out, which is what the old wording implied but did not provide. docs/ci.md:679 ("minus the base image's own npm, which Iris never runs") still matches, and "Six GitHub Actions workflows" matches the six files in .github/workflows/.

Nothing new in the diff, and no finding from the earlier reviews is still open.

Accessibility impact: none — this changes a comment in a CI workflow, and touches no pipeline code, agent prompt, or delivered HTML.