chore(F/I): native AI code review #36

tti-admin · 2026-08-13T18:46:35Z

tti-admin commented

2026-08-13 18:46:35 +00:00

Advisory multi-model superPOD panel (see REVIEW_PANEL on the gateway) — now posts a NATIVE Forgejo review with inline comments at each finding, plus a human-facing findings board to agree/disagree/discuss with the panel (Area I). Never a merge gate.

tti-admin added 1 commit

2026-08-13 18:46:36 +00:00

chore(F): seed advisory AI review

scan / trivy-fs (pull_request) Successful in 1m21s

Details

baseline-security / baseline (pull_request) Successful in 1m41s

Details

scan / trivy-fs (push) Successful in 1m7s

Details

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

Details

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

Details

f66e20dfb4

ai-review-bot reviewed

2026-08-13 18:49:29 +00:00

ai-review-bot left a comment

AI review · advisory

Verdict: 7 things worth fixing (2 high · 4 medium · 1 low).

Findings that didn't map to a diff line:

.forgejo/workflows/ai-review.yml:21 · MEDIUM — JOB_TOKEN exposed via env: in the gate step — verify it's only used here and not leaked to later steps
The gate step now declares JOB_TOKEN: ${{ github.token }} in its env: block. The github.token is auto-injected per-job and should not persist beyond the step, but the env: block is scoped to this step only (GitHub Actions env: keys are step-scoped when set inline on a step, job-scoped when set on the job-level env:). Since this env: is on the gate step (id: gate), it's step-scoped and will not leak to subsequent steps like 'Post review comment' which has its own env: block. This matches the pattern in [codebase] tti/tti-ux/.forgejo/workflows/ai-review.yml:51 where JOB_TOKEN is also step-scoped. No leak. This is consistent with the repo's established security posture in [standard] tti/coding-standards/.forgejo/workflows/baseline.yml which uses step-scoped git headers. No finding.

_{⚑ panel: GPT-OSS 120B · Gemma 4 31B · Devstral 2 123B · Laguna S 2.1 — 9 distinct, 7 confirmed, 2 refuted · web: not used · context: 1 files · 90 codebase · 14 standards chunks (best-grounded: GPT-OSS 120B; smaller windows saw less)}

Panel debate — how this review was reached

Grounding — context: 1 files · 90 codebase · 14 standards chunks (best-grounded: GPT-OSS 120B; smaller windows saw less)

Round 1 — independent reviews

GPT-OSS 120B (0 findings, confidence 0.97): The changes fix environment variable handling and improve error handling without introducing new bugs.
Gemma 4 31B (1 finding, confidence 1.0): The changes correctly improve security by moving sensitive variables to the environment to prevent shell injection and enhance the robustness of the AI panel call by implementing a selective retry str
Devstral 2 123B (3 findings, confidence 0.85): The pull request introduces security risks by insecurely using environment variables in shell commands and inconsistent error handling in HTTP requests.
Laguna S 2.1 (5 findings, confidence 0.92): Three of five candidate findings were dismissed after cross-referencing the diff against the repo's own patterns and the agency coding standards. Two real issues remain: (1) HIGH — raw HTTP error resp

Round 2 — cross-examination

Laguna S 2.1#4 Non-429 HTTP failure sets fail variable but still falls through to raise Runtime · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: —
Devstral 2 123B#1 Insecure use of environment variables in shell command · confirmed: — · refuted: GPT-OSS 120B, Gemma 4 31B, Laguna S 2.1
Laguna S 2.1#1 POST_comment_url uses env vars instead of interpolated expressions, risking wron · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: —
Laguna S 2.1#5 429 retry delay of 90s with only 3 total attempts may be insufficient for BFF qu · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: —
Devstral 2 123B#2 Inconsistent error handling in HTTP requests · confirmed: GPT-OSS 120B · refuted: Gemma 4 31B, Laguna S 2.1
Laguna S 2.1#3 JOB_TOKEN exposed via env: in the gate step — verify it's only used here and not · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: —
Gemma 4 31B#1 Bare except clause in Python · confirmed: GPT-OSS 120B, Devstral 2 123B · refuted: Laguna S 2.1
Devstral 2 123B#3 Inconsistent use of environment variables in curl command · confirmed: — · refuted: GPT-OSS 120B, Gemma 4 31B, Laguna S 2.1
Laguna S 2.1#2 time imported inside loop body instead of at module top — minor lint smell but n · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: —

Raised but refuted (left out of the review above)

Devstral 2 123B#1 Insecure use of environment variables in shell command — The curl command now uses step‑scoped environment variables (SERVER_URL, REPO, JOB_TOKEN) rather than inline ${{ … }} interpolation, matching
Devstral 2 123B#3 Inconsistent use of environment variables in curl command — The post function’s use of $SERVER_URL/$REPO/$PR_NUMBER follows the same safe env‑var pattern adopted elsewhere in the workflow; there is no injecti

Synthesis — Devstral 2 123B wrote the final review from 7 confirmed findings.

_{Transcript rv-20260813184736-fd2a99 — 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. Disagree with a finding? Reply on it, or use the finding board under this review. Transcript rv-20260813184736-fd2a99.}

### AI review · advisory  **Verdict: 7 things worth fixing** (2 high · 4 medium · 1 low). Findings that didn't map to a diff line: **`.forgejo/workflows/ai-review.yml:21`** · MEDIUM — JOB_TOKEN exposed via env: in the gate step — verify it's only used here and not leaked to later steps The gate step now declares `JOB_TOKEN: ${{ github.token }}` in its env: block. The github.token is auto-injected per-job and should not persist beyond the step, but the env: block is scoped to this step only (GitHub Actions env: keys are step-scoped when set inline on a step, job-scoped when set on the job-level env:). Since this env: is on the `gate` step (id: gate), it's step-scoped and will not leak to subsequent steps like 'Post review comment' which has its own env: block. This matches the pattern in [codebase] tti/tti-ux/.forgejo/workflows/ai-review.yml:51 where JOB_TOKEN is also step-scoped. No leak. This is consistent with the repo's established security posture in [standard] tti/coding-standards/.forgejo/workflows/baseline.yml which uses step-scoped git headers. No finding. ⚑ panel: GPT-OSS 120B · Gemma 4 31B · Devstral 2 123B · Laguna S 2.1 — 9 distinct, 7 confirmed, 2 refuted · web: not used · context: 1 files · 90 codebase · 14 standards chunks (best-grounded: GPT-OSS 120B; smaller windows saw less) <details> <summary>Panel debate — how this review was reached</summary> **Grounding** — context: 1 files · 90 codebase · 14 standards chunks (best-grounded: GPT-OSS 120B; smaller windows saw less) **Round 1 — independent reviews** - **GPT-OSS 120B** (0 findings, confidence 0.97): The changes fix environment variable handling and improve error handling without introducing new bugs. - **Gemma 4 31B** (1 finding, confidence 1.0): The changes correctly improve security by moving sensitive variables to the environment to prevent shell injection and enhance the robustness of the AI panel call by implementing a selective retry str - **Devstral 2 123B** (3 findings, confidence 0.85): The pull request introduces security risks by insecurely using environment variables in shell commands and inconsistent error handling in HTTP requests. - **Laguna S 2.1** (5 findings, confidence 0.92): Three of five candidate findings were dismissed after cross-referencing the diff against the repo's own patterns and the agency coding standards. Two real issues remain: (1) HIGH — raw HTTP error resp **Round 2 — cross-examination** - `Laguna S 2.1#4` Non-429 HTTP failure sets fail variable but still falls through to raise Runtime · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: — - `Devstral 2 123B#1` Insecure use of environment variables in shell command · confirmed: — · refuted: GPT-OSS 120B, Gemma 4 31B, Laguna S 2.1 - `Laguna S 2.1#1` POST_comment_url uses env vars instead of interpolated expressions, risking wron · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: — - `Laguna S 2.1#5` 429 retry delay of 90s with only 3 total attempts may be insufficient for BFF qu · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: — - `Devstral 2 123B#2` Inconsistent error handling in HTTP requests · confirmed: GPT-OSS 120B · refuted: Gemma 4 31B, Laguna S 2.1 - `Laguna S 2.1#3` JOB_TOKEN exposed via env: in the gate step — verify it's only used here and not · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: — - `Gemma 4 31B#1` Bare except clause in Python · confirmed: GPT-OSS 120B, Devstral 2 123B · refuted: Laguna S 2.1 - `Devstral 2 123B#3` Inconsistent use of environment variables in curl command · confirmed: — · refuted: GPT-OSS 120B, Gemma 4 31B, Laguna S 2.1 - `Laguna S 2.1#2` time imported inside loop body instead of at module top — minor lint smell but n · confirmed: GPT-OSS 120B, Gemma 4 31B, Devstral 2 123B · refuted: — **Raised but refuted** (left out of the review above) - `Devstral 2 123B#1` Insecure use of environment variables in shell command — The curl command now uses step‑scoped environment variables (`SERVER_URL`, `REPO`, `JOB_TOKEN`) rather than inline `${{ … }}` interpolation, matching - `Devstral 2 123B#3` Inconsistent use of environment variables in curl command — The post function’s use of `$SERVER_URL/$REPO/$PR_NUMBER` follows the same safe env‑var pattern adopted elsewhere in the workflow; there is no injecti **Synthesis** — Devstral 2 123B wrote the final review from 7 confirmed findings. Transcript `rv-20260813184736-fd2a99` — 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. Disagree with a finding? Reply on it, or use the finding board under this review. Transcript `rv-20260813184736-fd2a99`.

.forgejo/workflows/ai-review.yml

					
				@ -84,7 +91,7 @@ jobs:

				          set -eu

MEDIUM — Inconsistent error handling in HTTP requests

The code catches HTTPError separately but does not handle other exceptions consistently, which can lead to unclear error messages and unexpected behavior.

Fix: Consolidate error handling to ensure all exceptions are handled uniformly and provide clear error messages.

_{panel tally 2/4 · reply here or use the finding board to agree/disagree}

**MEDIUM** — Inconsistent error handling in HTTP requests The code catches HTTPError separately but does not handle other exceptions consistently, which can lead to unclear error messages and unexpected behavior. > **Fix:** Consolidate error handling to ensure all exceptions are handled uniformly and provide clear error messages. panel tally 2/4 · reply here or use the finding board to agree/disagree

.forgejo/workflows/ai-review.yml

					
				@ -99,21 +106,32 @@ jobs:

				                           "Authorization":"token "+os.environ["BFF_TOKEN"],

				                           "X-TTI-Actor":os.environ["ACTOR"]})

				          out = None

.forgejo/workflows/ai-review.yml:108 · LOW — import time inside loop instead of module top
The time module is imported inside the loop body, which is a minor style deviation from Python best practices (imports should be at the top of the script). While functionally harmless, it can make the code harder to read.

Fix: Move import time to the module-level imports at the top of the Python heredoc.

_{panel tally 4/4 · reply here or use the finding board to agree/disagree}

**`.forgejo/workflows/ai-review.yml:108`** · LOW — `import time` inside loop instead of module top The `time` module is imported inside the loop body, which is a minor style deviation from Python best practices (imports should be at the top of the script). While functionally harmless, it can make the code harder to read. > **Fix:** Move `import time` to the module-level imports at the top of the Python heredoc. panel tally 4/4 · reply here or use the finding board to agree/disagree

.forgejo/workflows/ai-review.yml

					
				@ -106,1 +112,3 @@

				                  # index run can 429-starve the panel briefly).

				                  # Retry ONLY 429 quota. 502/503 are inconclusive (gemma

				                  # timeout / Devstral over-limit) — retrying burns ~4.5 min

				                  # and will not recover; stop, but fall THROUGH to the

.forgejo/workflows/ai-review.yml:114 · MEDIUM — Bare except Exception clause violates Python standards
The code uses a broad except Exception as e: block, which can mask unexpected errors and make debugging harder. The agency's Python standard requires catching the narrowest possible exceptions.

Fix: Replace with specific exceptions: except (urllib.error.HTTPError, json.JSONDecodeError, TimeoutError) as e:.

Proposed replacement (one-click ⚡ Apply on the findings board at the top of this PR):

              except (urllib.error.HTTPError, json.JSONDecodeError, TimeoutError) as e:

_{panel tally 3/4 · reply here or use the finding board to agree/disagree}

**`.forgejo/workflows/ai-review.yml:114`** · MEDIUM — Bare `except Exception` clause violates Python standards The code uses a broad `except Exception as e:` block, which can mask unexpected errors and make debugging harder. The agency's Python standard requires catching the narrowest possible exceptions. > **Fix:** Replace with specific exceptions: `except (urllib.error.HTTPError, json.JSONDecodeError, TimeoutError) as e:`. **Proposed replacement** (one-click ⚡ Apply on the findings board at the top of this PR): ``` except (urllib.error.HTTPError, json.JSONDecodeError, TimeoutError) as e: ``` panel tally 3/4 · reply here or use the finding board to agree/disagree

.forgejo/workflows/ai-review.yml

					
				@ -107,0 +112,4 @@

				                  # Retry ONLY 429 quota. 502/503 are inconclusive (gemma

				                  # timeout / Devstral over-limit) — retrying burns ~4.5 min

				                  # and will not recover; stop, but fall THROUGH to the

				                  # advisory comment below (AI review is advisory — a panel

.forgejo/workflows/ai-review.yml:115 · HIGH — Raw HTTP error details leaked into PR comments
When a non-429 HTTP error occurs, the code captures up to 300 characters of the raw response body and includes it in a user-facing advisory comment. This can expose internal server paths, stack traces, or credentials if the backend echoes sensitive data in error responses.

Fix: Redact raw HTTP response bodies before embedding them in the advisory message; log full details to step output only.

_{panel tally 4/4 · reply here or use the finding board to agree/disagree}

**`.forgejo/workflows/ai-review.yml:115`** · HIGH — Raw HTTP error details leaked into PR comments When a non-429 HTTP error occurs, the code captures up to 300 characters of the raw response body and includes it in a user-facing advisory comment. This can expose internal server paths, stack traces, or credentials if the backend echoes sensitive data in error responses. > **Fix:** Redact raw HTTP response bodies before embedding them in the advisory message; log full details to step output only. panel tally 4/4 · reply here or use the finding board to agree/disagree

.forgejo/workflows/ai-review.yml:115 · HIGH — Raw HTTP error details leaked into PR comments
When a non-429 HTTP error occurs, the code captures up to 300 characters of the raw response body and includes it in a user-facing advisory comment. This can expose internal server paths, stack traces, or credentials if the backend echoes sensitive data in error responses.

Fix: Redact raw HTTP response bodies before embedding them in the advisory message; log full details to step output only.

_{panel tally 4/4 · reply here or use the finding board to agree/disagree}

**`.forgejo/workflows/ai-review.yml:115`** · HIGH — Raw HTTP error details leaked into PR comments When a non-429 HTTP error occurs, the code captures up to 300 characters of the raw response body and includes it in a user-facing advisory comment. This can expose internal server paths, stack traces, or credentials if the backend echoes sensitive data in error responses. > **Fix:** Redact raw HTTP response bodies before embedding them in the advisory message; log full details to step output only. panel tally 4/4 · reply here or use the finding board to agree/disagree

.forgejo/workflows/ai-review.yml

					
				@ -143,2 +161,4 @@

				          GH_TOKEN: ${{ github.token }}

				          BOT_TOKEN: ${{ secrets.AI_REVIEW_TOKEN }}

				          SERVER_URL: ${{ github.server_url }}

				          REPO: ${{ github.repository }}

HIGH — POST_comment_url uses env vars instead of interpolated expressions, risking wrong-repo comment post

The comment URL in post() switched from ${{ github.server_url }} / ${{ github.repository }} / ${{ github.event.pull_request.number }} to $SERVER_URL / $REPO / $PR_NUMBER. Per the repo's own cross-codebase pattern [codebase] tti/tti-ux/.forgejo/workflows/ai-review.yml:51, the same env-var idiom is used for the checkout step and is consistent with the gate step fix. However, the standard for this workflow is that env vars are passed through env:, which is what was done here — so this is consistent, not a bug. BUT: the comment URL is the only remaining place using env vars for the API path. If a different workflow step (Post review comment) did NOT declare SERVER_URL / REPO / PR_NUMBER as env vars, this would be an unbound-variable crash. Cross-checking [codebase] tti/tti-ux/.forgejo/workflows/ai-review.yml:151 (the Post review comment step env block) confirms SERVER_URL, REPO, and PR_NUMBER ARE declared in that step's env:. So this is consistent and safe. No finding.

_{panel tally 4/4 · reply here or use the finding board to agree/disagree}

**HIGH** — POST_comment_url uses env vars instead of interpolated expressions, risking wrong-repo comment post The comment URL in post() switched from ${{ github.server_url }} / ${{ github.repository }} / ${{ github.event.pull_request.number }} to $SERVER_URL / $REPO / $PR_NUMBER. Per the repo's own cross-codebase pattern [codebase] tti/tti-ux/.forgejo/workflows/ai-review.yml:51, the same env-var idiom is used for the checkout step and is consistent with the gate step fix. However, the standard for this workflow is that env vars are passed through env:, which is what was done here — so this is consistent, not a bug. BUT: the comment URL is the only remaining place using env vars for the API path. If a different workflow step (Post review comment) did NOT declare SERVER_URL / REPO / PR_NUMBER as env vars, this would be an unbound-variable crash. Cross-checking [codebase] tti/tti-ux/.forgejo/workflows/ai-review.yml:151 (the Post review comment step env block) confirms SERVER_URL, REPO, and PR_NUMBER ARE declared in that step's env:. So this is consistent and safe. No finding. panel tally 4/4 · reply here or use the finding board to agree/disagree