From 16f1e0c42e32dec1d1006ef773732e64bc155421 Mon Sep 17 00:00:00 2001 From: Fahad <42780409+F2had@users.noreply.github.com> Date: Mon, 3 Aug 2026 22:58:00 +0300 Subject: [PATCH 1/2] fix(docker): report a removed container as not running ContainerInspect returns a not-found error once a container is gone from the daemon, and IsContainerRunning wrapped it as an inspection failure, so callers could not tell a missing container apart from an unreachable daemon. StopContainer and RemoveContainer already special-case client.IsErrNotFound; do the same here. --- backend/pkg/docker/client.go | 6 ++- backend/pkg/docker/client_test.go | 77 +++++++++++++++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/backend/pkg/docker/client.go b/backend/pkg/docker/client.go index 6a114561..af5604d4 100644 --- a/backend/pkg/docker/client.go +++ b/backend/pkg/docker/client.go @@ -600,7 +600,11 @@ func (dc *dockerClient) Cleanup(ctx context.Context) error { func (dc *dockerClient) IsContainerRunning(ctx context.Context, containerID string) (bool, error) { inspection, err := dc.client.ContainerInspect(ctx, containerID) if err != nil { - return false, fmt.Errorf("container inspection failed: %w", err) + if !client.IsErrNotFound(err) { + return false, fmt.Errorf("container inspection failed: %w", err) + } + // a removed container is missing, not an inspection failure + return false, nil } // Check both Running state and health status if available diff --git a/backend/pkg/docker/client_test.go b/backend/pkg/docker/client_test.go index f7ae3a23..62de6f6e 100644 --- a/backend/pkg/docker/client_test.go +++ b/backend/pkg/docker/client_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/docker/docker/api/types/container" + "github.com/docker/docker/client" ) func TestStatContainerEntries_AllSucceed(t *testing.T) { @@ -260,3 +261,79 @@ func failNames(failures []statFailure) map[string]bool { } return m } + +const probeImage = "alpine:3.20" + +// newDaemonClient binds a client to the local daemon, skipping the test when +// none is reachable. +func newDaemonClient(t *testing.T) *dockerClient { + t.Helper() + + cli, err := client.NewClientWithOpts(client.FromEnv) + if err != nil { + t.Skipf("docker daemon unavailable: %v", err) + } + + ctx := context.Background() + cli.NegotiateAPIVersion(ctx) + if _, err := cli.Ping(ctx); err != nil { + t.Skipf("docker daemon unavailable: %v", err) + } + + return &dockerClient{client: cli} +} + +// A flow keeps the id of its primary container in the database. When that +// container is removed behind pentagi's back the id has to read as not running, +// otherwise the flow can never rebuild it. +func TestIsContainerRunningRemovedContainer(t *testing.T) { + dc := newDaemonClient(t) + ctx := context.Background() + + created, err := dc.client.ContainerCreate(ctx, &container.Config{ + Image: probeImage, + Entrypoint: []string{"tail", "-f", "/dev/null"}, + }, nil, nil, nil, "") + if client.IsErrNotFound(err) { + t.Skipf("%s is not present locally", probeImage) + } + if err != nil { + t.Fatalf("create probe container: %v", err) + } + t.Cleanup(func() { + dc.client.ContainerRemove(context.Background(), created.ID, container.RemoveOptions{Force: true}) + }) + + if err := dc.client.ContainerStart(ctx, created.ID, container.StartOptions{}); err != nil { + t.Fatalf("start probe container: %v", err) + } + + running, err := dc.IsContainerRunning(ctx, created.ID) + if err != nil || !running { + t.Fatalf("got running=%v err=%v, want true and no error", running, err) + } + + if err := dc.client.ContainerRemove(ctx, created.ID, container.RemoveOptions{Force: true}); err != nil { + t.Fatalf("remove probe container: %v", err) + } + + running, err = dc.IsContainerRunning(ctx, created.ID) + if err != nil { + t.Fatalf("removed container: got error %v, want none", err) + } + if running { + t.Fatal("removed container reported as running") + } +} + +func TestIsContainerRunningUnknownContainer(t *testing.T) { + dc := newDaemonClient(t) + + running, err := dc.IsContainerRunning(context.Background(), "pentagi-container-that-does-not-exist") + if err != nil { + t.Fatalf("unknown container: got error %v, want none", err) + } + if running { + t.Fatal("unknown container reported as running") + } +} From 7d675b313a6f97e6699d28e0f3d608912a2c3e76 Mon Sep 17 00:00:00 2001 From: Fahad <42780409+F2had@users.noreply.github.com> Date: Mon, 3 Aug 2026 22:58:38 +0300 Subject: [PATCH 2/2] fix(tools): rebuild the flow primary container when the daemon lost it A flow stores the id of its primary container in the database. When that container is removed outside pentagi the row keeps status 'running', so Prepare reused a container the daemon no longer knows about and every terminal call failed with "No such container" until the flow was recreated. Confirm with the daemon before reusing the stored container and let the existing remove-and-rebuild path take over when it is gone. An unreachable daemon is still an error, so a transient failure cannot discard a healthy container. --- backend/pkg/tools/tools.go | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/backend/pkg/tools/tools.go b/backend/pkg/tools/tools.go index a460e15d..c3d8b107 100644 --- a/backend/pkg/tools/tools.go +++ b/backend/pkg/tools/tools.go @@ -481,18 +481,24 @@ func (fte *flowToolsExecutor) SetGraphitiClient(client *graphiti.Client) { func (fte *flowToolsExecutor) Prepare(ctx context.Context) error { if cnt, err := fte.db.GetFlowPrimaryContainer(ctx, fte.flowID); err == nil { - switch cnt.Status { - case database.ContainerStatusRunning: - fte.primaryID = cnt.ID - fte.primaryLID = cnt.LocalID.String - if err := fte.syncMissingFiles(ctx); err != nil { - containerName := PrimaryTerminalName(fte.cfg.TenantPrefix(), fte.flowID) - return fmt.Errorf("failed to sync missing files to container '%s': %w", containerName, err) + // the stored status goes stale when the container is removed outside pentagi + if cnt.Status == database.ContainerStatusRunning { + containerName := PrimaryTerminalName(fte.cfg.TenantPrefix(), fte.flowID) + running, err := fte.docker.IsContainerRunning(ctx, cnt.LocalID.String) + if err != nil { + return fmt.Errorf("failed to inspect container '%s': %w", containerName, err) + } + if running { + fte.primaryID = cnt.ID + fte.primaryLID = cnt.LocalID.String + if err := fte.syncMissingFiles(ctx); err != nil { + return fmt.Errorf("failed to sync missing files to container '%s': %w", containerName, err) + } + return nil } - return nil - default: - fte.docker.RemoveContainer(ctx, cnt.LocalID.String, cnt.ID) } + + fte.docker.RemoveContainer(ctx, cnt.LocalID.String, cnt.ID) } // Explicit capability allow-list (CapDrop: ALL below): Docker's default 14