chennaji9 opened a new pull request, #6841:
URL: https://github.com/apache/rocketmq-dashboard/pull/6841
### Which Issue(s) This PR Fixes
None — small self-contained fix found by inspection (no issue tracked).
### Brief Description
`LlmConfigService.fromGeneralSettings` derives the runtime `enabled` flag as
`!requiresApiKey(provider) || hasText(apiKey)`, where `requiresApiKey` is
provider-based (`!"ollama".equals(provider)`). But the readiness contract in
`LlmConfigVO.isReady()` explicitly allows CLI engines without a key — "CLI
engines authenticate through the subprocess environment" — and the connection
test (`testConfig` → `testCliEngine`) only checks `provider.available()`.
So a non-ollama provider on a CLI engine (claude-code/qoder) with no stored
API key: the connection test reports success ("CLI is available"), yet
`getConfig()` derives `enabled=false` and `isReady()` never reaches its CLI
branch — sending a message then fails with a not-ready configuration even
though the locally authenticated CLI is exactly the supported setup.
The derived flag is now engine-aware: CLI engines no longer require a stored
API key to be enabled, mirroring `isReady()`'s existing CLI semantics. HTTP
engines and the ollama/provider rules are unchanged (an HTTP engine on a remote
provider still requires a key; the env-token override in `getConfig()` still
applies). The odd `!!` double negation in the expression is normalized away.
### How Did You Test This Change?
`mvn -B -ntp test -Dtest=LlmConfigServiceTest` → **Tests run: 33, Failures:
0** (30 existing + 2 new + 1 clarified).
New cases in `LlmConfigServiceTest` (mocked settings, no CLI process
started):
- `getConfigKeepsCliEnginesEnabledWithoutAStoredApiKey` — provider
`deepseek`, engine `claude-code`, no key: enabled and ready;
- `getConfigStillRequiresAStoredKeyForHttpEnginesOnRemoteProviders` — same
provider on engine `http` with no key: not enabled, not ready.
Red side: against the unfixed service, the CLI case fails (`enabled=false`);
the HTTP case passes on both sides, pinning that the requirement for HTTP
engines is untouched.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [x] Tests added or updated for non-trivial changes (2 new red-green tests)
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
(n/a — no UI change)
- [x] Architecture constraints stay green (same module, no new dependency)
- [x] New source files carry the ASF license header (n/a — no new source
file)
- [x] Documentation touched where behaviour changed (n/a — the derived flag
now honours the readiness contract isReady() already documents)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
