luigidemasi commented on code in PR #128:
URL: https://github.com/apache/camel-kit/pull/128#discussion_r4366345340
##########
camel-kit-core/src/main/java/io/github/luigidemasi/camelkit/generator/OpenCodeConfigMerger.java:
##########
@@ -77,7 +82,15 @@ private void removeObjectEntries(JsoncObjectEditor editor,
ObjectNode generated,
editor.removeObjectMembers(field, generatedObject.fieldNames());
}
- private String mergeGenerated(ExistingConfiguration existing, ObjectNode
generated) throws IOException {
+ private void removeCitrusEntries(JsoncObjectEditor editor) {
+ editor.removeObjectMembers("mcp",
java.util.Collections.singletonList("citrus").iterator());
+ editor.removeObjectMembers("permission",
+ List.of("citrus_*").iterator());
+ }
+
+ private String mergeGenerated(
+ ExistingConfiguration existing, ObjectNode generated, boolean
withCitrusMcp)
+ throws IOException {
if (isCurrentAndOrdered(existing, generated)) {
Review Comment:
[P2] Include Citrus removal in the early-return condition
The new cleanup at lines 116–118 is bypassed when `isCurrentAndOrdered`
returns true. Reproduced by initializing OpenCode with Citrus, moving the valid
`citrus_*: ask` permission before `camel_*` and `camel-knowledge_*`, then
regenerating without `--with-citrus-mcp`: `citrus.mcp.enabled=false` is saved,
but `mcp.citrus` and `permission.citrus_*` remain and doctor reports PASS.
Those disjoint permission rules can be reordered without changing their
meaning, so this leaves Citrus enabled after a legitimate opt-out. Removing
only the Citrus permission before regeneration also leaves the server behind
and makes doctor fail.
Please allow the early return only when no Citrus server or permission entry
needs removal (as the Antigravity path now does), and add regression tests for
these existing-config shapes while preserving unrelated settings.
Please also cover opt-out with Citrus in a lower `opencode.json` layer and a
higher `opencode.jsonc` layer: inverting the new removal condition in
`removeManagedEntries` still passes all 33 `OpenCodeGeneratorTest` tests. The
current production code passes that layered probe, but the regression is not
protected.
##########
docs/modules/ROOT/pages/commands.adoc:
##########
@@ -31,6 +31,8 @@ camel-kit init --here [options]
| `--source-platform` | `auto` | Source platform of a migration: `mulesoft`,
`biztalk`, `camel` or `auto`
| `--citrus-version` | the configured `citrus.version` ({citrus-version}) |
Citrus version for the test schemas
and the generated test dependencies
+| `--with-citrus-mcp` | `false` | Include the Citrus MCP server in agent
configuration. When omitted, no Citrus
+ server is generated; use this flag if your project needs Citrus test
integration.
Review Comment:
[P2] Identify the opt-in flag and changed default as development-only
The new option and the two-server default are still presented without a
version qualifier here and on the agent/MCP pages. The installation guide
identifies 0.4.1 as the latest release and explicitly promises to mark
unreleased features as development snapshot only. I checked the released 0.4.1
CLI: this option exits with `Unknown option: '--with-citrus-mcp'`; its
templates still include Citrus by default. A reader following the release
installation therefore gets both a rejected command and different startup
behavior.
Please mark the new flag/default as development snapshot only and
distinguish the released behavior, using a shared explanation/link where
appropriate. Also update the PR summary's remaining claim that doctor skips
Citrus checks when disabled: the corrected implementation and changelog
validate any present entry. This is the remaining version/description portion
of the earlier documentation request.
--
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]
