atiaomar1978-hub commented on PR #25368:
URL: https://github.com/apache/camel/pull/25368#issuecomment-5200084729
## Code review fixes (Bugbot + Grok)
_AI-generated summary on behalf of atiaomar1978-hub via Cursor Cloud Agent._
This PR went through automated review (Bugbot + Grok). Below is what was
addressed before marking ready for review.
### Blockers fixed
| Finding | Fix |
|---------|-----|
| **Catalog metadata drift** — `argSchema` missing from `camel-catalog` |
Regenerated and committed `catalog/camel-catalog/.../ai-tool.json` and related
generated docs |
| **NPE on JSON `null`** — `Jsoner.deserialize("null")` returned Java
`null`, then `getClass()` NPE'd | Null-safe parsing in
`AiToolParameterHelper.parseJsonObject()` and
`AiToolSpecToLangChain4j.buildSchemaFromJson()` |
### Security / correctness fixes
| Finding | Fix |
|---------|-----|
| **Empty/missing `properties`** — `{"type":"object"}` passed validation →
empty allowlist | `validateRootSchemaObject()` now requires a non-empty
top-level `properties` map |
| **Required names not in `properties`** — orphan `required` entries caused
executor confusion | Startup validation rejects `required` entries not declared
in `properties` |
| **Required check used unfiltered `arguments` map** — undeclared required
names could pass validation but never reach headers | `AiToolExecutor` now
validates required args against `argsCopy` after allowlist filtering |
| **Malformed `properties`/`required` types** — `ClassCastException` on bad
schemas | Type-checked validation with clear `IllegalArgumentException`
messages |
### LangChain4j adapter
| Finding | Fix |
|---------|-----|
| **Root schemas with `$schema`/`$id` metadata rejected** | Added metadata
stripping fallback before `JsonSchemaElementJsonUtils.fromMap()` |
| **Internal LC4j API usage** | Kept `JsonSchemaElementJsonUtils` (no stable
public alternative); documented limitation for schemas with unsupported root
keywords (`format`, `patternProperties`, object-valued `additionalProperties`) |
### Documentation / metadata
| Finding | Fix |
|---------|-----|
| **`parameter..` description corruption** (annotation strips `*`) |
Rephrased to "parameter multi-value options" |
| **Allowlist behavior undocumented** | Added docs clarifying Camel always
allowlists top-level `properties` names regardless of `additionalProperties` |
### Tests added (AssertJ)
- Invalid JSON / JSON `null`
- Wrong root type, missing/empty `properties`
- Invalid `required` array type
- Required name not in `properties`
- Invalid `properties` type (non-object)
- Executor: required-but-undeclared name rejected after filtering
- Existing nested schema, mutual exclusion, and LangChain4j conversion tests
retained
### Known limitations (accepted)
- LangChain4j conversion may still fail for root schemas with keywords LC4j
cannot model as `JsonObjectSchema` (clear error message at startup/conversion
time)
- `AiToolSpecToLangChain4jTest` still uses JUnit assertions in pre-existing
methods (AssertJ used in new ai-tool tests)
Commits addressing review feedback: `f777588`, `f22f932`.
--
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]
