From efa32a7712bb11db1a1057840bcf6d3799fb486c Mon Sep 17 00:00:00 2001 From: Kyle Isom Date: Thu, 26 Mar 2026 15:13:20 -0700 Subject: [PATCH] Fix container name handling for hyphenated service names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract ContainerNameFor and SplitContainerName into names.go. ContainerNameFor handles single-component services where service name equals component name (e.g., mc-proxy → "mc-proxy" not "mc-proxy-mc-proxy"). SplitContainerName checks known services from the registry before falling back to naive split on "-", fixing mc-proxy being misidentified as service "mc" component "proxy". Also fixes podman ps JSON parsing (Command field is []string not string) found during deployment. Co-Authored-By: Claude Opus 4.6 (1M context) --- deploy/systemd/mcp-agent.service | 2 ++ internal/agent/deploy.go | 2 +- internal/agent/lifecycle.go | 8 ++++---- internal/agent/names.go | 34 ++++++++++++++++++++++++++++++++ internal/agent/status.go | 18 +++++------------ internal/agent/status_test.go | 31 ++++++++++++++++++++++++++--- internal/runtime/podman.go | 2 +- 7 files changed, 75 insertions(+), 22 deletions(-) create mode 100644 internal/agent/names.go diff --git a/deploy/systemd/mcp-agent.service b/deploy/systemd/mcp-agent.service index 0d8a274..090f1eb 100644 --- a/deploy/systemd/mcp-agent.service +++ b/deploy/systemd/mcp-agent.service @@ -11,6 +11,8 @@ RestartSec=5 User=mcp Group=mcp +Environment=HOME=/srv/mcp +Environment=XDG_RUNTIME_DIR=/run/user/%U NoNewPrivileges=true ProtectSystem=strict diff --git a/internal/agent/deploy.go b/internal/agent/deploy.go index 9591527..fb0692c 100644 --- a/internal/agent/deploy.go +++ b/internal/agent/deploy.go @@ -49,7 +49,7 @@ func (a *Agent) Deploy(ctx context.Context, req *mcpv1.DeployRequest) (*mcpv1.De // deployComponent handles the full deploy lifecycle for a single component. func (a *Agent) deployComponent(ctx context.Context, serviceName string, cs *mcpv1.ComponentSpec, active bool) *mcpv1.ComponentResult { compName := cs.GetName() - containerName := serviceName + "-" + compName + containerName := ContainerNameFor(serviceName, compName) desiredState := "running" if !active { diff --git a/internal/agent/lifecycle.go b/internal/agent/lifecycle.go index c7f5f1b..4e5b618 100644 --- a/internal/agent/lifecycle.go +++ b/internal/agent/lifecycle.go @@ -27,7 +27,7 @@ func (a *Agent) StopService(ctx context.Context, req *mcpv1.StopServiceRequest) var results []*mcpv1.ComponentResult for _, c := range components { - containerName := req.GetName() + "-" + c.Name + containerName := ContainerNameFor(req.GetName(), c.Name) r := &mcpv1.ComponentResult{Name: c.Name, Success: true} if err := a.Runtime.Stop(ctx, containerName); err != nil { @@ -94,7 +94,7 @@ func (a *Agent) RestartService(ctx context.Context, req *mcpv1.RestartServiceReq // startComponent removes any existing container and runs a fresh one from // the registry spec, then updates state to running. func startComponent(ctx context.Context, a *Agent, service string, c *registry.Component) *mcpv1.ComponentResult { - containerName := service + "-" + c.Name + containerName := ContainerNameFor(service, c.Name) r := &mcpv1.ComponentResult{Name: c.Name, Success: true} // Remove any pre-existing container; ignore errors for non-existent ones. @@ -118,7 +118,7 @@ func startComponent(ctx context.Context, a *Agent, service string, c *registry.C // restartComponent stops, removes, and re-creates a container without // changing the desired_state in the registry. func restartComponent(ctx context.Context, a *Agent, service string, c *registry.Component) *mcpv1.ComponentResult { - containerName := service + "-" + c.Name + containerName := ContainerNameFor(service, c.Name) r := &mcpv1.ComponentResult{Name: c.Name, Success: true} _ = a.Runtime.Stop(ctx, containerName) @@ -142,7 +142,7 @@ func restartComponent(ctx context.Context, a *Agent, service string, c *registry // componentToSpec builds a runtime.ContainerSpec from a registry Component. func componentToSpec(service string, c *registry.Component) runtime.ContainerSpec { return runtime.ContainerSpec{ - Name: service + "-" + c.Name, + Name: ContainerNameFor(service, c.Name), Image: c.Image, Network: c.Network, User: c.UserSpec, diff --git a/internal/agent/names.go b/internal/agent/names.go new file mode 100644 index 0000000..33355ae --- /dev/null +++ b/internal/agent/names.go @@ -0,0 +1,34 @@ +package agent + +import "strings" + +// ContainerNameFor returns the expected container name for a service and +// component. For single-component services where the component name equals +// the service name, the container name is just the service name (e.g., +// "mc-proxy" not "mc-proxy-mc-proxy"). +func ContainerNameFor(service, component string) string { + if service == component { + return service + } + return service + "-" + component +} + +// SplitContainerName splits a container name into service and component parts. +// It checks known service names first to handle names like "mc-proxy" where a +// naive split on "-" would produce the wrong result. If no known service +// matches, it falls back to splitting on the first "-". +func SplitContainerName(name string, knownServices map[string]bool) (service, component string) { + if knownServices[name] { + return name, name + } + for svc := range knownServices { + prefix := svc + "-" + if strings.HasPrefix(name, prefix) && len(name) > len(prefix) { + return svc, name[len(prefix):] + } + } + if i := strings.Index(name, "-"); i >= 0 { + return name[:i], name[i+1:] + } + return name, name +} diff --git a/internal/agent/status.go b/internal/agent/status.go index 4be4691..9c1e337 100644 --- a/internal/agent/status.go +++ b/internal/agent/status.go @@ -3,7 +3,6 @@ package agent import ( "context" "fmt" - "strings" "time" mcpv1 "git.wntrmute.dev/kyle/mcp/gen/mcp/v1" @@ -75,7 +74,10 @@ func (a *Agent) liveCheckServices(ctx context.Context) ([]*mcpv1.ServiceInfo, er } var result []*mcpv1.ServiceInfo + knownServices := make(map[string]bool, len(services)) for _, svc := range services { + knownServices[svc.Name] = true + components, err := registry.ListComponents(a.DB, svc.Name) if err != nil { return nil, fmt.Errorf("list components for %q: %w", svc.Name, err) @@ -87,7 +89,7 @@ func (a *Agent) liveCheckServices(ctx context.Context) ([]*mcpv1.ServiceInfo, er } for _, comp := range components { - containerName := svc.Name + "-" + comp.Name + containerName := ContainerNameFor(svc.Name, comp.Name) ci := &mcpv1.ComponentInfo{ Name: comp.Name, Image: comp.Image, @@ -116,7 +118,7 @@ func (a *Agent) liveCheckServices(ctx context.Context) ([]*mcpv1.ServiceInfo, er continue } - svcName, compName := splitContainerName(c.Name) + svcName, compName := SplitContainerName(c.Name, knownServices) result = append(result, &mcpv1.ServiceInfo{ Name: svcName, @@ -210,13 +212,3 @@ func (a *Agent) GetServiceStatus(ctx context.Context, req *mcpv1.GetServiceStatu RecentEvents: protoEvents, }, nil } - -// splitContainerName splits a container name like "metacrypt-api" into service -// and component parts. If there is no hyphen, the whole name is used as both -// the service and component name. -func splitContainerName(name string) (service, component string) { - if i := strings.Index(name, "-"); i >= 0 { - return name[:i], name[i+1:] - } - return name, name -} diff --git a/internal/agent/status_test.go b/internal/agent/status_test.go index 8a894c5..cbc962f 100644 --- a/internal/agent/status_test.go +++ b/internal/agent/status_test.go @@ -253,22 +253,47 @@ func TestGetServiceStatus_IgnoreSkipsDrift(t *testing.T) { } func TestSplitContainerName(t *testing.T) { + known := map[string]bool{ + "metacrypt": true, + "mc-proxy": true, + "mcr": true, + } tests := []struct { name string service string comp string }{ {"metacrypt-api", "metacrypt", "api"}, - {"metacrypt-web-ui", "metacrypt", "web-ui"}, + {"metacrypt-web", "metacrypt", "web"}, + {"mc-proxy", "mc-proxy", "mc-proxy"}, + {"mcr-api", "mcr", "api"}, {"standalone", "standalone", "standalone"}, + {"unknown-thing", "unknown", "thing"}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - svc, comp := splitContainerName(tt.name) + svc, comp := SplitContainerName(tt.name, known) if svc != tt.service || comp != tt.comp { - t.Fatalf("splitContainerName(%q) = (%q, %q), want (%q, %q)", + t.Fatalf("SplitContainerName(%q) = (%q, %q), want (%q, %q)", tt.name, svc, comp, tt.service, tt.comp) } }) } } + +func TestContainerNameFor(t *testing.T) { + tests := []struct { + service, component, want string + }{ + {"metacrypt", "api", "metacrypt-api"}, + {"mc-proxy", "mc-proxy", "mc-proxy"}, + {"mcr", "web", "mcr-web"}, + } + for _, tt := range tests { + got := ContainerNameFor(tt.service, tt.component) + if got != tt.want { + t.Fatalf("ContainerNameFor(%q, %q) = %q, want %q", + tt.service, tt.component, got, tt.want) + } + } +} diff --git a/internal/runtime/podman.go b/internal/runtime/podman.go index 230649d..dbea148 100644 --- a/internal/runtime/podman.go +++ b/internal/runtime/podman.go @@ -179,7 +179,7 @@ type podmanPSEntry struct { Names []string `json:"Names"` Image string `json:"Image"` State string `json:"State"` - Command string `json:"Command"` + Command []string `json:"Command"` } // List returns information about all containers.