feat(config): add MCP server configuration support#239
Merged
feloy merged 3 commits intoopenkaiden:mainfrom Apr 10, 2026
Merged
feat(config): add MCP server configuration support#239feloy merged 3 commits intoopenkaiden:mainfrom
feloy merged 3 commits intoopenkaiden:mainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 24 minutes and 1 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds MCP (Model Context Protocol) server support: workspace schema for command-based (stdio) and SSE servers, validation and name-based merge behavior, Agent API extension with SetMCPServers, Claude implementation to update .claude.json, and manager wiring to apply MCP settings during instance creation. (45 words) Changes
Sequence DiagramsequenceDiagram
actor User
participant Manager
participant Config
participant Agent as ClaudeAgent
participant Settings as AgentSettings
User->>Manager: Add(workspace with MCP)
Manager->>Config: Merge(base, workspaceOverride)
Config-->>Manager: mergedConfig (includes Mcp)
Manager->>Agent: SetMCPServers(settings, mergedConfig.Mcp)
Agent->>Settings: Read existing .claude.json (or init {})
Agent->>Settings: Merge/Populate mcpServers from Commands (stdio) and Servers (sse)
Agent->>Settings: Marshal JSON and write back into settings map
Agent-->>Manager: Return updated settings
Manager->>Manager: Inject settings into runtime context
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkg/agent/agent.go (1)
26-48:⚠️ Potential issue | 🟠 Major
Agentinterface is still missing requiredSkillsDir() string.While adding
SetMCPServers, the interface should also exposeSkillsDir()to match the repository’s agent contract.Suggested interface adjustment
type Agent interface { // Name returns the agent name (e.g., "claude", "goose"). Name() string + // SkillsDir returns the container path (using $HOME) where skills should be mounted. + SkillsDir() string // SkipOnboarding modifies agent settings to skip onboarding prompts. SkipOnboarding(settings map[string][]byte, workspaceSourcesPath string) (map[string][]byte, error)Based on learnings: Applies to pkg/agent/**/*.go : The
Agentinterface (pkg/agent/agent.go) must exposeSkillsDir() stringwhich returns the container path using the$HOMEvariable where skill directories should be mounted.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/agent/agent.go` around lines 26 - 48, The Agent interface is missing the required SkillsDir() string method; add SkillsDir() string to the Agent interface declaration alongside existing methods (Name, SkipOnboarding, SetModel, SetMCPServers) so implementations must return the container path (using $HOME) where skill directories are mounted; update any implementing types (e.g., concrete agent structs that implement Agent) to include a SkillsDir method returning the appropriate path.
🧹 Nitpick comments (3)
pkg/instances/manager_test.go (1)
3552-3669: Add an MCP error-propagation test formanager.Add().Current coverage checks call/no-call only. Please add a subtest where
SetMCPServers()returns an error and assertAdd()fails with the expected wrapped message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/instances/manager_test.go` around lines 3552 - 3669, Add a third subtest to TestManager_Add_AppliesAgentMCPServers that registers an agent whose SetMCPServers(ctx, ...) method returns a controlled error and then calls manager.Add(...) with a workspace MCP config; assert that Add returns a non-nil error and that its error string wraps/includes the expected message from SetMCPServers (e.g., "SetMCPServers" or the specific error text). Locate the tracking agent implementation used in tests (newTrackingAgent or create a small failingTrackingAgent) and make it return the error from SetMCPServers, call manager.Add with WorkspaceConfig.Mcp set, and check the returned error matches the wrapped message to ensure propagation.skills/working-with-config-system/SKILL.md (1)
72-82: Consider restructuring to improve readability.The static analysis tool flagged that three successive sentences begin with "If". While the content is correct, consider restructuring for better flow.
📝 Suggested improvement
-**Implementation:** `manager.readAgentSettings(storageDir, agentName)` in `pkg/instances/manager.go` walks this directory and returns a `map[string][]byte` (relative forward-slash path → content). If the agent is registered in the agent registry, the manager calls the agent's `SkipOnboarding()` method to modify the settings. If a model ID is provided, the manager then calls the agent's `SetModel()` method. If the merged config contains MCP servers, the manager calls the agent's `SetMCPServers()` method. The final map is passed to the runtime via `runtime.CreateParams.AgentSettings`. The Podman runtime writes the files into the build context and adds a `COPY --chown=agent:agent agent-settings/. /home/agent/` instruction to the Containerfile. The model is also stored directly in the `instance` struct and persisted in `instances.json` via `InstanceData.Model`. +**Implementation:** `manager.readAgentSettings(storageDir, agentName)` in `pkg/instances/manager.go` walks this directory and returns a `map[string][]byte` (relative forward-slash path → content). For registered agents, the manager calls `SkipOnboarding()` to modify the settings. When a model ID is provided, the manager then calls `SetModel()`. Similarly, when the merged config contains MCP servers, the manager calls `SetMCPServers()`. The final map is passed to the runtime via `runtime.CreateParams.AgentSettings`. The Podman runtime writes the files into the build context and adds a `COPY --chown=agent:agent agent-settings/. /home/agent/` instruction to the Containerfile. The model is also stored directly in the `instance` struct and persisted in `instances.json` via `InstanceData.Model`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/working-with-config-system/SKILL.md` around lines 72 - 82, Rewrite the paragraph in SKILL.md to improve flow by combining or reordering the three consecutive sentences that start with "If": reference the implementation steps involving manager.readAgentSettings, the agent registry call to SkipOnboarding(), the conditional SetModel(modelID) call, and the conditional SetMCPServers() call so they read as a cohesive sequence rather than three standalone "If" sentences; keep the same facts (map returned by readAgentSettings, model stored in InstanceData.Model, agent settings passed via runtime.CreateParams.AgentSettings, Podman COPY instruction) and preserve mention of where MCP servers are merged and how they are applied to agents (SetMCPServers), but restructure into 2–3 varied-start sentences for better readability.pkg/agent/claude_test.go (1)
499-516: Consider using JSON comparison instead of string comparison.The test at line 513 compares the raw JSON bytes as strings. This works but is fragile - if the implementation changes JSON formatting (e.g., adds indentation or reorders keys), the test would fail even though the content is semantically identical. Consider parsing the result and comparing the
hasCompletedOnboardingfield directly, similar to other tests in this file.♻️ Suggested improvement
// Settings should be returned unchanged - if string(result[ClaudeJSONPath]) != `{"hasCompletedOnboarding": true}` { - t.Errorf("SetMCPServers() with nil MCP modified settings unexpectedly: %s", result[ClaudeJSONPath]) + var config map[string]interface{} + if err := json.Unmarshal(result[ClaudeJSONPath], &config); err != nil { + t.Fatalf("Failed to parse result JSON: %v", err) + } + if _, hasMCP := config["mcpServers"]; hasMCP { + t.Error("SetMCPServers() with nil MCP should not add mcpServers") + } + if config["hasCompletedOnboarding"] != true { + t.Error("hasCompletedOnboarding was not preserved") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/agent/claude_test.go` around lines 499 - 516, The test TestClaude_SetMCPServers_NilMCP should stop comparing raw JSON strings and instead parse the returned JSON bytes from result[ClaudeJSONPath] (from the SetMCPServers call on NewClaude) and assert the hasCompletedOnboarding field value; update the test to json.Unmarshal the bytes into a map or struct and check that hasCompletedOnboarding == true (and fail the test if unmarshal errors or the field isn't the expected boolean).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/config/merger.go`:
- Around line 166-185: The mergeMCP logic must prevent same-name entries from
different MCP types (Commands vs Servers) surviving and later colliding in the
flattened mcpServers key: update mergeMCP (and corresponding logic used in the
187-255 region) to detect cross-type name conflicts by building a name->type map
that prefers entries from the override side (respecting
Agent>Project>Global>Workspace precedence) and then filter out or shadow entries
of the other type from the base; specifically, run conflict resolution after
calling mergeMCPCommands and mergeMCPServers (or merge earlier by merging name
maps in mergeMCP), ensuring names present in override commands remove any base
servers with the same name and names present in override servers remove any base
commands with the same name so the final result cannot contain both types with
the same name that would later write to the same mcpServers key.
- Around line 257-274: copyMCP currently performs shallow copies of the Commands
and Servers slices which still alias nested pointer fields
(workspace.McpCommand.Args, workspace.McpCommand.Env, and
workspace.McpServer.Headers); update copyMCP to deep-copy each command and
server instead of using copy(): iterate over *mcp.Commands and for each
workspace.McpCommand allocate a new workspace.McpCommand, copy primitive fields,
and deep-copy any pointer fields (allocate new slices/maps for Args and Env and
copy their contents), then append to cmdsCopy; do the same for *mcp.Servers and
deep-copy Headers into new allocations before assigning result.Commands and
result.Servers so the returned McpConfiguration has no shared mutable references
with the input.
---
Outside diff comments:
In `@pkg/agent/agent.go`:
- Around line 26-48: The Agent interface is missing the required SkillsDir()
string method; add SkillsDir() string to the Agent interface declaration
alongside existing methods (Name, SkipOnboarding, SetModel, SetMCPServers) so
implementations must return the container path (using $HOME) where skill
directories are mounted; update any implementing types (e.g., concrete agent
structs that implement Agent) to include a SkillsDir method returning the
appropriate path.
---
Nitpick comments:
In `@pkg/agent/claude_test.go`:
- Around line 499-516: The test TestClaude_SetMCPServers_NilMCP should stop
comparing raw JSON strings and instead parse the returned JSON bytes from
result[ClaudeJSONPath] (from the SetMCPServers call on NewClaude) and assert the
hasCompletedOnboarding field value; update the test to json.Unmarshal the bytes
into a map or struct and check that hasCompletedOnboarding == true (and fail the
test if unmarshal errors or the field isn't the expected boolean).
In `@pkg/instances/manager_test.go`:
- Around line 3552-3669: Add a third subtest to
TestManager_Add_AppliesAgentMCPServers that registers an agent whose
SetMCPServers(ctx, ...) method returns a controlled error and then calls
manager.Add(...) with a workspace MCP config; assert that Add returns a non-nil
error and that its error string wraps/includes the expected message from
SetMCPServers (e.g., "SetMCPServers" or the specific error text). Locate the
tracking agent implementation used in tests (newTrackingAgent or create a small
failingTrackingAgent) and make it return the error from SetMCPServers, call
manager.Add with WorkspaceConfig.Mcp set, and check the returned error matches
the wrapped message to ensure propagation.
In `@skills/working-with-config-system/SKILL.md`:
- Around line 72-82: Rewrite the paragraph in SKILL.md to improve flow by
combining or reordering the three consecutive sentences that start with "If":
reference the implementation steps involving manager.readAgentSettings, the
agent registry call to SkipOnboarding(), the conditional SetModel(modelID) call,
and the conditional SetMCPServers() call so they read as a cohesive sequence
rather than three standalone "If" sentences; keep the same facts (map returned
by readAgentSettings, model stored in InstanceData.Model, agent settings passed
via runtime.CreateParams.AgentSettings, Podman COPY instruction) and preserve
mention of where MCP servers are merged and how they are applied to agents
(SetMCPServers), but restructure into 2–3 varied-start sentences for better
readability.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 77a0c914-0877-4d04-ba42-6df11c3faeea
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
AGENTS.mdREADME.mdgo.modpkg/agent/agent.gopkg/agent/claude.gopkg/agent/claude_test.gopkg/agent/cursor.gopkg/agent/goose.gopkg/agent/registry_test.gopkg/config/config.gopkg/config/config_test.gopkg/config/merger.gopkg/config/merger_test.gopkg/instances/manager.gopkg/instances/manager_test.goskills/working-with-config-system/SKILL.mdskills/working-with-instances-manager/SKILL.md
Allows users to configure MCP servers (command-based and URL-based) in workspace.json and in the multi-level config system (projects.json, agents.json). The manager calls SetMCPServers() on the registered agent after SetModel(), writing the merged MCP config into agent settings. - Upgrade kdn-api/workspace-configuration/go to v0.2.0 which adds McpConfiguration, McpCommand, and McpServer types - Add SetMCPServers() to the Agent interface; Claude writes command servers as type "stdio" and URL servers as type "sse" into the top-level mcpServers key of .claude.json (user scope); Goose and Cursor are no-ops - Add MCP merge logic to ConfigMerger (merge by name, override wins) - Add MCP validation: non-empty name/command/url required; names must be unique across both commands and servers combined since they are written to a single flat mcpServers map - Wire SetMCPServers() into manager.Add() after SetModel() - Add tests for all new functionality - Update README.md, CLAUDE.md, and skills documentation Closes openkaiden#210 Co-Authored-By: Claude Code (claude-sonnet-4-6) <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
pkg/config/merger.go (2)
217-250:⚠️ Potential issue | 🟡 MinorDeep-copy nested MCP fields before returning merged configs.
These helpers move
workspace.McpCommandandworkspace.McpServervalues around by assignment/copy(), but their nestedArgs,Env, andHeadersmembers are still shared. That means the returned config can alias the inputs even though these helpers are documented as producing independent copies. Please confirm the v0.2.0 workspace types still use those pointer-bearing fields, then clone each entry deeply in both the merge helpers andcopyMCP().Also applies to: 252-304
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pkg/config/merger.go` around lines 217 - 250, The merge helpers currently copy workspace.McpCommand and workspace.McpServer by value but leave nested pointer-bearing fields (Args, Env, Headers) shared; update mergeMCPCommands, the corresponding mergeMCPServers (lines ~252-304), and copyMCP to perform deep clones of each entry: allocate a fresh workspace.McpCommand/McpServer for each merged item and clone slice/map fields (Args, Env, Headers, etc.) into new slices/maps so no returned struct aliases the inputs; first verify the v0.2.0 workspace types still define those fields as pointer-bearing/compound types, then implement the deep-copy logic in those three functions.
193-215:⚠️ Potential issue | 🟠 MajorResolve cross-type MCP name collisions during merge.
mergeMCP()currently merges commands and servers independently, so a lower-precedenceserverand a higher-precedencecommandwith the sameNamecan both survive. Claude later flattens both lists into onemcpServersmap, which lets the lower-precedence entry silently overwrite the higher-precedence one depending on write order. Filter same-name command/server clashes as part of the merge, with the override side winning in both directions. A regression test for server→command and command→server clashes would pin this.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@skills/working-with-config-system/SKILL.md`:
- Around line 186-195: Clarify that MCP server names must be globally unique
across both lists: update the field descriptions for `mcp` so `commands[].name`
and `servers[].name` explicitly state the name must be unique across all MCP
entries (not just within their respective `commands` or `servers` lists),
mentioning that the loader flattens entries into a single `mcpServers` map; edit
the bullets for `commands` and `servers` to include this cross-list uniqueness
requirement and reference the `mcp` top-level field and `mcpServers` map so
readers know why duplicates are rejected.
---
Duplicate comments:
In `@pkg/config/merger.go`:
- Around line 217-250: The merge helpers currently copy workspace.McpCommand and
workspace.McpServer by value but leave nested pointer-bearing fields (Args, Env,
Headers) shared; update mergeMCPCommands, the corresponding mergeMCPServers
(lines ~252-304), and copyMCP to perform deep clones of each entry: allocate a
fresh workspace.McpCommand/McpServer for each merged item and clone slice/map
fields (Args, Env, Headers, etc.) into new slices/maps so no returned struct
aliases the inputs; first verify the v0.2.0 workspace types still define those
fields as pointer-bearing/compound types, then implement the deep-copy logic in
those three functions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97a2a8ab-c8b8-476c-b03e-0b4323f80152
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
AGENTS.mdREADME.mdgo.modpkg/agent/agent.gopkg/agent/claude.gopkg/agent/claude_test.gopkg/agent/cursor.gopkg/agent/goose.gopkg/agent/registry_test.gopkg/config/config.gopkg/config/config_test.gopkg/config/merger.gopkg/config/merger_test.gopkg/instances/manager.gopkg/instances/manager_test.goskills/working-with-config-system/SKILL.mdskills/working-with-instances-manager/SKILL.md
✅ Files skipped from review due to trivial changes (4)
- go.mod
- AGENTS.md
- pkg/agent/claude_test.go
- README.md
🚧 Files skipped from review as they are similar to previous changes (7)
- pkg/agent/registry_test.go
- pkg/instances/manager.go
- pkg/config/config.go
- skills/working-with-instances-manager/SKILL.md
- pkg/config/merger_test.go
- pkg/agent/claude.go
- pkg/instances/manager_test.go
Two bugs in the MCP merge logic identified in PR openkaiden#239 review: - Cross-type name collision: mergeMCP merged commands and servers independently, so a base server and an override command with the same name both survived. When agents flatten both lists into a single mcpServers map the lower-precedence entry would silently overwrite the higher-precedence one. Fix filters the losing type after each independent merge using the override name sets. - Shallow copy: mergeMCPCommands, mergeMCPServers, and copyMCP copied McpCommand/McpServer by value, leaving Args, Env, and Headers pointer fields aliasing the originals. Added deepCopyMcpCommand and deepCopyMcpServer helpers used at all three call sites. Also clarifies in the config-system skill doc that MCP server names must be unique across both commands and servers lists. Co-Authored-By: Claude Code (claude-sonnet-4-6) <noreply@anthropic.com> Signed-off-by: Philippe Martin <phmartin@redhat.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
skills/working-with-config-system/SKILL.md (1)
149-153: Avoid normalizing inline auth tokens in repo-scoped examples.
workspace.jsonis presented here as shared project config, so anAuthorization: Bearer ...example makes it easy for users to copy real credentials into source control. A clearly non-secret placeholder, or a note to put authenticated MCP entries in user-level config, would be safer.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/working-with-config-system/SKILL.md` around lines 149 - 153, The example in SKILL.md shows a repo-scoped workspace.json entry with an inline Authorization header containing a realistic bearer token; replace the inline secret with a non-sensitive placeholder (e.g., "Bearer <TOKEN>") or move the authenticated MCP entry into user-level config and add a short note telling readers not to commit real credentials; update the example for the "remote-api" object and add the brief guidance near the workspace.json snippet to avoid encouraging committing secrets.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@pkg/config/merger.go`:
- Around line 367-388: copyMCP currently returns a non-nil McpConfiguration even
when input has empty Commands/Servers; update copyMCP (which calls
deepCopyMcpCommand and deepCopyMcpServer) to normalize empty MCP configs to nil
by treating empty slices as absent: after copying, if both result.Commands and
result.Servers are nil or point to zero-length slices, return nil (or set them
to nil) so callers (e.g., mergedConfig.Mcp checks and SetMCPServers) behave the
same as mergeMCP for empty inputs.
In `@skills/working-with-config-system/SKILL.md`:
- Around line 81-83: The documentation fails to state that merging is name-based
across different types (commands vs servers) so a higher-precedence entry of one
type can remove a lower-precedence entry of the other type if they share the
same name; update the SKILL.md text that describes manager.readAgentSettings and
the merge behavior to explicitly say that pkg/config/merger.go performs
name-based merging across both command and server lists (i.e., entries with the
same name, regardless of type, are overridden by higher-precedence entries), and
mirror this clarification where the doc mentions MCP/server merging and the
runtime steps (including references to SetMCPServers, SetModel, SkipOnboarding)
so readers understand cross-type overrides.
---
Nitpick comments:
In `@skills/working-with-config-system/SKILL.md`:
- Around line 149-153: The example in SKILL.md shows a repo-scoped
workspace.json entry with an inline Authorization header containing a realistic
bearer token; replace the inline secret with a non-sensitive placeholder (e.g.,
"Bearer <TOKEN>") or move the authenticated MCP entry into user-level config and
add a short note telling readers not to commit real credentials; update the
example for the "remote-api" object and add the brief guidance near the
workspace.json snippet to avoid encouraging committing secrets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3262b202-929a-4815-b459-5493cd73785a
📒 Files selected for processing (3)
pkg/config/merger.gopkg/config/merger_test.goskills/working-with-config-system/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/config/merger_test.go
Comment on lines
+81
to
+83
| MCP servers from all configuration levels are merged before being passed to the agent, with higher-precedence levels overriding lower ones by server `name`. | ||
|
|
||
| **Implementation:** `manager.readAgentSettings(storageDir, agentName)` in `pkg/instances/manager.go` walks this directory and returns a `map[string][]byte` (relative forward-slash path → content). If the agent is registered in the agent registry, the manager calls the agent's `SkipOnboarding()` method to modify the settings. If a model ID is provided, the manager then calls the agent's `SetModel()` method. If the merged config contains MCP servers, the manager calls the agent's `SetMCPServers()` method. The final map is passed to the runtime via `runtime.CreateParams.AgentSettings`. The Podman runtime writes the files into the build context and adds a `COPY --chown=agent:agent agent-settings/. /home/agent/` instruction to the Containerfile. The model is also stored directly in the `instance` struct and persisted in `instances.json` via `InstanceData.Model`. |
There was a problem hiding this comment.
Document cross-type overrides explicitly.
These sections still read like commands and servers merge independently by type. In pkg/config/merger.go, Lines 193-258 also remove a lower-precedence command when a higher-precedence server reuses the same name, and vice versa, so the docs should describe this as name-based merging across both lists.
Also applies to: 345-347
🧰 Tools
🪛 LanguageTool
[style] ~83-~83: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... calls the agent's SetModel() method. If the merged config contains MCP servers,...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@skills/working-with-config-system/SKILL.md` around lines 81 - 83, The
documentation fails to state that merging is name-based across different types
(commands vs servers) so a higher-precedence entry of one type can remove a
lower-precedence entry of the other type if they share the same name; update the
SKILL.md text that describes manager.readAgentSettings and the merge behavior to
explicitly say that pkg/config/merger.go performs name-based merging across both
command and server lists (i.e., entries with the same name, regardless of type,
are overridden by higher-precedence entries), and mirror this clarification
where the doc mentions MCP/server merging and the runtime steps (including
references to SetMCPServers, SetModel, SkipOnboarding) so readers understand
cross-type overrides.
benoitf
approved these changes
Apr 10, 2026
Signed-off-by: Philippe Martin <phmartin@redhat.com>
MarsKubeX
added a commit
to MarsKubeX/kortex-cli
that referenced
this pull request
Apr 13, 2026
The Agent interface now requires SetMCPServers after openkaiden#239 was merged. Add a no-op implementation (like goose/cursor) since OpenCode does not yet support MCP configuration through agent settings files. Closes openkaiden#213 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com>
MarsKubeX
added a commit
that referenced
this pull request
Apr 13, 2026
* feat(agent): add OpenCode agent with Podman runtime support Add OpenCode as a supported AI agent with default Podman runtime configuration, automatic onboarding (SkipOnboarding, SetModel), and agent registration. The installer places the binary in ~/.opencode/bin/ which is outside the container's ENV PATH, so the default config symlinks it into ~/.local/bin/. Closes #213 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com> * feat(agent): add no-op SetMCPServers to OpenCode agent The Agent interface now requires SetMCPServers after #239 was merged. Add a no-op implementation (like goose/cursor) since OpenCode does not yet support MCP configuration through agent settings files. Closes #213 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com> * docs: add OpenCode agent to project documentation Update README, AGENTS.md, CLI help examples, and skill guides to include OpenCode as a supported agent alongside Claude, Goose, and Cursor. Closes #213 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com> --------- Signed-off-by: Marcel Bertagnini <mbertagn@redhat.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allows users to configure MCP servers (command-based and URL-based) in workspace.json and in the multi-level config system (projects.json, agents.json). The manager calls SetMCPServers() on the registered agent after SetModel(), writing the merged MCP config into agent settings.
Implementation for Claude agent only.
Closes #210