From a058dc287a21c1397f73f35481ecdb39d6b0329c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 5 Apr 2026 02:32:52 +0000 Subject: [PATCH] Simplify findContainerFlexible: drop name-to-ID resolution, just search all hosts If the direct host lookup fails (wrong host ID, host name used instead, etc.), simply fall back to searching across all hosts instead of trying to resolve host names to IDs. https://claude.ai/code/session_01EoVbj5ecZzsZfroGiNALp3 --- internal/cloud/tools_helpers.go | 23 +++++------------------ internal/cloud/tools_test.go | 8 +++++--- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/internal/cloud/tools_helpers.go b/internal/cloud/tools_helpers.go index 4fff7644..aff72bb6 100644 --- a/internal/cloud/tools_helpers.go +++ b/internal/cloud/tools_helpers.go @@ -66,32 +66,22 @@ func logHostErrors(errs []error) { } // findContainerFlexible finds a container by ID, optionally scoped to a specific host. -// When hostID is empty, it searches across all hosts by listing all containers and matching by ID. -// When hostID is provided, it also tries to resolve host names to host IDs for LLM-friendliness. +// When hostID is provided and valid, it uses direct lookup for efficiency. +// Otherwise it searches across all hosts since container IDs are unique. func findContainerFlexible(hostID string, containerID string, hostService ToolHostService, labels container.ContainerLabels) (*container_support.ContainerService, error) { if containerID == "" { return nil, fmt.Errorf("container_id is required") } - // If host is provided, try direct lookup first, then try resolving name to ID + // Try direct lookup if host is provided if hostID != "" { cs, err := hostService.FindContainer(hostID, containerID, labels) if err == nil { return cs, nil } - - // Try treating hostID as a host name and resolve to actual ID - for _, h := range hostService.Hosts() { - if strings.EqualFold(h.Name, hostID) { - cs, err := hostService.FindContainer(h.ID, containerID, labels) - if err == nil { - return cs, nil - } - } - } } - // Search across all hosts by listing all containers + // Fall back to searching across all hosts containers, errs := hostService.ListAllContainers(labels) logHostErrors(errs) @@ -101,8 +91,5 @@ func findContainerFlexible(hostID string, containerID string, hostService ToolHo } } - if hostID != "" { - return nil, fmt.Errorf("container %s not found on host %s", containerID, hostID) - } - return nil, fmt.Errorf("container %s not found on any host", containerID) + return nil, fmt.Errorf("container %s not found", containerID) } diff --git a/internal/cloud/tools_test.go b/internal/cloud/tools_test.go index 062a0045..d19948ab 100644 --- a/internal/cloud/tools_test.go +++ b/internal/cloud/tools_test.go @@ -209,16 +209,18 @@ func TestExecuteTool_RestartContainer_WithoutHostID(t *testing.T) { assert.Equal(t, "abc123", action.ContainerId) } -func TestExecuteTool_RestartContainer_WithHostName(t *testing.T) { +func TestExecuteTool_RestartContainer_WithWrongHost(t *testing.T) { mockClient := &MockClientService{} mockClient.On("ContainerAction", mock.Anything, mock.Anything, container.Restart).Return(nil) cs := container_support.NewContainerService(mockClient, container.Container{ID: "abc123"}) mockHost := &MockHostService{} - // First FindContainer with host name "my-server" fails, then we resolve name to ID + // LLM passes wrong host value, direct lookup fails, falls back to searching all hosts mockHost.On("FindContainer", "my-server", "abc123", container.ContainerLabels(nil)).Return(nil, fmt.Errorf("host not found")) - mockHost.On("Hosts").Return([]container.Host{{ID: "local", Name: "my-server"}}) + mockHost.On("ListAllContainers", container.ContainerLabels(nil)).Return([]container.Container{ + {ID: "abc123", Name: "nginx", Image: "nginx:latest", State: "running", Host: "local"}, + }, nil) mockHost.On("FindContainer", "local", "abc123", container.ContainerLabels(nil)).Return(cs, nil) argsJSON := `{"container_id": "abc123", "host_id": "my-server"}`