nimmo/forgejo-auto-pr-review
Watch
1
0
Fork
You've already forked forgejo-auto-pr-review
 0
Code Issues 5 Pull requests Projects Releases Packages 1 Activity Actions

Graceful first-PR config fallback #22

New issue
Open
opened 2026-07-23 17:17:25 +01:00 by nimmo · 0 comments
nimmo commented 2026-07-23 17:17:25 +01:00
Owner
Copy link

Summary

When a pull request introduces .forgejo/ai-review.yml itself (the first PR that adds the review configuration to a repository), the review workflow fails with provider.model or OLLAMA_MODEL is required because the action reads the configuration from the base commit (the trusted revision), and the base commit does not yet contain the file.

This is a chicken-and-egg problem: the very first PR that adds the review setup cannot itself be reviewed by it.

Reproduction

  1. Create a new repository without any .forgejo/ai-review.yml.
  2. Add the standard AI review workflow (.forgejo/workflows/ai-review.yml) and a new .forgejo/ai-review.yml describing the provider.
  3. Open a pull request that contains both files.
  4. The review job fails immediately with: 
    forgejo-ai-review: provider.model or OLLAMA_MODEL is required
  5. Inspecting reviewer.py confirms the cause: fetch_config() returns {} on 404 (the base commit does not have the file yet), and provider_request() then raises ValueError("provider.model or OLLAMA_MODEL is required") because provider.get("model", os.environ.get("OLLAMA_MODEL")) finds neither.

Suggested behavior

The action should degrade gracefully when the trusted-base config is missing, instead of hard-failing:

  1. Detect missing config. If fetch_config() returns {} (or throws a non-404 error), log a clear diagnostic rather than letting the missing provider.model raise later.
  2. Allow model from env vars as a fallback. OLLAMA_MODEL and OLLAMA_BASE_URL are already consulted as fallbacks in provider_request(); the same fallback chain should also apply when the config object is empty so a degraded review (no analysis.commands, default review settings) can still run.
  3. Disable deterministic checks when config is empty. If analysis.commands is missing, the run_checks() loop simply iterates over nothing — that already works. The reviewer should mention in the rendered comment that deterministic checks were not configured.
  4. Emit a warning in the published comment so the user knows the review ran with defaults rather than the repository's preferred configuration.

Workaround (for context)

Until this is fixed, consumers can hard-code the fallback in the workflow step that calls the action:

- uses: https://git.nimmog.uk/nimmo/forgejo-auto-pr-review@main
  env:
    OLLAMA_MODEL: qwen3.6:35b-a3b-q8_0
    OLLAMA_BASE_URL: http://nova.mesh:11434
  with:
    token: ${{ secrets.AI_REVIEW_FORGEJO_TOKEN }}
    pull-request: ${{ inputs.pull_request }}

The reviewer already reads these as fallbacks, so this gives a degraded review on the bootstrap PR. But pushing the model/endpoint URL into two places (config file and workflow env) is brittle and duplicates a value that should live in one place.

Environment

  • forgejo-auto-pr-review revision: current main (commit 1d35f73 at time of writing)
  • Repository introducing the setup: redview (Node.js Express app), PR #1
  • Base config path: .forgejo/ai-review.yml
  • Provider: ollama on nova.mesh:11434, model qwen3.6:35b-a3b-q8_0

Happy to send a PR against this repo once we agree on the desired behavior.

## Summary When a pull request introduces `.forgejo/ai-review.yml` itself (the first PR that adds the review configuration to a repository), the review workflow fails with `provider.model or OLLAMA_MODEL is required` because the action reads the configuration from the **base commit** (the trusted revision), and the base commit does not yet contain the file. This is a chicken-and-egg problem: the very first PR that adds the review setup cannot itself be reviewed by it. ## Reproduction 1. Create a new repository without any `.forgejo/ai-review.yml`. 2. Add the standard AI review workflow (`.forgejo/workflows/ai-review.yml`) and a new `.forgejo/ai-review.yml` describing the provider. 3. Open a pull request that contains both files. 4. The `review` job fails immediately with: ``` forgejo-ai-review: provider.model or OLLAMA_MODEL is required ``` 5. Inspecting `reviewer.py` confirms the cause: `fetch_config()` returns `{}` on 404 (the base commit does not have the file yet), and `provider_request()` then raises `ValueError("provider.model or OLLAMA_MODEL is required")` because `provider.get("model", os.environ.get("OLLAMA_MODEL"))` finds neither. ## Suggested behavior The action should degrade gracefully when the trusted-base config is missing, instead of hard-failing: 1. **Detect missing config.** If `fetch_config()` returns `{}` (or throws a non-404 error), log a clear diagnostic rather than letting the missing `provider.model` raise later. 2. **Allow model from env vars as a fallback.** `OLLAMA_MODEL` and `OLLAMA_BASE_URL` are already consulted as fallbacks in `provider_request()`; the same fallback chain should also apply when the config object is empty so a degraded review (no `analysis.commands`, default review settings) can still run. 3. **Disable deterministic checks when config is empty.** If `analysis.commands` is missing, the `run_checks()` loop simply iterates over nothing — that already works. The reviewer should mention in the rendered comment that deterministic checks were not configured. 4. **Emit a warning in the published comment** so the user knows the review ran with defaults rather than the repository's preferred configuration. ## Workaround (for context) Until this is fixed, consumers can hard-code the fallback in the workflow step that calls the action: ```yaml - uses: https://git.nimmog.uk/nimmo/forgejo-auto-pr-review@main env: OLLAMA_MODEL: qwen3.6:35b-a3b-q8_0 OLLAMA_BASE_URL: http://nova.mesh:11434 with: token: ${{ secrets.AI_REVIEW_FORGEJO_TOKEN }} pull-request: ${{ inputs.pull_request }} ``` The reviewer already reads these as fallbacks, so this gives a degraded review on the bootstrap PR. But pushing the model/endpoint URL into two places (config file and workflow env) is brittle and duplicates a value that should live in one place. ## Environment - `forgejo-auto-pr-review` revision: current `main` (commit `1d35f73` at time of writing) - Repository introducing the setup: redview (Node.js Express app), PR #1 - Base config path: `.forgejo/ai-review.yml` - Provider: ollama on `nova.mesh:11434`, model `qwen3.6:35b-a3b-q8_0` Happy to send a PR against this repo once we agree on the desired behavior.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
main
fix/action-yaml-parser
issue-4-normalize-analysis
No results found.
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
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/forgejo-auto-pr-review#22
No description provided.