From bb72d70586e953ed0ca9027a3e7b7f08d82b76c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Sun, 26 Jul 2026 17:54:12 +0100 Subject: [PATCH] refactor(backend): remove dead code and deprecated shims (#1965) Delete unreachable/unused backend symbols confirmed by staticcheck U1000 and golang.org/x/tools/deadcode, plus deprecated back-compat aliases kept alive only by their own tests. Removed (unused): - parseGitHubRepoURL, syncTaskStateForBulkWorkflowMove (task/service) - resolvePrimaryRepoInfo (orchestrator/executor) - resolveWorkflowAgentProfile, workflowDefaultAgentProfile wrappers (mcp/handlers) - cleanupExitedContainer (runtime/lifecycle) + orphaned fmt import - VscodeManager.setEnv (agentctl/server/process) - repoRootDir const (cmd/preview) Removed (deprecated shims): - Canceller.CancelSessionAndNotify alias -> callers use DetachSessionAndNotify - ScriptPlaceholder type alias (live scriptPlaceholders var kept) - Placeholder{QueueRun,ClearDecisions,QueueRunForEachParticipant}Callback no-op structs and their self-referential test No behavior change; build, vet, and affected-package tests pass. Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 4.8 (1M context) --- apps/backend/cmd/preview/build.go | 1 - .../runtime/lifecycle/manager_lifecycle.go | 50 ------------------- .../agentctl/server/process/vscode.go | 9 ---- .../internal/clarification/canceller.go | 6 --- .../internal/clarification/canceller_test.go | 10 ++-- .../backend/internal/mcp/handlers/handlers.go | 10 ---- .../orchestrator/executor/executor_resume.go | 16 ------ .../task/handlers/script_placeholders.go | 5 -- .../internal/task/service/service_tasks.go | 14 ------ .../internal/task/service/service_workflow.go | 25 ---------- .../workflow/engine/phase2_callbacks.go | 36 ------------- .../internal/workflow/engine/phase2_test.go | 20 -------- .../backend/internal/workflow/engine/types.go | 3 +- 13 files changed, 6 insertions(+), 199 deletions(-) diff --git a/apps/backend/cmd/preview/build.go b/apps/backend/cmd/preview/build.go index ea602a0bc1..b37bb3edd9 100644 --- a/apps/backend/cmd/preview/build.go +++ b/apps/backend/cmd/preview/build.go @@ -15,7 +15,6 @@ import ( // appsDir is the apps/ workspace root relative to the working directory (apps/backend/). const appsDir = ".." -const repoRootDir = "../.." // goDockerImage is used to cross-compile CGO binaries on non-linux/amd64 hosts. const goDockerImage = "golang:1.26-bookworm" diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_lifecycle.go b/apps/backend/internal/agent/runtime/lifecycle/manager_lifecycle.go index 2a17521700..ef71fbaa92 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager_lifecycle.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_lifecycle.go @@ -3,7 +3,6 @@ package lifecycle import ( "context" "errors" - "fmt" "sync" "time" @@ -231,55 +230,6 @@ func (m *Manager) StopAllAgents(ctx context.Context) error { const stopReasonStaleExecutionCleanup = "stale execution cleanup" -// cleanupExitedContainer handles cleanup for a single exited container. -func (m *Manager) cleanupExitedContainer(ctx context.Context, containerID string) { - execution, tracked := m.executionStore.GetByContainerID(containerID) - if !tracked { - return - } - - // Get the Docker executor's ContainerManager from the registry. - var containerMgr *ContainerManager - if m.executorRegistry != nil { - if backend, berr := m.executorRegistry.GetBackend(executor.NameDocker); berr == nil { - if dockerExec, ok := backend.(*DockerExecutor); ok { - containerMgr = dockerExec.ContainerMgr() - } - } - } - if containerMgr == nil { - m.logger.Warn("docker container manager unavailable, cannot clean up container", - zap.String("container_id", containerID)) - return - } - - // Get container info to get exit code - info, err := containerMgr.GetContainerInfo(ctx, containerID) - if err != nil { - m.logger.Warn("failed to get container info during cleanup", - zap.String("container_id", containerID), - zap.Error(err)) - return - } - - // Mark execution as completed - errorMsg := "" - if info.ExitCode != 0 { - errorMsg = fmt.Sprintf("container exited with code %d", info.ExitCode) - } - _ = m.MarkCompleted(execution.ID, info.ExitCode, errorMsg) - - // Remove the container - if err := containerMgr.RemoveContainer(ctx, containerID, false); err != nil { - m.logger.Warn("failed to remove container during cleanup", - zap.String("container_id", containerID), - zap.Error(err)) - } - - // Remove the execution from tracking so new agents can be launched - m.RemoveExecution(execution.ID) -} - // CleanupStaleExecutionBySessionID cleans up a stale execution: stops the runtime // instance, closes the client connection, and removes it from tracking. // diff --git a/apps/backend/internal/agentctl/server/process/vscode.go b/apps/backend/internal/agentctl/server/process/vscode.go index 1ccadaa1cc..b16d2fd3e5 100644 --- a/apps/backend/internal/agentctl/server/process/vscode.go +++ b/apps/backend/internal/agentctl/server/process/vscode.go @@ -93,15 +93,6 @@ func NewVscodeManager( } } -func (v *VscodeManager) setEnv(env map[string]string) { - v.mu.Lock() - defer v.mu.Unlock() - v.env = make(map[string]string, len(env)) - for key, value := range env { - v.env[key] = value - } -} - // Start launches the code-server process asynchronously. // It returns immediately after setting status to "installing" and spawns // a background goroutine that resolves the binary, writes theme settings, diff --git a/apps/backend/internal/clarification/canceller.go b/apps/backend/internal/clarification/canceller.go index b0865cbbd5..1325e3f459 100644 --- a/apps/backend/internal/clarification/canceller.go +++ b/apps/backend/internal/clarification/canceller.go @@ -132,12 +132,6 @@ func (c *Canceller) DetachSessionAndNotify(ctx context.Context, sessionID string return c.detachSessionBundles(ctx, sessionID) } -// Deprecated: use DetachSessionAndNotify. This alias remains so older tests and -// integrations keep the pre-rename entrypoint while detach semantics settle. -func (c *Canceller) CancelSessionAndNotify(ctx context.Context, sessionID string) int { - return c.DetachSessionAndNotify(ctx, sessionID) -} - // ExpireSessionAndNotify cancels in-memory waiters and marks clarification // messages expired so the overlay closes and history shows a timed-out entry. // TODO: wire this into terminal teardown paths that should close the overlay diff --git a/apps/backend/internal/clarification/canceller_test.go b/apps/backend/internal/clarification/canceller_test.go index 3f2b443300..10cc7f3c0d 100644 --- a/apps/backend/internal/clarification/canceller_test.go +++ b/apps/backend/internal/clarification/canceller_test.go @@ -88,7 +88,7 @@ func TestCanceller_MarksDetachedOnDisconnect(t *testing.T) { }, }} - cancelled := c.CancelSessionAndNotify(context.Background(), "s1") + cancelled := c.DetachSessionAndNotify(context.Background(), "s1") if cancelled != 1 { t.Fatalf("expected 1 cancelled, got %d", cancelled) } @@ -138,7 +138,7 @@ func TestCanceller_ExpireSessionAndNotify_MarksExpired(t *testing.T) { func TestCanceller_NoMessagesToUpdate(t *testing.T) { c, repo, _ := newTestCanceller(t, map[string][]*taskmodels.Message{}) - if got := c.CancelSessionAndNotify(context.Background(), "nonexistent"); got != 0 { + if got := c.DetachSessionAndNotify(context.Background(), "nonexistent"); got != 0 { t.Errorf("expected 0 cancelled, got %d", got) } if len(repo.updated) != 0 { @@ -159,7 +159,7 @@ func TestCanceller_PublishesMessageUpdatedEvent(t *testing.T) { Metadata: map[string]any{"status": "pending"}, }} - c.CancelSessionAndNotify(context.Background(), "s1") + c.DetachSessionAndNotify(context.Background(), "s1") if len(eventBus.events) != 1 { t.Fatalf("expected 1 event, got %d", len(eventBus.events)) @@ -179,7 +179,7 @@ func TestCanceller_MultiQuestion_MarksAllMessagesDetached(t *testing.T) { {ID: "m3", TaskSessionID: "s1", Metadata: map[string]any{"status": "pending", "question_id": "q3"}}, } - cancelled := c.CancelSessionAndNotify(context.Background(), "s1") + cancelled := c.DetachSessionAndNotify(context.Background(), "s1") if cancelled != 1 { t.Fatalf("expected 1 cancelled bundle, got %d", cancelled) } @@ -233,7 +233,7 @@ func TestCanceller_Idempotent_SkipsAnsweredMessages(t *testing.T) { {ID: "m2", TaskSessionID: "s1", Metadata: map[string]any{"status": "pending"}}, } - c.CancelSessionAndNotify(context.Background(), "s1") + c.DetachSessionAndNotify(context.Background(), "s1") if len(repo.updated) != 1 { t.Fatalf("expected 1 message update (only the pending one), got %d", len(repo.updated)) diff --git a/apps/backend/internal/mcp/handlers/handlers.go b/apps/backend/internal/mcp/handlers/handlers.go index 85b9703e70..2c72dd833a 100644 --- a/apps/backend/internal/mcp/handlers/handlers.go +++ b/apps/backend/internal/mcp/handlers/handlers.go @@ -1023,11 +1023,6 @@ func (h *Handlers) mcpTaskAgentProfileDefault(ctx context.Context, explicitAgent return usermodels.NormalizeMCPTaskAgentProfileDefault(settings.MCPTaskAgentProfileDefault), nil } -func (h *Handlers) resolveWorkflowAgentProfile(ctx context.Context, workflowStepID, workflowID string) string { - profileID, _ := h.resolveWorkflowAgentProfileWithError(ctx, workflowStepID, workflowID) - return profileID -} - func (h *Handlers) resolveWorkflowAgentProfileWithError(ctx context.Context, workflowStepID, workflowID string) (string, error) { profileID, resolvedWorkflowID := h.resolveWorkflowControllerAgentProfile(ctx, workflowStepID, workflowID) if profileID != "" { @@ -1075,11 +1070,6 @@ func (h *Handlers) resolveWorkflowStartStepAgentProfile(ctx context.Context, wor return startStepAgentProfile(resp.Steps) } -func (h *Handlers) workflowDefaultAgentProfile(ctx context.Context, workflowID string) string { - profileID, _ := h.workflowDefaultAgentProfileWithError(ctx, workflowID) - return profileID -} - func (h *Handlers) workflowDefaultAgentProfileWithError(ctx context.Context, workflowID string) (string, error) { if workflowID == "" || h.taskSvc == nil { return "", nil diff --git a/apps/backend/internal/orchestrator/executor/executor_resume.go b/apps/backend/internal/orchestrator/executor/executor_resume.go index c2e29f97d8..98bd09ffb2 100644 --- a/apps/backend/internal/orchestrator/executor/executor_resume.go +++ b/apps/backend/internal/orchestrator/executor/executor_resume.go @@ -70,22 +70,6 @@ type repoInfo struct { Repository *models.Repository } -// resolvePrimaryRepoInfo fetches and resolves the primary repository info for a task. -func (e *Executor) resolvePrimaryRepoInfo(ctx context.Context, taskID string) (*repoInfo, error) { - info := &repoInfo{} - primaryTaskRepo, err := e.repo.GetPrimaryTaskRepository(ctx, taskID) - if err != nil { - e.logger.Error("failed to get primary task repository", - zap.String("task_id", taskID), - zap.Error(err)) - return nil, err - } - if primaryTaskRepo == nil { - return info, nil - } - return e.resolveTaskRepoInfo(ctx, primaryTaskRepo) -} - // resolveAllRepoInfo returns the resolved repository info for every repository // linked to the task, ordered by Position. Returns a single-element slice for // single-repo tasks and an empty slice for repo-less tasks (e.g. quick chat). diff --git a/apps/backend/internal/task/handlers/script_placeholders.go b/apps/backend/internal/task/handlers/script_placeholders.go index 128b298c50..0360abcee0 100644 --- a/apps/backend/internal/task/handlers/script_placeholders.go +++ b/apps/backend/internal/task/handlers/script_placeholders.go @@ -2,10 +2,5 @@ package handlers import "github.com/kandev/kandev/internal/scriptengine" -// ScriptPlaceholder describes a template variable available in prepare/cleanup scripts. -// -// Deprecated: Use scriptengine.PlaceholderInfo directly. -type ScriptPlaceholder = scriptengine.PlaceholderInfo - // scriptPlaceholders is the registry of all available script template placeholders. var scriptPlaceholders = scriptengine.DefaultPlaceholders diff --git a/apps/backend/internal/task/service/service_tasks.go b/apps/backend/internal/task/service/service_tasks.go index e390d8c4c9..20d94cd6a1 100644 --- a/apps/backend/internal/task/service/service_tasks.go +++ b/apps/backend/internal/task/service/service_tasks.go @@ -966,20 +966,6 @@ func (s *Service) probeProviderDefaultBranchIfMissing( return probed } -// parseGitHubRepoURL parses a GitHub repository URL into owner and name. -// Supports: https://github.com/owner/repo, github.com/owner/repo, -// https://github.com/owner/repo.git, with optional trailing slashes. -func parseGitHubRepoURL(rawURL string) (owner, name string, err error) { - provider, owner, name, _, parseErr := parseRemoteRepositoryURL(rawURL, "") - if parseErr != nil { - return "", "", parseErr - } - if provider != providerGitHub { - return "", "", fmt.Errorf("not a GitHub URL") - } - return owner, name, nil -} - // resolvePRNumber returns the GitHub PR number for a repository input. Prefers // the explicit PRNumber field; falls back to parsing a /pull/ path out of // GitHubURL when present. Returns 0 when no PR is identified. diff --git a/apps/backend/internal/task/service/service_workflow.go b/apps/backend/internal/task/service/service_workflow.go index 12908c61a0..130674ed3b 100644 --- a/apps/backend/internal/task/service/service_workflow.go +++ b/apps/backend/internal/task/service/service_workflow.go @@ -466,31 +466,6 @@ func (s *Service) syncTaskStateForWorkflowMove(ctx context.Context, task *models return nil } -func (s *Service) syncTaskStateForBulkWorkflowMove(ctx context.Context, task *models.Task, oldStepID, targetStepID string, targetIsTerminal, sourceIsTerminal, sourceTerminalKnown bool) error { - if targetIsTerminal { - if !models.IsTerminalTaskState(task.State) { - task.State = v1.TaskStateCompleted - } - return nil - } - if oldStepID == targetStepID || task.State != v1.TaskStateCompleted { - return nil - } - - oldTerminal := sourceIsTerminal - if !sourceTerminalKnown { - var err error - oldTerminal, err = s.terminalWorkflowStep(ctx, oldStepID) - if err != nil { - return err - } - } - if oldTerminal { - task.State = v1.TaskStateTODO - } - return nil -} - func (s *Service) pullNextTaskOnVacate(ctx context.Context, vacatedStepID, excludeTaskID string) { vacatedStep := s.pullEnabledStep(ctx, vacatedStepID) if vacatedStep == nil { diff --git a/apps/backend/internal/workflow/engine/phase2_callbacks.go b/apps/backend/internal/workflow/engine/phase2_callbacks.go index ea5cd39ef3..634a6a9ecf 100644 --- a/apps/backend/internal/workflow/engine/phase2_callbacks.go +++ b/apps/backend/internal/workflow/engine/phase2_callbacks.go @@ -382,45 +382,9 @@ func queueRunForEachParticipantReason(in ActionInput) string { return string(in.Trigger) } -// PlaceholderQueueRunCallback is preserved as a typed alias for backward -// compatibility with the Phase 2 prep slice. New code should construct -// QueueRunCallback directly. -// -// Deprecated: Use QueueRunCallback. -type PlaceholderQueueRunCallback struct{} - -// Execute returns ErrActionNotYetWired so accidental use is loud. -func (PlaceholderQueueRunCallback) Execute(_ context.Context, _ ActionInput) (ActionResult, error) { - return ActionResult{}, ErrActionNotYetWired -} - -// PlaceholderClearDecisionsCallback is preserved for backward compatibility. -// -// Deprecated: Use ClearDecisionsCallback. -type PlaceholderClearDecisionsCallback struct{} - -// Execute returns ErrActionNotYetWired so accidental use is loud. -func (PlaceholderClearDecisionsCallback) Execute(_ context.Context, _ ActionInput) (ActionResult, error) { - return ActionResult{}, ErrActionNotYetWired -} - -// PlaceholderQueueRunForEachParticipantCallback is preserved for backward -// compatibility. -// -// Deprecated: Use QueueRunForEachParticipantCallback. -type PlaceholderQueueRunForEachParticipantCallback struct{} - -// Execute returns ErrActionNotYetWired so accidental use is loud. -func (PlaceholderQueueRunForEachParticipantCallback) Execute(_ context.Context, _ ActionInput) (ActionResult, error) { - return ActionResult{}, ErrActionNotYetWired -} - // Compile-time interface assertions. var ( _ ActionCallback = QueueRunCallback{} _ ActionCallback = ClearDecisionsCallback{} _ ActionCallback = QueueRunForEachParticipantCallback{} - _ ActionCallback = PlaceholderQueueRunCallback{} - _ ActionCallback = PlaceholderClearDecisionsCallback{} - _ ActionCallback = PlaceholderQueueRunForEachParticipantCallback{} ) diff --git a/apps/backend/internal/workflow/engine/phase2_test.go b/apps/backend/internal/workflow/engine/phase2_test.go index a027b029cc..3e4b19d5c3 100644 --- a/apps/backend/internal/workflow/engine/phase2_test.go +++ b/apps/backend/internal/workflow/engine/phase2_test.go @@ -2,7 +2,6 @@ package engine import ( "context" - "errors" "testing" wfmodels "github.com/kandev/kandev/internal/workflow/models" @@ -90,25 +89,6 @@ func TestClearDecisionsAction_Construction(t *testing.T) { } } -func TestPhase2Placeholders_ReturnNotYetWired(t *testing.T) { - cases := []struct { - name string - cb ActionCallback - }{ - {"queue_run", PlaceholderQueueRunCallback{}}, - {"clear_decisions", PlaceholderClearDecisionsCallback{}}, - {"queue_run_for_each_participant", PlaceholderQueueRunForEachParticipantCallback{}}, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - _, err := tc.cb.Execute(context.Background(), ActionInput{}) - if !errors.Is(err, ErrActionNotYetWired) { - t.Fatalf("expected ErrActionNotYetWired, got %v", err) - } - }) - } -} - func TestPhase2Placeholders_NotRegisteredInDefaultRegistry(t *testing.T) { // Empty MapRegistry must not resolve the new action kinds. This pins the // invariant that the Phase 2 actions are declared but inert until the diff --git a/apps/backend/internal/workflow/engine/types.go b/apps/backend/internal/workflow/engine/types.go index 25e1c719a3..194a4ff9fa 100644 --- a/apps/backend/internal/workflow/engine/types.go +++ b/apps/backend/internal/workflow/engine/types.go @@ -150,8 +150,7 @@ type RunCodeReviewAction struct { } // QueueRunAction represents the Phase 2 "queue a run on a target task/agent" -// action. The action is declared but its callback is not yet wired into the -// engine; see PlaceholderQueueRunCallback. +// action. The callback is QueueRunCallback. // // Targets: "primary" | "participant_role:" | "agent_profile_id:" | // "workspace.ceo_agent" (resolved at engine evaluation time, post-wire-up).