-
Notifications
You must be signed in to change notification settings - Fork 13
feat: add Crush harness support #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Greptile Overview
Greptile Summary
Adds comprehensive Crush harness support to Bridle, enabling profile management, MCP installation, and skills integration for the Crush AI coding assistant.
Key Changes:
- Added Crush harness recognition across all CLI, TUI, and installation modules
- Implemented
crush.jsonMCP configuration with top-level"mcp"key (similar to OpenCode) - Added model extraction supporting both simple
"model"key and nested"models.large.model"/"models.small.model"paths - Included comprehensive test coverage for MCP read/write and profile operations
- Improved test isolation in
config/manager/mod.rsby replacing unsafe env var manipulation with proper mutex-based locking - Temporarily using git dependencies from
edlsh/harness-barnbranch until upstream PR merges
Implementation Quality:
- Consistent pattern matching across all modules (CLI, installer, uninstaller, TUI, etc.)
- Proper JSON handling (not JSONC) for Crush config files
- Good test coverage including integration tests for profile creation and MCP installation
Confidence Score: 4/5
- Safe to merge with minor enhancement opportunity for MCP extraction
- Implementation is thorough and consistent across all modules with good test coverage. The missing Crush-specific MCP extractor means detailed server info won't display in profile views (falls back to generic extraction), but this doesn't affect functionality - only UI completeness. Test isolation improvements are solid. Dependency on feature branch is temporary and documented.
- Check
src/config/manager/extraction/mod.rsfor the optional MCP extraction enhancement
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| src/config/manager/extraction/mod.rs | 4/5 | Added extract_model_crush() for model extraction, but missing extract_mcp_from_crush_config() - Crush falls back to generic MCP extraction which loses detailed server info |
| src/install/mcp_config.rs | 5/5 | Added Crush support with "mcp" key correctly, consistent with OpenCode pattern, handles JSON (not JSONC) |
| src/install/mcp_installer.rs | 5/5 | Added Crush harness parsing and config path mapping, includes comprehensive test coverage |
| tests/mcp_installation.rs | 5/5 | Added test for Crush JSON read/write operations with MCP config, validates schema and structure |
| tests/cli_integration.rs | 5/5 | Added comprehensive Crush profile tests covering model extraction and profile creation from current config |
| src/config/manager/mod.rs | 5/5 | Improved test isolation by replacing unsafe env var manipulation with proper mutex-based locking |
| Cargo.toml | 4/5 | Switched to git dependencies from edlsh/harness-barn feat branch (temporary until upstream merge) |
Sequence Diagram
sequenceDiagram
participant User
participant CLI
participant ProfileMgr as Profile Manager
participant McpInstaller as MCP Installer
participant McpConfig as MCP Config
participant CrushFS as Crush Filesystem
Note over User,CrushFS: Crush Harness Installation Flow
User->>CLI: bridle install owner/repo crush my-profile
CLI->>ProfileMgr: resolve_harness("crush")
ProfileMgr-->>CLI: HarnessKind::Crush
CLI->>McpInstaller: install_mcp(name, server, target)
McpInstaller->>McpInstaller: parse_harness_kind("crush")
McpInstaller->>McpInstaller: get_profile_config_path(Crush)
Note right of McpInstaller: Returns profile_dir/crush.json
McpInstaller->>McpConfig: read_mcp_config(Crush, path)
McpConfig->>McpConfig: get_mcp_key(Crush)
Note right of McpConfig: Returns "mcp" key
McpConfig->>CrushFS: read crush.json
CrushFS-->>McpConfig: JSON content
McpConfig->>McpConfig: Parse JSON & extract "mcp" section
McpConfig-->>McpInstaller: Existing MCP servers map
McpInstaller->>McpConfig: write_mcp_config(Crush, path, servers)
McpConfig->>CrushFS: Read existing crush.json
CrushFS-->>McpConfig: Current config
McpConfig->>McpConfig: Merge new MCP into "mcp" key
McpConfig->>CrushFS: Write updated crush.json
CrushFS-->>McpConfig: Success
McpConfig-->>McpInstaller: Success
McpInstaller-->>CLI: McpInstallOutcome::Installed
CLI-->>User: Installation successful
Note over User,CrushFS: Profile Switch Flow
User->>CLI: bridle profile switch crush my-profile
CLI->>ProfileMgr: switch_profile(crush, my-profile)
ProfileMgr->>CrushFS: Copy profile/crush/my-profile/* to ~/.config/crush/
Note right of ProfileMgr: Includes crush.json with MCP config
ProfileMgr-->>CLI: Success
CLI-->>User: Profile switched
Note over User,CrushFS: Model Extraction Flow
User->>CLI: bridle profile show crush my-profile
CLI->>ProfileMgr: extract_model(crush_harness, profile_path)
ProfileMgr->>ProfileMgr: extract_model_crush(profile_path)
ProfileMgr->>CrushFS: Read crush.json
CrushFS-->>ProfileMgr: JSON content
ProfileMgr->>ProfileMgr: Try "model" key
alt model key exists
ProfileMgr-->>CLI: model value
else check models.large
ProfileMgr->>ProfileMgr: Try "models.large.model"
ProfileMgr-->>CLI: large model value
else check models.small
ProfileMgr->>ProfileMgr: Try "models.small.model"
ProfileMgr-->>CLI: small model value
end
CLI-->>User: Display model info
Additional Comments (1)
Then implement Prompt To Fix With AIThis is a comment left during a code review.
Path: src/config/manager/extraction/mod.rs
Line: 61:67
Comment:
Missing `"crush"` case - falls back to `extract_mcp_generic()` which only extracts server names, not detailed info (type, command, args, url). Consider adding:
```suggestion
match harness.id() {
"opencode" => extract_mcp_from_opencode_config(profile_path),
"crush" => extract_mcp_from_crush_config(profile_path),
"amp-code" => extract_mcp_from_ampcode_config(profile_path),
"claude-code" => extract_mcp_from_claudecode_config(profile_path),
"goose" => extract_mcp_from_goose_config(profile_path),
_ => extract_mcp_generic(harness, profile_path),
}
```
Then implement `extract_mcp_from_crush_config()` similar to `extract_mcp_from_opencode_config()` (lines 11-55) but reading `crush.json` instead of `opencode.jsonc`.
How can I resolve this? If you propose a fix, please make it concise. |
Summary
crush.jsonwith top-levelmcpkey.Dependency
Depends on neiii/harness-barn#7 (using
edlsh/harness-barnbranchfeat/add-crush-harnessuntil merged).Testing
cargo fmt -- --checkcargo clippy -- -D warningscargo test