Add OpenCode harness support - #120
Merged
Merged
Conversation
Reviewer's GuideAdds first-class OpenCode harness support (skills, roles, MCP, doctor checks, detection, and config path handling) plus updates docs to treat OpenCode as a primary managed harness, with tests covering all new behavior. Sequence diagram for OpenCode skills sync using SkillsNativeRootsequenceDiagram
participant dotagents
participant Harness
participant openCodeReadsAgentsSkills
participant FileSystem
dotagents->>Harness: SkillsNativeRoot(repoRoot, home)
Harness->>openCodeReadsAgentsSkills: repoRoot, home
openCodeReadsAgentsSkills-->>Harness: bool
alt [SkillsNativeRoot returns true]
note over dotagents,FileSystem: OpenCode reads ~/.agents/skills natively
dotagents-->>FileSystem: skip os.MkdirAll(report.SkillRoot)
dotagents-->>FileSystem: skip skill mirror writes
else [SkillsNativeRoot returns false]
dotagents->>FileSystem: os.MkdirAll(report.SkillRoot, 0o755)
note over dotagents,FileSystem: standard symlink-based skill mirror
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
inspectAgent, theSkillsNativeRootshortcut is applied for any harness with a non-nil predicate, regardless ofSkillsmode; consider guarding this withh.Skills == SkillsSymlinkto match the struct’s documented intent and avoid surprises for future harnesses. - For
openCodeReadsAgentsSkills, you currently rely onsameResolvedPathwhich follows symlinks; if users intentionally symlink~/.agentselsewhere this may make the behavior less obvious, so it could be worth either documenting that or tightening the comparison to only the literal~/.agentspath.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `inspectAgent`, the `SkillsNativeRoot` shortcut is applied for any harness with a non-nil predicate, regardless of `Skills` mode; consider guarding this with `h.Skills == SkillsSymlink` to match the struct’s documented intent and avoid surprises for future harnesses.
- For `openCodeReadsAgentsSkills`, you currently rely on `sameResolvedPath` which follows symlinks; if users intentionally symlink `~/.agents` elsewhere this may make the behavior less obvious, so it could be worth either documenting that or tightening the comparison to only the literal `~/.agents` path.
## Individual Comments
### Comment 1
<location path="cmd/dotagents/opencode_test.go" line_range="184-193" />
<code_context>
+func TestOpenCodeMCPPreservesUnmanagedKeysAndServers(t *testing.T) {
</code_context>
<issue_to_address>
**suggestion (testing):** Consider testing preservation/merge behavior for existing environment fields in MCP entries
The current test covers unmanaged top-level keys and servers, plus adding the managed `linkedin` server. Please also add a case where the MCP entry already has an `environment` block, verifying that `patchOpenCodeMCPServer` merges new env keys into the existing map (without overwriting or dropping unrelated keys). This will make the merge behavior explicit and protect against regressions.
Suggested implementation:
```golang
func TestOpenCodeMCPPreservesUnmanagedKeysAndServers(t *testing.T) {
t.Setenv("XDG_CONFIG_HOME", "")
home := t.TempDir()
configPath := filepath.Join(home, ".config", "opencode", "opencode.json")
writeSyncTestFile(t, configPath, []byte(`{
"$schema": "https://opencode.ai/config.json",
"theme": "opencode",
"mcp": {
"existing-remote": {"type": "remote", "url": "https://example.test/mcp", "enabled": true},
"existing-local": {"type": "local", "command": ["node", "server.js"], "enabled": true},
"linkedin": {
"type": "remote",
"url": "https://example.test/linkedin-mcp",
"enabled": true,
"environment": {
"PRESERVE_ME": "keep"
}
}
}
```
To fully implement the requested behavior verification (that `patchOpenCodeMCPServer` merges new environment keys into an existing environment map without overwriting or dropping unrelated keys), you should:
1. Ensure this test invokes whatever helper currently patches the MCP config (likely `patchOpenCodeMCPServer` or a wrapper) after `writeSyncTestFile` has created the initial config containing the `linkedin` MCP entry with the `PRESERVE_ME` env key.
2. After the patch is applied, read `configPath` back into the same config struct used in other tests (e.g., `opencode.Config` if present) and locate the `linkedin` MCP entry.
3. Add assertions such as:
- The `linkedin` MCP entry still has `environment["PRESERVE_ME"] == "keep"`.
- The `environment` map now contains at least one additional key compared to the initial config (e.g., `len(env) > 1`), confirming that new managed env keys were merged in rather than replacing the map.
4. If your implementation of `patchOpenCodeMCPServer` is expected to add specific environment keys (e.g., API tokens), you can also assert that those keys are present alongside `PRESERVE_ME` to make the merge behavior even more explicit.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+184
to
+193
| func TestOpenCodeMCPPreservesUnmanagedKeysAndServers(t *testing.T) { | ||
| t.Setenv("XDG_CONFIG_HOME", "") | ||
| home := t.TempDir() | ||
| configPath := filepath.Join(home, ".config", "opencode", "opencode.json") | ||
| writeSyncTestFile(t, configPath, []byte(`{ | ||
| "$schema": "https://opencode.ai/config.json", | ||
| "theme": "opencode", | ||
| "mcp": { | ||
| "existing-remote": {"type": "remote", "url": "https://example.test/mcp", "enabled": true}, | ||
| "existing-local": {"type": "local", "command": ["node", "server.js"], "enabled": true} |
There was a problem hiding this comment.
suggestion (testing): Consider testing preservation/merge behavior for existing environment fields in MCP entries
The current test covers unmanaged top-level keys and servers, plus adding the managed linkedin server. Please also add a case where the MCP entry already has an environment block, verifying that patchOpenCodeMCPServer merges new env keys into the existing map (without overwriting or dropping unrelated keys). This will make the merge behavior explicit and protect against regressions.
Suggested implementation:
func TestOpenCodeMCPPreservesUnmanagedKeysAndServers(t *testing.T) {
t.Setenv("XDG_CONFIG_HOME", "")
home := t.TempDir()
configPath := filepath.Join(home, ".config", "opencode", "opencode.json")
writeSyncTestFile(t, configPath, []byte(`{
"$schema": "https://opencode.ai/config.json",
"theme": "opencode",
"mcp": {
"existing-remote": {"type": "remote", "url": "https://example.test/mcp", "enabled": true},
"existing-local": {"type": "local", "command": ["node", "server.js"], "enabled": true},
"linkedin": {
"type": "remote",
"url": "https://example.test/linkedin-mcp",
"enabled": true,
"environment": {
"PRESERVE_ME": "keep"
}
}
}To fully implement the requested behavior verification (that patchOpenCodeMCPServer merges new environment keys into an existing environment map without overwriting or dropping unrelated keys), you should:
- Ensure this test invokes whatever helper currently patches the MCP config (likely
patchOpenCodeMCPServeror a wrapper) afterwriteSyncTestFilehas created the initial config containing thelinkedinMCP entry with thePRESERVE_MEenv key. - After the patch is applied, read
configPathback into the same config struct used in other tests (e.g.,opencode.Configif present) and locate thelinkedinMCP entry. - Add assertions such as:
- The
linkedinMCP entry still hasenvironment["PRESERVE_ME"] == "keep". - The
environmentmap now contains at least one additional key compared to the initial config (e.g.,len(env) > 1), confirming that new managed env keys were merged in rather than replacing the map.
- The
- If your implementation of
patchOpenCodeMCPServeris expected to add specific environment keys (e.g., API tokens), you can also assert that those keys are present alongsidePRESERVE_MEto make the merge behavior even more explicit.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37faab7fa2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // openCodeConfigPath returns the shared OpenCode config file that also holds the | ||
| // managed `mcp` block. | ||
| func openCodeConfigPath(home string) string { | ||
| return filepath.Join(openCodeConfigDir(home), "opencode.json") |
There was a problem hiding this comment.
When a user keeps their global MCP configuration in ~/.config/opencode/opencode.jsonc (OpenCode documents JSONC support and uses that filename in examples), every inspect/import/patch path here looks only at opencode.json, so dotagents misses existing native servers and writes a separate file instead of merging with the user's real config; this breaks setup import and can create conflicting duplicate MCP definitions for the same server name.
Useful? React with 👍 / 👎.
| {Name: agentDroid, Enabled: true, SkillRoot: "~/.factory/skills", AgentRoot: "~/.factory/droids", Detect: "droid"}, | ||
| {Name: agentHermes, Enabled: true, SkillRoot: "~/.hermes/skills", Detect: "hermes"}, | ||
| {Name: agentOMP, Enabled: true, SkillRoot: "~/.omp/agent/skills", AgentRoot: "~/.omp/agent/agents", Detect: "omp"}, | ||
| {Name: agentOpenCode, Enabled: true, SkillRoot: "~/.config/opencode/skills", AgentRoot: "~/.config/opencode/agents", Detect: "opencode"}, |
There was a problem hiding this comment.
When XDG_CONFIG_HOME is set, the MCP adapter and duplicate-skill doctor check use $XDG_CONFIG_HOME/opencode, but first-run setup persists ~/.config/opencode/{skills,agents} here. Users on an XDG config home will have roles and custom-root skill mirrors written to a directory OpenCode does not read, while MCP goes to the XDG directory, leaving the OpenCode harness only partially synced after setup.
Useful? React with 👍 / 👎.
| read: readOpenCodeMCPServer, | ||
| rootKey: "mcp", | ||
| }), | ||
| Roles: &RolesCapability{Extension: ".md", Render: renderOpenCodeAgentRole}, |
There was a problem hiding this comment.
With OpenCode roles enabled here, setup scans ~/.config/opencode/agents/*.md, but OpenCode markdown agents use the filename as the agent name and their frontmatter normally lacks the canonical name field. The current generic Markdown importer therefore fails canonical parsing, falls back to wrapping the entire original file (including --- frontmatter) into the instructions, and replaces fields like description, mode, and model; accepting such an import loses the native role metadata instead of normalizing it.
Useful? React with 👍 / 👎.
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
1 participant
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.
Implements docs/plans/launch-2026-07-14/09-opencode-spec.md (research: 08-opencode-research.md). Motivated by the Kimi K3 wave: OpenCode is the go-to open harness for it.
~/.agents/skills/, so no mirror when the config root is~/.agents(avoids double-listing); custom roots mirror into~/.config/opencode/skills/.doctorwarns on duplicate skill names across both locations.agents/*.mdrendered to~/.config/opencode/agents/<name>.mdwith frontmatter mapping and anopencode:per-harness override block (model,temperature,mode; defaultmode: subagent).opencode.jsonundermcp—command+argsfolded into OpenCode's singlecommandarray,env→environment; unmanaged keys and remote servers untouched. Import reverse-translates.opencodebinary; XDG-aware config root; setup import scans existing agents + MCP.Full
go test ./...green. Implemented by the opus builder from the spec; docs conflict with #119 resolved during rebase.Summary by Sourcery
Add first-class OpenCode harness support with native skill consumption, agent role rendering, MCP config integration, and tooling updates.
New Features:
Enhancements:
Documentation:
Tests: