Add automated AI pull-request review and workflow lint gate #1

Merged
nimmo merged 1 commit from feat/ai-pr-review into main 2026-07-23 17:47:10 +01:00
Owner

Summary

Mirrors the forgejo-auto-pr-review setup from nixos-config onto redview, using the same shared ollama infrastructure (nova.mesh:11434, qwen3.6:35b-a3b-q8_0).

Changes

  • .forgejo/workflows/ai-review.yml (new) — triggers an advisory AI review on opened, reopened, synchronize, and ready_for_review events, plus workflow_dispatch. Runs on the nova runner. Same fork-guard and concurrency cancellation as nixos-config.
  • .forgejo/ai-review.yml (new) — provider config and deterministic gate. Uses npm test (120s timeout) as the authoritative check; this exercises the 16-test suite including the schema migration tests and openapi-routes.test.js, so the AI review always has evidence that routes are documented and migrations are intact.
  • .forgejo/ai-review.md (new) — architecture-specific guidance for the AI reviewer: appended-only db.js migrations, the @openapi JSDoc policy, lib/path-safety.js for any new file-serving code, the tagger/Dockerfile matrix, and the no-build-step vanilla JS SPA rule.
  • .forgejo/workflows/docker-build.yml (modified) — new lint job that downloads actionlint via curl and runs it across all workflows before any Docker build is started. The forgejo action URL for forgejo-auto-pr-review is in the actionlint ignore list (same approach as nixos-config's attic-cache.yml). build-amd now needs: lint, so a failed lint blocks the three image builds. Container builds are now gated on github.event_name == 'push' — PRs only run the lint job, no image builds or registry pushes.
  • .github/actionlint.yaml (new) — declares the docker/nova self-hosted runner labels, ignores the forgejo action URL format, and ignores forgejo.* expression-context warnings (actionlint doesn't understand the Forgejo extension).

Deployment note

The AI_REVIEW_FORGEJO_TOKEN secret must be set on the redview repository (or org) in Forgejo before the review workflow can post comments. Without it, the review job will fail to publish its advisory comment but the workflow itself will still run end-to-end.

Why npm test as the deterministic gate

The 16 test files give the reviewer strong evidence about the codebase without needing any external infrastructure:

  • openapi-routes.test.js — every route has a matching @openapi block
  • db-migrations.test.js — migration framework produces correct schema
  • auth.test.js, dedup.test.js, etc. — business logic gates

This catches the most common classes of regression (undocumented routes, broken migrations, broken business logic) before the AI even sees the diff.

Why PRs skip the container builds

Building three image variants (AMD, NVIDIA, CPU) and pushing them to the registry on every commit to every PR branch is a lot of wasted CI time and registry churn. The docker-build.yml workflow now distinguishes:

  • push events — full pipeline: prepare tags → notify start → lint → build AMD → build NVIDIA → build CPU → notify on failure. The lint job still runs first as a gate.
  • pull_request events — only the lint job runs. The prepare_tags, notify-start, three build-* jobs, and notify-failure are all skipped via if: github.event_name == 'push'.

This keeps the build/push pipeline for the actual deployment (push to main) and uses the lighter lint check on PRs.

Known limitation: this PR's own AI review

The reviewer reads .forgejo/ai-review.yml from the base commit of the PR (the trusted revision, so a PR can't change its own review config). On the very first PR that introduces the config, the base doesn't have it yet, so the review job will fail with provider.model or OLLAMA_MODEL is required. This is upstream behaviour and tracked at nimmo/forgejo-auto-pr-review#22 — every PR after this one will be reviewed normally.

Test plan

  • Confirm the lint job passes on this PR (validates workflow syntax with actionlint)
  • Confirm PRs skip container builds (only lint runs for pull_request)
  • Confirm the AI_REVIEW_FORGEJO_TOKEN secret is set on the repo/org, then merge
  • Open a draft PR after merging to verify the AI review is generated as a comment
## Summary Mirrors the `forgejo-auto-pr-review` setup from `nixos-config` onto `redview`, using the same shared ollama infrastructure (`nova.mesh:11434`, `qwen3.6:35b-a3b-q8_0`). ### Changes - **`.forgejo/workflows/ai-review.yml`** (new) — triggers an advisory AI review on `opened`, `reopened`, `synchronize`, and `ready_for_review` events, plus `workflow_dispatch`. Runs on the `nova` runner. Same fork-guard and concurrency cancellation as nixos-config. - **`.forgejo/ai-review.yml`** (new) — provider config and deterministic gate. Uses `npm test` (120s timeout) as the authoritative check; this exercises the 16-test suite including the schema migration tests and `openapi-routes.test.js`, so the AI review always has evidence that routes are documented and migrations are intact. - **`.forgejo/ai-review.md`** (new) — architecture-specific guidance for the AI reviewer: appended-only `db.js` migrations, the `@openapi` JSDoc policy, `lib/path-safety.js` for any new file-serving code, the tagger/Dockerfile matrix, and the no-build-step vanilla JS SPA rule. - **`.forgejo/workflows/docker-build.yml`** (modified) — new `lint` job that downloads `actionlint` via `curl` and runs it across all workflows before any Docker build is started. The forgejo action URL for `forgejo-auto-pr-review` is in the `actionlint` ignore list (same approach as nixos-config's `attic-cache.yml`). `build-amd` now `needs: lint`, so a failed lint blocks the three image builds. **Container builds are now gated on `github.event_name == 'push'`** — PRs only run the `lint` job, no image builds or registry pushes. - **`.github/actionlint.yaml`** (new) — declares the `docker`/`nova` self-hosted runner labels, ignores the forgejo action URL format, and ignores `forgejo.*` expression-context warnings (actionlint doesn't understand the Forgejo extension). ### Deployment note The `AI_REVIEW_FORGEJO_TOKEN` secret must be set on the `redview` repository (or org) in Forgejo before the review workflow can post comments. Without it, the review job will fail to publish its advisory comment but the workflow itself will still run end-to-end. ### Why `npm test` as the deterministic gate The 16 test files give the reviewer strong evidence about the codebase without needing any external infrastructure: - `openapi-routes.test.js` — every route has a matching `@openapi` block - `db-migrations.test.js` — migration framework produces correct schema - `auth.test.js`, `dedup.test.js`, etc. — business logic gates This catches the most common classes of regression (undocumented routes, broken migrations, broken business logic) before the AI even sees the diff. ### Why PRs skip the container builds Building three image variants (AMD, NVIDIA, CPU) and pushing them to the registry on every commit to every PR branch is a lot of wasted CI time and registry churn. The `docker-build.yml` workflow now distinguishes: - **`push` events** — full pipeline: prepare tags → notify start → lint → build AMD → build NVIDIA → build CPU → notify on failure. The lint job still runs first as a gate. - **`pull_request` events** — only the `lint` job runs. The `prepare_tags`, `notify-start`, three `build-*` jobs, and `notify-failure` are all skipped via `if: github.event_name == 'push'`. This keeps the build/push pipeline for the actual deployment (push to `main`) and uses the lighter lint check on PRs. ### Known limitation: this PR's own AI review The reviewer reads `.forgejo/ai-review.yml` from the **base commit** of the PR (the trusted revision, so a PR can't change its own review config). On the very first PR that introduces the config, the base doesn't have it yet, so the review job will fail with `provider.model or OLLAMA_MODEL is required`. This is upstream behaviour and tracked at https://git.nimmog.uk/nimmo/forgejo-auto-pr-review/issues/22 — every PR after this one will be reviewed normally. ### Test plan - [x] Confirm the `lint` job passes on this PR (validates workflow syntax with `actionlint`) - [x] Confirm PRs skip container builds (only `lint` runs for `pull_request`) - [ ] Confirm the `AI_REVIEW_FORGEJO_TOKEN` secret is set on the repo/org, then merge - [ ] Open a draft PR after merging to verify the AI review is generated as a comment
feat(ci): add AI pull-request review and workflow lint gate
Some checks failed
Build and Push Docker Image / prepare_tags (push) Successful in 1s
Build and Push Docker Image / notify-start (push) Successful in 1s
Build and Push Docker Image / lint (push) Failing after 2s
Build and Push Docker Image / build-amd (push) Has been skipped
Build and Push Docker Image / build-nvidia (push) Has been skipped
Build and Push Docker Image / build-cpu (push) Has been skipped
Build and Push Docker Image / notify-failure (push) Successful in 1s
AI pull-request review / review (pull_request) Failing after 4s
cdfc846f00
Mirror the forgejo-auto-pr-review setup from nixos-config to redview.

- Add .forgejo/workflows/ai-review.yml that triggers an advisory review on
  opened/reopened/synchronize/ready_for_review events and via manual
  workflow_dispatch. Runs on the nova runner, skips fork PRs to protect
  the comment token, and uses concurrency cancellation per PR.
- Add .forgejo/ai-review.yml with the shared ollama provider
  (qwen3.6:35b-a3b-q8_0 on nova.mesh:11434) and use npm test as the
  deterministic gate so the review has evidence that the schema
  migrations, API route coverage, and business logic all pass.
- Add .forgejo/ai-review.md with architecture-specific guidance for the
  Express + better-sqlite3 + vanilla JS SPA stack: appended-only db.js
  migrations, the @openapi JSDoc policy enforced by
  openapi-routes.test.js, the lib/path-safety.js requirement for any
  new file-serving code, and the tagger/Dockerfile matrix.
- Extend .forgejo/workflows/docker-build.yml with a lint job that
  installs actionlint via curl and runs it across the workflows before
  any docker build is started, gating build-amd on its success.

Requires the AI_REVIEW_FORGEJO_TOKEN secret to be set on the redview
repository before the review workflow can post comments.
nimmo force-pushed feat/ai-pr-review from cdfc846f00
Some checks failed
Build and Push Docker Image / prepare_tags (push) Successful in 1s
Build and Push Docker Image / notify-start (push) Successful in 1s
Build and Push Docker Image / lint (push) Failing after 2s
Build and Push Docker Image / build-amd (push) Has been skipped
Build and Push Docker Image / build-nvidia (push) Has been skipped
Build and Push Docker Image / build-cpu (push) Has been skipped
Build and Push Docker Image / notify-failure (push) Successful in 1s
AI pull-request review / review (pull_request) Failing after 4s
to f45fda0f91
Some checks failed
Build and Push Docker Image / prepare_tags (pull_request) Has been skipped
Build and Push Docker Image / notify-start (pull_request) Has been skipped
Build and Push Docker Image / lint (pull_request) Successful in 2s
AI pull-request review / review (pull_request) Failing after 4s
Build and Push Docker Image / build-amd (pull_request) Has been skipped
Build and Push Docker Image / build-nvidia (pull_request) Has been skipped
Build and Push Docker Image / build-cpu (pull_request) Has been skipped
Build and Push Docker Image / notify-failure (pull_request) Has been skipped
Build and Push Docker Image / prepare_tags (push) Successful in 1s
Build and Push Docker Image / lint (push) Successful in 3s
Build and Push Docker Image / notify-start (push) Successful in 1s
Build and Push Docker Image / build-amd (push) Successful in 14s
Build and Push Docker Image / build-nvidia (push) Successful in 15s
Build and Push Docker Image / build-cpu (push) Successful in 15s
Build and Push Docker Image / notify-failure (push) Has been skipped
2026-07-23 17:18:12 +01:00
Compare
nimmo merged commit f45fda0f91 into main 2026-07-23 17:47:10 +01:00
nimmo deleted branch feat/ai-pr-review 2026-07-23 17:47:10 +01:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
nimmo/redview!1
No description provided.