Skip to content

revert(dotAI): rollback LangChain4J integration (PRs #35150, #35445, #35456)#35494

Merged
ihoffmann-dot merged 4 commits intorelease/v26.04.28-01from
dot-ai-langchain-rollback
Apr 28, 2026
Merged

revert(dotAI): rollback LangChain4J integration (PRs #35150, #35445, #35456)#35494
ihoffmann-dot merged 4 commits intorelease/v26.04.28-01from
dot-ai-langchain-rollback

Conversation

@ihoffmann-dot
Copy link
Copy Markdown
Member

Summary

Reverts the LangChain4J integration and related changes merged in:

Restores dotAI to the state at tag v26.04.28-01 prior to those merges.

Test plan

  • Verify dotAI app configuration UI works as before
  • Verify AI completions, embeddings, and image generation function with the original OpenAI client

@github-actions github-actions Bot added Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code labels Apr 28, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 28, 2026

Claude finished @ihoffmann-dot's task in 2m 42s —— View job


PR Review

  • Read key changed files
  • Review API contract changes
  • Review app config / secrets schema changes
  • Review data migration risk
  • Review test coverage
  • Post final findings

Issues Found

Bug — AIAppListener.notify() silent logic error (AIAppListener.java:70-73)

When hostAPI.find(...) returns null (host not found or lookup fails), resetModels is correctly skipped, but ConfigService.INSTANCE.config(host) is called with null. ConfigService.config(null) falls back to the system host, so there is no NPE — but validation is then run against the system host config instead of the site that triggered the event. Any misconfiguration on the actual site will not be reported to the user. The fix is to return early when host == null.

// Current
Optional.ofNullable(host).ifPresent(found -> AIModels.get().resetModels(found.getHostname()));
final AppConfig appConfig = ConfigService.INSTANCE.config(host);   // silently uses system host when host == null

// Should be
if (host == null) { return; }
AIModels.get().resetModels(host.getHostname());
final AppConfig appConfig = ConfigService.INSTANCE.config(host);

Fix this →


Bug — AppConfig.isEnabled() is over-restrictive (AppConfig.java, new isEnabled())

return Stream.of(apiUrl, apiImageUrl, apiEmbeddingsUrl, apiKey).allMatch(StringUtils::isNotBlank);

This requires all four fields — including apiImageUrl and apiEmbeddingsUrl — to be non-blank. Any deployment that uses only text completions (no image/embeddings URLs configured) will get isEnabled() = false. The old logic correctly checked whether at least one model was operational. This gates getOrPullSupportedModels (which returns empty set when disabled) and the constructor's loadModels guard — so a partially configured site won't load models even if the API key and text model are set.


Bug — AIModels.notify() wrong default and type (AIModels.java:303)

} else if (event.getKey().contains(AI_MODELS_FETCH_TIMEOUT_KEY)) {
    modelsFetchTimeout.set(Config.getIntProperty(AI_MODELS_FETCH_TIMEOUT_KEY, 14));  // wrong

Uses Config.getIntProperty with default 14 ms, but resolveModelsFetchTimeout() (the canonical initializer) uses Config.getLongProperty(..., 4000). The notify handler would reset the timeout to 14 ms if a system-table update fires with any matching key prefix, effectively hanging all subsequent model fetches. Should be Config.getLongProperty(AI_MODELS_FETCH_TIMEOUT_KEY, 4000).

Fix this →


Info leak — CompletionsResource.getConfig() dumps all config keys (CompletionsResource.java:167-175)

for (final AppKeys config : AppKeys.values()) {
    map.put(config.key, appConfig.getConfig(config));
}

This iterates every AppKeys enum value and writes it into the response. The API key is then masked two lines later, but internal settings like rate limits (textModelTokensPerMinute, apiPerMinute), endpoint URL overrides, cache TTL params, and model name lists are now exposed to any authenticated backend user. The previous getConfig only returned the redacted providerConfig blob and debugLogging. This is a new surface that wasn't present before the LangChain integration and should be scoped explicitly.


Dead / mismatched constants — AppKeys.AI_MODELS_CACHE_TTL / AI_MODELS_CACHE_SIZE (AppKeys.java:52-53, AIModels.java:52-53)

AppKeys declares:

AI_MODELS_CACHE_TTL("com.dotcms.ai.models.supported.ttl", "28800"),
AI_MODELS_CACHE_SIZE("com.dotcms.ai.models.supported.size", "64");

AIModels uses private constants:

private static final int AI_MODELS_CACHE_TTL = 28800;
private static final int AI_MODELS_CACHE_SIZE = 256;

The AppKeys entries are never read by AIModels — they are dead config keys. Worse, the cache size defaults disagree (64 in AppKeys vs 256 in AIModels). These keys should either be read by AIModels or removed from AppKeys.


Silent breakage — sites configured under LangChain4J schema

Sites that stored providerConfig in app_secrets during the LangChain4J window will have that key silently ignored after this rollback; the restored code reads only flat keys (apiKey, textModelNames, etc.). There is no migration, no startup warning, no log. dotAI will appear unconfigured for those sites until they are manually reconfigured. The previous Claude comment proposed a shim in AppConfig — the concern stands.


Minor — AppConfig.toString() format regression (AppConfig.java:312-314)

"  model=" + model + "',\n" +       // was: "  model='" + model + "',\n"
"  imageModel=" + imageModel + "',\n" +
"  embeddingsModel=" + embeddingsModel + "',\n" +

Opening single-quote is missing, producing malformed model=AIModel{...}', output. Low severity but worth a one-char fix.


AIModels.activateModels() marks models ACTIVE without support verification

activateModels sets all configured models to ModelStatus.ACTIVE without querying OpenAI's /v1/models to confirm they are actually available. The support-validation path only runs through getOrPullSupportedModelsAIAppValidator, which is called asynchronously on secret-save events. A misconfigured model name (e.g. a typo) is therefore silently marked ACTIVE and only fails at request time. This is a design choice but worth flagging: the activation and validation flows are decoupled with no fallback if validation never fires.

@ihoffmann-dot ihoffmann-dot changed the base branch from main to release/v26.04.28-01 April 28, 2026 19:16
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Apr 28, 2026

Pull Request Unsafe to Rollback!!!

Category: M-3 — REST / GraphQL / Headless API Contract Change
Risk Level: 🟡 MEDIUM
Why it's unsafe: This rollback removes two REST API additions that were shipped with the LangChain4J integration. Any client or integration that was already built against the LangChain4J contracts (PUT endpoint for saving provider config, siteId query parameter on GET config) will break after this rollback is deployed. Additionally, the dotAI App configuration schema changes from a single JSON providerConfig secret to individual flat-parameter secrets (apiKey, textModelNames, etc.), meaning any site-level providerConfig values already stored in app_secrets will be silently ignored by the reverted code — dotAI will not function until those sites are manually reconfigured with the individual params.
Code that makes it unsafe:

  • dotCMS/src/main/java/com/dotcms/ai/rest/CompletionsResource.java: @PUT @Path("/config") endpoint (saveConfig) is removed entirely; @QueryParam("siteId") is removed from getConfig
  • dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml: PUT /v1/ai/completions/config operation block removed; siteId parameter block removed from GET /v1/ai/completions/config
  • dotCMS/src/main/java/com/dotcms/ai/app/AppKeys.java: PROVIDER_CONFIG("providerConfig", null) enum value removed; replaced by individual model-param keys (TEXT_MODEL_NAMES, IMAGE_MODEL_NAMES, etc.)
  • dotCMS/src/main/resources/apps/dotAI.yml: providerConfig parameter block replaced by individual flat params (apiKey, textModelNames, imageModelNames, embeddingsModelNames, etc.)

Alternative (if possible): To minimize breakage, keep the PUT /v1/ai/completions/config endpoint in place but have it return HTTP 410 Gone, and keep the siteId parameter accepted (but ignored) on the GET endpoint. For the App config migration, add a startup migration shim in AppConfig that reads a stored providerConfig blob and writes it out as individual flat secrets before falling back to the new schema — this prevents silent data loss for sites already configured under the LangChain4J format.

@ihoffmann-dot ihoffmann-dot linked an issue Apr 28, 2026 that may be closed by this pull request
5 tasks
@dotCMS dotCMS deleted a comment from github-actions Bot Apr 28, 2026
@ihoffmann-dot ihoffmann-dot merged commit 7149dce into release/v26.04.28-01 Apr 28, 2026
14 checks passed
@ihoffmann-dot ihoffmann-dot deleted the dot-ai-langchain-rollback branch April 28, 2026 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI: Not Safe To Rollback Area : Backend PR changes Java/Maven backend code Area : Frontend PR changes Angular/TypeScript frontend code

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

[FEATURE] dotAI: LangChain4J integration — Phase 1 (OpenAI)

3 participants