chore(ai-review): the gateway review route moved #25

A-Guevara · 2026-09-23T17:20:29Z

A-Guevara commented

2026-09-23 17:20:29 +00:00

The AI review gateway renamed its routes (nis/forgejo-stack ADR-0002) and kept no aliases, so the workflow this repo carries now gets a 404 and posts a could-not-review comment. This re-seeds the canonical ai/ai-review.yml, which calls the new route. Merge it to get AI review back on this repository. Advisory only, never a merge gate.

A-Guevara added 1 commit

2026-09-23 17:20:29 +00:00

chore(ai-review): the gateway review route moved (ADR-0002)

ai-review / review (pull_request) Successful in 1m7s

Details

ci / build (push) Successful in 16s

Details

baseline-security / baseline (push) Successful in 1m59s

Details

baseline-security / baseline (pull_request) Successful in 2m2s

Details

d920e60c63

ai-review-bot reviewed

2026-09-23 17:24:50 +00:00

ai-review-bot left a comment

AI review · advisory

Verdict: 1 thing worth fixing (1 low) — 2 confirmed, near-duplicates merged.

_{⚑ panel: GPT-OSS 120B · Gemma 4 31B · Devstral 2 123B · Laguna S 2.1 — 3 distinct, 2 confirmed, 1 below threshold, 0 refuted · web: not used · context: 1 files under review · 39 codebase · 6 standards chunks}

Panel debate — how this review was reached

Grounding — context: 1 files under review · 39 codebase · 6 standards chunks

Round 1 — independent reviews

GPT-OSS 120B (0 findings, confidence 0.97 · thought 602 + wrote 44 of 16.0k tokens · 6 s): The changed lines correct the API endpoint and update related references without introducing bugs or security issues.
Gemma 4 31B (0 findings, confidence 1.0 · thought 1.2k + wrote 215 of 16.0k tokens · 33 s): The changes correctly update the AI review gateway route and corresponding transcript URL to match the new RESTful pattern, and the implementation is consistent with the existing script's logic and se
Devstral 2 123B (0 findings, confidence 1.0 · wrote 71 of 16.0k tokens · 9 s): The changes in the PR are correct and consistent with the codebase standards. The route update aligns with the documented ADR-0002 and maintains the same security and error-handling practices.
Laguna S 2.1 (3 findings, confidence 0.85 · wrote 1.2k of 65.5k tokens · 10 s): Two genuine concerns: (1) medium — the body shape changed (repo/pr removed) to match the new path-based route, but there's no validation that the gateway's /v1/repos/{o}/{r}/pulls/{n}/reviews endpoint

Round 2 — cross-examination

Laguna S 2.1#2 PR_NUMBER parsed as int with no validation can crash int() on empty/malformed · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: — · support 3
Laguna S 2.1#1 Body no longer carries repo/pr after route moved to path · confirmed: Devstral 2 123B · refuted: GPT-OSS 120B, Gemma 4 31B · support 1
Laguna S 2.1#3 Transcript URL path singular/plural now matches comment header · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: — · support 3

Not posted (support < 2, or contested at a severity where one refutation vetoes)

Laguna S 2.1#1 Body no longer carries repo/pr after route moved to path (support 1)

Synthesis — Laguna S 2.1 wrote the final review from 2 confirmed findings (+1 below threshold) · promotion: support ≥ 2, and no refutation at high severity.

_{Transcript rv-20260923172355-999bd2 — full round outputs, web results, and model reasoning are viewable by anyone with access to this repository via the AI gateway.}

_{Advisory — never a merge gate. Findings are ordered by how well the panel's own evidence checks out, strongest first. React 👍/👎 on any inline comment to tell it whether it was worth flagging — that is the only feedback this system gets, and every threshold in it is tuned from those reactions. Transcript rv-20260923172355-999bd2.}

### AI review · advisory  **Verdict: 1 thing worth fixing** (1 low) — 2 confirmed, near-duplicates merged. ⚑ panel: GPT-OSS 120B · Gemma 4 31B · Devstral 2 123B · Laguna S 2.1 — 3 distinct, 2 confirmed, 1 below threshold, 0 refuted · web: not used · context: 1 files under review · 39 codebase · 6 standards chunks <details> <summary>Panel debate — how this review was reached</summary> **Grounding** — context: 1 files under review · 39 codebase · 6 standards chunks **Round 1 — independent reviews** - **GPT-OSS 120B** (0 findings, confidence 0.97 · thought 602 + wrote 44 of 16.0k tokens · 6 s): The changed lines correct the API endpoint and update related references without introducing bugs or security issues. - **Gemma 4 31B** (0 findings, confidence 1.0 · thought 1.2k + wrote 215 of 16.0k tokens · 33 s): The changes correctly update the AI review gateway route and corresponding transcript URL to match the new RESTful pattern, and the implementation is consistent with the existing script's logic and se - **Devstral 2 123B** (0 findings, confidence 1.0 · wrote 71 of 16.0k tokens · 9 s): The changes in the PR are correct and consistent with the codebase standards. The route update aligns with the documented ADR-0002 and maintains the same security and error-handling practices. - **Laguna S 2.1** (3 findings, confidence 0.85 · wrote 1.2k of 65.5k tokens · 10 s): Two genuine concerns: (1) medium — the body shape changed (repo/pr removed) to match the new path-based route, but there's no validation that the gateway's /v1/repos/{o}/{r}/pulls/{n}/reviews endpoint **Round 2 — cross-examination** - `Laguna S 2.1#2` PR_NUMBER parsed as int with no validation can crash int() on empty/malformed · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: — · support 3 - `Laguna S 2.1#1` Body no longer carries repo/pr after route moved to path · confirmed: Devstral 2 123B · refuted: GPT-OSS 120B, Gemma 4 31B · support 1 - `Laguna S 2.1#3` Transcript URL path singular/plural now matches comment header · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: — · support 3 **Not posted** (support < 2, or contested at a severity where one refutation vetoes) - `Laguna S 2.1#1` Body no longer carries repo/pr after route moved to path (support 1) **Synthesis** — Laguna S 2.1 wrote the final review from 2 confirmed findings (+1 below threshold) · promotion: support ≥ 2, and no refutation at high severity. Transcript `rv-20260923172355-999bd2` — full round outputs, web results, and model reasoning are viewable by anyone with access to this repository via the AI gateway. </details> Advisory — never a merge gate. Findings are ordered by how well the panel's own evidence checks out, strongest first. React 👍/👎 on any inline comment to tell it whether it was worth flagging — that is the only feedback this system gets, and every threshold in it is tuned from those reactions. Transcript `rv-20260923172355-999bd2`.

.forgejo/workflows/ai-review.yml

					
				@ -148,3 +149,3 @@

				                  md += ("\n\n<sub>Full transcript: " + os.environ["SERVER_URL"]

				                         + "/-/ai/v1/review/" + out["review_id"] + "</sub>")

				                         + "/-/ai/v1/reviews/" + out["review_id"] + "</sub>")

				          except Exception as e:

LOW — Transcript URL path singular/plural now matches comment header

The route changed from /-/ai/v1/review/{id} to /-/ai/v1/reviews/{id}. This is consistent with the plural 'reviews' in the BFF URL path just above (line 103) and the file header comment (line 3) which now says POST /v1/repos/{o}/{r}/pulls/{n}/reviews. The transcript link is a secondary ACL-gated UI route, so correctness here is cosmetic, but it is at least internally consistent post-change. No defect — noting for completeness against cross-codebase consistency.

Fix: No change needed — the plural 'reviews' now matches both the BFF route (line 103) and the header comment (line 3).

_{Evidence: strong evidence — two or more reviewers, quote verified against the diff, anchored on a changed line (panel 4/4).
👍 if this was worth flagging · 👎 if it was not — a reaction on this comment is the whole feedback loop.}

**LOW** — Transcript URL path singular/plural now matches comment header The route changed from /-/ai/v1/review/{id} to /-/ai/v1/reviews/{id}. This is consistent with the plural 'reviews' in the BFF URL path just above (line 103) and the file header comment (line 3) which now says POST /v1/repos/{o}/{r}/pulls/{n}/reviews. The transcript link is a secondary ACL-gated UI route, so correctness here is cosmetic, but it is at least internally consistent post-change. No defect — noting for completeness against cross-codebase consistency. > **Fix:** No change needed — the plural 'reviews' now matches both the BFF route (line 103) and the header comment (line 3). Evidence: **strong evidence** — two or more reviewers, quote verified against the diff, anchored on a changed line (panel 4/4). 👍 if this was worth flagging · 👎 if it was not — a reaction on this comment is the whole feedback loop.

.forgejo/workflows/ai-review.yml

					
				@ -103,1 +101,3 @@

				          req = urllib.request.Request(os.environ["BFF_URL"]+"/v1/review",

				          # Forgejo-shaped route (ADR-0002): repo and PR live in the PATH and the

				          # gateway checks the caller can see that repo before it reads the diff.

				          req = urllib.request.Request(os.environ["BFF_URL"] + "/v1/repos/" + os.environ["REPO"]

HIGH — PR_NUMBER parsed as int with no validation can crash int() on empty/malformed

int(os.environ.get("PR_NUMBER") or 0) is robust when PR_NUMBER is unset (falls to 0 via the or 0). However, if github.event.pull_request.number is somehow empty-but-truthy or non-numeric (e.g. a forged event payload, or the env var set to a non-int string by a misbehaving trigger), int() raises ValueError inside the urllib.request.Request URL construction — this happens BEFORE the try/except retry loop, so the whole python heredoc aborts and /tmp/review.md is left empty, which the next step tolerates (non-fatal) but loses the review entirely without the documented fallthrough. This is inconsistent with the defensive posture elsewhere in the file (e.g. the or 0 guard, the retry/fallthrough design).

Fix: Wrap the PR number coercion in a helper that validates it is a positive integer before building the URL, falling back to the same 0 sentinel the rest of the code already tolerates, so a malformed number cannot abort the heredoc outside the retry/fallthrough block.

_{Evidence: strong evidence — two or more reviewers, quote verified against the diff, anchored on a changed line (panel 4/4).
👍 if this was worth flagging · 👎 if it was not — a reaction on this comment is the whole feedback loop.}

**HIGH** — PR_NUMBER parsed as int with no validation can crash int() on empty/malformed int(os.environ.get("PR_NUMBER") or 0) is robust when PR_NUMBER is unset (falls to 0 via the `or 0`). However, if github.event.pull_request.number is somehow empty-but-truthy or non-numeric (e.g. a forged event payload, or the env var set to a non-int string by a misbehaving trigger), int() raises ValueError inside the urllib.request.Request URL construction — this happens BEFORE the try/except retry loop, so the whole python heredoc aborts and /tmp/review.md is left empty, which the next step tolerates (non-fatal) but loses the review entirely without the documented fallthrough. This is inconsistent with the defensive posture elsewhere in the file (e.g. the `or 0` guard, the retry/fallthrough design). > **Fix:** Wrap the PR number coercion in a helper that validates it is a positive integer before building the URL, falling back to the same 0 sentinel the rest of the code already tolerates, so a malformed number cannot abort the heredoc outside the retry/fallthrough block. Evidence: **strong evidence** — two or more reviewers, quote verified against the diff, anchored on a changed line (panel 4/4). 👍 if this was worth flagging · 👎 if it was not — a reaction on this comment is the whole feedback loop.