diff --git a/internal/agent/service.go b/internal/agent/service.go index e7fb7929..2e05a567 100644 --- a/internal/agent/service.go +++ b/internal/agent/service.go @@ -1206,6 +1206,8 @@ func (s *Service) Delete(ctx context.Context, id string) error { return fmt.Errorf("agent %q not found", id) } + s.stopLifecycleAgent(id) + if runtimeImpl, err := s.runtimeForKind(strings.TrimSpace(existing.RuntimeKind)); err == nil && strings.TrimSpace(existing.BoxID) != "" { if err := runtimeImpl.Delete(ctx, runtimeHandleForAgent(existing)); err != nil && !sandbox.IsNotFound(err) { return fmt.Errorf("remove agent box: %w", err) @@ -1220,15 +1222,8 @@ func (s *Service) Delete(ctx context.Context, id string) error { return homeErr } if rt != nil { - boxIDOrName := strings.TrimSpace(existing.BoxID) - if boxIDOrName == "" { - boxIDOrName = existing.Name - } - if _, resolvedKey, resolveErr := s.resolveAgentBox(ctx, rt, existing); resolveErr == nil && strings.TrimSpace(resolvedKey) != "" { - boxIDOrName = resolvedKey - } - if err := s.forceRemoveBox(ctx, rt, boxIDOrName); err != nil && !sandbox.IsNotFound(err) { - return fmt.Errorf("remove agent box: %w", err) + if err := s.stopAndForceRemoveBox(ctx, rt, existing); err != nil { + return err } _ = s.closeRuntime(runtimeHome, rt) } @@ -1264,7 +1259,30 @@ func (s *Service) Delete(ctx context.Context, id string) error { return err } s.mu.Unlock() - s.stopLifecycleAgent(id) + return nil +} + +func (s *Service) stopAndForceRemoveBox(ctx context.Context, rt sandbox.Runtime, got Agent) error { + boxIDOrName := strings.TrimSpace(got.BoxID) + if boxIDOrName == "" { + boxIDOrName = strings.TrimSpace(got.Name) + } + box, resolvedKey, err := s.resolveAgentBox(ctx, rt, got) + if err == nil && box != nil { + if key := strings.TrimSpace(resolvedKey); key != "" { + boxIDOrName = key + } + if stopErr := s.stopBox(ctx, box, sandbox.StopOptions{}); stopErr != nil && !sandbox.IsNotFound(stopErr) { + _ = s.closeBox(box) + return fmt.Errorf("stop agent box: %w", stopErr) + } + _ = s.closeBox(box) + } else if err != nil && !sandbox.IsNotFound(err) { + return fmt.Errorf("resolve agent box: %w", err) + } + if err := s.forceRemoveBox(ctx, rt, boxIDOrName); err != nil && !sandbox.IsNotFound(err) { + return fmt.Errorf("remove agent box: %w", err) + } return nil } @@ -1294,7 +1312,11 @@ func removeAllWithRetry(path string) error { } func isRetryableRemoveAllError(err error) bool { - return errors.Is(err, syscall.ENOTEMPTY) || strings.Contains(strings.ToLower(err.Error()), "directory not empty") + if errors.Is(err, syscall.ENOTEMPTY) || errors.Is(err, syscall.EACCES) { + return true + } + lower := strings.ToLower(err.Error()) + return strings.Contains(lower, "directory not empty") || strings.Contains(lower, "permission denied") } func (s *Service) List() []Agent { diff --git a/internal/agent/service_test.go b/internal/agent/service_test.go index a04405f6..92a9e7e5 100644 --- a/internal/agent/service_test.go +++ b/internal/agent/service_test.go @@ -2569,17 +2569,30 @@ func TestDeletePrefersBoxIDOverName(t *testing.T) { defer ResetTestHooks() var removed string + var calls []string + testStopBoxHook = func(_ *Service, _ context.Context, _ sandbox.Instance, opts sandbox.StopOptions) error { + calls = append(calls, "stop") + return nil + } testForceRemoveBoxHook = func(_ *Service, _ context.Context, _ sandbox.Runtime, idOrName string) error { + calls = append(calls, "remove") removed = idOrName return nil } - testGetBoxHook = func(_ *Service, _ context.Context, _ sandbox.Runtime, _ string) (sandbox.Instance, error) { + testGetBoxHook = func(_ *Service, _ context.Context, _ sandbox.Runtime, idOrName string) (sandbox.Instance, error) { + if idOrName == "box-123" { + return &fakeInstance{}, nil + } return nil, fmt.Errorf("%w: missing", sandbox.ErrNotFound) } defer func() { + testStopBoxHook = nil testForceRemoveBoxHook = nil }() + homeDir := t.TempDir() + t.Setenv("HOME", homeDir) + svc, err := NewService(testModelConfig(), config.ServerConfig{}, "manager-image:test", "") if err != nil { t.Fatalf("NewService() error = %v", err) @@ -2600,6 +2613,68 @@ func TestDeletePrefersBoxIDOverName(t *testing.T) { if removed != "box-123" { t.Fatalf("ForceRemove() target = %q, want %q", removed, "box-123") } + if strings.Join(calls, ",") != "stop,remove" { + t.Fatalf("Delete() sandbox calls = %q, want stop then remove", strings.Join(calls, ",")) + } +} + +func TestDeleteStopsBoxBeforeRemoveOnLegacyPath(t *testing.T) { + SetTestHooks( + func(_ *Service, _ string) (sandbox.Runtime, error) { return &fakeRuntime{}, nil }, + nil, + ) + defer ResetTestHooks() + + var calls []string + testGetBoxHook = func(_ *Service, _ context.Context, _ sandbox.Runtime, idOrName string) (sandbox.Instance, error) { + if idOrName == "alice" { + return &fakeInstance{}, nil + } + return nil, fmt.Errorf("%w: missing", sandbox.ErrNotFound) + } + testStopBoxHook = func(_ *Service, _ context.Context, _ sandbox.Instance, _ sandbox.StopOptions) error { + calls = append(calls, "stop") + return nil + } + testForceRemoveBoxHook = func(_ *Service, _ context.Context, _ sandbox.Runtime, idOrName string) error { + calls = append(calls, "remove:"+idOrName) + return nil + } + defer func() { + testGetBoxHook = nil + testStopBoxHook = nil + testForceRemoveBoxHook = nil + }() + + homeDir := t.TempDir() + t.Setenv("HOME", homeDir) + + svc, err := NewService(testModelConfig(), config.ServerConfig{}, "manager-image:test", "") + if err != nil { + t.Fatalf("NewService() error = %v", err) + } + svc.agents["u-alice"] = Agent{ + ID: "u-alice", + Name: "alice", + Role: RoleWorker, + Status: "running", + CreatedAt: time.Date(2026, 4, 1, 11, 0, 0, 0, time.UTC), + } + + agentHome, err := agentHomeDir("alice") + if err != nil { + t.Fatalf("agentHomeDir() error = %v", err) + } + if err := os.MkdirAll(filepath.Join(agentHome, config.RuntimeHomeDirName), 0o755); err != nil { + t.Fatalf("os.MkdirAll(agent runtime) error = %v", err) + } + + if err := svc.Delete(context.Background(), "u-alice"); err != nil { + t.Fatalf("Delete() error = %v", err) + } + if strings.Join(calls, ",") != "stop,remove:alice" { + t.Fatalf("Delete() sandbox calls = %q, want stop then remove:alice", strings.Join(calls, ",")) + } } func TestDeleteFallsBackToNameWhenStoredBoxIDIsStale(t *testing.T) { diff --git a/internal/runtime/sandboxgateway/runtime.go b/internal/runtime/sandboxgateway/runtime.go index 05bc9408..56fd250a 100644 --- a/internal/runtime/sandboxgateway/runtime.go +++ b/internal/runtime/sandboxgateway/runtime.go @@ -212,6 +212,10 @@ func (r *Runtime) Delete(ctx context.Context, h agentruntime.Handle) error { boxIDOrName := "" box, resolvedKey, err := r.deps.ResolveBox(ctx, rt, got) if err == nil { + if stopErr := r.deps.StopBox(ctx, box, sandbox.StopOptions{}); stopErr != nil && !sandbox.IsNotFound(stopErr) { + _ = r.deps.CloseBox(box) + return stopErr + } info, infoErr := r.infoForBox(ctx, h, box) _ = r.deps.CloseBox(box) if infoErr != nil { diff --git a/internal/runtime/sandboxgateway/runtime_test.go b/internal/runtime/sandboxgateway/runtime_test.go index 520fb2cd..f866c1ed 100644 --- a/internal/runtime/sandboxgateway/runtime_test.go +++ b/internal/runtime/sandboxgateway/runtime_test.go @@ -115,6 +115,29 @@ func TestStartWaitsForDockerReadiness(t *testing.T) { } } +func TestDeleteStopsBoxBeforeForceRemove(t *testing.T) { + var calls []string + deps := testGatewayDeps(func() string { return "docker" }, func(context.Context, sandbox.Instance, string, []string, io.Writer) (int, error) { + return 0, nil + }) + deps.StopBox = func(context.Context, sandbox.Instance, sandbox.StopOptions) error { + calls = append(calls, "stop") + return nil + } + deps.ForceRemoveBox = func(context.Context, sandbox.Runtime, string) error { + calls = append(calls, "remove") + return nil + } + + rt := New(deps) + if err := rt.Delete(context.Background(), agentruntime.Handle{RuntimeID: "rt-u-manager", HandleID: "box-1"}); err != nil { + t.Fatalf("Delete() error = %v", err) + } + if strings.Join(calls, ",") != "stop,remove" { + t.Fatalf("Delete() sandbox calls = %q, want stop then remove", strings.Join(calls, ",")) + } +} + func testGatewayDeps(providerName func() string, run func(context.Context, sandbox.Instance, string, []string, io.Writer) (int, error)) Dependencies { return Dependencies{ RuntimeKind: agentruntime.KindOpenClawSandbox, diff --git a/internal/templates/embed/picoclaw-manager/agent.toml b/internal/templates/embed/picoclaw-manager/agent.toml index 2902dd7a..b8f77111 100644 --- a/internal/templates/embed/picoclaw-manager/agent.toml +++ b/internal/templates/embed/picoclaw-manager/agent.toml @@ -5,5 +5,4 @@ runtime_kind = "picoclaw_sandbox" updated_at = "2026-05-27T00:00:00Z" [image] -# ref = "opencsg-registry.cn-beijing.cr.aliyuncs.com/opencsghq/picoclaw:2026.5.27" -ref = "picoclaw:dev1" +ref = "opencsg-registry.cn-beijing.cr.aliyuncs.com/opencsghq/picoclaw:2026.5.27" \ No newline at end of file diff --git a/internal/templates/embed/picoclaw-worker/agent.toml b/internal/templates/embed/picoclaw-worker/agent.toml index b4c9e5dd..a0725ff1 100644 --- a/internal/templates/embed/picoclaw-worker/agent.toml +++ b/internal/templates/embed/picoclaw-worker/agent.toml @@ -5,5 +5,4 @@ runtime_kind = "picoclaw_sandbox" updated_at = "2026-05-27T00:00:00Z" [image] -# ref = "opencsg-registry.cn-beijing.cr.aliyuncs.com/opencsghq/picoclaw:2026.5.27" -ref = "picoclaw:dev1" +ref = "opencsg-registry.cn-beijing.cr.aliyuncs.com/opencsghq/picoclaw:2026.5.27" \ No newline at end of file