feat(dotAI): Implement new dotAI config page

[KevinDavilaDotCMS]

0817.mov

Summary

Redesigns the dotAI provider configuration page (per-site Chat/Embeddings/Image capabilities plus shared settings) and adds a real "Test Connection" flow, driven entirely by provider metadata from the backend so new providers need no frontend changes.

Backend

• New POST /v1/ai/providers/test/{capability} endpoint (AiProviderResource) that builds the real LangChain4j model for a capability and issues one minimal live call.
• Generic requiredUnless field metadata (ProviderField) so a provider can declare "either this field or that sibling field is required" (e.g. Azure's model/deploymentName) without any provider-specific frontend code.
• Guards against exfiltrating a masked credential (*****) to an attacker-controlled endpoint/provider during connection testing (targetsStoredDestination).
• Capability-aware default timeout for the test call (longer for IMAGE, since real generation routinely exceeds the chat/embeddings default).
• Extracted shared host-resolution logic (lenient vs. strict) into AiHostResolver, used consistently by both the save and test-connection endpoints.

Frontend

• New dot-ai-config-detail page: white background, per-capability cards (Chat/Embeddings/Image), a shared settings card, and an "Additional properties" escape hatch for provider fields not yet modeled.
• Dynamic field rendering (dot-ai-dynamic-field) purely from provider metadata — text/number/secret inputs, required-field markers, and cross-field requiredUnless validation.
• A SECRET field with an already-saved value renders as read-only plain text (no reveal toggle, since the real secret never reaches the browser); editing it switches back to a masked password input so a newly-typed secret isn't shown in clear text.
• "Test Connection" per capability, with the Cancel button appearing only when the form actually differs from the last-saved state.
• All new UI text localized via Language.properties (apps.ai.* keys).

Fixes from review

Addressed the review feedback that was in-scope for this PR (see PR comments for full discussion): additional-properties no longer silently override a real field with the same name and are cleared on provider switch; non-string saved values (e.g. Vertex's JSON credentials) round-trip without String() corruption; the connection-test capability parsing is locale-independent; the requiredUnless cross-field validity fix no longer depends on incidental Angular re-render timing; the page now shows the site it's configuring; and the page blocks Save (instead of silently saving an empty/default config) when the initial load fails.

Testing

• New/updated Jest specs for dot-ai-capability-card, dot-ai-dynamic-field, dot-ai-config-detail, and dot-ai-config.constants.
• New/updated JUnit tests for AiProviderResource, ProviderConnectionTester, and provider metadata.

Checklist

• Tests
• Translations
• Security Implications Contemplated (add notes if applicable)

[claude]

Claude finished @KevinDavilaDotCMS's task in 2m 39s —— View job

---

dotAI Config Page — Re-review

Re-reviewed the latest commit (bf3b4525) against the prior review findings. The great majority of blocking/actionable items are now genuinely fixed in code. One new consistency gap remains, plus the items the author explicitly scoped out.

New Issues

• 🟡 Medium: dot-ai-settings-card.component.ts:136-141 — buildPayloadSection() merges additional properties last with no knownKeys guard, so a manually-added additional-property row whose key collides with a fixed/advanced setting (e.g. imageSize, rolePrompt, embeddingsThreads) silently overwrites the real control's value on save. This is the same class of bug that was flagged HIGH on the capability card and fixed there (dot-ai-capability-card.component.ts:254 now filters !knownNames.has(key)), but the equivalent guard wasn't mirrored here. Lower severity than the card case because it isn't reachable via hydration (known keys are excluded at :94) — it requires the user to type a colliding key by hand. Suggest applying the same knownKeys.has(key) skip in the loop for consistency. Fix this →

Resolved

• ✅ dot-ai-config.constants.ts:210 — imageSize added to SETTINGS_COMMON_FIELDS; no longer double-hydrated into dropdown + additional-property row (was HIGH).
• ✅ dot-ai-config-detail.component.ts:68,159 + .html:53 — loadFailed state now blocks Save ([disabled]="loadFailed()") and save() returns early, so a failed load no longer wipes stored config (was HIGH).
• ✅ dot-ai-capability-card.component.ts:257 — additional properties no longer override a real provider field (!knownNames.has(key)); selectProvider now clears them on provider switch (:189) (was MEDIUM).
• ✅ dot-ai-capability-card.component.ts:296 / constants:151 — non-string saved values round-trip via stringifyForField/parseIfJson instead of String() → no more "[object Object]" corruption (was MEDIUM-LOW).
• ✅ ProviderConnectionTester.java:53,89 — per-capability default timeout (60s for IMAGE, 10s otherwise) prevents spurious image-test timeouts (was MEDIUM).
• ✅ AiProviderResource.java:167,193 — capability parsing uses Locale.ROOT for both toUpperCase/toLowerCase (was LOW, tr_TR breakage).
• ✅ AiProviderResource.java:182 — test path now uses resolveHostStrict (was: lenient fallback resolving masked creds against a different site).
• ✅ dot-ai-capability-card.component.ts:328-333 — requiredUnless recheck now also recomputes the group status (group.updateValueAndValidity), so a hydrated Azure config saved with only deploymentName no longer loads as falsely invalid / Save-blocked.
• ✅ dot-ai-dynamic-field.component.ts:45-71 — currentValue signal tracks live keystrokes, so a freshly-typed secret over the ***** placeholder flips back to the masked p-password input instead of clear text (was LOW security).
• ✅ dot-ai-config-detail.component.ts:79 + .html:10-14 — page now surfaces the configured site name.
• ✅ core-web/.sdkmanrc — removed from the PR.

Existing

These were acknowledged by the author as out-of-scope follow-ups (reasonable, but tracking them here):

• 🟡 Medium: ProviderConnectionTester.java:100-118 — BedrockRuntimeClient/VertexAI (AutoCloseable) built per test call and never closed; repeated "Test Connection" clicks leak connection pools/threads. Pre-existing in LangChain4jModelFactory's build path, not introduced here. Worth a follow-up.
• 🟡 Medium: AiProviderResource.java:153-155 + openapi.yaml — test-connection request body is typed String with @Schema(implementation = Map.class), so the generated spec emits type: string for a JSON-object body. A typed request DTO would fix both the spec and the "@Schema must match actual type" rule.
• 🟡 Medium: AiProviderResource.java:209 — ProviderConfig is @JsonIgnoreProperties(ignoreUnknown = true), so free-form "Additional properties" are dropped before the test client is built; a green result may not reflect what Save will actually store. Consider surfacing "test ignores additional properties" in the UI.
• 🟡 Medium: dot-ai-capability-card.component.ts:224 / ProviderConnectionTester.friendlyMessage — raw provider SDK error text is forwarded to the UI; some SDKs embed the request URL (which can carry an API key as a query param). Also mixed i18n semantics in testResult.message (local keys vs. raw backend strings) only works because DotMessageService.get() echoes unknown keys.
• 🟢 Low: dot-ai-dynamic-field.component.ts:78 — humanizeFieldName() derives English labels from field names, so provider field labels ship untranslated unlike the rest of the page.

Semgrep's LangChain4j finding was correctly triaged as acceptable risk (fixed hardcoded test prompt, no untrusted input).

Nothing blocking from my pass — the one new item (settings-card additional-property collision) is non-blocking but cheap to close for parity with the capability-card fix.
· 36826-dotai-provider-configuration-ui

[github-actions]

🐳 PR Docker test image

Latest build for commit bf3b452 pushed to dotcms/dotcms-test:

docker pull dotcms/dotcms-test:pr-37048-36826-dotai-provider-configuration-ui
docker pull dotcms/dotcms-test:pr-37048-36826-dotai-provider-configuration-ui_bf3b452

[rjvelazco]

Issues

Important

1. onlySelf: true leaves the parent FormGroup stale-INVALID

• File: dot-ai-capability-card.component.ts:~320 (the requiredUnlessFields recheck).
• Mechanism: model is constructed before deploymentName exists.
• Result: its validator sees no parent, returns an error, and the group caches INVALID.
• The recheck flips model to VALID but onlySelf: true skips parent recalculation.
• User impact: an Azure config saved with only deploymentName loads as invalid.
• Consequence: Save and Test Connection stay blocked until the user touches a field.
• Contradiction: the spec at line 254 asserts the opposite, so that test should be failing.
• Verify: pnpm nx test dotcms-ui --testPathPattern=dot-ai-capability-card.
• Fix: drop onlySelf: true, keep emitEvent: false.

2. Bedrock and Vertex clients are never closed

• File: ProviderConnectionTester.java:~87 (test()).
• Issue: each call builds a model wrapping an AutoCloseable SDK client, then discards it.
• Affected: BedrockRuntimeClient / BedrockRuntimeAsyncClient and VertexAI.
• Why it matters: repeated admin "Test connection" clicks accumulate connection pools and threads.
• Verify: whether LangChain4j closes these itself or registers a shutdown hook.
• Fix: close the client after the test call if it does not.

3. Test path uses lenient host resolution, save uses strict

• File: AiProviderResource.java (testConnection calls AiHostResolver.resolveHost).
• Behavior: an unresolvable siteId silently falls back to the current host.
• Effect: masked credentials then resolve against a different site's stored config.
• Risk: an admin gets a misleading "success" for a site they never tested.
• Fix: use resolveHostStrict for consistency with PUT /v1/ai/completions/config.

Minor

• Locale-sensitive enum parsing: capability.toUpperCase() breaks under tr_TR.
• Fix: pass Locale.ROOT to both toUpperCase() and toLowerCase().
• Dynamic field labels bypass i18n: humanizeFieldName() derives English from field names.
• Consequence: every provider field label ships untranslated, unlike the rest of the page.
• Raw provider errors reach the UI: friendlyMessage() forwards SDK exception text verbatim.
• Risk: some SDKs embed the request URL, which can carry the API key as a query param.
• Mixed message semantics: testResult.message holds i18n keys locally, raw English from the server.
• Fragility: this only works because DotMessageService.get() echoes unknown keys.
• Unrelated file: core-web/.sdkmanrc pins Java 25 inside the frontend directory.
• Question: does this belong in this PR, or at the repo root?
• Stale subscriptions: rebuildFieldsGroup re-subscribes on every provider switch.
• Impact: harmless in practice, since discarded groups never emit again.
• Check unused imports in CompletionsResource after the four private methods were removed.

Recommendations

• Add a spec for dot-ai-config-detail.component.ts: dirty tracking and payload assembly are untested.
• Confirm clearing behavior: buildPayloadSection drops empty values from the payload.
• Open question: can a user actually unset a previously-saved optional field like temperature?
• Consider a contract test asserting every strategy's configFields throws only for unsupported capabilities.
• Untick items: the PR body still has placeholder text ("change 1", "original screenshot") and unchecked boxes.

[zJaaal]

Review: correctness pass on the new dotAI config page

Read every hunk plus surrounding context. 9 inline findings below, ordered roughly by severity - the two flagged HIGH are both silent-data-loss paths in the save flow, so they are the ones worth looking at first.

One more finding that has no good inline anchor:

LOW - dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml (/v1/ai/providers/test/{capability}): the generated requestBody schema is type: string, taken from the String body parameter, even though the endpoint expects a JSON object. The @RequestBody(content = @Content(schema = @Schema(implementation = Map.class))) annotation is not winning over the parameter type, so clients generated from the spec will send a JSON-encoded string. A typed request DTO would fix both the spec and the CLAUDE.md "@Schema must match the actual type" rule.

Checked and confirmed not bugs (so they do not get re-litigated)

• The updateValueAndValidity({ onlySelf: true }) recheck in rebuildFieldsGroup does leave the parent group's aggregate status stale (verified with a throwaway jest probe: the child flips to valid, the group does not). It is harmless in practice because FormGroupDirective.addControl re-validates each control with parent propagation on render, and PrimeNG's p-panel projects its content even when collapsed - so every requiredUnless field is always registered and the group status is repaired before any user action. Worth knowing it depends on that, though.
• AiProviderResource:199 does not leak the resolved secret through the Jackson 400 message: Jackson 2.16+ disables INCLUDE_SOURCE_IN_LOCATION by default (verified against 2.17.2, the version in bom/application/pom.xml), so the source snippet is redacted.
• API_ENDPOINT + /providers resolves correctly to /api/v1/ai/providers.
• pInputText does expose an invalid input in PrimeNG 21.1.3, so the [invalid] bindings compile.
• Every key in SETTINGS_ADVANCED_FIELDS matches an AppKeys settingsKey.
• ProviderConfigMerger.containsMasked / containsMaskedCredential exist with the semantics the new resource assumes, and ProviderConfig is @JsonIgnoreProperties(ignoreUnknown = true), so the free-form additional properties do not break deserialization.

---

Automated review authored by Claude (Claude Code), posted from @zJaaal's account.

[ihoffmann-dot]

It looks good overall! In addition to Jal's feedback I found these:

🟡 The page no longer shows which site's configuration is being edited

providerConfig is per-site and the route carries a site id, but the removed header was what displayed app.sites?.[0]?.name. On a multi-site instance this invites a mistake: pasting a production API key into another site's configuration without noticing.

Suggestion: surface the site name in the page heading

🟢 Non-blocking: Test Connection silently ignores additional properties

The additional properties escape hatch lets users add keys the form doesn't model, and those keys are persisted on save. But the test endpoint deserializes the body into ProviderConfig, which is @JsonIgnoreProperties, so they're dropped before the provider client is built. A green "Connection successful" therefore doesn't reflect the configuration that will actually be stored, and if the extra property was the one that made it work, the test gives false confidence.

Suggestion: Just flagging it. Options if you want to address it later: reject unknown keys with a notice, or state in the UI that the test ignores them.

[KevinDavilaDotCMS]

Went through the rest of the feedback and fixed everything actionable in the last commit (data-loss on load failure, additional-properties override/clear-on-switch, imageSize duplication, image test timeout, locale-sensitive parsing, secret re-masking, missing site name, host-resolution consistency). A few remaining items I'm intentionally leaving as-is for this PR — flagging why instead of silently skipping them:

Bedrock/Vertex client leak on Test Connection (@rjvelazco, also flagged by the automated review) — real concern, but it's a pre-existing gap in how LangChain4jModelFactory builds these clients generally, not something introduced by the test-connection endpoint. Scoping a proper fix (closing/pooling BedrockRuntimeClient/VertexAI) to this PR would mean touching the shared model-building path for a feature this PR doesn't otherwise change. Worth its own follow-up.

openapi.yaml schema is type: string instead of an object (@zJaaal) — correct, the @Schema(implementation = Map.class) annotation isn't winning over the raw String body parameter type. Fixing it means introducing a typed request DTO for the test-connection body, which is more surface area than this PR's scope; the endpoint's actual behavior (accepts a JSON object) isn't affected, just the generated spec's accuracy for downstream client generation.

Test Connection ignores additional properties (@ihoffmann-dot) — accurate: ProviderConfig is @JsonIgnoreProperties(ignoreUnknown = true), so an extra-property key that happens to matter isn't exercised by the test call. Given how narrow that case is (an unmodeled field that's also load-bearing for connectivity), I'd rather leave it as a known limitation than add scope now; happy to open a follow-up issue if this turns out to bite someone in practice.

Mixed i18n semantics in testResult.message (@rjvelazco) — also accurate: some messages are local i18n keys, others are raw strings from the backend, and it only reads correctly because DotMessageService.get() echoes unknown keys back unchanged. Untangling that cleanly means deciding how (or whether) to localize arbitrary provider error text, which felt like a separate, bigger conversation than this PR's scope.