fix(docker): track executor containers for cleanup

This commit is contained in:
José Almeida
2026-05-03 17:42:02 +01:00
parent 09057324b3
commit 3bc9e1bc02
17 changed files with 640 additions and 111 deletions
@@ -65,6 +65,7 @@ type ContainerInfo struct {
FinishedAt time.Time
ExitCode int
Health string
Labels map[string]string
}
// Client wraps the Docker client.
@@ -345,6 +346,7 @@ func (c *Client) GetContainerInfo(ctx context.Context, containerID string) (*Con
State: inspect.State.Status,
Status: inspect.State.Status,
ExitCode: inspect.State.ExitCode,
Labels: inspect.Config.Labels,
}
// Parse timestamps
@@ -544,6 +546,7 @@ func (c *Client) ListContainers(ctx context.Context, labels map[string]string) (
Image: ctr.Image,
State: ctr.State,
Status: ctr.Status,
Labels: ctr.Labels,
}
infos = append(infos, info)
}
+24 -17
View File
@@ -20,12 +20,13 @@ type buildImageRequest struct {
// containerResponse is the JSON representation of a container in list responses.
type containerResponse struct {
ID string `json:"id"`
Name string `json:"name"`
Image string `json:"image"`
State string `json:"state"`
Status string `json:"status"`
StartedAt time.Time `json:"started_at"`
ID string `json:"id"`
Name string `json:"name"`
Image string `json:"image"`
State string `json:"state"`
Status string `json:"status"`
StartedAt time.Time `json:"started_at"`
Labels map[string]string `json:"labels,omitempty"`
}
// stopContainerRequest is the optional JSON body for POST /api/v1/docker/containers/:id/stop.
@@ -130,22 +131,28 @@ func handleListContainers(clientProvider ClientProvider, log *logger.Logger) gin
return
}
resp := make([]containerResponse, len(containers))
for i, ctr := range containers {
resp[i] = containerResponse{
ID: ctr.ID,
Name: ctr.Name,
Image: ctr.Image,
State: ctr.State,
Status: ctr.Status,
StartedAt: ctr.StartedAt,
}
}
resp := newContainerResponses(containers)
c.JSON(http.StatusOK, gin.H{"containers": resp})
}
}
func newContainerResponses(containers []ContainerInfo) []containerResponse {
resp := make([]containerResponse, len(containers))
for i, ctr := range containers {
resp[i] = containerResponse{
ID: ctr.ID,
Name: ctr.Name,
Image: ctr.Image,
State: ctr.State,
Status: ctr.Status,
StartedAt: ctr.StartedAt,
Labels: ctr.Labels,
}
}
return resp
}
// parseLabelsQuery extracts label filters from the "labels" query parameter.
// Expected format: "key1=value1,key2=value2".
func parseLabelsQuery(c *gin.Context) map[string]string {
@@ -0,0 +1,30 @@
package docker
import (
"testing"
"time"
)
func TestContainerResponsesIncludeLabels(t *testing.T) {
containers := []ContainerInfo{{
ID: "container-1",
Name: "kandev-agent-1",
Image: "kandev/agent:test",
State: "running",
Status: "Up 2 seconds",
StartedAt: time.Unix(100, 0),
Labels: map[string]string{
"kandev.executor_profile_id": "profile-1",
"kandev.task_id": "task-1",
},
}}
got := newContainerResponses(containers)
if got[0].Labels["kandev.executor_profile_id"] != "profile-1" {
t.Fatalf("labels = %#v, want executor profile label", got[0].Labels)
}
if got[0].Labels["kandev.task_id"] != "task-1" {
t.Fatalf("labels = %#v, want task label", got[0].Labels)
}
}
@@ -22,25 +22,28 @@ import (
const (
dockerAgentctlInstancePortBase = 41001
dockerAgentctlInstancePortMax = 41100
boolStringTrue = "true"
)
// ContainerConfig holds configuration for launching a Docker container
type ContainerConfig struct {
AgentConfig agents.Agent
WorkspacePath string // If empty, workspace is not mounted (will clone inside container)
TaskID string
TaskDescription string
Model string
SessionID string
Credentials map[string]string
ProfileInfo *AgentProfileInfo
InstanceID string
MainRepoGitDir string // Path to main repo's .git directory (for worktrees)
McpServers []McpServerConfig
McpMode string
PrepareScript string // Script to run inside container before agent starts (e.g., clone repo)
ImageTagOverride string // If set, replaces the agent runtime's default image (e.g. profile.config.image_tag)
BootstrapNonce string // one-time nonce for agentctl handshake (set internally)
AgentConfig agents.Agent
WorkspacePath string // If empty, workspace is not mounted (will clone inside container)
TaskID string
TaskEnvironmentID string
TaskDescription string
Model string
SessionID string
ExecutorProfileID string
Credentials map[string]string
ProfileInfo *AgentProfileInfo
InstanceID string
MainRepoGitDir string // Path to main repo's .git directory (for worktrees)
McpServers []McpServerConfig
McpMode string
PrepareScript string // Script to run inside container before agent starts (e.g., clone repo)
ImageTagOverride string // If set, replaces the agent runtime's default image (e.g. profile.config.image_tag)
BootstrapNonce string // one-time nonce for agentctl handshake (set internally)
}
// ContainerManager handles Docker container lifecycle operations
@@ -419,14 +422,20 @@ exec /usr/local/bin/agentctl`,
Memory: memoryBytes,
CPUQuota: cpuQuota,
Labels: map[string]string{
"kandev.managed": "true",
"kandev.instance_id": config.InstanceID,
"kandev.task_id": config.TaskID,
"kandev.session_id": config.SessionID,
"kandev.managed": boolStringTrue,
"kandev.instance_id": config.InstanceID,
"kandev.task_id": config.TaskID,
"kandev.session_id": config.SessionID,
"kandev.task_environment_id": config.TaskEnvironmentID,
"com.kandev.image": imageName,
},
AutoRemove: false, // We manage cleanup ourselves
}
if config.ExecutorProfileID != "" {
containerCfg.Labels["kandev.executor_profile_id"] = config.ExecutorProfileID
containerCfg.Labels["kandev.profile_id"] = config.ExecutorProfileID
}
if config.ProfileInfo != nil && config.ProfileInfo.ProfileID != "" {
containerCfg.Labels["kandev.profile_id"] = config.ProfileInfo.ProfileID
}
@@ -588,7 +597,7 @@ func generateBootstrapNonce() (string, error) {
// ListManagedContainers returns all containers managed by kandev
func (cm *ContainerManager) ListManagedContainers(ctx context.Context) ([]docker.ContainerInfo, error) {
return cm.dockerClient.ListContainers(ctx, map[string]string{
"kandev.managed": "true",
"kandev.managed": boolStringTrue,
})
}
@@ -128,6 +128,32 @@ func TestBuildContainerConfig_ImageTagOverrideWins(t *testing.T) {
}
}
func TestBuildContainerConfig_LabelsExecutorProfileAndTaskEnvironment(t *testing.T) {
cm := newCMTest(t)
cfg := ContainerConfig{
AgentConfig: newConfigStubAgent(),
InstanceID: "0123456789abcdef",
TaskID: "task-1",
SessionID: "session-1",
TaskEnvironmentID: "env-1",
ExecutorProfileID: "profile-1",
ImageTagOverride: "kandev/agent:custom",
}
got, err := cm.buildContainerConfig(cfg)
if err != nil {
t.Fatalf("buildContainerConfig: %v", err)
}
assertLabel(t, got.Labels, "kandev.managed", boolStringTrue)
assertLabel(t, got.Labels, "kandev.task_id", "task-1")
assertLabel(t, got.Labels, "kandev.session_id", "session-1")
assertLabel(t, got.Labels, "kandev.task_environment_id", "env-1")
assertLabel(t, got.Labels, "kandev.executor_profile_id", "profile-1")
assertLabel(t, got.Labels, "kandev.profile_id", "profile-1")
assertLabel(t, got.Labels, "com.kandev.image", "kandev/agent:custom")
}
func TestBuildContainerConfig_PublishesAgentctlPorts(t *testing.T) {
cm := newCMTest(t)
cfg := ContainerConfig{
@@ -151,6 +177,13 @@ func TestBuildContainerConfig_PublishesAgentctlPorts(t *testing.T) {
assertEnvContains(t, got.Env, "AGENTCTL_INSTANCE_PORT_MAX=41100")
}
func assertLabel(t *testing.T, labels map[string]string, key, want string) {
t.Helper()
if labels[key] != want {
t.Fatalf("label %s = %q, want %q in %#v", key, labels[key], want, labels)
}
}
func assertHasPortBinding(t *testing.T, bindings []docker.PortBindingConfig, port int) {
t.Helper()
for _, binding := range bindings {
@@ -40,7 +40,7 @@ func getMetadataBool(metadata map[string]interface{}, key string) bool {
case bool:
return raw
case string:
return strings.EqualFold(strings.TrimSpace(raw), "true")
return strings.EqualFold(strings.TrimSpace(raw), boolStringTrue)
default:
return false
}
@@ -147,15 +147,17 @@ func (r *DockerExecutor) CreateInstance(ctx context.Context, req *ExecutorCreate
// Convert ExecutorCreateRequest to ContainerConfig
// Note: WorkspacePath is empty to skip mounting - we clone inside container
containerCfg := ContainerConfig{
AgentConfig: req.AgentConfig,
WorkspacePath: "", // Empty = no workspace mount, we'll clone instead
TaskID: req.TaskID,
SessionID: req.SessionID,
InstanceID: req.InstanceID,
Credentials: req.Env, // Env contains credentials from the caller
McpServers: req.McpServers,
PrepareScript: prepareScript,
ImageTagOverride: getMetadataString(req.Metadata, MetadataKeyImageTagOverride),
AgentConfig: req.AgentConfig,
WorkspacePath: "", // Empty = no workspace mount, we'll clone instead
TaskID: req.TaskID,
TaskEnvironmentID: req.TaskEnvironmentID,
SessionID: req.SessionID,
ExecutorProfileID: getMetadataString(req.Metadata, "executor_profile_id"),
InstanceID: req.InstanceID,
Credentials: req.Env, // Env contains credentials from the caller
McpServers: req.McpServers,
PrepareScript: prepareScript,
ImageTagOverride: getMetadataString(req.Metadata, MetadataKeyImageTagOverride),
}
// Use ContainerManager to launch container (includes nonce handshake)
@@ -254,3 +254,30 @@ func TestResetTaskEnvironment_PushBranchFailureAbortsResetBeforeTeardown(t *test
t.Error("expected environment row to be preserved when push fails")
}
}
func TestPerformTaskCleanup_TearsDownTaskEnvironmentAndDeletesRow(t *testing.T) {
env := &models.TaskEnvironment{
ID: "env-1",
TaskID: "task-1",
ContainerID: "container-abc",
}
repo := &stubEnvRepo{env: env}
destroyer := &stubDestroyer{}
svc := newResetTestService(t, repo)
svc.SetEnvironmentDestroyer(destroyer)
errs := svc.performTaskCleanup(context.Background(), "task-1", nil, nil, taskEnvironmentCleanup{
env: env,
deleteRow: true,
}, false)
if len(errs) != 0 {
t.Fatalf("unexpected cleanup errors: %v", errs)
}
if len(destroyer.containerCalls) != 1 || destroyer.containerCalls[0] != "container-abc" {
t.Fatalf("expected container teardown, got %v", destroyer.containerCalls)
}
if !repo.deleted {
t.Fatal("expected task environment row to be deleted")
}
}
@@ -23,6 +23,11 @@ type taskStopTarget struct {
executionID string
}
type taskEnvironmentCleanup struct {
env *models.TaskEnvironment
deleteRow bool
}
// Task operations
// CreateTask creates a new task and publishes a task.created event.
@@ -419,6 +424,7 @@ func (s *Service) ArchiveTask(ctx context.Context, id string) error {
}
}
}
taskEnv := s.gatherTaskEnvironmentForCleanup(ctx, id)
// 3. Set archived_at in DB
if err := s.tasks.ArchiveTask(ctx, id); err != nil {
@@ -439,8 +445,9 @@ func (s *Service) ArchiveTask(ctx context.Context, id string) error {
// 6. Background: Stop agents and cleanup worktrees
// Note: isEphemeral=false for archive to preserve quick-chat directories
if len(stopTargets) > 0 || s.worktreeCleanup != nil || len(sessions) > 0 {
s.runAsyncTaskCleanup(id, sessions, worktrees, stopTargets, false,
envCleanup := taskEnvironmentCleanup{env: taskEnv, deleteRow: true}
if len(stopTargets) > 0 || s.worktreeCleanup != nil || len(sessions) > 0 || taskEnv != nil {
s.runAsyncTaskCleanup(id, sessions, worktrees, stopTargets, envCleanup, false,
"task archived", "failed to stop session on task archive", "task archive cleanup completed")
}
@@ -468,6 +475,7 @@ func (s *Service) DeleteTask(ctx context.Context, id string) error {
}
worktrees := s.gatherWorktreesForDelete(ctx, id)
taskEnv := s.gatherTaskEnvironmentForCleanup(ctx, id)
// 3. Get active sessions for stopping agents (sync, fast)
// Must query before delete since DB records will be gone
@@ -497,8 +505,9 @@ func (s *Service) DeleteTask(ctx context.Context, id string) error {
// 6. Return immediately - all remaining cleanup is async
// 7. Background: Stop agents and cleanup worktrees
if len(stopTargets) > 0 || s.worktreeCleanup != nil || len(sessions) > 0 || task.IsEphemeral {
s.runAsyncTaskCleanup(id, sessions, worktrees, stopTargets, task.IsEphemeral,
envCleanup := taskEnvironmentCleanup{env: taskEnv, deleteRow: false}
if len(stopTargets) > 0 || s.worktreeCleanup != nil || len(sessions) > 0 || task.IsEphemeral || taskEnv != nil {
s.runAsyncTaskCleanup(id, sessions, worktrees, stopTargets, envCleanup, task.IsEphemeral,
"task deleted", "failed to stop session on task delete", "task cleanup completed")
}
@@ -531,11 +540,26 @@ func (s *Service) gatherWorktreesForDelete(ctx context.Context, taskID string) [
return worktrees
}
func (s *Service) gatherTaskEnvironmentForCleanup(ctx context.Context, taskID string) *models.TaskEnvironment {
if s.taskEnvironments == nil {
return nil
}
env, err := s.taskEnvironments.GetTaskEnvironmentByTaskID(ctx, taskID)
if err != nil {
s.logger.Warn("failed to lookup task environment for cleanup",
zap.String("task_id", taskID),
zap.Error(err))
return nil
}
return env
}
func (s *Service) runAsyncTaskCleanup(
id string,
sessions []*models.TaskSession,
worktrees []*worktree.Worktree,
stopTargets []taskStopTarget,
envCleanup taskEnvironmentCleanup,
isEphemeral bool,
stopReason, stopFailMsg, cleanupMsg string,
) {
@@ -565,7 +589,7 @@ func (s *Service) runAsyncTaskCleanup(
}
}
cleanupErrors := s.performTaskCleanup(cleanupCtx, id, sessions, worktrees, isEphemeral)
cleanupErrors := s.performTaskCleanup(cleanupCtx, id, sessions, worktrees, envCleanup, isEphemeral)
if len(cleanupErrors) > 0 {
s.logger.Warn(cleanupMsg+" with errors",
@@ -613,10 +637,14 @@ func (s *Service) performTaskCleanup(
taskID string,
sessions []*models.TaskSession,
worktrees []*worktree.Worktree,
envCleanup taskEnvironmentCleanup,
isEphemeral bool,
) []error {
var errs []error
errs = append(errs, s.cleanupTaskEnvironment(ctx, taskID, envCleanup)...)
worktrees = excludeEnvironmentWorktree(worktrees, envCleanup.env)
// Cleanup worktrees
if len(worktrees) > 0 {
if cleaner, ok := s.worktreeCleanup.(WorktreeBatchCleaner); ok {
@@ -669,6 +697,47 @@ func (s *Service) performTaskCleanup(
return errs
}
func (s *Service) cleanupTaskEnvironment(
ctx context.Context,
taskID string,
cleanup taskEnvironmentCleanup,
) []error {
if cleanup.env == nil {
return nil
}
if err := s.teardownEnvironmentResources(ctx, cleanup.env); err != nil {
s.logger.Warn("failed to teardown task environment during task cleanup",
zap.String("task_id", taskID),
zap.String("env_id", cleanup.env.ID),
zap.Error(err))
return []error{fmt.Errorf("teardown task environment %s: %w", cleanup.env.ID, err)}
}
if cleanup.deleteRow {
if err := s.taskEnvironments.DeleteTaskEnvironment(ctx, cleanup.env.ID); err != nil {
s.logger.Warn("failed to delete task environment row during task cleanup",
zap.String("task_id", taskID),
zap.String("env_id", cleanup.env.ID),
zap.Error(err))
return []error{fmt.Errorf("delete task environment row %s: %w", cleanup.env.ID, err)}
}
}
return nil
}
func excludeEnvironmentWorktree(worktrees []*worktree.Worktree, env *models.TaskEnvironment) []*worktree.Worktree {
if env == nil || env.WorktreeID == "" || len(worktrees) == 0 {
return worktrees
}
filtered := worktrees[:0]
for _, wt := range worktrees {
if wt == nil || wt.ID == env.WorktreeID {
continue
}
filtered = append(filtered, wt)
}
return filtered
}
// ListTasks returns all tasks for a workflow
func (s *Service) ListTasks(ctx context.Context, workflowID string) ([]*models.Task, error) {
tasks, err := s.tasks.ListTasks(ctx, workflowID)
@@ -9,6 +9,7 @@ import { useSecrets } from "@/hooks/domains/settings/use-secrets";
import {
updateExecutorProfile,
deleteExecutorProfile,
removeDockerContainer,
fetchLocalGitIdentity,
listScriptPlaceholders,
} from "@/lib/api/domains/settings-api";
@@ -33,6 +34,7 @@ import {
DockerSections,
SpritesSections,
} from "@/components/settings/profile-edit/profile-runtime-sections";
import { useDockerProfileContainers } from "@/components/settings/profile-edit/docker-sections";
import {
ProfileHeader,
ProfileFormActions,
@@ -232,23 +234,27 @@ function useProfilePersistence(executor: Executor, profile: ExecutorProfile) {
[executor.id, profile.id, executors, setExecutors],
);
const remove = useCallback(async () => {
setDeleting(true);
try {
await deleteExecutorProfile(executor.id, profile.id);
setExecutors(
executors.map((e: Executor) =>
e.id === executor.id
? { ...e, profiles: e.profiles?.filter((p) => p.id !== profile.id) }
: e,
),
);
router.push(EXECUTORS_ROUTE);
} catch {
setDeleting(false);
setDeleteDialogOpen(false);
}
}, [executor.id, profile.id, executors, setExecutors, router]);
const remove = useCallback(
async (beforeDelete?: () => Promise<void>) => {
setDeleting(true);
try {
await beforeDelete?.();
await deleteExecutorProfile(executor.id, profile.id);
setExecutors(
executors.map((e: Executor) =>
e.id === executor.id
? { ...e, profiles: e.profiles?.filter((p) => p.id !== profile.id) }
: e,
),
);
router.push(EXECUTORS_ROUTE);
} catch {
setDeleting(false);
setDeleteDialogOpen(false);
}
},
[executor.id, profile.id, executors, setExecutors, router],
);
return { saving, error, deleting, deleteDialogOpen, setDeleteDialogOpen, save, remove };
}
@@ -483,6 +489,7 @@ function ProfileEditForm({ executor, profile }: { executor: Executor; profile: E
const { items: secrets } = useSecrets();
const persistence = useProfilePersistence(executor, profile);
const form = useProfileFormState(executor, profile);
const relatedContainers = useDockerProfileContainers(profile.id, form.isDocker);
const spritesTokenMissing = form.isSprites && !form.spritesSecretId;
const handleSave = () => {
@@ -497,6 +504,18 @@ function ProfileEditForm({ executor, profile }: { executor: Executor; profile: E
});
};
const handleDelete = (options?: { removeRelatedDockerContainers?: boolean }) => {
const beforeDelete = options?.removeRelatedDockerContainers
? async () => {
await Promise.all(
relatedContainers.containers.map((container) => removeDockerContainer(container.id)),
);
await relatedContainers.refresh();
}
: undefined;
void persistence.remove(beforeDelete);
};
return (
<div className="space-y-8">
<ProfileHeader
@@ -523,8 +542,9 @@ function ProfileEditForm({ executor, profile }: { executor: Executor; profile: E
<DeleteProfileDialog
open={persistence.deleteDialogOpen}
onOpenChange={persistence.setDeleteDialogOpen}
onDelete={persistence.remove}
onDelete={handleDelete}
deleting={persistence.deleting}
relatedDockerContainerCount={form.isDocker ? relatedContainers.containers.length : 0}
/>
</div>
);
@@ -6,6 +6,7 @@ import { Badge } from "@kandev/ui/badge";
import { Button } from "@kandev/ui/button";
import { Card, CardContent } from "@kandev/ui/card";
import { Separator } from "@kandev/ui/separator";
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
import { useAppStore } from "@/components/state-provider";
import { useSecrets } from "@/hooks/domains/settings/use-secrets";
import {
@@ -27,7 +28,10 @@ import {
rowsToEnvVars,
} from "@/components/settings/profile-edit/env-vars-card";
import { ScriptCard } from "@/components/settings/profile-edit/script-card";
import { DockerfileBuildCard } from "@/components/settings/profile-edit/docker-sections";
import {
DockerfileBuildCard,
type DockerBuildSuccess,
} from "@/components/settings/profile-edit/docker-sections";
import { SpritesApiKeyCard } from "@/components/settings/profile-edit/sprites-api-key-card";
import { NetworkPoliciesCard } from "@/components/settings/profile-edit/sprites-sections";
import {
@@ -144,13 +148,20 @@ function CreateProfileHeader({
function CreateFormActions({
saving,
saveDisabled,
disabledReason,
onSave,
}: {
saving: boolean;
saveDisabled: boolean;
disabledReason: string | null;
onSave: () => void;
}) {
const router = useRouter();
const createButton = (
<Button onClick={onSave} disabled={saveDisabled} className="cursor-pointer">
{saving ? "Creating..." : "Create Profile"}
</Button>
);
return (
<div className="flex items-center justify-end gap-2">
<Button
@@ -160,9 +171,18 @@ function CreateFormActions({
>
Cancel
</Button>
<Button onClick={onSave} disabled={saveDisabled} className="cursor-pointer">
{saving ? "Creating..." : "Create Profile"}
</Button>
{disabledReason ? (
<Tooltip>
<TooltipTrigger asChild>
<span className="inline-flex" data-testid="create-profile-disabled-tooltip">
{createButton}
</span>
</TooltipTrigger>
<TooltipContent>{disabledReason}</TooltipContent>
</Tooltip>
) : (
createButton
)}
</div>
);
}
@@ -327,7 +347,7 @@ function useCreateGitIdentityState(isRemote: boolean) {
}
function useCreateProfileFormState(executorType: string) {
const [name, setName] = useState("");
const [name, setName] = useState(() => (executorType === "local_docker" ? "Docker" : ""));
const [mcpPolicy, setMcpPolicy] = useState("");
const [prepareScript, setPrepareScript] = useState("");
const [cleanupScript, setCleanupScript] = useState("");
@@ -337,6 +357,7 @@ function useCreateProfileFormState(executorType: string) {
const remoteAuth = useCreateRemoteAuthState(executorType);
const [dockerfile, setDockerfile] = useState("");
const [imageTag, setImageTag] = useState("");
const [builtDockerImage, setBuiltDockerImage] = useState<DockerBuildSuccess | null>(null);
const flags = useCreateRemoteFlags(executorType);
const gitIdentity = useCreateGitIdentityState(flags.isRemote);
const mcpPolicyError = useMemo(() => validateMcpPolicy(mcpPolicy), [mcpPolicy]);
@@ -357,6 +378,17 @@ function useCreateProfileFormState(executorType: string) {
return vars;
}, [envVarRows, flags.isSprites, spritesSecretId]);
const recordDockerBuildSuccess = useCallback((result: DockerBuildSuccess) => {
setBuiltDockerImage(result);
}, []);
const dockerImageBuilt =
!flags.isDocker ||
(Boolean(dockerfile.trim()) &&
Boolean(imageTag.trim()) &&
builtDockerImage?.dockerfile === dockerfile &&
builtDockerImage?.imageTag === imageTag.trim());
const prepareDesc = flags.isRemote
? "Runs inside the execution environment before the agent starts. Type {{ to see available placeholders."
: "Runs on the host machine before the agent starts.";
@@ -390,6 +422,8 @@ function useCreateProfileFormState(executorType: string) {
setDockerfile,
imageTag,
setImageTag,
recordDockerBuildSuccess,
dockerImageBuilt,
gitUserName: gitIdentity.gitUserName,
setGitUserName: gitIdentity.setGitUserName,
gitUserEmail: gitIdentity.gitUserEmail,
@@ -415,6 +449,10 @@ function useCreateProfileSave(
const handleSave = useCallback(async () => {
if (!form.name.trim() || form.mcpPolicyError) return;
if (form.isDocker && !form.dockerImageBuilt) {
setError("Build this Docker image before creating the profile.");
return;
}
if (form.isSprites && !form.spritesSecretId) {
setError("Sprites API key is required.");
return;
@@ -483,6 +521,7 @@ function CreateProfileSections({
onDockerfileChange={form.setDockerfile}
imageTag={form.imageTag}
onImageTagChange={form.setImageTag}
onBuildSuccess={form.recordDockerBuildSuccess}
/>
)}
{form.isRemote && (
@@ -544,6 +583,23 @@ function CreateProfileSections({
);
}
function getCreateDisabledReason(
form: ReturnType<typeof useCreateProfileFormState>,
spritesTokenMissing: boolean,
saving: boolean,
) {
if (saving) return "Creating profile...";
if (!form.name.trim()) return "Enter a profile name.";
if (form.mcpPolicyError) return form.mcpPolicyError;
if (spritesTokenMissing) return "Add a Sprites API key before creating the profile.";
if (form.isDocker) {
if (!form.imageTag.trim()) return "Enter an image tag before creating the profile.";
if (!form.dockerfile.trim()) return "Add Dockerfile content before creating the profile.";
if (!form.dockerImageBuilt) return "Build this Docker image before creating the profile.";
}
return null;
}
function CreateProfileForm({
executorType,
typeInfo,
@@ -555,6 +611,7 @@ function CreateProfileForm({
const form = useCreateProfileFormState(executorType);
const { saving, error, handleSave } = useCreateProfileSave(form, typeInfo.executorId);
const spritesTokenMissing = form.isSprites && !form.spritesSecretId;
const disabledReason = getCreateDisabledReason(form, spritesTokenMissing, saving);
return (
<div className="space-y-8">
@@ -570,9 +627,8 @@ function CreateProfileForm({
{error && <p className="text-sm text-destructive">{error}</p>}
<CreateFormActions
saving={saving}
saveDisabled={
!form.name.trim() || Boolean(form.mcpPolicyError) || spritesTokenMissing || saving
}
saveDisabled={Boolean(disabledReason)}
disabledReason={disabledReason}
onSave={handleSave}
/>
</div>
@@ -36,7 +36,8 @@ RUN apt-get update \\
WORKDIR /workspace
`;
type BuildStatus = "idle" | "building" | "success" | "failed";
export type BuildStatus = "idle" | "building" | "success" | "failed";
export type DockerBuildSuccess = { dockerfile: string; imageTag: string };
/** Parse a Docker JSON stream line into a human-readable string. */
function parseDockerLine(raw: string): string | null {
@@ -103,7 +104,7 @@ async function readDockerStream(
return hasError;
}
function useBuildStream() {
function useBuildStream(onBuildSuccess?: (result: DockerBuildSuccess) => void) {
const [buildStatus, setBuildStatus] = useState<BuildStatus>("idle");
const [buildLog, setBuildLog] = useState("");
@@ -130,14 +131,18 @@ function useBuildStream() {
return;
}
const hasError = await readDockerStream(reader, appendLog);
setBuildStatus(hasError ? "failed" : "success");
const nextStatus = hasError ? "failed" : "success";
setBuildStatus(nextStatus);
if (nextStatus === "success") {
onBuildSuccess?.({ dockerfile, imageTag: tag.trim() });
}
} catch (err) {
setBuildStatus("failed");
const msg = err instanceof Error ? err.message : "Unknown error";
setBuildLog((prev) => prev + `\nBuild failed: ${msg}`);
}
},
[appendLog],
[appendLog, onBuildSuccess],
);
return { buildStatus, buildLog, runBuild };
@@ -148,6 +153,7 @@ type DockerfileBuildCardProps = {
onDockerfileChange: (v: string) => void;
imageTag: string;
onImageTagChange: (v: string) => void;
onBuildSuccess?: (result: DockerBuildSuccess) => void;
};
export function DockerfileBuildCard({
@@ -155,8 +161,9 @@ export function DockerfileBuildCard({
onDockerfileChange,
imageTag,
onImageTagChange,
onBuildSuccess,
}: DockerfileBuildCardProps) {
const { buildStatus, buildLog, runBuild } = useBuildStream();
const { buildStatus, buildLog, runBuild } = useBuildStream(onBuildSuccess);
const logRef = useRef<HTMLPreElement>(null);
useEffect(() => {
@@ -253,7 +260,41 @@ function ContainersEmptyState({ loading }: { loading: boolean }) {
</div>
);
}
return <p className="py-4 text-sm text-muted-foreground">No running containers.</p>;
return <p className="py-4 text-sm text-muted-foreground">No Docker containers.</p>;
}
export function useDockerProfileContainers(profileId: string, enabled = true) {
const [containers, setContainers] = useState<DockerContainer[]>([]);
const [loading, setLoading] = useState(enabled);
const refresh = useCallback(async () => {
if (!enabled) {
setContainers([]);
setLoading(false);
return;
}
setLoading(true);
try {
const result = await listDockerContainers({
labels: { "kandev.executor_profile_id": profileId },
});
setContainers(result.containers ?? []);
} catch {
setContainers([]);
} finally {
setLoading(false);
}
}, [enabled, profileId]);
useEffect(() => {
void refresh();
}, [refresh]);
return { containers, loading, refresh };
}
function containerTaskLabel(container: DockerContainer) {
return container.labels?.["kandev.task_id"] ?? "Untracked";
}
function ContainerRow({
@@ -269,9 +310,12 @@ function ContainerRow({
}) {
const isLoading = actionLoading === container.id;
return (
<TableRow>
<TableRow data-testid={`docker-container-row-${container.id}`}>
<TableCell className="font-mono text-sm">{container.name}</TableCell>
<TableCell className="text-sm">{container.image}</TableCell>
<TableCell className="font-mono text-xs" data-testid="docker-container-task">
{containerTaskLabel(container)}
</TableCell>
<TableCell>
<Badge variant={container.state === "running" ? "default" : "secondary"}>
{container.status}
@@ -312,26 +356,9 @@ function ContainerRow({
}
export function DockerContainersCard({ profileId }: { profileId: string }) {
const [containers, setContainers] = useState<DockerContainer[]>([]);
const [loading, setLoading] = useState(true);
const { containers, loading, refresh } = useDockerProfileContainers(profileId);
const [actionLoading, setActionLoading] = useState<string | null>(null);
const refresh = useCallback(async () => {
setLoading(true);
try {
const result = await listDockerContainers({ labels: { "kandev.profile_id": profileId } });
setContainers(result.containers ?? []);
} catch {
setContainers([]);
} finally {
setLoading(false);
}
}, [profileId]);
useEffect(() => {
void refresh();
}, [refresh]);
const handleAction = useCallback(
async (id: string, action: (id: string) => Promise<void>) => {
setActionLoading(id);
@@ -348,7 +375,7 @@ export function DockerContainersCard({ profileId }: { profileId: string }) {
return (
<Card>
<CardHeader>
<CardTitle>Running Containers</CardTitle>
<CardTitle>Docker Containers</CardTitle>
<CardDescription>Docker containers created by this profile.</CardDescription>
</CardHeader>
<CardContent>
@@ -360,6 +387,7 @@ export function DockerContainersCard({ profileId }: { profileId: string }) {
<TableRow>
<TableHead>Name</TableHead>
<TableHead>Image</TableHead>
<TableHead>Task</TableHead>
<TableHead>Status</TableHead>
<TableHead className="w-[100px]" />
</TableRow>
@@ -1,9 +1,12 @@
"use client";
import { useState } from "react";
import { useRouter } from "next/navigation";
import { IconTrash } from "@tabler/icons-react";
import { Badge } from "@kandev/ui/badge";
import { Button } from "@kandev/ui/button";
import { Checkbox } from "@kandev/ui/checkbox";
import { Label } from "@kandev/ui/label";
import { Separator } from "@kandev/ui/separator";
import {
Dialog,
@@ -100,26 +103,57 @@ export function DeleteProfileDialog({
onOpenChange,
onDelete,
deleting,
relatedDockerContainerCount = 0,
}: {
open: boolean;
onOpenChange: (v: boolean) => void;
onDelete: () => void;
onDelete: (options?: { removeRelatedDockerContainers?: boolean }) => void;
deleting: boolean;
relatedDockerContainerCount?: number;
}) {
const [removeRelatedContainers, setRemoveRelatedContainers] = useState<boolean | null>(null);
const hasRelatedContainers = relatedDockerContainerCount > 0;
const shouldRemoveRelatedContainers = hasRelatedContainers && (removeRelatedContainers ?? true);
const handleOpenChange = (nextOpen: boolean) => {
if (!nextOpen) setRemoveRelatedContainers(null);
onOpenChange(nextOpen);
};
return (
<Dialog open={open} onOpenChange={onOpenChange}>
<Dialog open={open} onOpenChange={handleOpenChange}>
<DialogContent>
<DialogHeader>
<DialogTitle>Delete Profile</DialogTitle>
<DialogDescription>Are you sure? This action cannot be undone.</DialogDescription>
</DialogHeader>
{hasRelatedContainers && (
<div className="space-y-3 rounded-md border p-3">
<p className="text-sm text-muted-foreground">
{relatedDockerContainerCount} related Docker{" "}
{relatedDockerContainerCount === 1 ? "container" : "containers"} will also be removed.
</p>
<div className="flex items-center gap-2">
<Checkbox
id="remove-related-docker-containers"
checked={shouldRemoveRelatedContainers}
onCheckedChange={(checked) => setRemoveRelatedContainers(checked === true)}
/>
<Label htmlFor="remove-related-docker-containers" className="cursor-pointer text-sm">
Remove related Docker containers
</Label>
</div>
</div>
)}
<DialogFooter>
<Button variant="outline" onClick={() => onOpenChange(false)} className="cursor-pointer">
Cancel
</Button>
<Button
variant="destructive"
onClick={onDelete}
onClick={() =>
onDelete({ removeRelatedDockerContainers: shouldRemoveRelatedContainers })
}
disabled={deleting}
className="cursor-pointer"
>
@@ -69,6 +69,22 @@ afterEach(() => {
});
describe("RemoteCredentialsCard", () => {
it("does not crash when an auth spec has no methods", async () => {
vi.mocked(listRemoteCredentials).mockResolvedValueOnce({
auth_specs: [
{
id: "empty-agent",
display_name: "Empty Agent",
methods: null as never,
},
],
});
renderRemoteCredentialsCard();
expect(await screen.findByText("Empty Agent")).toBeTruthy();
});
it("allows selecting file auth even when local file detection reports missing files", async () => {
const onChange = vi.fn();
renderRemoteCredentialsCard(onChange);
@@ -117,7 +117,8 @@ export function RemoteCredentialsCard({
/>
)}
{authSpecs.map((spec) => {
const envMethod = spec.methods.find((m) => m.type === "env");
const methods = getSpecMethods(spec);
const envMethod = methods.find((m) => m.type === "env");
return (
<AuthSection
key={spec.id}
@@ -139,6 +140,10 @@ export function RemoteCredentialsCard({
);
}
function getSpecMethods(spec: RemoteAuthSpec): RemoteAuthMethod[] {
return Array.isArray(spec.methods) ? spec.methods : [];
}
function GitIdentityAccordionItem({
mode,
onModeChange,
@@ -306,9 +311,10 @@ function AuthSection({
onMethodSecretChange: (methodId: string, secretId: string | null) => void;
secrets: SecretListItem[];
}) {
const envMethod = spec.methods.find((m) => m.type === "env");
const fileMethod = spec.methods.find((m) => m.type === "files");
const ghTokenMethod = spec.methods.find((m) => m.type === "gh_cli_token");
const methods = getSpecMethods(spec);
const envMethod = methods.find((m) => m.type === "env");
const fileMethod = methods.find((m) => m.type === "files");
const ghTokenMethod = methods.find((m) => m.type === "gh_cli_token");
const hasOnlyEnv = envMethod && !fileMethod && !ghTokenMethod;
// `choice` is derived from props so the configured-status badge updates live
@@ -28,6 +28,15 @@ function dockerState(containerID: string): string {
return res.stdout.trim();
}
async function waitForDockerContainerRemoved(containerID: string, message: string): Promise<void> {
await expect
.poll(() => dockerInspectExists(containerID), {
timeout: 60_000,
message,
})
.toBe(false);
}
async function waitForLatestSessionDone(
apiClient: import("../../helpers/api-client").ApiClient,
taskId: string,
@@ -94,6 +103,62 @@ test.describe("Docker executor — launch + reuse + recovery", () => {
expect(dockerInspectExists(env!.container_id!), "container should exist on host").toBe(true);
});
test("archives a task and removes its Docker container", async ({ apiClient, seedData }) => {
test.setTimeout(180_000);
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
"Docker Archive Cleanup",
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [seedData.repositoryId],
executor_profile_id: seedData.dockerExecutorProfileId,
},
);
await waitForLatestSessionDone(apiClient, task.id, 1, "Waiting for archive cleanup session");
const env = await apiClient.getTaskEnvironment(task.id);
expect(env?.container_id).toBeTruthy();
const containerID = env!.container_id!;
try {
await apiClient.archiveTask(task.id);
await waitForDockerContainerRemoved(containerID, "Archived task should remove container");
} finally {
dockerRemove(containerID);
}
});
test("deletes a task and removes its Docker container", async ({ apiClient, seedData }) => {
test.setTimeout(180_000);
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
"Docker Delete Cleanup",
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [seedData.repositoryId],
executor_profile_id: seedData.dockerExecutorProfileId,
},
);
await waitForLatestSessionDone(apiClient, task.id, 1, "Waiting for delete cleanup session");
const env = await apiClient.getTaskEnvironment(task.id);
expect(env?.container_id).toBeTruthy();
const containerID = env!.container_id!;
try {
await apiClient.deleteTask(task.id);
await waitForDockerContainerRemoved(containerID, "Deleted task should remove container");
} finally {
dockerRemove(containerID);
}
});
test("restarts an externally stopped container and reuses the task environment", async ({
apiClient,
seedData,
@@ -12,6 +12,129 @@ import { test, expect } from "../../fixtures/test-base";
* editor would use — bypassing CodeMirror, which is awkward to drive.
*/
test.describe("Docker executor profile persistence", () => {
test("new Docker profile defaults the name and requires a successful image build", async ({
testPage,
apiClient,
}) => {
test.setTimeout(60_000);
let createdProfileId: string | null = null;
await testPage.route("**/api/v1/docker/build", async (route) => {
await route.fulfill({
status: 200,
contentType: "application/x-ndjson",
body: JSON.stringify({ stream: "Successfully built\n" }) + "\n",
});
});
try {
await testPage.goto("/settings/executors/new/local_docker");
await expect(testPage.locator("#profile-name")).toHaveValue("Docker", {
timeout: 10_000,
});
await testPage.getByRole("button", { name: "Use defaults" }).click();
const createButton = testPage.getByRole("button", { name: "Create Profile" });
await expect(createButton).toBeDisabled();
await testPage.getByTestId("create-profile-disabled-tooltip").hover();
await expect(
testPage.getByText("Build this Docker image before creating the profile."),
).toBeVisible();
await testPage.getByRole("button", { name: "Build Image" }).click();
await expect(testPage.getByText("Success", { exact: true })).toBeVisible({
timeout: 10_000,
});
await expect(createButton).toBeEnabled();
await createButton.click();
await expect(testPage).toHaveURL(/\/settings\/executors\/[^/]+$/);
createdProfileId = testPage.url().split("/").pop() ?? null;
expect(createdProfileId).toBeTruthy();
} finally {
if (createdProfileId) {
await apiClient.deleteExecutorProfile(createdProfileId).catch(() => {});
}
}
});
test("profile page lists related Docker containers and can remove them during profile deletion", async ({
testPage,
apiClient,
}) => {
test.setTimeout(60_000);
const exec = await apiClient.createExecutor("e2e-docker-containers", "local_docker");
const profile = await apiClient.createExecutorProfile(exec.id, {
name: "containers",
config: {
image_tag: "kandev/e2e:test",
dockerfile: "FROM busybox\nWORKDIR /workspace\n",
},
});
const containers = [
{
id: "container-1",
name: "kandev-agent-test",
image: "kandev/e2e:test",
state: "running",
status: "Up 5 seconds",
labels: {
"kandev.executor_profile_id": profile.id,
"kandev.task_id": "task-abc123",
"kandev.task_environment_id": "env-123",
},
},
];
let visibleContainers = [...containers];
const removed: string[] = [];
await testPage.route("**/api/v1/docker/containers?*", async (route) => {
await route.fulfill({
status: 200,
contentType: "application/json",
body: JSON.stringify({ containers: visibleContainers }),
});
});
await testPage.route("**/api/v1/docker/containers/container-1", async (route) => {
if (route.request().method() !== "DELETE") {
await route.fallback();
return;
}
removed.push("container-1");
visibleContainers = [];
await route.fulfill({
status: 200,
contentType: "application/json",
body: JSON.stringify({ success: true }),
});
});
try {
await testPage.goto(`/settings/executors/${profile.id}`);
const row = testPage.getByTestId("docker-container-row-container-1");
await expect(row).toBeVisible({ timeout: 10_000 });
await expect(row.getByTestId("docker-container-task")).toContainText("task-abc123");
await expect(row.getByText("Up 5 seconds")).toBeVisible();
await testPage.getByRole("button", { name: "Delete Profile" }).click();
await expect(
testPage.getByText("1 related Docker container will also be removed."),
).toBeVisible();
await expect(
testPage.getByRole("checkbox", { name: "Remove related Docker containers" }),
).toBeChecked();
await testPage.getByRole("button", { name: "Delete" }).click();
await expect.poll(() => removed).toContain("container-1");
} finally {
await apiClient.deleteExecutor(exec.id).catch(() => {});
}
});
test("Dockerfile content and image tag round-trip through Save", async ({
testPage,
apiClient,
+1
View File
@@ -359,6 +359,7 @@ export type DockerContainer = {
state: string;
status: string;
started_at?: string;
labels?: Record<string, string>;
};
export function buildDockerImageUrl(payload: {