luigidemasi commented on code in PR #128:
URL: https://github.com/apache/camel-kit/pull/128#discussion_r4364295958
##########
camel-kit-core/src/main/java/io/github/luigidemasi/camelkit/command/InitCommand.java:
##########
@@ -48,6 +48,10 @@ public class InitCommand extends CamelKitCommand {
defaultValue = "default")
public String citrusVersion;
+ @Option(names = {"--with-citrus-mcp"},
+ description = "Include the Citrus MCP server in agent
configuration (default: off)")
+ public boolean withCitrusMcp;
Review Comment:
Required before merge (finding 1).
**[P2] Expose and forward the option through `camel kit init`**
(`camel-kit-core/src/main/java/io/github/luigidemasi/camelkit/command/InitCommand.java:51-53`).
- **What this is:** The standalone command now accepts
`--with-citrus-mcp`; the Camel JBang plugin uses the separate `KitInitCommand`
adapter.
- **Problem:** That adapter has neither the option nor an assignment to
`InitCommand.withCitrusMcp`. Plugin initialization now always disables Citrus,
and passing the new flag is rejected as an unknown option.
- **Fix:** Add the option to `KitInitCommand`, forward its value, and
extend `CamelKitCommandParityTest` to exercise both default and opted-in MCP
output through both command surfaces.
- **If we skip it:** Plugin users lose the previously available Citrus
integration with no supported way to enable it through their command.
##########
camel-kit-core/src/main/java/io/github/luigidemasi/camelkit/service/DoctorService.java:
##########
@@ -738,8 +730,17 @@ private void checkCodexMcp(Path root, Path mcpFile,
List<DoctorFinding> findings
root, mcpFile, servers, "camel", expectations.camelMcpTools(),
findings);
boolean knowledgeOk = checkCodexMcpServer(
root, mcpFile, servers, "camel-knowledge",
expectations.knowledgeMcpTools(), findings);
- boolean citrusOk = checkCodexMcpServer(
- root, mcpFile, servers, "citrus",
expectations.citrusMcpTools(), findings);
+ boolean codexCitrusEnabled =
"true".equals(config.getProperty("citrus.mcp.enabled"));
+ if (!config.containsKey("citrus.mcp.enabled") &&
config.containsKey("citrus.mcp.version")) {
+ codexCitrusEnabled = true;
+ }
+ boolean citrusOk;
+ if (codexCitrusEnabled || servers.get("citrus") instanceof TomlTable) {
+ citrusOk = checkCodexMcpServer(
+ root, mcpFile, servers, "citrus",
expectations.citrusMcpTools(), findings);
+ } else {
+ citrusOk = true;
Review Comment:
Required before merge (finding 3).
**[P2] Validate a present Codex Citrus entry even when its type is wrong**
(`camel-kit-core/src/main/java/io/github/luigidemasi/camelkit/service/DoctorService.java:738-742`).
- **What this is:** Doctor must still validate Citrus when a user has
configured it, even if the saved opt-in is false.
- **Problem:** The presence check uses `instanceof TomlTable`. With
`citrus.mcp.enabled=false`, `[mcp_servers]` followed by `citrus =
"invalid-server"` is skipped and doctor reports MCP PASS. The installed Codex
parser rejects that value with `expected struct RawMcpServerConfig`.
- **Fix:** Use `servers.contains("citrus")` to check whether the key
exists, then let `checkCodexMcpServer` validate its type. Add regression cases
for malformed present values with opt-in absent and explicitly false.
- **If we skip it:** Doctor certifies a configuration that prevents the
agent from loading.
##########
CHANGELOG.md:
##########
@@ -19,6 +19,10 @@ and this project adheres to [Semantic
Versioning](https://semver.org/spec/v2.0.0
### Changed
+- **Make Citrus MCP server opt-in (#109)** — `camel-kit init` no longer writes
Citrus MCP server
+ configuration by default. Pass `--with-citrus-mcp` to include it. Workspaces
created before this
+ change that have `citrus.mcp.version` but no `citrus.mcp.enabled` are
treated as implicitly opted
+ in for backward compatibility. `camel-kit doctor` skips Citrus health checks
when not opted in.
Review Comment:
Required before merge (finding 4).
**[P2] Document the new default and opt-in in the active guides**
(`CHANGELOG.md:22-25`; `docs/modules/ROOT/pages/commands.adoc`;
`camel-kit-core/src/main/resources/skills/shared/mcp-setup.md`).
- **What this is:** This changes the default generated workspace and
introduces a public option. `CLAUDE.md` requires corresponding documentation
updates.
- **Problem:** The command reference omits `--with-citrus-mcp`, the
generated shared guide still says Camel Kit configures Citrus, and the
command/architecture pages still describe three required MCP servers. The new
changelog also says doctor skips Citrus checks whenever opt-in is off, although
correctly implemented present-server checks still run and can fail.
- **Fix:** Document the development-version default, how to enable Citrus
on both command surfaces, regeneration behavior, legacy compatibility and
cached-reference fallback. State that doctor permits an absent disabled server
but validates any present server; align the changelog and PR description.
- **If we skip it:** Users and generated agents receive conflicting setup
and troubleshooting instructions. Website impact is **required**; update the
Antora sources in this PR.
##########
camel-kit-core/src/main/java/io/github/luigidemasi/camelkit/generator/McpConfigGenerator.java:
##########
@@ -31,9 +31,10 @@ void generate(InitContext ctx, WorkflowManifest workflow)
throws InvalidAgentCon
QuteTemplateEngine qute = new QuteTemplateEngine();
String template =
TemplateUtils.readTemplate(ctx.agent().mcpConfigTemplatePath());
- String processed = qute.renderString(template,
templateData(ctx.distribution(), workflow));
+ String processed
+ = qute.renderString(template,
templateData(ctx.distribution(), workflow, ctx.withCitrusMcp()));
Review Comment:
Required before merge (finding 2).
**[P2] Remove generated Citrus entries when regenerating without opt-in**
(`camel-kit-core/src/main/java/io/github/luigidemasi/camelkit/generator/McpConfigGenerator.java:34-39`).
- **What this is:** The templates now omit Citrus when disabled, but
OpenCode and Antigravity merge into existing configuration.
- **Problem:** Initialize either target with Citrus, then initialize the
same workspace without the flag using `--force`: properties say
`citrus.mcp.enabled=false`, but the previous Citrus runner remains configured
and doctor reports MCP PASS. The same happens when the retained OpenCode server
is in a lower-precedence configuration file. These mergers only remove names
present in the newly rendered template, which no longer contains Citrus.
- **Fix:** Remove previously generated Citrus server entries when
disabled, including the generated `citrus_*` permission and all OpenCode
configuration layers, while preserving unrelated user configuration. Add
enabled-to-disabled regeneration tests for both targets and layered OpenCode.
- **If we skip it:** Existing migration workspaces still start the
unwanted server and carry its tools despite the recorded opt-out, leaving the
overhead from #109 in place.
--
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]
