From 6483eb80447a4d4e7fa1859b011e25f770468050 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Sat, 23 May 2026 21:22:49 +0100 Subject: [PATCH] refactor: remove dead code, deduplicate defaultHome, simplify mcpEntry + kilo render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dead code removed: - BuildProviders, buildModels, ProviderEntry (no production callers) - ZedLanguageModels (shadowed by adapter-local version, wrong schema) - GatewayProviderID deprecated constant (zero references) - Corresponding dead tests for BuildProviders Deduplication: - Extract common.DefaultHome() to internal/adapter/common/home.go - All 7 adapters call common.DefaultHome() instead of local copies (-49 lines) Simplification: - mcp.go: merge MCPClaudeStyle/MCPCrushStyle branches (identical URL path, one-line difference in stdio path) — clearer intent, fewer lines - kilo.go: extract opencode.jsonc merge to mergeOpenCodeJSONC() helper, Render() drops from 92 to ~55 lines - goose.go: strings.IndexByte → strings.Cut in splitProviderModel --- internal/adapter/common/blocks.go | 84 -------------------- internal/adapter/common/common_test.go | 26 ------- internal/adapter/common/home.go | 12 +++ internal/adapter/common/mcp.go | 34 ++------ internal/adapters/cagent/cagent.go | 10 +-- internal/adapters/claudecode/claudecode.go | 10 +-- internal/adapters/crush/crush.go | 10 +-- internal/adapters/goose/goose.go | 14 +--- internal/adapters/kilo/kilo.go | 90 ++++++++++------------ internal/adapters/opencode/opencode.go | 10 +-- internal/adapters/zed/zed.go | 10 +-- 11 files changed, 68 insertions(+), 242 deletions(-) create mode 100644 internal/adapter/common/home.go diff --git a/internal/adapter/common/blocks.go b/internal/adapter/common/blocks.go index da6cdba..e657ce0 100644 --- a/internal/adapter/common/blocks.go +++ b/internal/adapter/common/blocks.go @@ -11,10 +11,6 @@ import ( const fallbackGatewayProviderID = "harness-sync-gateway" -// GatewayProviderID is the canonical provider key used across all adapters. -// Deprecated: prefer GatewayProviderKey(gatewayURL) for a stable URL-derived key. -const GatewayProviderID = fallbackGatewayProviderID - // GatewayProviderKey returns a stable provider key derived from the gateway URL. // Format: "hs-" with dots and special chars normalised to dashes. // Falls back to "harness-sync-gateway" when the URL is empty or unparseable. @@ -34,52 +30,6 @@ func GatewayProviderKey(gatewayURL string) string { return "hs-" + r.Replace(host) } -// ProviderEntry is a generic provider record. Adapters may massage further. -type ProviderEntry = map[string]any - -// BuildProviders returns the canonical-to-generic provider list: gateway first -// (if configured), then any upstreams. Each entry has id, name, base_url, -// api_key, and (for the gateway) a models list. -func BuildProviders(p *canonical.Profile) []ProviderEntry { - out := make([]ProviderEntry, 0, len(p.Upstreams)+1) - if p.Gateway.URL != "" { - key := GatewayProviderKey(p.Gateway.URL) - gw := ProviderEntry{ - "id": key, - "name": "harness-sync gateway", - "base_url": p.Gateway.URL, - "api_key": p.Gateway.Token, - } - if models := buildModels(p.Models); len(models) > 0 { - gw["models"] = models - } - out = append(out, gw) - } - for _, up := range p.Upstreams { - e := ProviderEntry{"id": up.Name, "name": up.Name} - if up.BaseURL != "" { - e["base_url"] = up.BaseURL - } - if up.APIKey != "" { - e["api_key"] = up.APIKey - } - out = append(out, e) - } - return out -} - -func buildModels(models []canonical.Model) []map[string]any { - out := make([]map[string]any, 0, len(models)) - for _, m := range models { - e := map[string]any{"id": m.ID} - if m.Alias != "" { - e["alias"] = m.Alias - } - out = append(out, e) - } - return out -} - // crushModel builds a 10-field model entry required by the crush schema. func crushModel(id string) map[string]any { return map[string]any{ @@ -209,40 +159,6 @@ func ProvidersAsCagentMap(p *canonical.Profile) map[string]any { return out } -// ZedLanguageModels returns the language_models.openai block for Zed when a -// gateway URL is configured. -func ZedLanguageModels(p *canonical.Profile) map[string]any { - if p.Gateway.URL == "" { - return map[string]any{} - } - models := make([]map[string]any, 0) - for _, m := range p.Models { - models = append(models, map[string]any{ - "name": m.ID, - "display_name": func() string { - if m.Alias != "" { - return m.Alias - } - return m.ID - }(), - "max_tokens": 8192, - }) - } - if len(models) == 0 && p.Gateway.DefaultModel != "" { - models = append(models, map[string]any{ - "name": p.Gateway.DefaultModel, - "display_name": p.Gateway.DefaultModel, - "max_tokens": 8192, - }) - } - return map[string]any{ - "openai": map[string]any{ - "api_url": p.Gateway.URL, - "available_models": models, - }, - } -} - // GooseCustomProviderFile returns the JSON body and provider ID for the goose // custom_providers/.json file. Returns nil body when Gateway.URL is empty. func GooseCustomProviderFile(p *canonical.Profile) ([]byte, string) { diff --git a/internal/adapter/common/common_test.go b/internal/adapter/common/common_test.go index 009b9a8..9f10de3 100644 --- a/internal/adapter/common/common_test.go +++ b/internal/adapter/common/common_test.go @@ -9,32 +9,6 @@ import ( "github.com/lukaszraczylo/harness-sync/internal/canonical" ) -func TestBuildProvidersGatewayFirst(t *testing.T) { - p := &canonical.Profile{ - Gateway: canonical.Gateway{URL: "https://gw", Token: "t", DefaultModel: "x"}, - Models: []canonical.Model{{ID: "m1", Alias: "a1"}}, - Upstreams: []canonical.Upstream{ - {Name: "anthropic", APIKey: "k", BaseURL: "https://api.anthropic.com"}, - }, - } - out := BuildProviders(p) - require.Len(t, out, 2) - assert.Equal(t, "hs-gw", out[0]["id"]) - assert.Equal(t, "anthropic", out[1]["id"]) - assert.NotNil(t, out[0]["models"]) -} - -func TestBuildProvidersNoGateway(t *testing.T) { - p := &canonical.Profile{ - Upstreams: []canonical.Upstream{ - {Name: "anthropic", APIKey: "k"}, - }, - } - out := BuildProviders(p) - require.Len(t, out, 1) - assert.Equal(t, "anthropic", out[0]["id"]) -} - func TestBuildMCPMap(t *testing.T) { reg := &canonical.MCPRegistry{Servers: []canonical.MCPServer{ {Name: "filepuff", Command: "/bin/x", Transport: "stdio"}, diff --git a/internal/adapter/common/home.go b/internal/adapter/common/home.go new file mode 100644 index 0000000..78620e9 --- /dev/null +++ b/internal/adapter/common/home.go @@ -0,0 +1,12 @@ +package common + +import "os" + +// DefaultHome returns the current user's home directory, or "" on error. +func DefaultHome() string { + h, err := os.UserHomeDir() + if err != nil { + return "" + } + return h +} diff --git a/internal/adapter/common/mcp.go b/internal/adapter/common/mcp.go index 79fca4a..8568236 100644 --- a/internal/adapter/common/mcp.go +++ b/internal/adapter/common/mcp.go @@ -31,32 +31,7 @@ func BuildMCPMapStyled(reg *canonical.MCPRegistry, style MCPStyle) map[string]an func mcpEntry(s canonical.MCPServer, style MCPStyle) map[string]any { e := map[string]any{} switch style { - case MCPClaudeStyle: - // Claude Code's actual entry shape uses "type" (not "transport"). - // Default to stdio when no URL is set. - t := s.Transport - if s.URL != "" { - if t == "" { - t = "http" - } - e["type"] = t - e["url"] = s.URL - } else { - if t == "" { - t = "stdio" - } - e["type"] = t - if s.Command != "" { - e["command"] = s.Command - } - if len(s.Args) > 0 { - e["args"] = s.Args - } - if len(s.Env) > 0 { - e["env"] = s.Env - } - } - case MCPCrushStyle: + case MCPClaudeStyle, MCPCrushStyle: if s.URL != "" { t := s.Transport if t == "" { @@ -65,7 +40,12 @@ func mcpEntry(s canonical.MCPServer, style MCPStyle) map[string]any { e["type"] = t e["url"] = s.URL } else { - e["type"] = "stdio" + // Claude honours Transport field; Crush always uses "stdio". + t := "stdio" + if style == MCPClaudeStyle && s.Transport != "" { + t = s.Transport + } + e["type"] = t if s.Command != "" { e["command"] = s.Command } diff --git a/internal/adapters/cagent/cagent.go b/internal/adapters/cagent/cagent.go index ed0ebf8..7a22159 100644 --- a/internal/adapters/cagent/cagent.go +++ b/internal/adapters/cagent/cagent.go @@ -29,21 +29,13 @@ func WithHome(h string) Option { return func(a *Adapter) { a.home = h } } // New returns a new Adapter with the given options applied. func New(opts ...Option) *Adapter { - a := &Adapter{home: defaultHome()} + a := &Adapter{home: common.DefaultHome()} for _, o := range opts { o(a) } return a } -func defaultHome() string { - h, err := os.UserHomeDir() - if err != nil { - return "" - } - return h -} - // Name returns the harness identifier. func (a *Adapter) Name() string { return name } diff --git a/internal/adapters/claudecode/claudecode.go b/internal/adapters/claudecode/claudecode.go index 4c409b2..b019223 100644 --- a/internal/adapters/claudecode/claudecode.go +++ b/internal/adapters/claudecode/claudecode.go @@ -25,21 +25,13 @@ func WithHome(h string) Option { return func(a *Adapter) { a.home = h } } // New returns a new Adapter with the given options applied. func New(opts ...Option) *Adapter { - a := &Adapter{home: defaultHome()} + a := &Adapter{home: common.DefaultHome()} for _, o := range opts { o(a) } return a } -func defaultHome() string { - h, err := os.UserHomeDir() - if err != nil { - return "" - } - return h -} - // Name returns the harness identifier. func (a *Adapter) Name() string { return name } diff --git a/internal/adapters/crush/crush.go b/internal/adapters/crush/crush.go index 43de32b..5b17382 100644 --- a/internal/adapters/crush/crush.go +++ b/internal/adapters/crush/crush.go @@ -23,21 +23,13 @@ func WithHome(h string) Option { return func(a *Adapter) { a.home = h } } // New returns a new Adapter with the given options applied. func New(opts ...Option) *Adapter { - a := &Adapter{home: defaultHome()} + a := &Adapter{home: common.DefaultHome()} for _, o := range opts { o(a) } return a } -func defaultHome() string { - h, err := os.UserHomeDir() - if err != nil { - return "" - } - return h -} - // Name returns the harness identifier. func (a *Adapter) Name() string { return name } diff --git a/internal/adapters/goose/goose.go b/internal/adapters/goose/goose.go index 0f0ba30..ccff6a5 100644 --- a/internal/adapters/goose/goose.go +++ b/internal/adapters/goose/goose.go @@ -32,21 +32,13 @@ func WithHome(h string) Option { return func(a *Adapter) { a.home = h } } // New returns a new Adapter with the given options applied. func New(opts ...Option) *Adapter { - a := &Adapter{home: defaultHome()} + a := &Adapter{home: common.DefaultHome()} for _, o := range opts { o(a) } return a } -func defaultHome() string { - h, err := os.UserHomeDir() - if err != nil { - return "" - } - return h -} - // Name returns the harness identifier. func (a *Adapter) Name() string { return name } @@ -152,8 +144,8 @@ func buildExtensions(servers []canonical.MCPServer) map[string]any { // splitProviderModel splits "provider/model" into its parts. // If no slash, provider is "anthropic" and the whole string is the model. func splitProviderModel(s string) (string, string) { - if idx := strings.IndexByte(s, '/'); idx >= 0 { - return s[:idx], s[idx+1:] + if p, m, ok := strings.Cut(s, "/"); ok { + return p, m } return "anthropic", s } diff --git a/internal/adapters/kilo/kilo.go b/internal/adapters/kilo/kilo.go index 5ca03b8..c06596a 100644 --- a/internal/adapters/kilo/kilo.go +++ b/internal/adapters/kilo/kilo.go @@ -27,21 +27,13 @@ func WithHome(h string) Option { return func(a *Adapter) { a.home = h } } // New returns a new Adapter with the given options applied. func New(opts ...Option) *Adapter { - a := &Adapter{home: defaultHome()} + a := &Adapter{home: common.DefaultHome()} for _, o := range opts { o(a) } return a } -func defaultHome() string { - h, err := os.UserHomeDir() - if err != nil { - return "" - } - return h -} - // Name returns the harness identifier. func (a *Adapter) Name() string { return name } @@ -110,50 +102,15 @@ func (a *Adapter) Render(b *canonical.Bundle) (*adapter.FileSet, error) { // Also merge into opencode.jsonc when present — kilo reads it as its // primary config. Delete the enabled_providers filter so our provider is - // visible alongside user-defined providers. Deep-merge the provider map - // so existing providers (e.g. "llmgw") are preserved. + // visible alongside user-defined providers. ocPath := filepath.Join(base, "opencode.jsonc") - if ocRaw, readErr := os.ReadFile(ocPath); readErr == nil { - // Parse existing content (supports JSONC comments internally). - var ocBase map[string]any - clean := common.StripJSONComments(string(ocRaw)) - _ = json.Unmarshal([]byte(clean), &ocBase) - if ocBase == nil { - ocBase = map[string]any{} - } - - ocOverlay := map[string]any{ - "enabled_providers": nil, // delete filter - } - - // Deep-merge provider: add our entries INTO the existing provider map, - // then absorb any duplicate providers that share the same gateway URL. - if newProv, ok := overlay["provider"].(map[string]any); ok && len(newProv) > 0 { - existingProv, _ := ocBase["provider"].(map[string]any) - if existingProv == nil { - existingProv = map[string]any{} - } - maps.Copy(existingProv, newProv) - // Absorb providers at the same URL into harness-sync-gateway, - // preserving user-crafted display names and custom limits. - merged := common.AbsorbDuplicateProviders(existingProv, common.GatewayProviderKey(b.Profile.Gateway.URL), b.Profile.Gateway.URL) - ocOverlay["provider"] = merged - } - // Copy model/small_model/mcp from the kilo.json overlay. - for _, k := range []string{"model", "small_model", "mcp"} { - if v, ok := overlay[k]; ok { - ocOverlay[k] = v - } - } - - ocMerged, mergeErr := common.MergeJSONKeys([]byte(clean), ocOverlay) - if mergeErr != nil { - return nil, mergeErr - } + if ocContent, mergeErr := mergeOpenCodeJSONC(ocPath, overlay, b.Profile.Gateway.URL); mergeErr != nil { + return nil, mergeErr + } else if ocContent != nil { fs.Add(adapter.File{ Dest: ocPath, Kind: adapter.RenderedFile, - Content: ocMerged, + Content: ocContent, NoMerge: true, }) } @@ -161,6 +118,41 @@ func (a *Adapter) Render(b *canonical.Bundle) (*adapter.FileSet, error) { return fs, nil } +// mergeOpenCodeJSONC deep-merges our overlay into the existing opencode.jsonc +// file, deleting the enabled_providers filter and deduplicating gateway providers. +// Returns nil content (no error) when the file does not exist. +func mergeOpenCodeJSONC(path string, overlay map[string]any, gatewayURL string) ([]byte, error) { + ocRaw, err := os.ReadFile(path) + if err != nil { + return nil, nil //nolint:nilerr // file absent is not an error + } + + var ocBase map[string]any + clean := common.StripJSONComments(string(ocRaw)) + _ = json.Unmarshal([]byte(clean), &ocBase) + if ocBase == nil { + ocBase = map[string]any{} + } + + ocOverlay := map[string]any{"enabled_providers": nil} + + if newProv, ok := overlay["provider"].(map[string]any); ok && len(newProv) > 0 { + existingProv, _ := ocBase["provider"].(map[string]any) + if existingProv == nil { + existingProv = map[string]any{} + } + maps.Copy(existingProv, newProv) + ocOverlay["provider"] = common.AbsorbDuplicateProviders(existingProv, common.GatewayProviderKey(gatewayURL), gatewayURL) + } + for _, k := range []string{"model", "small_model", "mcp"} { + if v, ok := overlay[k]; ok { + ocOverlay[k] = v + } + } + + return common.MergeJSONKeys([]byte(clean), ocOverlay) +} + // Import reads kilo config from home and returns a canonical ImportResult. func (a *Adapter) Import(home string) (*adapter.ImportResult, error) { return importFrom(home) diff --git a/internal/adapters/opencode/opencode.go b/internal/adapters/opencode/opencode.go index a6ee5d9..f732f64 100644 --- a/internal/adapters/opencode/opencode.go +++ b/internal/adapters/opencode/opencode.go @@ -24,21 +24,13 @@ func WithHome(h string) Option { return func(a *Adapter) { a.home = h } } // New returns a new Adapter with the given options applied. func New(opts ...Option) *Adapter { - a := &Adapter{home: defaultHome()} + a := &Adapter{home: common.DefaultHome()} for _, o := range opts { o(a) } return a } -func defaultHome() string { - h, err := os.UserHomeDir() - if err != nil { - return "" - } - return h -} - // Name returns the harness identifier. func (a *Adapter) Name() string { return name } diff --git a/internal/adapters/zed/zed.go b/internal/adapters/zed/zed.go index 7fc542e..e944175 100644 --- a/internal/adapters/zed/zed.go +++ b/internal/adapters/zed/zed.go @@ -28,21 +28,13 @@ func WithHome(h string) Option { return func(a *Adapter) { a.home = h } } // New returns a new Adapter with the given options applied. func New(opts ...Option) *Adapter { - a := &Adapter{home: defaultHome()} + a := &Adapter{home: common.DefaultHome()} for _, o := range opts { o(a) } return a } -func defaultHome() string { - h, err := os.UserHomeDir() - if err != nil { - return "" - } - return h -} - // Name returns the harness identifier. func (a *Adapter) Name() string { return name }