[AI-3] Add the LLM connection administration page by tangopium · Pull Request #24880 · opf/openproject

tangopium · 2026-08-22T14:55:00Z

Ticket

What are you trying to accomplish?

PR 2 of 11 in the AI-3 stack. This part adds the admin page itself: the "LLM settings" menu entry under Artificial Intelligence (AI), the connection form (enabled, API format, base URL, write-only API key) and the Contract/Service write path. The update contract probes the server when credentials change, so a failed Connect persists nothing, and probing is guarded on changed attributes so a form render never fires an outbound request. Inline errors distinguish an unreachable host, rejected credentials, a non-OpenAI endpoint and an SSRF-filtered address, the latter naming the allowlist environment variable.

Merge checklist

Added/updated tests
Behind the llm_connection feature flag

Stacked on #24879.

github-actions · 2026-08-22T14:55:35Z

Caution

The provided work package version does not match the core version

Details:

Work package URL: https://community.openproject.org/work_packages/66020
Work package version: not set
Core version: 17.9.0

Please make sure that:

The work package version OR your pull request target branch is correct

github-actions · 2026-08-22T14:55:36Z

Caution

The Enterprise plan field is not set on the work package

Details:

Work package URL: https://community.openproject.org/work_packages/66020

Please make sure that:

The work package Enterprise plan field is set

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fcf7e8fc1c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

github-actions · 2026-08-22T15:27:02Z

Warning

Flaky specs

rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]
rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24880, linked for reference only):

- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]`
- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]`

Treat this as a standalone task, unrelated to PR #24880. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24880 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @tangopium to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @tangopium, and request a review from @tangopium.
On every commit, set @tangopium as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

Administration, Artificial Intelligence (AI), LLM settings: API format, base URL and API key, behind the llm_connection feature flag. Saving is connecting. The contract probes the server before anything is written, following the storages precedent where the Nextcloud credentials validator adds a contract error on 401 and stops the write, so a failed connect persists nothing. The probe fires only when base URL, API key or API format changed. Without that guard every unrelated save, and every form render that builds a model through SetAttributesService, would hit the server. Registry-backed formats skip it entirely: there is nothing at their base URL to probe. The API key is write-only. The stored value is never sent to the browser, a blank submission keeps the current key, and the Stimulus controller wipes the field on turbo:before-cache so the back button cannot restore a typed secret. The base URL is normalised only by trimming whitespace and a trailing slash; the version segment is never rewritten, because silently editing an administrator's URL makes the eventual failure harder to diagnose. A 404 for the model list is not a failure: a server can speak chat perfectly while offering no list, so the save succeeds and later parts of this stack give the administrator ways to name models by hand. Part 2 of the AI-3 stack. https://community.openproject.org/work_packages/66020

github-actions · 2026-08-22T16:13:03Z

Warning

Flaky specs

rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24880, linked for reference only):

- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:2]`

Treat this as a standalone task, unrelated to PR #24880. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24880 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @tangopium to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @tangopium, and request a review from @tangopium.
On every commit, set @tangopium as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

github-actions · 2026-08-23T01:11:19Z

Warning

Flaky specs

rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]

🤖 Ask Copilot to investigate

Copy the prompt below into a new comment on this PR to delegate the investigation to GitHub Copilot. It will look into the flakiness and open a separate pull request with you as reviewer.

@copilot The following spec(s) are flaky in CI (first seen on PR #24880, linked for reference only):

- `rspec ./spec/features/work_packages/table/switch_types_spec.rb[1:1:1]`

Treat this as a standalone task, unrelated to PR #24880. Create a new branch from origin/dev and open a new pull request targeting dev — do not stack it on PR #24880 or reuse that branch.

Follow the playbook in docs/development/testing/handling-flaky-tests/README.md to find the root cause and fix the underlying race — do not skip, delete, or weaken the spec to make it pass; disabling is a last resort per the playbook, and only with a bug ticket. Verify the fix by running the spec(s) repeatedly (e.g. `script/bulk_run_rspec --run-count 10`).

If you cannot reproduce the flake or are not confident in a fix after reasonable investigation, do not fabricate a change or skip the spec to force CI green. Instead, leave the pull request in draft and document what you tried, the suspected cause, and any leads in its description, then assign @tangopium to take over.

Once the fix is verified, title the PR after the spec(s) it fixes, and use the PR description to explain the root cause, how the change resolves it, and the before/after results. Label the PR `flaky-spec`, assign @tangopium, and request a review from @tangopium.
On every commit, set @tangopium as the sole co-author with a `Co-authored-by:` trailer (use their GitHub no-reply email so it links to their account), so it is traceable who dispatched the fix.

thykel

First pass done 🚀

One more thing that comes to mind: These new Admin pages are likely gonna be gated by a subscription tier. The current consensus is that they will be available in Professional and above.

That topic is too big to roll into this PR, so we should definitely establish a separate PR on for it. [Lookbook resource]

thykel · 2026-08-28T10:43:40Z

+      I18n.t("admin.llm_connections.form.api_key_caption#{'_stored' if key}")
+    end
+
+    def api_key_placeholder


So, when we come to this form for the first time, the field is empty.

Then, if the form submission fails (because of a bogus URL), the "API key" field gets cleared and suddenly displays the asterisk placeholder.

Shall we fix this?

For starters, that placeholder should only be displayed if there is an actual value stored already.

However, we probably also want to retain the API key value even once the error hits -- the rest of the form is also retained anyway.

thykel · 2026-08-28T10:43:40Z

+module Admin
+  class LlmConnectionsController < ApplicationController
+    include OpTurbo::ComponentStream
+    include PaginationHelper


🤖 PaginationHelper is not used anywhere in this PR — show is empty and nothing paginates until the model table arrives in #24883. Worth moving to the PR that needs it.

thykel · 2026-08-28T10:43:40Z

+
+    def render_form_with_errors
+      update_via_turbo_stream(component: ::LlmConnections::FormComponent.new(@connection))
+      # The HTML fallback re-renders the whole page, which needs everything the


🤖 This comment describes a show that assigns several things, but in this PR show is def show; end. The comment becomes accurate in #24883. Either trim it here or move it with the code it describes.

thykel · 2026-08-28T10:43:40Z

+      contract.errors.add(:api_key, :invalid_api_key)
+    when Llm::Client::TimeoutError
+      contract.errors.add(attribute, :request_timed_out)
+    when Llm::Client::ConnectionError


🤖 Faraday::SSLError and HTTPX's TLS failures both reach this branch as ConnectionError, so a certificate problem is reported as "could not be reached. Please ensure the LLM server is running and reachable from OpenProject."

The acceptance criteria ask for a distinct SSL error under Base URL. The field placement is already right; only the message is missing. Given that plain http is deliberately allowed for on-premise servers, a TLS failure against an https URL is a case an administrator will hit, and "the server is not running" sends them the wrong way.

If this is deferred on the grounds that the ticket calls its own list incomplete, that is reasonable — worth saying so on the ticket rather than leaving it implicit.

thykel · 2026-08-28T10:43:40Z

+// gives the administrator feedback while that happens, and keeps the typed API
+// key out of Turbo's page cache.
+export default class extends Controller {
+  static targets = ['submitButton', 'progressBanner', 'secretInput'];


🤖 The acceptance criteria include: "When LLM settings checkbox is disabled the form underneath and the LLM panel are hidden."

This controller handles the progress banner and secret clearing, and nothing keys off the Enabled checkbox — the form and (from #24883) the model panel stay visible when it is unchecked. The flag works correctly at runtime, so this is presentation only, but it is an unimplemented criterion rather than a deliberate deviation as far as I can tell.

thykel · 2026-08-28T14:40:28Z

+
+    validates :base_url, presence: true
+    validates :api_format, inclusion: { in: Llm::Adapters::FORMATS }
+    # Bedrock and Vertex AI can be discovered against, but not called: they need


Duplicated comment, let's keep only the one at the source.

thykel · 2026-08-28T14:41:23Z

+    attribute :default_chat_model_id
+    attribute :default_embedding_model_id
+
+    validates :base_url, presence: true


Let's move this next to the other one :)

thykel · 2026-08-28T14:46:46Z

Ouch, what happened with the newline purge? Let's keep this lean, as we use an automated linter.

thykel · 2026-08-28T14:49:16Z

+
+module LlmConnections
+  class ConnectionForm < ApplicationForm
+    form do |f|


Following our Lookbook, let's wrap this in a fieldset_group.

thykel · 2026-08-28T14:51:15Z

+        add_model_description: "If the server does not expose a model list, enter the model name exactly as the server expects it. Manually added models are kept when the list is refreshed."
+        add_model_submit: "Add model"
+        description: "Connect OpenProject to a server that speaks the OpenAI API, so AI features can use it."
+        refresh_models: "Refresh models"


These are not relevant in this PR yet, shall we do a move?

thykel · 2026-08-29T21:45:02Z

+module LlmConnections
+  class ConnectionForm < ApplicationForm
+    form do |f|
+      f.check_box(


Based on our design principles (and the original Figma designs), we only want to expose the rest of the form once "Enable LLMs for this instance" is ticked as enabled.

This was referenced Aug 22, 2026

[AI-3] Let admins disconnect or remove the stored API key #24881

Open

[#66020] Add LLM connection settings to admin AI configuration #24851

Closed

chatgpt-codex-connector Bot reviewed Aug 22, 2026

View reviewed changes

Comment thread config/routes.rb

Comment thread app/validators/llm_server_validator.rb Outdated

tangopium force-pushed the feature/66020-pr02-connection-admin-page branch from fcf7e8f to 9ae73a8 Compare August 22, 2026 15:05

tangopium changed the title ~~[#66020] Add the LLM connection administration page~~ [AI-3] Add the LLM connection administration page Aug 22, 2026

tangopium force-pushed the feature/66020-pr02-connection-admin-page branch from 9ae73a8 to 3dd92bc Compare August 22, 2026 15:51

tangopium force-pushed the feature/66020-pr02-connection-admin-page branch from 3dd92bc to 0b2833b Compare August 22, 2026 16:16

tangopium requested a review from thykel August 22, 2026 21:39

tangopium self-assigned this Aug 22, 2026

thykel reviewed Aug 28, 2026

View reviewed changes

thykel reviewed Aug 29, 2026

View reviewed changes

Uh oh!

Conversation

tangopium commented Aug 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

What are you trying to accomplish?

Merge checklist

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

github-actions Bot commented Aug 22, 2026

Uh oh!

github-actions Bot commented Aug 23, 2026

Uh oh!

thykel left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

2 participants

tangopium commented Aug 22, 2026 •

edited

Loading

thykel left a comment •

edited

Loading