123123213weqw commented on PR #4472:
URL:
https://github.com/apache/rocketmq-dashboard/pull/4472#issuecomment-5757306434
Agreed on both halves, and thanks for splitting them instead of rejecting
the pair.
**(a) kept.** `@ToString.Exclude` stays, and its test is renamed to
`toStringShouldNotExposeTheProviderTokenTest`. Your reading is right and the
description now says so: there is no leaking call site today — `LlmProperties`
is only read field by field (`ClaudeCodeAgentProvider`,
`CliProcessEnvironment`, `LlmConfigService`), nothing logs the bean, and
`/actuator/configprops` goes through getters with key sanitization rather than
`toString()`. The exclusion is cheap prevention, not a fix for a live leak.
**(b) dropped.** I could not produce the reachable path you asked for. Both
providers report constants (`claude-code`, `qoder`), so
`forEngine("claude-code")` resolves today, and the duplicate case needs a third
implementation that does not exist. `AgentProviderRegistry` and its test are
reverted, so the pull request is now the two-line annotation plus its test;
title and description are updated to the narrower scope.
```
$ cd server && mvn -B -ntp test
-Dtest=LlmPropertiesTest,AgentProviderRegistryTest
[INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0
[INFO] BUILD SUCCESS
```
Thanks also for the explicit "I cannot find a reachable defect" — that was
the answer I needed to choose between dropping and justifying, and the
tests-asserting-unreachable-behaviour argument is one I will carry into the
next handful of PRs.
--
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]
