feat: add DefaultEnv config field for enable commands#44
Conversation
Add a DefaultEnv map[string]string field to Config that is automatically
merged into the server env when enable writes a config for any editor
(Claude, VSCode, Cursor). User-provided --env values take precedence
on conflict.
This solves the problem where editors launch MCP server subprocesses
with a minimal PATH that cannot find executables managed by mise, asdf,
homebrew, nix, or installed to non-standard paths.
Consumers can now capture PATH (or any other env vars) at config time:
&ophis.Config{
DefaultEnv: map[string]string{
"PATH": os.Getenv("PATH"),
},
}
Backwards compatible: nil DefaultEnv preserves current behavior.
Closes njayp#39 (follow-up)
There was a problem hiding this comment.
Review Summary
This PR successfully adds the DefaultEnv feature to allow configuring default environment variables for MCP servers. The implementation is solid and well-tested.
Implementation Quality
✅ Clean design: The DefaultEnv field is added to the Config struct with clear documentation
✅ Backwards compatible: nil DefaultEnv preserves current behavior
✅ Consistent implementation: The merge logic is identical across all three editors (Claude, VSCode, Cursor)
✅ User precedence: User-provided --env values correctly override defaults
✅ Comprehensive tests: Tests cover all key scenarios:
- Default env written to config
- User override behavior
- nil/empty DefaultEnv handling
- Mixed default + user env
Code Review
Merge Logic (claude/enable.go:66-77, vscode/enable.go:71-81, cursor/enable.go:68-78):
The implementation correctly:
- Creates a new map to avoid mutating the config
- Copies default env first
- Overlays user env (giving user values precedence)
- Only sets
server.Envif non-empty
Threading: The defaultEnv is properly threaded through:
Config→root.Command()→ editor commands →enableCommand()→ merge logic
Documentation: README.md and docs/config.md provide clear examples and explain the PATH problem this solves.
No Issues Found
No bugs, security issues, or significant design problems identified. This is ready to merge.
What does this PR do?
Add a DefaultEnv map[string]string field to Config that is automatically merged into the server env when enable writes a config for any editor (Claude, VSCode, Cursor). User-provided --env values take precedence on conflict.
This solves the problem where editors launch MCP server subprocesses with a minimal PATH that cannot find executables managed by mise, asdf, homebrew, nix, or installed to non-standard paths.
Consumers can now capture PATH (or any other env vars) at config time:
Backwards compatible: nil DefaultEnv preserves current behavior.
Related Issues
Closes #39 (follow-up)
Checklist