luigidemasi commented on code in PR #149:
URL: https://github.com/apache/camel-kit/pull/149#discussion_r4512797982
##########
kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/OpenCodeGenerator.java:
##########
@@ -1,51 +1,360 @@
package org.apache.camel.kit.generator;
+import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Locale;
import java.util.Map;
+import java.util.Set;
+import java.util.regex.Pattern;
+import org.apache.camel.kit.config.OpenCodeProjectConfig;
import org.apache.camel.kit.util.AnsiColors;
+import com.fasterxml.jackson.databind.JsonNode;
+
public class OpenCodeGenerator extends DefaultGenerator {
private static final String[] AGENTS = {
"brainstormer", "planner", "implementer",
"validator", "tester", "migrator", "executor",
- "researcher", "reviewer"
+ "researcher", "reviewer", "camel-ship-worker"
};
+ private static final Set<String> BUILTIN_PERMISSION_PREFIXES =
Set.of("external");
+
+ private static final Pattern SUBSTITUTION_PATTERN =
Pattern.compile("\\{(env|file):[^}]+}");
+
+ private static final List<String> GLOBAL_CONFIG_FILES =
List.of("opencode.json", "opencode.jsonc");
+
+ private static final List<String> ANCESTOR_CONFIG_FILES = List.of(
+ "opencode.json", "opencode.jsonc",
+ ".opencode/opencode.json", ".opencode/opencode.jsonc");
+
private final QuteTemplateEngine templateEngine = new QuteTemplateEngine();
@Override
public void preflight(InitContext ctx) throws Exception {
- new
OpenCodeConfigMerger().validateExisting(ctx.projectDir().resolve(ctx.agent().mcpConfigPath()));
+ try {
+ new
OpenCodeConfigMerger().validateExisting(ctx.projectDir().resolve(ctx.agent().mcpConfigPath()));
+ } catch (Exception e) {
+ // OpenCode substitutes {env:}/{file:} before parsing, so a
project config that is invalid as raw
+ // JSON may still be valid for the host. Initialization aborts
here, so remove the installed worker:
+ // its denial list can no longer be shown to match the effective
MCP server set. Other validation
+ // failures (e.g. duplicate keys) keep the "fail before any
project writes" contract.
+ if (projectConfigMayContainSubstitution(ctx)) {
Review Comment:
[P1] Invalidate the worker on duplicate-key preflight failures too
The substitution-only condition deliberately preserves the worker for
duplicate keys, but those configs can still load in OpenCode. For example, a
project file with two `mcp` properties, where the second introduces `probe`, is
rejected by Kit's strict duplicate detection while the host uses the latter
value. `InitService` stops in preflight, so the unconditional cleanup in
`generate()` is never reached.
I initialized a workspace, added that config before reinitialization, and
ran the actual CLI with `init --ai opencode --here --force --silent
--no-fetch`. For both `opencode.json` and `.opencode/opencode.json`, it returns
1 and retains the worker byte-for-byte without `probe_*: deny`. Actual OpenCode
2.2.5 child sessions then call `probe.mark` through `execute` and write the
harmless canary; matching namespace-denial controls block both.
Remove this substitution-only exemption: a failed project-config preflight
must invalidate the managed worker whenever its effective permissions cannot be
established. Keep rejecting duplicate config and preserving user configuration,
but adjust `duplicateOpenCodeMcpFailsBeforeAnyProjectWrites` to allow this
safety cleanup and add a new-server duplicate-key reinit regression.
_AI-generated by Codex on behalf of @luigidemasi._
##########
kit/camel-kit-core/src/main/java/org/apache/camel/kit/generator/OpenCodeGenerator.java:
##########
@@ -1,51 +1,360 @@
package org.apache.camel.kit.generator;
+import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashMap;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Locale;
import java.util.Map;
+import java.util.Set;
+import java.util.regex.Pattern;
+import org.apache.camel.kit.config.OpenCodeProjectConfig;
import org.apache.camel.kit.util.AnsiColors;
+import com.fasterxml.jackson.databind.JsonNode;
+
public class OpenCodeGenerator extends DefaultGenerator {
private static final String[] AGENTS = {
"brainstormer", "planner", "implementer",
"validator", "tester", "migrator", "executor",
- "researcher", "reviewer"
+ "researcher", "reviewer", "camel-ship-worker"
};
+ private static final Set<String> BUILTIN_PERMISSION_PREFIXES =
Set.of("external");
+
+ private static final Pattern SUBSTITUTION_PATTERN =
Pattern.compile("\\{(env|file):[^}]+}");
+
+ private static final List<String> GLOBAL_CONFIG_FILES =
List.of("opencode.json", "opencode.jsonc");
+
+ private static final List<String> ANCESTOR_CONFIG_FILES = List.of(
+ "opencode.json", "opencode.jsonc",
+ ".opencode/opencode.json", ".opencode/opencode.jsonc");
+
private final QuteTemplateEngine templateEngine = new QuteTemplateEngine();
@Override
public void preflight(InitContext ctx) throws Exception {
- new
OpenCodeConfigMerger().validateExisting(ctx.projectDir().resolve(ctx.agent().mcpConfigPath()));
+ try {
+ new
OpenCodeConfigMerger().validateExisting(ctx.projectDir().resolve(ctx.agent().mcpConfigPath()));
+ } catch (Exception e) {
+ // OpenCode substitutes {env:}/{file:} before parsing, so a
project config that is invalid as raw
+ // JSON may still be valid for the host. Initialization aborts
here, so remove the installed worker:
+ // its denial list can no longer be shown to match the effective
MCP server set. Other validation
+ // failures (e.g. duplicate keys) keep the "fail before any
project writes" contract.
+ if (projectConfigMayContainSubstitution(ctx)) {
+
Files.deleteIfExists(ctx.projectDir().resolve(".opencode/agents/camel-ship-worker.md"));
Review Comment:
[P2] Reject a symlinked agents directory before deleting the worker
`Files.deleteIfExists` follows symlinks in parent directories.
`InitService.requireRealAgentDirectories` currently checks the skills and
commands paths, so a real `.opencode` directory containing `agents ->
<outside-directory>` passes that guard. This new preflight failure path then
deletes `<outside-directory>/camel-ship-worker.md`.
In an isolated actual-CLI probe, I created that symlink and an outside
sentinel with the worker filename, then supplied the structural-substitution
project config. Both heads return 1, but `7ef2d32a` preserves the outside file
and `ceba494b` deletes it; the symlink itself remains intact. Validation
failure now causes a destructive operation outside the selected workspace.
Apply the existing no-symlink directory boundary to the agents output path
before either new cleanup call. Cover the failing-preflight case and assert
that the outside file remains unchanged.
_AI-generated by Codex on behalf of @luigidemasi._
--
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]
