From 6729789d58d71b652c0d5e33e549eb1fba74df29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20AZA=C3=8FS?= Date: Fri, 22 May 2026 23:48:38 +0200 Subject: [PATCH] feat(agent): add Oh My Pi ACP agent (#971) --- .../agent/agents/logos/omp_acp_dark.svg | 1 + .../agent/agents/logos/omp_acp_light.svg | 1 + .../agent/agents/new_acp_agents_test.go | 7 + apps/backend/internal/agent/agents/omp_acp.go | 121 ++++++++++++++++++ .../internal/agent/registry/registry.go | 1 + .../internal/agentctl/server/api/agent.go | 12 +- .../agentctl/server/api/agent_test.go | 42 ++++++ .../internal/agentctl/server/config/config.go | 7 +- .../agentctl/server/config/config_test.go | 40 ++++++ .../agentctl/server/utility/acp_executor.go | 14 +- .../server/utility/probe_allowlist_test.go | 42 ++++++ 11 files changed, 277 insertions(+), 11 deletions(-) create mode 100644 apps/backend/internal/agent/agents/logos/omp_acp_dark.svg create mode 100644 apps/backend/internal/agent/agents/logos/omp_acp_light.svg create mode 100644 apps/backend/internal/agent/agents/omp_acp.go create mode 100644 apps/backend/internal/agentctl/server/utility/probe_allowlist_test.go diff --git a/apps/backend/internal/agent/agents/logos/omp_acp_dark.svg b/apps/backend/internal/agent/agents/logos/omp_acp_dark.svg new file mode 100644 index 0000000000..c1f972e84f --- /dev/null +++ b/apps/backend/internal/agent/agents/logos/omp_acp_dark.svg @@ -0,0 +1 @@ +Oh My Pi diff --git a/apps/backend/internal/agent/agents/logos/omp_acp_light.svg b/apps/backend/internal/agent/agents/logos/omp_acp_light.svg new file mode 100644 index 0000000000..b2b8d3bf77 --- /dev/null +++ b/apps/backend/internal/agent/agents/logos/omp_acp_light.svg @@ -0,0 +1 @@ +Oh My Pi diff --git a/apps/backend/internal/agent/agents/new_acp_agents_test.go b/apps/backend/internal/agent/agents/new_acp_agents_test.go index 0ae3279f6c..ffeca6eec8 100644 --- a/apps/backend/internal/agent/agents/new_acp_agents_test.go +++ b/apps/backend/internal/agent/agents/new_acp_agents_test.go @@ -104,6 +104,13 @@ var newACPAgentSpecs = []struct { passthroughArgv: []string{"traecli"}, installViaNpm: false, }}, + {func() Agent { return NewOmpACP() }, acpAgentSpec{ + id: "omp-acp", displayName: "omp", detectBinaries: []string{"omp"}, + expectedArgv: []string{"omp", "acp"}, + inferenceArgv: []string{"omp", "acp"}, + passthroughArgv: []string{"omp"}, + installViaNpm: false, + }}, } func TestNewACPAgents_IDAndDisplay(t *testing.T) { diff --git a/apps/backend/internal/agent/agents/omp_acp.go b/apps/backend/internal/agent/agents/omp_acp.go new file mode 100644 index 0000000000..cd2e570347 --- /dev/null +++ b/apps/backend/internal/agent/agents/omp_acp.go @@ -0,0 +1,121 @@ +//nolint:dupl,goconst // Native-binary ACP agents (Cursor, Kimi, Kiro, Omp, Qoder, Trae) follow the same minimal scaffold; differences are the binary name and subcommand. Shared literals (`CLI Passthrough`, `Show terminal directly instead of chat interface`, `{workspace}`) live in every peer file by convention. +package agents + +import ( + "context" + _ "embed" + "time" + + "github.com/kandev/kandev/internal/agent/usage" + "github.com/kandev/kandev/pkg/agent" +) + +//go:embed logos/omp_acp_light.svg +var ompACPLogoLight []byte + +//go:embed logos/omp_acp_dark.svg +var ompACPLogoDark []byte + +const ompACPBin = "omp" + +var ( + _ Agent = (*OmpACP)(nil) + _ PassthroughAgent = (*OmpACP)(nil) + _ InferenceAgent = (*OmpACP)(nil) +) + +// OmpACP implements Agent for the Oh My Pi (omp) coding agent via its native +// `omp acp` subcommand. Distributed as a single binary (bun-installed); BYO +// API key — omp reads any of the dozen provider env vars (ANTHROPIC_API_KEY, +// OPENAI_API_KEY, GEMINI_API_KEY, ...) directly. +type OmpACP struct { + StandardPassthrough +} + +func NewOmpACP() *OmpACP { + return &OmpACP{ + StandardPassthrough: StandardPassthrough{ + PermSettings: emptyPermSettings, + Cfg: PassthroughConfig{ + Supported: true, + Label: "CLI Passthrough", + Description: "Show terminal directly instead of chat interface", + PassthroughCmd: NewCommand(ompACPBin), + ModelFlag: NewParam("--model", "{model}"), + ResumeFlag: NewParam("-c"), + SessionResumeFlag: NewParam("--resume"), + IdleTimeout: 3 * time.Second, + BufferMaxBytes: DefaultBufferMaxBytes, + }, + }, + } +} + +func (a *OmpACP) ID() string { return "omp-acp" } +func (a *OmpACP) Name() string { return "Oh My Pi ACP Agent" } +func (a *OmpACP) DisplayName() string { return ompACPBin } +func (a *OmpACP) Description() string { + return "Oh My Pi (omp) coding agent using the ACP protocol via the `omp acp` subcommand." +} +func (a *OmpACP) Enabled() bool { return true } +func (a *OmpACP) DisplayOrder() int { return 18 } + +func (a *OmpACP) Logo(v LogoVariant) []byte { + if v == LogoDark { + return ompACPLogoDark + } + return ompACPLogoLight +} + +func (a *OmpACP) IsInstalled(ctx context.Context) (*DiscoveryResult, error) { + result, err := Detect(ctx, WithCommand(ompACPBin)) + if err != nil { + return result, err + } + result.SupportsMCP = true + result.Capabilities = DiscoveryCapabilities{ + SupportsSessionResume: true, + } + return result, nil +} + +func (a *OmpACP) BuildCommand(opts CommandOptions) Command { + return Cmd(ompACPBin, "acp").Build() +} + +func (a *OmpACP) Runtime() *RuntimeConfig { + canRecover := true + return &RuntimeConfig{ + Cmd: Cmd(ompACPBin, "acp").Build(), + WorkingDir: "{workspace}", + Env: map[string]string{}, + ResourceLimits: ResourceLimits{MemoryMB: 4096, CPUCores: 2.0, Timeout: time.Hour}, + Protocol: agent.ProtocolACP, + ProjectSkillDir: ".omp/skills", + UserSkillDir: ".omp/agent/skills", + SessionConfig: SessionConfig{ + NativeSessionResume: true, + CanRecover: &canRecover, + SessionDirTemplate: "{home}/.omp", + }, + } +} + +func (a *OmpACP) RemoteAuth() *RemoteAuth { return nil } + +func (a *OmpACP) InstallScript() string { + return "bun install -g @oh-my-pi/pi-coding-agent" +} + +func (a *OmpACP) PermissionSettings() map[string]PermissionSetting { + return emptyPermSettings +} + +func (a *OmpACP) InferenceConfig() *InferenceConfig { + return &InferenceConfig{ + Supported: true, + Command: NewCommand(ompACPBin, "acp"), + } +} + +func (a *OmpACP) BillingType() usage.BillingType { return defaultBillingType() } diff --git a/apps/backend/internal/agent/registry/registry.go b/apps/backend/internal/agent/registry/registry.go index ea03bd2626..fcd2ebb705 100644 --- a/apps/backend/internal/agent/registry/registry.go +++ b/apps/backend/internal/agent/registry/registry.go @@ -61,6 +61,7 @@ func (r *Registry) LoadDefaults() { agents.NewKiroACP(), agents.NewQoderACP(), agents.NewTraeACP(), + agents.NewOmpACP(), agents.NewMockAgent(), } diff --git a/apps/backend/internal/agentctl/server/api/agent.go b/apps/backend/internal/agentctl/server/api/agent.go index 5cf0a06d62..84cfda0e38 100644 --- a/apps/backend/internal/agentctl/server/api/agent.go +++ b/apps/backend/internal/agentctl/server/api/agent.go @@ -313,8 +313,10 @@ func (s *Server) handleWSInitialize(ctx context.Context, msg *ws.Message) *ws.Me } // injectKandevMcpServers prepends the local kandev MCP server to the list of MCP servers. -// Both SSE and HTTP variants are injected - the agent's capability filtering will select -// the appropriate one based on what the agent supports (SSE for Claude Code, HTTP for Codex ACP). +// Both HTTP and SSE variants are injected - the agent's capability filtering will select +// the appropriate one based on what the agent supports. HTTP is listed first so that +// when an agent advertises both transports the "first surviving entry wins" dedup keeps +// the HTTP entry (modern streamable MCP); SSE remains as a fallback for SSE-only agents. // Any existing kandev server in the list is filtered out to avoid duplicates. func (s *Server) injectKandevMcpServers(mcpServers []types.McpServer) []types.McpServer { kandevMcpSse := types.McpServer{ @@ -328,15 +330,15 @@ func (s *Server) injectKandevMcpServers(mcpServers []types.McpServer) []types.Mc URL: fmt.Sprintf("http://localhost:%d%s", s.cfg.Port, mcpPathHTTP), } filtered := make([]types.McpServer, 0, len(mcpServers)+2) - filtered = append(filtered, kandevMcpSse, kandevMcpHttp) + filtered = append(filtered, kandevMcpHttp, kandevMcpSse) for _, srv := range mcpServers { if srv.Name != kandevMcpServerName { filtered = append(filtered, srv) } } - s.logger.Debug("injected local kandev MCP servers (sse+http)", - zap.String("sse_url", kandevMcpSse.URL), + s.logger.Debug("injected local kandev MCP servers (http+sse)", zap.String("http_url", kandevMcpHttp.URL), + zap.String("sse_url", kandevMcpSse.URL), zap.Int("total_servers", len(filtered))) return filtered } diff --git a/apps/backend/internal/agentctl/server/api/agent_test.go b/apps/backend/internal/agentctl/server/api/agent_test.go index a160070da9..5707dc05c3 100644 --- a/apps/backend/internal/agentctl/server/api/agent_test.go +++ b/apps/backend/internal/agentctl/server/api/agent_test.go @@ -11,6 +11,7 @@ import ( "github.com/gorilla/websocket" "github.com/kandev/kandev/internal/agentctl/server/config" "github.com/kandev/kandev/internal/agentctl/server/process" + "github.com/kandev/kandev/internal/agentctl/types" "github.com/kandev/kandev/internal/common/logger" ws "github.com/kandev/kandev/pkg/websocket" ) @@ -490,3 +491,44 @@ func TestAgentStreamWS_MalformedMessage(t *testing.T) { t.Errorf("expected response after malformed message, got %q", resp.Type) } } + +// --- injectKandevMcpServers ordering --- + +// TestInjectKandevMcpServers_HttpFirst ensures the kandev MCP HTTP entry is injected +// before the SSE entry so that the capability-filter dedup (first surviving entry per +// name wins) prefers the HTTP transport whenever an agent advertises both +// mcpCapabilities.http and mcpCapabilities.sse. SSE-only agents still pick up the SSE +// fallback. +func TestInjectKandevMcpServers_HttpFirst(t *testing.T) { + srv := newTestServer(t) + + got := srv.injectKandevMcpServers(nil) + if len(got) != 2 { + t.Fatalf("expected 2 kandev entries, got %d: %+v", len(got), got) + } + if got[0].Name != kandevMcpServerName || got[0].Type != mcpTransportHTTP { + t.Errorf("expected first entry name=%q type=%q, got name=%q type=%q", + kandevMcpServerName, mcpTransportHTTP, got[0].Name, got[0].Type) + } + if got[1].Name != kandevMcpServerName || got[1].Type != mcpTransportSSE { + t.Errorf("expected second entry name=%q type=%q, got name=%q type=%q", + kandevMcpServerName, mcpTransportSSE, got[1].Name, got[1].Type) + } + + // Upstream non-kandev entries must be preserved and appended after the injected pair; + // any upstream "kandev" must be filtered out. + upstream := []types.McpServer{ + {Name: "other", Type: "stdio", Command: "x"}, + {Name: kandevMcpServerName, Type: mcpTransportSSE, URL: "http://stale/sse"}, + } + got = srv.injectKandevMcpServers(upstream) + if len(got) != 3 { + t.Fatalf("expected 3 entries (http+sse+other), got %d: %+v", len(got), got) + } + if got[0].Type != mcpTransportHTTP || got[1].Type != mcpTransportSSE { + t.Errorf("expected injected order http,sse; got %q,%q", got[0].Type, got[1].Type) + } + if got[2].Name != "other" || got[2].Command != "x" { + t.Errorf("expected upstream 'other' entry last, got %+v", got[2]) + } +} diff --git a/apps/backend/internal/agentctl/server/config/config.go b/apps/backend/internal/agentctl/server/config/config.go index 682be1590a..356e8b452c 100644 --- a/apps/backend/internal/agentctl/server/config/config.go +++ b/apps/backend/internal/agentctl/server/config/config.go @@ -465,7 +465,10 @@ const kandevMcpServerName = "kandev" // injectKandevMcpServer prepends the local kandev MCP server to the list of MCP servers. // This replaces any existing kandev server to avoid duplicates. // The kandev MCP server provides tools like ask_user_question to the agent. -// Both SSE and HTTP variants are injected - agent capability filtering will select the appropriate one. +// Both HTTP and SSE variants are injected - agent capability filtering will select the +// appropriate one. HTTP is listed first so that when an agent advertises both transports +// the "first surviving entry wins" dedup keeps the HTTP entry (modern streamable MCP); +// SSE remains as a fallback for SSE-only agents. func injectKandevMcpServer(servers []McpServerConfig, port int) []McpServerConfig { portStr := strconv.Itoa(port) kandevMcpSse := McpServerConfig{ @@ -481,7 +484,7 @@ func injectKandevMcpServer(servers []McpServerConfig, port int) []McpServerConfi // Filter out any existing kandev server and prepend the local ones result := make([]McpServerConfig, 0, len(servers)+2) - result = append(result, kandevMcpSse, kandevMcpHttp) + result = append(result, kandevMcpHttp, kandevMcpSse) for _, srv := range servers { if srv.Name != kandevMcpServerName { result = append(result, srv) diff --git a/apps/backend/internal/agentctl/server/config/config_test.go b/apps/backend/internal/agentctl/server/config/config_test.go index 2b6db8b737..bf9fac1af0 100644 --- a/apps/backend/internal/agentctl/server/config/config_test.go +++ b/apps/backend/internal/agentctl/server/config/config_test.go @@ -75,3 +75,43 @@ func TestGenerateSelfToken(t *testing.T) { t.Fatal("expected unique tokens, got identical") } } + +// TestInjectKandevMcpServer_HttpFirst is the McpServerConfig counterpart to +// api.TestInjectKandevMcpServers_HttpFirst. HTTP must appear before SSE so the +// downstream capability filter dedup keeps the HTTP transport for agents that +// advertise both transports. +func TestInjectKandevMcpServer_HttpFirst(t *testing.T) { + got := injectKandevMcpServer(nil, 12345) + if len(got) != 2 { + t.Fatalf("expected 2 kandev entries, got %d: %+v", len(got), got) + } + if got[0].Name != kandevMcpServerName || got[0].Type != "http" { + t.Errorf("expected first entry name=%q type=http, got name=%q type=%q", + kandevMcpServerName, got[0].Name, got[0].Type) + } + if got[0].URL != "http://localhost:12345/mcp" { + t.Errorf("expected http URL .../mcp, got %q", got[0].URL) + } + if got[1].Name != kandevMcpServerName || got[1].Type != "sse" { + t.Errorf("expected second entry name=%q type=sse, got name=%q type=%q", + kandevMcpServerName, got[1].Name, got[1].Type) + } + if got[1].URL != "http://localhost:12345/sse" { + t.Errorf("expected sse URL .../sse, got %q", got[1].URL) + } + + upstream := []McpServerConfig{ + {Name: "other", Type: "stdio", Command: "x"}, + {Name: kandevMcpServerName, Type: "sse", URL: "http://stale/sse"}, + } + got = injectKandevMcpServer(upstream, 12345) + if len(got) != 3 { + t.Fatalf("expected 3 entries (http+sse+other), got %d: %+v", len(got), got) + } + if got[0].Type != "http" || got[1].Type != "sse" { + t.Errorf("expected injected order http,sse; got %q,%q", got[0].Type, got[1].Type) + } + if got[2].Name != "other" || got[2].Command != "x" { + t.Errorf("expected upstream 'other' entry last, got %+v", got[2]) + } +} diff --git a/apps/backend/internal/agentctl/server/utility/acp_executor.go b/apps/backend/internal/agentctl/server/utility/acp_executor.go index 24df809aa4..66d0a0b319 100644 --- a/apps/backend/internal/agentctl/server/utility/acp_executor.go +++ b/apps/backend/internal/agentctl/server/utility/acp_executor.go @@ -475,10 +475,16 @@ func derefString(p *string) string { // is not derived from untrusted input — even though the value is // semantically the same as the base name taken from InferenceConfig.Command. var allowedProbeCommands = map[string]string{ - "npx": "npx", - "auggie": "auggie", - "opencode": "opencode", - "mock-agent": "mock-agent", + "auggie": "auggie", + "cursor-agent": "cursor-agent", + "kimi": "kimi", + "kiro-cli-chat": "kiro-cli-chat", + "mock-agent": "mock-agent", + "npx": "npx", + "omp": "omp", + "opencode": "opencode", + "qodercli": "qodercli", + "traecli": "traecli", } // resolveProbeCommand validates and returns a hard-coded executable name for diff --git a/apps/backend/internal/agentctl/server/utility/probe_allowlist_test.go b/apps/backend/internal/agentctl/server/utility/probe_allowlist_test.go new file mode 100644 index 0000000000..c6d2ef0edb --- /dev/null +++ b/apps/backend/internal/agentctl/server/utility/probe_allowlist_test.go @@ -0,0 +1,42 @@ +package utility + +import ( + "testing" + + "github.com/kandev/kandev/internal/agent/agents" + "github.com/kandev/kandev/internal/agent/registry" + "github.com/kandev/kandev/internal/common/logger" +) + +// TestProbeAllowlist_CoversEveryEnabledInferenceAgent guards against the +// `command "" is not an allowed ACP probe command` regression: every +// agent the registry exposes as an InferenceAgent must have its primary +// command (filepath.Base of cfg.Command.Args()[0]) listed in +// allowedProbeCommands, otherwise the host-utility probe rejects it the +// moment a user installs that CLI on PATH. +func TestProbeAllowlist_CoversEveryEnabledInferenceAgent(t *testing.T) { + t.Parallel() + + log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "json"}) + if err != nil { + t.Fatalf("logger.NewLogger: %v", err) + } + + reg := registry.NewRegistry(log) + reg.LoadDefaults() + + for _, ag := range reg.ListEnabled() { + ia, ok := ag.(agents.InferenceAgent) + if !ok { + continue + } + cfg := ia.InferenceConfig() + if cfg == nil || !cfg.Supported || cfg.Command.IsEmpty() { + continue + } + primary := cfg.Command.Args()[0] + if resolveProbeCommand(primary) == "" { + t.Errorf("agent %q: probe binary %q missing from allowedProbeCommands", ag.ID(), primary) + } + } +}