From 64c0a20b7500a2ec2724ef4d929faa20b93cb7ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Wed, 12 Aug 2026 22:48:00 +0100 Subject: [PATCH] test(backend): cover lifecycle manager coordination paths (#2590) Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 5 (1M context) --- .../environment_resolution_defaults_test.go | 154 ++++++ .../lifecycle/manager_events_routing_test.go | 210 ++++++++ .../manager_events_workspace_test.go | 328 +++++++++++++ .../manager_execution_passthrough_test.go | 342 +++++++++++++ .../manager_interaction_accessors_test.go | 402 +++++++++++++++ .../manager_interaction_session_ops_test.go | 463 ++++++++++++++++++ .../lifecycle/manager_launch_metadata_test.go | 421 ++++++++++++++++ .../manager_launch_workspace_test.go | 142 ++++++ .../manager_passthrough_launch_test.go | 422 ++++++++++++++++ .../manager_passthrough_mcpfiles_test.go | 285 +++++++++++ .../manager_passthrough_runtime_test.go | 419 ++++++++++++++++ .../manager_profile_capabilities_test.go | 369 ++++++++++++++ .../lifecycle/manager_remote_status_test.go | 338 +++++++++++++ .../manager_workspace_rescan_guard_test.go | 156 ++++++ .../lifecycle/process_runner_session_test.go | 317 ++++++++++++ .../lifecycle/session_history_resume_test.go | 153 ++++++ .../lifecycle/streams_callbacks_test.go | 184 +++++++ 17 files changed, 5105 insertions(+) create mode 100644 apps/backend/internal/agent/runtime/lifecycle/environment_resolution_defaults_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_events_routing_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_events_workspace_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_execution_passthrough_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_interaction_accessors_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_interaction_session_ops_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_launch_metadata_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_launch_workspace_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_launch_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_mcpfiles_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_runtime_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_profile_capabilities_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_remote_status_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/manager_workspace_rescan_guard_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/process_runner_session_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/session_history_resume_test.go create mode 100644 apps/backend/internal/agent/runtime/lifecycle/streams_callbacks_test.go diff --git a/apps/backend/internal/agent/runtime/lifecycle/environment_resolution_defaults_test.go b/apps/backend/internal/agent/runtime/lifecycle/environment_resolution_defaults_test.go new file mode 100644 index 0000000000..1a0f2f8610 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/environment_resolution_defaults_test.go @@ -0,0 +1,154 @@ +package lifecycle + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + + runtimeenv "github.com/kandev/kandev/internal/agent/runtime/environment" + settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" +) + +// TestAppendAgentRuntimeDefaultsFillsOnlyMissingKeys pins the runtime +// environment invariant at the launch-definition boundary: every +// Agent.Runtime().Env entry becomes a definition, but a higher-precedence +// definition already present for that key wins. +func TestAppendAgentRuntimeDefaultsFillsOnlyMissingKeys(t *testing.T) { + agentConfig, ok := newTestRegistry().Get("claude-acp") + require.True(t, ok) + runtimeEnv := agentConfig.Runtime().Env + require.NotEmpty(t, runtimeEnv, "the fixture agent must declare runtime env for this to be meaningful") + + var claimed string + for key := range runtimeEnv { + claimed = key + break + } + + definitions := []runtimeenv.Definition{ + {Key: claimed, Literal: "profile-wins", Origin: agentProfileOrigin}, + } + appendAgentRuntimeDefaults(&definitions, agentConfig) + + byKey := map[string]runtimeenv.Definition{} + for _, d := range definitions { + require.NotContains(t, byKey, d.Key, "duplicate definition for %q", d.Key) + byKey[d.Key] = d + } + require.Len(t, byKey, len(runtimeEnv), + "every Agent.Runtime().Env key must reach the launch environment exactly once") + require.Equal(t, "profile-wins", byKey[claimed].Literal, + "an existing higher-precedence definition must not be overwritten by the agent default") + require.Equal(t, agentProfileOrigin, byKey[claimed].Origin) + for key, value := range runtimeEnv { + if key == claimed { + continue + } + require.Equal(t, value, byKey[key].Literal, "runtime env %q lost its value", key) + require.Equal(t, managedAgentDefaultsOrigin, byKey[key].Origin) + } +} + +func TestAppendAgentRuntimeDefaultsIgnoresMissingConfig(t *testing.T) { + definitions := []runtimeenv.Definition{{Key: "EXISTING", Literal: "value"}} + + appendAgentRuntimeDefaults(&definitions, nil) + + require.Len(t, definitions, 1) +} + +func TestHasEnvironmentDefinition(t *testing.T) { + definitions := []runtimeenv.Definition{ + {Key: "PATH", Literal: "/usr/bin"}, + {Key: "ANTHROPIC_API_KEY", SecretID: "secret-1"}, + } + + require.True(t, hasEnvironmentDefinition(definitions, "PATH")) + require.True(t, hasEnvironmentDefinition(definitions, "ANTHROPIC_API_KEY")) + require.False(t, hasEnvironmentDefinition(definitions, "HTTPS_PROXY")) + require.False(t, hasEnvironmentDefinition(nil, "PATH")) +} + +func TestAppendMapDefinitionsIsDeterministicallyOrdered(t *testing.T) { + var definitions []runtimeenv.Definition + + appendMapDefinitions(&definitions, map[string]string{ + "ZED": "3", "ALPHA": "1", "MIKE": "2", + }, agentProfileOrigin) + + require.Equal(t, []runtimeenv.Definition{ + {Key: "ALPHA", Literal: "1", Origin: agentProfileOrigin}, + {Key: "MIKE", Literal: "2", Origin: agentProfileOrigin}, + {Key: "ZED", Literal: "3", Origin: agentProfileOrigin}, + }, definitions, "map iteration order must not leak into the launch environment") + + var empty []runtimeenv.Definition + appendMapDefinitions(&empty, nil, agentProfileOrigin) + require.Empty(t, empty) +} + +// TestAppendProfileDefinitionsDropsUnusableEntries pins the filter: a blank key +// or an entry with neither a literal nor a secret would produce an env var the +// agent cannot use. +func TestAppendProfileDefinitionsDropsUnusableEntries(t *testing.T) { + var definitions []runtimeenv.Definition + + appendProfileDefinitions(&definitions, []settingsmodels.ProfileEnvVar{ + {Key: " ", Value: "blank key"}, + {Key: "EMPTY"}, + {Key: "LITERAL", Value: "value"}, + {Key: "SECRET", SecretID: "secret-1"}, + }, agentProfileOrigin) + + require.Equal(t, []runtimeenv.Definition{ + {Key: "LITERAL", Literal: "value", Origin: agentProfileOrigin}, + {Key: "SECRET", SecretID: "secret-1", Origin: agentProfileOrigin}, + }, definitions) +} + +func TestApprovedSecretEnvironmentKeysOnlyIncludesWorkspaceScoped(t *testing.T) { + approved := approvedSecretEnvironmentKeys([]runtimeenv.Definition{ + {Key: "GLOBAL_SECRET", SecretID: "secret-1"}, + {Key: "REPO_TOKEN", SecretID: "secret-2", WorkspaceID: "ws-1"}, + {Key: " ", SecretID: "secret-3", WorkspaceID: "ws-1"}, + {Key: "ANOTHER_TOKEN", SecretID: "secret-4", WorkspaceID: "ws-1"}, + {Key: "REPO_TOKEN", SecretID: "secret-5", WorkspaceID: "ws-2"}, + }) + + require.Equal(t, []string{"ANOTHER_TOKEN", "REPO_TOKEN"}, approved, + "only workspace-scoped bindings may be forwarded, deduplicated and sorted") +} + +func TestResolveEnvironmentDefinitionLiteralNeedsNoSecretStore(t *testing.T) { + mgr := newTestManager(t) + + value, err := mgr.resolveEnvironmentDefinition(context.Background(), + runtimeenv.Definition{Key: "PATH", Literal: "/usr/bin"}) + + require.NoError(t, err) + require.Equal(t, "/usr/bin", value) +} + +func TestResolveEnvironmentDefinitionRequiresSecretStore(t *testing.T) { + mgr := newTestManager(t) + mgr.secretStore = nil + + _, err := mgr.resolveEnvironmentDefinition(context.Background(), + runtimeenv.Definition{Key: "TOKEN", SecretID: "secret-1"}) + + require.ErrorContains(t, err, "secret store unavailable") +} + +// TestResolveEnvironmentDefinitionRejectsUnscopedStoreForWorkspaceSecret pins +// the fail-closed rule: a workspace-scoped binding must never fall back to a +// global reveal, which would cross the workspace boundary. +func TestResolveEnvironmentDefinitionRejectsUnscopedStoreForWorkspaceSecret(t *testing.T) { + mgr := newTestManager(t) + mgr.secretStore = &mockSecretStore{} + + _, err := mgr.resolveEnvironmentDefinition(context.Background(), + runtimeenv.Definition{Key: "REPO_TOKEN", SecretID: "secret-1", WorkspaceID: "ws-1"}) + + require.ErrorContains(t, err, "workspace-scoped secret storage is unavailable") +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_events_routing_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_events_routing_test.go new file mode 100644 index 0000000000..534046a4d3 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_events_routing_test.go @@ -0,0 +1,210 @@ +package lifecycle + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/require" + + agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" + "github.com/kandev/kandev/internal/agentctl/types/streams" + "github.com/kandev/kandev/internal/events" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// TestHandleAvailableCommandsEventCachesAndPublishes pins both halves of the +// slash-command update: the execution caches the list for later reads, and the +// same list is published so the composer refreshes without a reload. +func TestHandleAvailableCommandsEventCachesAndPublishes(t *testing.T) { + h := newWorkspaceEventsHarness(t) + commands := []streams.AvailableCommand{ + {Name: "review", Description: "Review the diff"}, + {Name: "commit"}, + } + + h.mgr.handleAvailableCommandsEvent(h.exec, agentctl.AgentEvent{AvailableCommands: commands}) + + require.Equal(t, commands, h.mgr.GetAvailableCommandsForSession("session-1"), + "the cached list is what a late-joining subscriber reads back") + got := h.only(t) + require.Equal(t, events.AvailableCommandsUpdated, got.Event.Type) + payload, ok := got.Event.Data.(AvailableCommandsEventPayload) + require.True(t, ok, "payload type = %T", got.Event.Data) + require.Equal(t, commands, payload.AvailableCommands) +} + +func TestHandleAvailableCommandsEventIgnoresEmptyList(t *testing.T) { + h := newWorkspaceEventsHarness(t) + h.exec.SetAvailableCommands([]streams.AvailableCommand{{Name: "review"}}) + + h.mgr.handleAvailableCommandsEvent(h.exec, agentctl.AgentEvent{}) + + require.Zero(t, h.count(), "an empty list must not be published") + require.Len(t, h.mgr.GetAvailableCommandsForSession("session-1"), 1, + "an empty update must not wipe the commands the agent already reported") +} + +// TestHandleAgentEventDropsStaleMCPAttachment pins the identity check: a +// diagnostic from a replaced execution must not be attributed to the live one. +func TestHandleAgentEventDropsStaleMCPAttachment(t *testing.T) { + h := newWorkspaceEventsHarness(t) + + h.mgr.handleAgentEvent(h.exec, agentctl.AgentEvent{ + Type: streams.EventTypeMCPAttachment, + MCPAttachmentAttempt: &streams.MCPAttachmentAttempt{ + AttemptID: "attempt-1", + ExecutionID: "exec-replaced", + }, + }) + + require.Zero(t, h.count(), "an attachment attempt from a replaced execution is dropped") +} + +// TestHandleAgentEventStampsMCPAttachmentIdentity pins that the attempt is +// re-stamped with the live execution's identity before publication, and that a +// copy is stamped rather than the caller's struct. +func TestHandleAgentEventStampsMCPAttachmentIdentity(t *testing.T) { + h := newWorkspaceEventsHarness(t) + h.exec.AgentID = "claude-acp" + h.exec.Status = v1.AgentStatusReady + attempt := &streams.MCPAttachmentAttempt{AttemptID: "attempt-1"} + + h.mgr.handleAgentEvent(h.exec, agentctl.AgentEvent{ + Type: streams.EventTypeMCPAttachment, + MCPAttachmentAttempt: attempt, + }) + + got := h.only(t) + payload, ok := got.Event.Data.(AgentStreamEventPayload) + require.True(t, ok, "payload type = %T", got.Event.Data) + require.NotNil(t, payload.Data) + published := payload.Data.MCPAttachmentAttempt + require.NotNil(t, published) + require.Equal(t, "task-1", published.TaskID) + require.Equal(t, "session-1", published.SessionID) + require.Equal(t, "exec-1", published.ExecutionID) + require.Equal(t, "claude-acp", published.AgentID) + + require.Empty(t, attempt.ExecutionID, + "the caller's attempt must not be stamped in place; a copy is published") + require.Equal(t, v1.AgentStatusReady, h.exec.Status, + "attachment diagnostics are not model activity and must not flip Ready into Running") +} + +// TestHandleStreamDisconnectIgnoresSupersededGeneration pins the guard that +// keeps an old prompt's disconnect from failing the execution that replaced it. +func TestHandleStreamDisconnectIgnoresSupersededGeneration(t *testing.T) { + h := newWorkspaceEventsHarness(t) + h.exec.Status = v1.AgentStatusRunning + h.exec.promptDoneCh = make(chan PromptCompletionSignal, 1) + generation, err := h.mgr.BeginPrompt("exec-1") + require.NoError(t, err) + + h.mgr.handleStreamDisconnect(h.exec, errors.New("connection reset"), generation+1) + + require.NotEqual(t, v1.AgentStatusFailed, h.exec.Status, + "a disconnect from a superseded prompt generation must not fail the live execution") +} + +func TestHandleStreamDisconnectFailsOwningGeneration(t *testing.T) { + h := newWorkspaceEventsHarness(t) + h.exec.Status = v1.AgentStatusRunning + h.exec.promptDoneCh = make(chan PromptCompletionSignal, 1) + generation, err := h.mgr.BeginPrompt("exec-1") + require.NoError(t, err) + + h.mgr.handleStreamDisconnect(h.exec, errors.New("connection reset"), generation) + + require.Equal(t, v1.AgentStatusFailed, h.exec.Status, + "the owning prompt's disconnect fails the execution so the orchestrator can reconcile") +} + +func TestNotifyWorktreeMaterializedPublishesAgentctlReady(t *testing.T) { + h := newWorkspaceEventsHarness(t) + h.exec.TaskEnvironmentID = "env-1" + + h.mgr.NotifyWorktreeMaterialized(context.Background(), MaterializedWorktree{ + TaskID: "task-1", + SessionID: "session-1", + WorktreeID: "wt-1", + WorktreePath: "/work/task-1/backend", + WorktreeBranch: "kandev/feature", + TaskWorkspacePath: "/work/task-1", + }) + + got := h.only(t) + require.Equal(t, events.AgentctlReady, got.Subject) + payload, ok := got.Event.Data.(AgentctlEventPayload) + require.True(t, ok, "payload type = %T", got.Event.Data) + require.Equal(t, "task-1", payload.TaskID) + require.Equal(t, "session-1", payload.SessionID) + require.Equal(t, "exec-1", payload.AgentExecutionID, + "the live execution is resolved so the frontend can key the update") + require.Equal(t, "env-1", payload.TaskEnvironmentID) + require.Equal(t, "wt-1", payload.WorktreeID) + require.Equal(t, "/work/task-1/backend", payload.WorktreePath) + require.Equal(t, "kandev/feature", payload.WorktreeBranch) + require.Equal(t, "/work/task-1", payload.TaskWorkspacePath) +} + +func TestNotifyWorktreeMaterializedWithoutExecutionStillPublishes(t *testing.T) { + h := newWorkspaceEventsHarness(t) + + h.mgr.NotifyWorktreeMaterialized(context.Background(), MaterializedWorktree{ + TaskID: "task-2", SessionID: "session-unknown", WorktreeID: "wt-2", + }) + + payload, ok := h.only(t).Event.Data.(AgentctlEventPayload) + require.True(t, ok) + require.Empty(t, payload.AgentExecutionID, + "a worktree materialized before its execution exists still notifies the frontend") + require.Equal(t, "wt-2", payload.WorktreeID) +} + +func TestNotifyWorktreeMaterializedWithoutEventBusIsNoOp(t *testing.T) { + mgr := newTestManager(t) + mgr.eventPublisher = NewEventPublisher(nil, newTestLogger()) + + mgr.NotifyWorktreeMaterialized(context.Background(), MaterializedWorktree{SessionID: "session-1"}) +} + +func TestBranchSnapshotErrorWrapsCause(t *testing.T) { + cause := errors.New("write executors_running: disk full") + err := &BranchSnapshotError{Cause: cause} + + require.Equal(t, "branch renamed but branch snapshot persistence failed: write executors_running: disk full", + err.Error()) + require.ErrorIs(t, err, cause) + require.False(t, err.Retryable(), + "the branch already has its new name; retrying the Git op would target the wrong branch") + + var nilErr *BranchSnapshotError + require.Equal(t, "branch renamed but branch snapshot persistence failed", nilErr.Error()) + require.NoError(t, nilErr.Unwrap()) + require.Equal(t, "branch renamed but branch snapshot persistence failed", + (&BranchSnapshotError{}).Error()) +} + +func TestMetadataIntAcceptsJSONRoundTrippedShapes(t *testing.T) { + require.Zero(t, metadataInt(nil, "port")) + require.Equal(t, 8080, metadataInt(map[string]interface{}{"port": 8080}, "port")) + require.Equal(t, 8080, metadataInt(map[string]interface{}{"port": int64(8080)}, "port")) + require.Equal(t, 8080, metadataInt(map[string]interface{}{"port": float64(8080)}, "port"), + "a JSON round-trip turns integers into float64") + require.Equal(t, 8080, metadataInt(map[string]interface{}{"port": "8080"}, "port")) + require.Zero(t, metadataInt(map[string]interface{}{"port": "not-a-number"}, "port")) + require.Zero(t, metadataInt(map[string]interface{}{"port": true}, "port")) + require.Zero(t, metadataInt(map[string]interface{}{}, "port")) +} + +func TestAgentctlPIDFromExecution(t *testing.T) { + require.Zero(t, agentctlPIDFromExecution(nil)) + + execution := &AgentExecution{ID: "exec-1"} + require.Zero(t, agentctlPIDFromExecution(execution)) + + execution.setMetadataValue(MetadataKeySSHRemoteAgentctlPID, float64(4242)) + require.Equal(t, 4242, agentctlPIDFromExecution(execution), + "the remote PID survives a persistence round-trip as a float") +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_events_workspace_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_events_workspace_test.go new file mode 100644 index 0000000000..b62480386b --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_events_workspace_test.go @@ -0,0 +1,328 @@ +package lifecycle + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + + agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" + "github.com/kandev/kandev/internal/agentctl/types/streams" + "github.com/kandev/kandev/internal/events" +) + +// workspaceEventsHarness drives the workspace-stream callbacks the manager +// registers on the StreamManager and captures what reaches the event bus. +type workspaceEventsHarness struct { + mgr *Manager + bus *MockEventBusWithTracking + exec *AgentExecution +} + +func newWorkspaceEventsHarness(t *testing.T) *workspaceEventsHarness { + t.Helper() + mgr, eventBus := createTestManagerWithTracking() + cleanupManagerStopCh(t, mgr) + execution := &AgentExecution{ID: "exec-1", TaskID: "task-1", SessionID: "session-1"} + require.NoError(t, mgr.executionStore.Add(execution)) + return &workspaceEventsHarness{mgr: mgr, bus: eventBus, exec: execution} +} + +func (h *workspaceEventsHarness) only(t *testing.T) trackedEvent { + t.Helper() + h.bus.mu.Lock() + defer h.bus.mu.Unlock() + require.Len(t, h.bus.PublishedEvents, 1) + return h.bus.PublishedEvents[0] +} + +func (h *workspaceEventsHarness) count() int { + h.bus.mu.Lock() + defer h.bus.mu.Unlock() + return len(h.bus.PublishedEvents) +} + +// TestHandleGitCommitCreatedPublishesCommitPayload pins the projection the +// orchestrator's commit sync reads: every field of the agentctl notification +// must survive the translation, on the session-scoped git subject. +func TestHandleGitCommitCreatedPublishesCommitPayload(t *testing.T) { + h := newWorkspaceEventsHarness(t) + committedAt := time.Date(2026, 8, 12, 10, 30, 0, 0, time.UTC) + + h.mgr.handleGitCommitCreated(h.exec, &agentctl.GitCommitNotification{ + CommitSHA: "abc123", + ParentSHA: "parent456", + Message: "fix: stop the leak", + AuthorName: "Kandev Agent", + AuthorEmail: "agent@kandev.ai", + FilesChanged: 3, + Insertions: 42, + Deletions: 7, + CommittedAt: committedAt, + RepositoryName: "backend", + }) + + got := h.only(t) + require.Equal(t, events.BuildGitEventSubject("session-1"), got.Subject) + require.Equal(t, events.GitEvent, got.Event.Type) + payload, ok := got.Event.Data.(*GitEventPayload) + require.True(t, ok, "payload type = %T", got.Event.Data) + require.Equal(t, GitEventTypeCommitCreated, payload.Type) + require.Equal(t, "task-1", payload.TaskID) + require.Equal(t, "session-1", payload.SessionID) + require.Equal(t, "exec-1", payload.AgentID) + require.NotNil(t, payload.Commit) + require.Equal(t, "abc123", payload.Commit.CommitSHA) + require.Equal(t, "parent456", payload.Commit.ParentSHA) + require.Equal(t, "fix: stop the leak", payload.Commit.Message) + require.Equal(t, "Kandev Agent", payload.Commit.AuthorName) + require.Equal(t, "agent@kandev.ai", payload.Commit.AuthorEmail) + require.Equal(t, 3, payload.Commit.FilesChanged) + require.Equal(t, 42, payload.Commit.Insertions) + require.Equal(t, 7, payload.Commit.Deletions) + require.Equal(t, committedAt.Format(time.RFC3339), payload.Commit.CommittedAt) + require.Equal(t, "backend", payload.Commit.RepositoryName, + "the per-repo tag must survive so multi-repo workspaces attribute the commit correctly") +} + +// TestHandleGitResetDetectedPublishesResetPayload pins the HEAD-moved-backward +// projection the orchestrator uses to re-sync its commit list. +func TestHandleGitResetDetectedPublishesResetPayload(t *testing.T) { + h := newWorkspaceEventsHarness(t) + at := time.Date(2026, 8, 12, 11, 0, 0, 0, time.UTC) + + h.mgr.handleGitResetDetected(h.exec, &agentctl.GitResetNotification{ + PreviousHead: "head-after", + CurrentHead: "head-before", + RepositoryName: "backend", + Timestamp: at, + }) + + payload, ok := h.only(t).Event.Data.(*GitEventPayload) + require.True(t, ok) + require.Equal(t, GitEventTypeCommitsReset, payload.Type) + require.Equal(t, at.Format(time.RFC3339Nano), payload.Timestamp, + "the reset timestamp comes from the notification, not from publish time") + require.NotNil(t, payload.Reset) + require.Equal(t, "head-after", payload.Reset.PreviousHead) + require.Equal(t, "head-before", payload.Reset.CurrentHead) + require.Equal(t, "backend", payload.Reset.RepositoryName) +} + +// TestHandleBranchSwitchPublishesBaseCommit pins that the new base commit rides +// along; the orchestrator needs it to rebase its diff range. +func TestHandleBranchSwitchPublishesBaseCommit(t *testing.T) { + h := newWorkspaceEventsHarness(t) + at := time.Date(2026, 8, 12, 12, 0, 0, 0, time.UTC) + + h.mgr.handleBranchSwitch(h.exec, &agentctl.GitBranchSwitchNotification{ + PreviousBranch: "main", + CurrentBranch: "feature/cover", + CurrentHead: "head-sha", + BaseCommit: "base-sha", + RepositoryName: "backend", + Timestamp: at, + }) + + payload, ok := h.only(t).Event.Data.(*GitEventPayload) + require.True(t, ok) + require.Equal(t, GitEventTypeBranchSwitched, payload.Type) + require.NotNil(t, payload.BranchSwitch) + require.Equal(t, "main", payload.BranchSwitch.PreviousBranch) + require.Equal(t, "feature/cover", payload.BranchSwitch.CurrentBranch) + require.Equal(t, "head-sha", payload.BranchSwitch.CurrentHead) + require.Equal(t, "base-sha", payload.BranchSwitch.BaseCommit) + require.Equal(t, "backend", payload.BranchSwitch.RepositoryName) + require.Equal(t, at.Format(time.RFC3339Nano), payload.Timestamp) +} + +func TestHandleGitStatusUpdateUsesGitSubject(t *testing.T) { + h := newWorkspaceEventsHarness(t) + + h.mgr.handleGitStatusUpdate(h.exec, &agentctl.GitStatusUpdate{ + Timestamp: time.Now(), + Branch: "feature/cover", + Modified: []string{"main.go"}, + Ahead: 2, + }) + + got := h.only(t) + require.Equal(t, events.BuildGitEventSubject("session-1"), got.Subject) + payload, ok := got.Event.Data.(*GitEventPayload) + require.True(t, ok) + require.Equal(t, GitEventTypeStatusUpdate, payload.Type) + require.NotNil(t, payload.Status) + require.Equal(t, "feature/cover", payload.Status.Branch) + require.Equal(t, []string{"main.go"}, payload.Status.Modified) + require.Equal(t, 2, payload.Status.Ahead) +} + +func TestHandleFileChangeNotificationPublishesPathAndOperation(t *testing.T) { + h := newWorkspaceEventsHarness(t) + at := time.Date(2026, 8, 12, 13, 0, 0, 0, time.UTC) + + h.mgr.handleFileChangeNotification(h.exec, &agentctl.FileChangeNotification{ + Path: "src/main.go", + Operation: "modified", + Timestamp: at, + RepositoryName: "backend", + }) + + got := h.only(t) + require.Equal(t, events.BuildFileChangeSubject("session-1"), got.Subject) + require.Equal(t, events.FileChangeNotified, got.Event.Type) + payload, ok := got.Event.Data.(FileChangeEventPayload) + require.True(t, ok, "payload type = %T", got.Event.Data) + require.Equal(t, "task-1", payload.TaskID) + require.Equal(t, "exec-1", payload.AgentID) + require.Equal(t, "src/main.go", payload.Path) + require.Equal(t, "modified", payload.Operation) + require.Equal(t, at.Format(time.RFC3339Nano), payload.Timestamp) + require.Equal(t, "backend", payload.RepositoryName) +} + +func TestHandleShellOutputAndExitPublishTypedPayloads(t *testing.T) { + h := newWorkspaceEventsHarness(t) + + h.mgr.handleShellOutput(h.exec, "build succeeded\n") + h.mgr.handleShellExit(h.exec, 137) + + h.bus.mu.Lock() + published := append([]trackedEvent(nil), h.bus.PublishedEvents...) + h.bus.mu.Unlock() + require.Len(t, published, 2) + + require.Equal(t, events.BuildShellOutputSubject("session-1"), published[0].Subject) + output, ok := published[0].Event.Data.(ShellOutputEventPayload) + require.True(t, ok, "payload type = %T", published[0].Event.Data) + require.Equal(t, "output", output.Type) + require.Equal(t, "build succeeded\n", output.Data) + require.Equal(t, "session-1", output.SessionID) + + require.Equal(t, events.BuildShellExitSubject("session-1"), published[1].Subject) + exit, ok := published[1].Event.Data.(ShellExitEventPayload) + require.True(t, ok, "payload type = %T", published[1].Event.Data) + require.Equal(t, "exit", exit.Type) + require.Equal(t, 137, exit.Code, "the exit code is what the UI renders; it must not be dropped") +} + +func TestHandleProcessOutputAndStatusIgnoreNilPayloads(t *testing.T) { + h := newWorkspaceEventsHarness(t) + + h.mgr.handleProcessOutput(h.exec, nil) + h.mgr.handleProcessStatus(h.exec, nil) + + require.Zero(t, h.count(), "a nil notification must be dropped, not published as an empty event") +} + +func TestHandleProcessStatusPublishesScriptMetadata(t *testing.T) { + h := newWorkspaceEventsHarness(t) + exitCode := 2 + at := time.Date(2026, 8, 12, 14, 0, 0, 0, time.UTC) + + h.mgr.handleProcessStatus(h.exec, &agentctl.ProcessStatusUpdate{ + SessionID: "session-1", + ProcessID: "proc-9", + Command: "make test", + ScriptName: "setup", + WorkingDir: "/workspace", + Status: "failed", + ExitCode: &exitCode, + Timestamp: at, + }) + + got := h.only(t) + require.Equal(t, events.ProcessStatus, got.Event.Type) + payload, ok := got.Event.Data.(ProcessStatusEventPayload) + require.True(t, ok, "payload type = %T", got.Event.Data) + require.Equal(t, "proc-9", payload.ProcessID) + require.Equal(t, "make test", payload.Command) + require.Equal(t, "setup", payload.ScriptName) + require.Equal(t, "/workspace", payload.WorkingDir) + require.Equal(t, "failed", payload.Status) + require.NotNil(t, payload.ExitCode) + require.Equal(t, 2, *payload.ExitCode) +} + +func TestPublishAvailableCommandsCarriesCommandList(t *testing.T) { + h := newWorkspaceEventsHarness(t) + + h.mgr.eventPublisher.PublishAvailableCommands(h.exec, []streams.AvailableCommand{ + {Name: "review", Description: "Review the diff"}, + }) + + got := h.only(t) + require.Equal(t, events.BuildAvailableCommandsSubject("session-1"), got.Subject) + require.Equal(t, events.AvailableCommandsUpdated, got.Event.Type) + payload, ok := got.Event.Data.(AvailableCommandsEventPayload) + require.True(t, ok, "payload type = %T", got.Event.Data) + require.Len(t, payload.AvailableCommands, 1) + require.Equal(t, "review", payload.AvailableCommands[0].Name) + require.Equal(t, "Review the diff", payload.AvailableCommands[0].Description) +} + +func TestPublishPermissionRequestProjectsOptions(t *testing.T) { + h := newWorkspaceEventsHarness(t) + + h.mgr.eventPublisher.PublishPermissionRequest(h.exec, agentctl.AgentEvent{ + PendingID: "pending-1", + ToolCallID: "tool-1", + PermissionTitle: "Run `rm -rf build`", + ActionType: "execute", + ActionDetails: map[string]any{"command": "rm -rf build"}, + PermissionOptions: []agentctl.PermissionOption{ + {OptionID: "allow", Name: "Allow", Kind: "allow_once"}, + {OptionID: "deny", Name: "Deny", Kind: "reject_once"}, + }, + }) + + got := h.only(t) + require.Equal(t, events.BuildPermissionRequestSubject("session-1"), got.Subject) + payload, ok := got.Event.Data.(PermissionRequestEventPayload) + require.True(t, ok, "payload type = %T", got.Event.Data) + require.Equal(t, "permission_request", payload.Type) + require.Equal(t, "pending-1", payload.PendingID) + require.Equal(t, "tool-1", payload.ToolCallID) + require.Equal(t, "Run `rm -rf build`", payload.Title) + require.Equal(t, "execute", payload.ActionType) + require.Equal(t, map[string]any{"command": "rm -rf build"}, payload.ActionDetails) + require.Equal(t, []PermissionOption{ + {OptionID: "allow", Name: "Allow", Kind: "allow_once"}, + {OptionID: "deny", Name: "Deny", Kind: "reject_once"}, + }, payload.Options, "every option must reach the UI or the dialog renders unusable buttons") +} + +// TestEventPublisherWithoutBusIsInert pins that a publisher wired without an +// event bus (embedded / test configurations) is a silent no-op rather than a +// nil-map panic on every workspace notification. +func TestEventPublisherWithoutBusIsInert(t *testing.T) { + publisher := NewEventPublisher(nil, newTestLogger()) + execution := &AgentExecution{ID: "exec-1", TaskID: "task-1", SessionID: "session-1"} + + publisher.PublishGitEvent(&GitEventPayload{Type: GitEventTypeStatusUpdate, SessionID: "session-1"}) + publisher.PublishFileChange(execution, &agentctl.FileChangeNotification{Path: "a.go"}) + publisher.PublishShellOutput(execution, "hi") + publisher.PublishShellExit(execution, 0) + publisher.PublishPermissionRequest(execution, agentctl.AgentEvent{}) + publisher.PublishAvailableCommands(execution, nil) + publisher.PublishProcessOutput(execution, &agentctl.ProcessOutput{}) + publisher.PublishProcessStatus(execution, &agentctl.ProcessStatusUpdate{}) +} + +// TestPublishGitEventStampsTimestampWhenMissing pins the fallback: a payload +// built without a timestamp still reaches subscribers with one, so the frontend +// can order events. +func TestPublishGitEventStampsTimestampWhenMissing(t *testing.T) { + h := newWorkspaceEventsHarness(t) + + h.mgr.eventPublisher.PublishGitEvent(&GitEventPayload{ + Type: GitEventTypeStatusUpdate, + SessionID: "session-1", + }) + + payload, ok := h.only(t).Event.Data.(*GitEventPayload) + require.True(t, ok) + require.NotEmpty(t, payload.Timestamp) + _, err := time.Parse(time.RFC3339Nano, payload.Timestamp) + require.NoError(t, err) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_execution_passthrough_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_execution_passthrough_test.go new file mode 100644 index 0000000000..da22ccd0e2 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_execution_passthrough_test.go @@ -0,0 +1,342 @@ +package lifecycle + +import ( + "context" + "errors" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/agentruntime" + "github.com/kandev/kandev/internal/task/models" +) + +func TestResolveSessionRuntimeFallbackOrder(t *testing.T) { + ctx := context.Background() + + t.Run("empty session id", func(t *testing.T) { + mgr := newTestManager(t) + _, err := mgr.ResolveSessionRuntime(ctx, "") + require.ErrorContains(t, err, "session_id is required") + }) + + t.Run("in-memory execution wins", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {SessionID: "session-1", ExecutorType: string(models.ExecutorTypeSSH)}, + }} + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-1", SessionID: "session-1", RuntimeName: agentruntime.RuntimeDocker, + })) + + got, err := mgr.ResolveSessionRuntime(ctx, "session-1") + require.NoError(t, err) + require.Equal(t, agentruntime.RuntimeDocker, got, + "a live execution's runtime is authoritative over the persisted executor type") + }) + + t.Run("no provider configured", func(t *testing.T) { + mgr := newTestManager(t) + _, err := mgr.ResolveSessionRuntime(ctx, "session-1") + require.ErrorContains(t, err, "workspace info provider not configured") + }) + + t.Run("provider error", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{err: errors.New("db down")} + _, err := mgr.ResolveSessionRuntime(ctx, "session-1") + require.ErrorContains(t, err, "failed to resolve runtime for session session-1") + }) + + t.Run("session not found", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{}} + _, err := mgr.ResolveSessionRuntime(ctx, "session-1") + require.ErrorContains(t, err, "session session-1 not found") + }) + + t.Run("falls back to stored runtime name", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {SessionID: "session-1", RuntimeName: agentruntime.RuntimeSprites}, + }} + got, err := mgr.ResolveSessionRuntime(ctx, "session-1") + require.NoError(t, err) + require.Equal(t, agentruntime.RuntimeSprites, got, + "legacy records carry only a runtime name") + }) + + t.Run("defaults to standalone", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {SessionID: "session-1"}, + }} + got, err := mgr.ResolveSessionRuntime(ctx, "session-1") + require.NoError(t, err) + require.Equal(t, agentruntime.RuntimeStandalone, got) + }) +} + +func TestVerifyPassthroughEnabledRejectsNonPassthroughProfile(t *testing.T) { + mgr := newTestManager(t) + mgr.profileResolver = &mockPassthroughProfileResolver{cliPassthrough: false} + + _, err := mgr.verifyPassthroughEnabled(context.Background(), "session-1", "profile-1") + + require.ErrorContains(t, err, "is not configured for CLI passthrough mode") +} + +func TestVerifyPassthroughEnabledSurfacesResolveFailure(t *testing.T) { + mgr := newTestManager(t) + mgr.profileResolver = &mockPassthroughProfileResolver{err: errors.New("profile deleted")} + + _, err := mgr.verifyPassthroughEnabled(context.Background(), "session-1", "profile-1") + + require.ErrorContains(t, err, "failed to resolve profile profile-1") + require.ErrorContains(t, err, "profile deleted") +} + +func TestVerifyPassthroughEnabledWithoutResolver(t *testing.T) { + mgr := newTestManager(t) + mgr.profileResolver = nil + + _, err := mgr.verifyPassthroughEnabled(context.Background(), "session-1", "profile-1") + + require.ErrorContains(t, err, "has no profile configured for passthrough mode") +} + +// TestEnsurePassthroughExecutionChecksAccessBeforeCacheLookup pins that a +// non-owner cannot reach a cached execution: the scoping check runs before the +// execution store is consulted. +func TestEnsurePassthroughExecutionChecksAccessBeforeCacheLookup(t *testing.T) { + denied := errors.New("session access denied") + provider := &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {TaskID: "task-1", SessionID: "session-1", WorkspacePath: "/work"}, + }} + mgr := newTestManager(t) + mgr.workspaceInfoProvider = provider + mgr.SetSessionAccessChecker(func(context.Context, string) error { return denied }) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-1", SessionID: "session-1"})) + + _, err := mgr.EnsurePassthroughExecution(context.Background(), "session-1") + + require.ErrorIs(t, err, denied) + require.Zero(t, provider.sessionCalls, "authorization must run before any lookup work") +} + +// TestEnsurePassthroughExecutionResumesExistingExecution covers the recovery +// branch taken after a backend restart: an execution exists but its PTY does +// not, so the resume path runs (and here fails for lack of a runner). +func TestEnsurePassthroughExecutionResumesExistingExecution(t *testing.T) { + mgr := newTestManager(t) + mgr.profileResolver = &mockPassthroughProfileResolver{agentName: "claude-acp", cliPassthrough: true} + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-1", TaskID: "task-1", SessionID: "session-1", AgentProfileID: "profile-1", + // A stale process ID: the runner no longer knows this process. + PassthroughProcessID: "pty-dead", + })) + + _, err := mgr.EnsurePassthroughExecution(context.Background(), "session-1") + + require.ErrorContains(t, err, "resume passthrough session session-1", + "a stale process ID must fall through to the resume path rather than being returned as live") + require.ErrorContains(t, err, "interactive runner not available") +} + +func TestCreateExecutionFromSessionInfoRequiresProvider(t *testing.T) { + mgr := newTestManager(t) + + _, err := mgr.EnsurePassthroughExecution(context.Background(), "session-1") + + require.ErrorContains(t, err, "cannot restore session session-1: workspace info provider not configured") +} + +func TestCreateExecutionFromSessionInfoSurfacesProviderError(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{err: errors.New("db down")} + + _, err := mgr.EnsurePassthroughExecution(context.Background(), "session-1") + + require.ErrorContains(t, err, "get workspace info for session session-1") + require.ErrorContains(t, err, "db down") +} + +func TestCreateExecutionFromSessionInfoRequiresWorkspaceAndTask(t *testing.T) { + ctx := context.Background() + + t.Run("no workspace path", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {TaskID: "task-1", SessionID: "session-1"}, + }} + _, err := mgr.EnsurePassthroughExecution(ctx, "session-1") + require.ErrorIs(t, err, ErrSessionWorkspaceNotReady, + "the terminal handler retries on this sentinel instead of surfacing a hard failure") + }) + + t.Run("no task id", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {SessionID: "session-1", WorkspacePath: "/work"}, + }} + _, err := mgr.EnsurePassthroughExecution(ctx, "session-1") + require.ErrorContains(t, err, "session session-1 has no associated task ID") + }) +} + +// TestCreateExecutionFromSessionInfoRejectsNonPassthroughProfile pins that the +// terminal recovery path refuses to spawn a PTY for an ACP session. +func TestCreateExecutionFromSessionInfoRejectsNonPassthroughProfile(t *testing.T) { + mgr := newTestManager(t) + mgr.profileResolver = &mockPassthroughProfileResolver{cliPassthrough: false} + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": { + TaskID: "task-1", SessionID: "session-1", WorkspacePath: "/work", + ExecutionProfileID: "profile-1", + }, + }} + + _, err := mgr.EnsurePassthroughExecution(context.Background(), "session-1") + + require.ErrorContains(t, err, "is not configured for CLI passthrough mode") + _, exists := mgr.GetExecutionBySessionID("session-1") + require.False(t, exists, "a rejected passthrough recovery must not leave an execution behind") +} + +func TestApplyResumeIntentDerivesFromPreviousExecution(t *testing.T) { + fresh := &AgentExecution{ID: "exec-1"} + applyResumeIntent(fresh, &ExecutorCreateRequest{}) + require.False(t, fresh.isResumedSession, + "a session that never ran has no CLI conversation for a resume flag to attach to") + + recovered := &AgentExecution{ID: "exec-2"} + applyResumeIntent(recovered, &ExecutorCreateRequest{PreviousExecutionID: "exec-1"}) + require.True(t, recovered.isResumedSession) +} + +func TestWorkspaceProfileIDHelpers(t *testing.T) { + require.Empty(t, workspaceExecutionProfileID(nil)) + require.Empty(t, workspaceExecutorProfileID(nil)) + + info := &WorkspaceInfo{AgentProfileID: "office-1"} + require.Equal(t, "office-1", workspaceExecutionProfileID(info), + "a legacy session uses its Office identity as the execution profile") + + info.ExecutionProfileID = "exec-profile-1" + require.Equal(t, "exec-profile-1", workspaceExecutionProfileID(info)) + + info.ExecutorProfileID = "executor-profile-1" + require.Equal(t, "executor-profile-1", workspaceExecutorProfileID(info)) +} + +func TestGetOrEnsureExecutionRequiresSessionID(t *testing.T) { + mgr := newTestManager(t) + + _, err := mgr.GetOrEnsureExecution(context.Background(), "") + + require.ErrorContains(t, err, "session_id is required") +} + +func TestGetOrEnsureExecutionForEnvironmentValidatesProviderResponse(t *testing.T) { + ctx := context.Background() + + t.Run("empty environment id", func(t *testing.T) { + mgr := newTestManager(t) + _, err := mgr.GetOrEnsureExecutionForEnvironment(ctx, "") + require.ErrorContains(t, err, "task_environment_id is required") + }) + + t.Run("no provider", func(t *testing.T) { + mgr := newTestManager(t) + _, err := mgr.GetOrEnsureExecutionForEnvironment(ctx, "env-1") + require.ErrorContains(t, err, "workspace info provider not configured") + }) + + t.Run("provider returns a different environment", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{envInfos: map[string]*WorkspaceInfo{ + "env-1": {TaskEnvironmentID: "env-other", SessionID: "session-1", WorkspacePath: "/work"}, + }} + _, err := mgr.GetOrEnsureExecutionForEnvironment(ctx, "env-1") + require.ErrorContains(t, err, "workspace info resolved environment env-other, want env-1", + "a mismatched environment must fail loudly rather than silently binding the wrong workspace") + }) + + t.Run("environment without workspace path", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{envInfos: map[string]*WorkspaceInfo{ + "env-1": {TaskEnvironmentID: "env-1", SessionID: "session-1"}, + }} + _, err := mgr.GetOrEnsureExecutionForEnvironment(ctx, "env-1") + require.ErrorIs(t, err, ErrSessionWorkspaceNotReady) + }) + + t.Run("environment without a session", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{envInfos: map[string]*WorkspaceInfo{ + "env-1": {TaskEnvironmentID: "env-1", WorkspacePath: "/work"}, + }} + _, err := mgr.GetOrEnsureExecutionForEnvironment(ctx, "env-1") + require.ErrorContains(t, err, "task environment env-1 has no task session") + }) +} + +func TestEnsureWorkspaceExecutionForSessionRequiresSessionID(t *testing.T) { + mgr := newTestManager(t) + + _, err := mgr.EnsureWorkspaceExecutionForSession(context.Background(), "task-1", "") + + require.ErrorContains(t, err, "session_id is required") +} + +func TestEnsureWorkspaceExecutionLockedReusesEnvironmentExecution(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-new": { + TaskID: "task-1", SessionID: "session-new", TaskEnvironmentID: "env-1", WorkspacePath: "/work", + }, + }} + existing := &AgentExecution{ + ID: "exec-existing", TaskID: "task-1", SessionID: "session-old", TaskEnvironmentID: "env-1", + } + require.NoError(t, mgr.executionStore.Add(existing)) + + got, err := mgr.EnsureWorkspaceExecutionForSession(context.Background(), "task-1", "session-new") + + require.NoError(t, err) + require.Same(t, existing, got, + "sibling sessions in one task environment must share the existing execution, not create a second one") +} + +func TestEnsureWorkspaceExecutionLockedRequiresProviderAndInfo(t *testing.T) { + ctx := context.Background() + + t.Run("no provider", func(t *testing.T) { + mgr := newTestManager(t) + _, err := mgr.EnsureWorkspaceExecutionForSession(ctx, "task-1", "session-1") + require.ErrorContains(t, err, "workspace info provider not configured") + }) + + t.Run("provider error", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{err: errors.New("db down")} + _, err := mgr.EnsureWorkspaceExecutionForSession(ctx, "task-1", "session-1") + require.ErrorContains(t, err, "failed to get workspace info for session session-1") + }) + + t.Run("session not found", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{}} + _, err := mgr.EnsureWorkspaceExecutionForSession(ctx, "task-1", "session-1") + require.ErrorContains(t, err, "session session-1 not found") + }) + + t.Run("workspace not ready", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {TaskID: "task-1", SessionID: "session-1"}, + }} + _, err := mgr.EnsureWorkspaceExecutionForSession(ctx, "task-1", "session-1") + require.ErrorIs(t, err, ErrSessionWorkspaceNotReady) + }) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_interaction_accessors_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_interaction_accessors_test.go new file mode 100644 index 0000000000..22ada0ba6a --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_interaction_accessors_test.go @@ -0,0 +1,402 @@ +package lifecycle + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/agent/executor" + "github.com/kandev/kandev/internal/agentctl/types/streams" + "github.com/kandev/kandev/internal/events" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +func TestWasSessionInitializedReflectsExecutionFlag(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-init", sessionInitialized: true})) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-pending"})) + + require.True(t, mgr.WasSessionInitialized("exec-init")) + require.False(t, mgr.WasSessionInitialized("exec-pending")) + require.False(t, mgr.WasSessionInitialized("exec-absent"), "an unknown execution is not initialized") +} + +func TestGetAvailableCommandsForSessionReturnsCachedCommands(t *testing.T) { + mgr := newTestManager(t) + exec := &AgentExecution{ID: "exec-1", SessionID: "session-1"} + exec.SetAvailableCommands([]streams.AvailableCommand{ + {Name: "review", Description: "Review the diff"}, + {Name: "commit"}, + }) + require.NoError(t, mgr.executionStore.Add(exec)) + + got := mgr.GetAvailableCommandsForSession("session-1") + + require.Len(t, got, 2) + require.Equal(t, "review", got[0].Name) + require.Equal(t, "Review the diff", got[0].Description) + require.Equal(t, "commit", got[1].Name) + require.Nil(t, mgr.GetAvailableCommandsForSession("session-absent")) +} + +func TestGetModeStateForSessionReturnsCachedMode(t *testing.T) { + mgr := newTestManager(t) + exec := &AgentExecution{ID: "exec-1", SessionID: "session-1"} + exec.SetModeState(&CachedModeState{ + CurrentModeID: "plan", + AvailableModes: []streams.SessionModeInfo{{ID: "plan"}, {ID: "default"}}, + }) + require.NoError(t, mgr.executionStore.Add(exec)) + + state := mgr.GetModeStateForSession("session-1") + + require.NotNil(t, state) + require.Equal(t, "plan", state.CurrentModeID) + require.Len(t, state.AvailableModes, 2) + require.Nil(t, mgr.GetModeStateForSession("session-absent")) +} + +func TestGetModelStateForSessionReturnsCachedModel(t *testing.T) { + mgr := newTestManager(t) + exec := &AgentExecution{ID: "exec-1", SessionID: "session-1"} + exec.SetModelState(&CachedModelState{CurrentModelID: "claude-opus-5"}) + require.NoError(t, mgr.executionStore.Add(exec)) + + state := mgr.GetModelStateForSession("session-1") + + require.NotNil(t, state) + require.Equal(t, "claude-opus-5", state.CurrentModelID) + require.Nil(t, mgr.GetModelStateForSession("session-absent")) +} + +func TestGetPromptGenerationForSessionTracksBeginPrompt(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-1", SessionID: "session-1", Status: v1.AgentStatusReady, + promptDoneCh: make(chan PromptCompletionSignal, 1), + })) + + before, err := mgr.GetPromptGenerationForSession(context.Background(), "session-1") + require.NoError(t, err) + + generation, err := mgr.BeginPrompt("exec-1") + require.NoError(t, err) + require.Greater(t, generation, before, "BeginPrompt must advance the prompt generation") + + after, err := mgr.GetPromptGenerationForSession(context.Background(), "session-1") + require.NoError(t, err) + require.Equal(t, generation, after, + "the session-scoped read must observe the generation BeginPrompt handed out") + require.True(t, mgr.OwnsPromptGeneration("session-1", "exec-1", generation)) + require.False(t, mgr.OwnsPromptGeneration("session-1", "exec-1", generation+1)) +} + +func TestGetPromptGenerationForSessionUnknownSession(t *testing.T) { + mgr := newTestManager(t) + + _, err := mgr.GetPromptGenerationForSession(context.Background(), "session-absent") + + require.ErrorIs(t, err, ErrNoExecutionForSession) +} + +func TestBeginPromptUnknownExecution(t *testing.T) { + mgr := newTestManager(t) + + _, err := mgr.BeginPrompt("missing") + + require.ErrorContains(t, err, `execution "missing" not found`) +} + +func TestGetExecutionIDForSession(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-42", SessionID: "session-42"})) + + id, err := mgr.GetExecutionIDForSession(context.Background(), "session-42") + require.NoError(t, err) + require.Equal(t, "exec-42", id) + + id, err = mgr.GetExecutionIDForSession(context.Background(), "session-absent") + require.ErrorIs(t, err, ErrNoExecutionForSession) + require.Empty(t, id) +} + +func TestIsAgentCommandConfigured(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-agent", AgentCommand: "claude"})) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-workspace"})) + + require.True(t, mgr.IsAgentCommandConfigured("exec-agent")) + require.False(t, mgr.IsAgentCommandConfigured("exec-workspace"), + "a workspace-only execution has not been promoted to an agent execution") + require.False(t, mgr.IsAgentCommandConfigured("exec-absent")) +} + +func TestResolveTaskEnvironmentIDPrefersInMemoryExecution(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {SessionID: "session-1", TaskEnvironmentID: "env-from-db"}, + }} + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-1", SessionID: "session-1", TaskEnvironmentID: "env-in-memory", + })) + + got, err := mgr.ResolveTaskEnvironmentID(context.Background(), "session-1") + + require.NoError(t, err) + require.Equal(t, "env-in-memory", got) +} + +func TestResolveTaskEnvironmentIDErrors(t *testing.T) { + ctx := context.Background() + + t.Run("empty session id", func(t *testing.T) { + mgr := newTestManager(t) + _, err := mgr.ResolveTaskEnvironmentID(ctx, "") + require.ErrorContains(t, err, "session_id is required") + }) + + t.Run("execution without environment", func(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-1", SessionID: "session-1"})) + _, err := mgr.ResolveTaskEnvironmentID(ctx, "session-1") + require.ErrorContains(t, err, "has no task environment ID") + }) + + t.Run("no provider configured", func(t *testing.T) { + mgr := newTestManager(t) + _, err := mgr.ResolveTaskEnvironmentID(ctx, "session-1") + require.ErrorContains(t, err, "workspace info provider not configured") + }) + + t.Run("provider returns nil info", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{}} + _, err := mgr.ResolveTaskEnvironmentID(ctx, "session-1") + require.ErrorContains(t, err, "has no task environment ID") + }) + + t.Run("provider falls back for missing execution", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {SessionID: "session-1", TaskEnvironmentID: "env-from-db"}, + }} + got, err := mgr.ResolveTaskEnvironmentID(ctx, "session-1") + require.NoError(t, err) + require.Equal(t, "env-from-db", got) + }) +} + +// TestIsAgentRunningForSessionStartingGrace pins the bounded boot window: a +// STARTING execution is reported live without probing agentctl, but only until +// the grace expires. +func TestIsAgentRunningForSessionStartingGrace(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-fresh", SessionID: "session-fresh", + Status: v1.AgentStatusStarting, StartedAt: time.Now(), + })) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-stale", SessionID: "session-stale", + Status: v1.AgentStatusStarting, StartedAt: time.Now().Add(-2 * agentStartupLivenessGrace), + })) + + require.True(t, mgr.IsAgentRunningForSession(context.Background(), "session-fresh")) + require.False(t, mgr.IsAgentRunningForSession(context.Background(), "session-stale"), + "a starting execution past the grace window must fall through to probing and be reaped") + require.False(t, mgr.IsAgentRunningForSession(context.Background(), "session-absent")) +} + +func TestIsAgentRunningForSessionPassthroughWithoutRunner(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-pty", SessionID: "session-pty", Status: v1.AgentStatusReady, + PassthroughProcessID: "pty-1", StartedAt: time.Now(), + })) + + require.False(t, mgr.IsAgentRunningForSession(context.Background(), "session-pty"), + "without an interactive runner a passthrough process cannot be confirmed alive") +} + +func TestIsAgentReadyForPromptRequiresInitializedACPSession(t *testing.T) { + ctx := context.Background() + + t.Run("no execution", func(t *testing.T) { + mgr := newTestManager(t) + require.False(t, mgr.IsAgentReadyForPrompt(ctx, "session-absent")) + }) + + t.Run("not ready status", func(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-1", SessionID: "session-1", Status: v1.AgentStatusRunning, + })) + require.False(t, mgr.IsAgentReadyForPrompt(ctx, "session-1")) + }) + + t.Run("ready but session not initialized", func(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-1", SessionID: "session-1", Status: v1.AgentStatusReady, + agentctl: newReadyAgentctlClient(t, newTestLogger()), + })) + require.False(t, mgr.IsAgentReadyForPrompt(ctx, "session-1"), + "an uninitialized ACP session cannot take a prompt even when the status is ready") + }) + + t.Run("passthrough delegates to running check", func(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-pty", SessionID: "session-pty", Status: v1.AgentStatusReady, + IsPassthrough: true, StartedAt: time.Now(), + })) + require.False(t, mgr.IsAgentReadyForPrompt(ctx, "session-pty")) + }) +} + +func TestStopBySessionIDStopsResolvedExecution(t *testing.T) { + log := newTestRegistryLogger() + execRegistry := NewExecutorRegistry(log) + stopTracker := &mockStopTracker{name: executor.NameStandalone} + execRegistry.Register(stopTracker) + mgr := NewManager(newTestRegistry(), &MockEventBus{}, execRegistry, nil, nil, nil, ExecutorFallbackWarn, "", log) + cleanupManagerStopCh(t, mgr) + + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-stop", + TaskID: "task-stop", + SessionID: "session-stop", + RuntimeName: executor.NameStandalone, + Status: v1.AgentStatusRunning, + })) + + require.NoError(t, mgr.StopBySessionID(context.Background(), "session-stop", false)) + + require.True(t, stopTracker.stopCalled, "StopBySessionID must reach the executor backend") + require.Equal(t, "exec-stop", stopTracker.stoppedInstanceID) + _, exists := mgr.GetExecutionBySessionID("session-stop") + require.False(t, exists, "a stopped execution is removed from tracking") + + bus, ok := mgr.eventBus.(*MockEventBus) + require.True(t, ok) + require.NotEmpty(t, bus.PublishedEvents) + require.Equal(t, events.AgentStopped, bus.PublishedEvents[len(bus.PublishedEvents)-1].Type) +} + +func TestStopBySessionIDUnknownSession(t *testing.T) { + mgr := newTestManager(t) + + err := mgr.StopBySessionID(context.Background(), "session-absent", false) + + require.ErrorContains(t, err, `no agent running for session "session-absent"`) +} + +func TestStopAgentUnknownExecution(t *testing.T) { + mgr := newTestManager(t) + + err := mgr.StopAgent(context.Background(), "missing", false) + + require.ErrorIs(t, err, ErrExecutionNotFound) +} + +// TestStopAgentForcePassesForceToBackend pins that the force flag reaches the +// executor backend rather than being swallowed by the graceful-stop branch. +func TestStopAgentForcePassesForceToBackend(t *testing.T) { + log := newTestRegistryLogger() + execRegistry := NewExecutorRegistry(log) + stopTracker := &forceRecordingStopTracker{mockStopTracker: mockStopTracker{name: executor.NameStandalone}} + execRegistry.Register(stopTracker) + mgr := NewManager(newTestRegistry(), &MockEventBus{}, execRegistry, nil, nil, nil, ExecutorFallbackWarn, "", log) + cleanupManagerStopCh(t, mgr) + + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-force", SessionID: "session-force", RuntimeName: executor.NameStandalone, + })) + + require.NoError(t, mgr.StopAgentWithReason(context.Background(), "exec-force", StopReasonBackendShutdown, true)) + + require.True(t, stopTracker.forced, "force must be forwarded to StopInstance") + require.Equal(t, StopReasonBackendShutdown, stopTracker.stopReason) +} + +type forceRecordingStopTracker struct { + mockStopTracker + forced bool +} + +func (m *forceRecordingStopTracker) StopInstance(ctx context.Context, instance *ExecutorInstance, force bool) error { + m.forced = force + return m.mockStopTracker.StopInstance(ctx, instance, force) +} + +func TestSteerAgentWithDispatchCallbackUnknownExecution(t *testing.T) { + mgr := newTestManager(t) + + _, err := mgr.SteerAgentWithDispatchCallback(context.Background(), "missing", "hi", nil, false, nil) + + require.ErrorIs(t, err, ErrExecutionNotFound) +} + +func TestUpdateStatusUnknownExecution(t *testing.T) { + mgr := newTestManager(t) + + err := mgr.UpdateStatus("missing", v1.AgentStatusReady) + + require.ErrorContains(t, err, `execution "missing" not found`) +} + +func TestUpdateStatusTransitionsExecution(t *testing.T) { + mgr := newTestManager(t) + exec := &AgentExecution{ID: "exec-1", SessionID: "session-1", Status: v1.AgentStatusStarting} + require.NoError(t, mgr.executionStore.Add(exec)) + + require.NoError(t, mgr.UpdateStatus("exec-1", v1.AgentStatusRunning)) + + require.Equal(t, v1.AgentStatusRunning, exec.Status) +} + +func TestMarkBootReadyFromFailedOnlyPromotesFailedExecutions(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-ready", SessionID: "session-ready", Status: v1.AgentStatusReady, + })) + failed := &AgentExecution{ID: "exec-failed", SessionID: "session-failed", Status: v1.AgentStatusFailed} + require.NoError(t, mgr.executionStore.Add(failed)) + + require.NoError(t, mgr.markBootReadyFromFailed(context.Background(), "exec-ready"), + "a non-failed execution is a no-op, not an error") + require.ErrorContains(t, mgr.markBootReadyFromFailed(context.Background(), "missing"), + `execution "missing" not found`) + + require.NoError(t, mgr.markBootReadyFromFailed(context.Background(), "exec-failed")) + require.Equal(t, v1.AgentStatusReady, failed.Status) + + bus, ok := mgr.eventBus.(*MockEventBus) + require.True(t, ok) + require.Len(t, bus.PublishedEvents, 1, "only the failed execution publishes a boot-ready event") + require.Equal(t, events.AgentBootReady, bus.PublishedEvents[0].Type) +} + +func TestGetSessionAuthMethodsFallsBackByAgentID(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-claude", SessionID: "session-claude", AgentID: "claude-acp", + })) + cached := &AgentExecution{ID: "exec-cached", SessionID: "session-cached", AgentID: "claude-acp"} + cached.SetAuthMethods([]streams.AuthMethodInfo{{ID: "cached-method"}}) + require.NoError(t, mgr.executionStore.Add(cached)) + + fallback := mgr.GetSessionAuthMethods("session-claude") + require.Len(t, fallback, 1) + require.Equal(t, "claude-auth-login", fallback[0].ID) + require.NotNil(t, fallback[0].TerminalAuth) + require.Equal(t, "claude", fallback[0].TerminalAuth.Command) + + live := mgr.GetSessionAuthMethods("session-cached") + require.Len(t, live, 1) + require.Equal(t, "cached-method", live[0].ID, + "reported capabilities win over the static fallback") + + require.Nil(t, mgr.GetSessionAuthMethods("session-absent")) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_interaction_session_ops_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_interaction_session_ops_test.go new file mode 100644 index 0000000000..3087819a53 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_interaction_session_ops_test.go @@ -0,0 +1,463 @@ +package lifecycle + +import ( + "context" + "encoding/json" + "errors" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" + + agentctlclient "github.com/kandev/kandev/internal/agent/runtime/agentctl" + v1 "github.com/kandev/kandev/pkg/api/v1" + ws "github.com/kandev/kandev/pkg/websocket" +) + +// sessionOpsHarness wires a Manager to a live agentctl agent stream backed by +// the mock WS server, recording every request the manager makes so tests can +// assert the exact action and payload that reached agentctl rather than merely +// that the call returned nil. +type sessionOpsHarness struct { + mgr *Manager + exec *AgentExecution + mu sync.Mutex + requests []recordedStreamRequest +} + +type recordedStreamRequest struct { + action string + payload map[string]interface{} + raw []byte +} + +// decode unmarshals the captured wire payload into v. Needed for payloads whose +// fields carry `omitempty` (permission responses), where the typed shape — not +// the raw map — is what agentctl actually observes. +func (r recordedStreamRequest) decode(t *testing.T, v interface{}) { + t.Helper() + require.NoError(t, json.Unmarshal(r.raw, v)) +} + +func (h *sessionOpsHarness) record(msg ws.Message) { + payload := map[string]interface{}{} + raw := append([]byte(nil), msg.Payload...) + if len(raw) > 0 { + _ = json.Unmarshal(raw, &payload) + } + h.mu.Lock() + h.requests = append(h.requests, recordedStreamRequest{action: msg.Action, payload: payload, raw: raw}) + h.mu.Unlock() +} + +// only returns the single recorded request, failing when the manager made a +// different number of agentctl calls than expected. +func (h *sessionOpsHarness) only(t *testing.T) recordedStreamRequest { + t.Helper() + h.mu.Lock() + defer h.mu.Unlock() + require.Len(t, h.requests, 1, "expected exactly one agentctl stream request, got %+v", h.requests) + return h.requests[0] +} + +func (h *sessionOpsHarness) count() int { + h.mu.Lock() + defer h.mu.Unlock() + return len(h.requests) +} + +// newSessionOpsHarness builds a manager holding one execution whose agentctl +// client has a connected agent stream. The execution is registered in the +// store under both its execution ID and session ID. +func newSessionOpsHarness(t *testing.T) *sessionOpsHarness { + t.Helper() + + h := &sessionOpsHarness{} + mock := newMockAgentServer(t) + t.Cleanup(mock.Close) + mock.handler = func(msg ws.Message) *ws.Message { + h.record(msg) + resp, _ := ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{"success": true}) + return resp + } + + client := createTestClient(t, mock.server.URL) + t.Cleanup(client.Close) + streamCtx, streamCancel := context.WithCancel(context.Background()) + t.Cleanup(streamCancel) + require.NoError(t, client.StreamUpdates(streamCtx, func(agentctlclient.AgentEvent) {}, nil, nil)) + select { + case <-mock.wsConnected: + case <-time.After(2 * time.Second): + t.Fatal("mock agentctl server never saw the agent stream connect") + } + + h.mgr = newTestManager(t) + h.exec = &AgentExecution{ + ID: "exec-ops", + TaskID: "task-ops", + SessionID: "session-ops", + ACPSessionID: "acp-live", + Status: v1.AgentStatusReady, + sessionInitialized: true, + agentctl: client, + } + require.NoError(t, h.mgr.executionStore.Add(h.exec)) + return h +} + +// TestSetSessionModeUsesLiveACPSessionID pins the documented invariant on +// SetSessionMode: the ACP session ID sent to agentctl always comes from the +// execution, never from the caller's (possibly stale) copy. +func TestSetSessionModeUsesLiveACPSessionID(t *testing.T) { + h := newSessionOpsHarness(t) + + require.NoError(t, h.mgr.SetSessionMode(context.Background(), "exec-ops", "acp-stale-from-caller", "plan")) + + req := h.only(t) + require.Equal(t, "agent.session.set_mode", req.action) + require.Equal(t, "acp-live", req.payload["session_id"], + "SetSessionMode must send the execution's current ACP session ID, not the caller's argument") + require.Equal(t, "plan", req.payload["mode_id"]) +} + +func TestSetSessionModeBySessionIDResolvesExecution(t *testing.T) { + h := newSessionOpsHarness(t) + + require.NoError(t, h.mgr.SetSessionModeBySessionID(context.Background(), "session-ops", "accept-edits")) + + req := h.only(t) + require.Equal(t, "agent.session.set_mode", req.action) + require.Equal(t, "acp-live", req.payload["session_id"]) + require.Equal(t, "accept-edits", req.payload["mode_id"]) +} + +func TestSetSessionModeGuardsRejectBeforeReachingAgentctl(t *testing.T) { + ctx := context.Background() + + t.Run("unknown execution", func(t *testing.T) { + h := newSessionOpsHarness(t) + err := h.mgr.SetSessionMode(ctx, "missing", "", "plan") + require.ErrorContains(t, err, `execution "missing" not found`) + require.Zero(t, h.count(), "no agentctl call may be made for an unknown execution") + }) + + t.Run("no agentctl client", func(t *testing.T) { + h := newSessionOpsHarness(t) + require.NoError(t, h.mgr.executionStore.Add(&AgentExecution{ + ID: "exec-bare", SessionID: "session-bare", ACPSessionID: "acp", sessionInitialized: true, + })) + err := h.mgr.SetSessionMode(ctx, "exec-bare", "", "plan") + require.ErrorContains(t, err, "has no agentctl client") + require.Zero(t, h.count()) + }) + + t.Run("acp session not initialized", func(t *testing.T) { + h := newSessionOpsHarness(t) + h.exec.sessionInitialized = false + err := h.mgr.SetSessionMode(ctx, "exec-ops", "", "plan") + require.ErrorContains(t, err, "ACP session is not ready") + require.Zero(t, h.count()) + }) + + t.Run("acp session id empty", func(t *testing.T) { + h := newSessionOpsHarness(t) + h.exec.ACPSessionID = "" + err := h.mgr.SetSessionMode(ctx, "exec-ops", "", "plan") + require.ErrorContains(t, err, "ACP session is not ready") + require.Zero(t, h.count()) + }) + + t.Run("unknown session id", func(t *testing.T) { + h := newSessionOpsHarness(t) + err := h.mgr.SetSessionModeBySessionID(ctx, "nope", "plan") + require.ErrorContains(t, err, `no agent running for session "nope"`) + require.Zero(t, h.count()) + }) +} + +func TestSetSessionModelSendsModelToAgentctl(t *testing.T) { + h := newSessionOpsHarness(t) + + require.NoError(t, h.mgr.SetSessionModelBySessionID(context.Background(), "session-ops", "claude-opus-5")) + + req := h.only(t) + require.Equal(t, "agent.session.set_model", req.action) + require.Equal(t, "claude-opus-5", req.payload["model_id"]) + require.Empty(t, h.exec.metadataString(MetadataKeyModelOverride), + "an ACP session swaps the model in-place and must not persist a relaunch override") +} + +// TestSetSessionModelPassthroughPersistsOverrideAndRestarts pins the +// passthrough branch: a TUI agent has no live channel for the model, so the +// override is persisted and the process is relaunched instead. The restart is +// expected to fail here (no interactive runner is wired), which is what proves +// RestartAgentProcess was reached — but the override must already be stored. +func TestSetSessionModelPassthroughPersistsOverrideAndRestarts(t *testing.T) { + h := newSessionOpsHarness(t) + h.exec.PassthroughProcessID = "pty-1" + + err := h.mgr.SetSessionModel(context.Background(), "exec-ops", "gpt-5-codex") + + require.Error(t, err, "restart must surface the missing interactive runner") + require.ErrorContains(t, err, "interactive runner not available") + require.Equal(t, "gpt-5-codex", h.exec.metadataString(MetadataKeyModelOverride), + "passthrough model change must persist the override for the next launch") + require.Zero(t, h.count(), "passthrough agents have no ACP channel for set_model") +} + +func TestSetSessionModelGuards(t *testing.T) { + ctx := context.Background() + + t.Run("unknown execution", func(t *testing.T) { + h := newSessionOpsHarness(t) + require.ErrorContains(t, h.mgr.SetSessionModel(ctx, "missing", "m"), `execution "missing" not found`) + require.Zero(t, h.count()) + }) + + t.Run("no agentctl client", func(t *testing.T) { + h := newSessionOpsHarness(t) + require.NoError(t, h.mgr.executionStore.Add(&AgentExecution{ID: "exec-bare", SessionID: "session-bare"})) + require.ErrorContains(t, h.mgr.SetSessionModel(ctx, "exec-bare", "m"), "has no agentctl client") + require.Zero(t, h.count()) + }) + + t.Run("unknown session id", func(t *testing.T) { + h := newSessionOpsHarness(t) + require.ErrorContains(t, h.mgr.SetSessionModelBySessionID(ctx, "nope", "m"), `no agent running for session "nope"`) + require.Zero(t, h.count()) + }) +} + +func TestSetSessionConfigOptionSendsIDAndValue(t *testing.T) { + h := newSessionOpsHarness(t) + + require.NoError(t, h.mgr.SetSessionConfigOptionBySessionID(context.Background(), "session-ops", "reasoning", "high")) + + req := h.only(t) + require.Equal(t, "agent.session.set_config_option", req.action) + require.Equal(t, "reasoning", req.payload["config_id"]) + require.Equal(t, "high", req.payload["value"]) +} + +func TestSetSessionConfigOptionGuards(t *testing.T) { + ctx := context.Background() + + t.Run("unknown execution", func(t *testing.T) { + h := newSessionOpsHarness(t) + require.ErrorContains(t, h.mgr.SetSessionConfigOption(ctx, "missing", "c", "v"), `execution "missing" not found`) + require.Zero(t, h.count()) + }) + + t.Run("acp session not ready", func(t *testing.T) { + h := newSessionOpsHarness(t) + h.exec.ACPSessionID = "" + require.ErrorContains(t, h.mgr.SetSessionConfigOption(ctx, "exec-ops", "c", "v"), "ACP session is not ready") + require.Zero(t, h.count()) + }) + + t.Run("unknown session id", func(t *testing.T) { + h := newSessionOpsHarness(t) + require.ErrorContains(t, h.mgr.SetSessionConfigOptionBySessionID(ctx, "nope", "c", "v"), + `no agent running for session "nope"`) + require.Zero(t, h.count()) + }) +} + +func TestAuthenticateBySessionIDSendsMethodID(t *testing.T) { + h := newSessionOpsHarness(t) + + require.NoError(t, h.mgr.AuthenticateBySessionID(context.Background(), "session-ops", "claude-auth-login")) + + req := h.only(t) + require.Equal(t, "agent.session.authenticate", req.action) + require.Equal(t, "claude-auth-login", req.payload["method_id"]) +} + +func TestAuthenticateBySessionIDGuards(t *testing.T) { + ctx := context.Background() + + t.Run("unknown session", func(t *testing.T) { + h := newSessionOpsHarness(t) + require.ErrorContains(t, h.mgr.AuthenticateBySessionID(ctx, "nope", "m"), `no agent running for session "nope"`) + require.Zero(t, h.count()) + }) + + t.Run("no agentctl client", func(t *testing.T) { + h := newSessionOpsHarness(t) + require.NoError(t, h.mgr.executionStore.Add(&AgentExecution{ID: "exec-bare", SessionID: "session-bare"})) + require.ErrorContains(t, h.mgr.AuthenticateBySessionID(ctx, "session-bare", "m"), "has no agentctl client") + require.Zero(t, h.count()) + }) +} + +func TestRespondToPermissionForwardsDecision(t *testing.T) { + for _, tc := range []struct { + name string + optionID string + cancelled bool + }{ + {name: "approve", optionID: "allow", cancelled: false}, + {name: "deny", optionID: "deny", cancelled: false}, + {name: "dialog cancelled", optionID: "", cancelled: true}, + } { + t.Run(tc.name, func(t *testing.T) { + h := newSessionOpsHarness(t) + + require.NoError(t, h.mgr.RespondToPermissionBySessionID("session-ops", "pending-7", tc.optionID, tc.cancelled)) + + req := h.only(t) + require.Equal(t, "agent.permissions.respond", req.action) + var got struct { + PendingID string `json:"pending_id"` + OptionID string `json:"option_id"` + Cancelled bool `json:"cancelled"` + } + req.decode(t, &got) + require.Equal(t, "pending-7", got.PendingID) + require.Equal(t, tc.optionID, got.OptionID) + require.Equal(t, tc.cancelled, got.Cancelled) + }) + } +} + +func TestRespondToPermissionGuards(t *testing.T) { + t.Run("unknown execution", func(t *testing.T) { + h := newSessionOpsHarness(t) + require.ErrorContains(t, h.mgr.RespondToPermission("missing", "p", "allow", false), + "agent execution not found: missing") + require.Zero(t, h.count()) + }) + + t.Run("no agentctl client", func(t *testing.T) { + h := newSessionOpsHarness(t) + require.NoError(t, h.mgr.executionStore.Add(&AgentExecution{ID: "exec-bare", SessionID: "session-bare"})) + require.ErrorContains(t, h.mgr.RespondToPermission("exec-bare", "p", "allow", false), + "agent execution has no agentctl client") + require.Zero(t, h.count()) + }) + + t.Run("unknown session", func(t *testing.T) { + h := newSessionOpsHarness(t) + require.ErrorContains(t, h.mgr.RespondToPermissionBySessionID("nope", "p", "allow", false), + "no agent execution found for session: nope") + require.Zero(t, h.count()) + }) +} + +// TestCancelAgentBySessionIDPassthroughWritesCtrlC pins that a passthrough +// session is cancelled with a PTY Ctrl-C rather than an ACP agent.cancel, and +// that a failed write stays non-fatal so the caller still reconciles DB state. +func TestCancelAgentBySessionIDPassthroughSendsCtrlCNotACPCancel(t *testing.T) { + h := newSessionOpsHarness(t) + h.exec.PassthroughProcessID = "pty-1" + + // No interactive runner is wired, so WritePassthroughStdin fails — the + // documented non-fatal path. + require.NoError(t, h.mgr.CancelAgentBySessionID(context.Background(), "session-ops"), + "a failed PTY write must not surface as a cancel error") + require.Zero(t, h.count(), "passthrough cancel must not send agent.cancel over ACP") +} + +func TestCancelAgentBySessionIDACPPathSendsCancel(t *testing.T) { + h := newSessionOpsHarness(t) + + require.NoError(t, h.mgr.CancelAgentBySessionID(context.Background(), "session-ops")) + + req := h.only(t) + require.Equal(t, "agent.cancel", req.action) +} + +func TestCancelAgentBySessionIDUnknownSession(t *testing.T) { + h := newSessionOpsHarness(t) + + err := h.mgr.CancelAgentBySessionID(context.Background(), "nope") + + require.ErrorIs(t, err, ErrNoExecutionForSession) + require.Zero(t, h.count()) +} + +func TestCancelAgentRejectsExecutionWithoutAgentctl(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-bare", SessionID: "session-bare"})) + + err := mgr.CancelAgent(context.Background(), "exec-bare") + + require.ErrorContains(t, err, "has no agentctl client") +} + +func TestCancelAgentUnknownExecution(t *testing.T) { + mgr := newTestManager(t) + + err := mgr.CancelAgent(context.Background(), "missing") + + require.ErrorContains(t, err, `execution "missing" not found`) +} + +// TestSetMcpProvidersForSessionSendsProviders exercises the HTTP-backed MCP +// provider refresh and its documented no-op-on-missing-session contract. +func TestSetMcpProvidersForSessionRequiresSessionID(t *testing.T) { + mgr := newTestManager(t) + + require.ErrorContains(t, mgr.SetMcpProvidersForSession(context.Background(), "", nil), "session_id is required") +} + +func TestSetMcpProvidersForSessionMissingExecutionIsNoOp(t *testing.T) { + mgr := newTestManager(t) + + require.NoError(t, mgr.SetMcpProvidersForSession(context.Background(), "session-absent", []string{"github"}), + "an absent execution is a successful no-op: the next launch re-derives providers") +} + +func TestSetMcpProvidersForSessionRequiresAgentctl(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-bare", SessionID: "session-bare"})) + + err := mgr.SetMcpProvidersForSession(context.Background(), "session-bare", []string{"github"}) + + require.ErrorContains(t, err, "has no agentctl client") +} + +func TestSetMcpModeGuards(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-bare", SessionID: "session-bare"})) + + require.ErrorContains(t, mgr.SetMcpMode(context.Background(), "missing", "config"), `execution "missing" not found`) + require.ErrorContains(t, mgr.SetMcpMode(context.Background(), "exec-bare", "config"), "has no agentctl client") +} + +// TestCancelAgentEscalatesWhenStreamDisconnected covers the branch where the +// stream is torn down before the cancel lands: the manager escalates locally +// rather than surfacing the transport error. +func TestCancelAgentEscalatesWhenStreamDisconnected(t *testing.T) { + prevEsc := cancelEscalationTimeout + cancelEscalationTimeout = 50 * time.Millisecond + t.Cleanup(func() { cancelEscalationTimeout = prevEsc }) + + mock := newMockAgentServer(t) + t.Cleanup(mock.Close) + client := createTestClient(t, mock.server.URL) + t.Cleanup(client.Close) + + mgr := newTestManager(t) + promptFinished := make(chan struct{}) + exec := &AgentExecution{ + ID: "exec-disc", + SessionID: "session-disc", + Status: v1.AgentStatusRunning, + agentctl: client, + promptFinished: promptFinished, + promptDoneCh: make(chan PromptCompletionSignal, 1), + } + require.NoError(t, mgr.executionStore.Add(exec)) + + // The agent stream was never connected, so Cancel returns + // ErrAgentStreamNotConnected — the teardown-race path. + err := mgr.CancelAgent(context.Background(), "exec-disc") + + require.True(t, errors.Is(err, ErrCancelEscalated), + "a disconnected stream must escalate locally, got %v", err) + require.Equal(t, v1.AgentStatusReady, exec.Status, + "escalation must leave the execution ready so the workflow can advance") +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch_metadata_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_metadata_test.go new file mode 100644 index 0000000000..81c7b81fc8 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_metadata_test.go @@ -0,0 +1,421 @@ +package lifecycle + +import ( + "context" + "path/filepath" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/agent/executor" + "github.com/kandev/kandev/internal/task/models" +) + +func validTestRemoteContribution(number int, sourcePath string) models.RemoteContribution { + return models.RemoteContribution{ + Version: models.RemoteContributionVersion, + Provider: models.RemoteContributionProviderGitHub, + Kind: models.RemoteContributionKindPullRequest, + CanonicalURL: "https://github.com/acme/widget/pull/" + strconv.Itoa(number), + Number: number, + State: models.RemoteContributionStateOpen, + BaseBranch: "main", + HeadBranch: "feature/remote", + HeadSHA: strings.Repeat("a", 40), + SourceRepository: models.RemoteContributionRepository{ + Host: "github.com", Path: sourcePath, RemoteURL: "https://github.com/" + sourcePath + ".git", + }, + CollaborationAllowed: true, + } +} + +// TestBuildLaunchMetadataExecutorConfigWinsForTrustedKeys pins the security +// boundary: connection-routing keys always come from the configured executor +// record, while ordinary per-task keys keep the caller's value. +func TestBuildLaunchMetadataExecutorConfigWinsForTrustedKeys(t *testing.T) { + req := &LaunchRequest{ + Metadata: map[string]interface{}{ + MetadataKeySSHHost: "attacker.example.com", + MetadataKeySSHHostFingerprint: "SHA256:forged", + MetadataKeySSHUser: "root", + MetadataKeyCleanupScript: "task-cleanup.sh", + }, + ExecutorConfig: map[string]string{ + MetadataKeySSHHost: "trusted.example.com", + MetadataKeySSHHostFingerprint: "SHA256:pinned", + MetadataKeySSHUser: "deploy", + MetadataKeyCleanupScript: "executor-cleanup.sh", + MetadataKeySSHWorkdirRoot: "/srv/kandev", + }, + } + + metadata := buildLaunchMetadata(req, "", "", "") + + require.Equal(t, "trusted.example.com", metadata[MetadataKeySSHHost], + "a task metadata payload must never be able to pivot the SSH host") + require.Equal(t, "SHA256:pinned", metadata[MetadataKeySSHHostFingerprint], + "the pinned host key must not be overridable by request metadata") + require.Equal(t, "deploy", metadata[MetadataKeySSHUser]) + require.Equal(t, "task-cleanup.sh", metadata[MetadataKeyCleanupScript], + "untrusted keys keep the caller's value when present") + require.Equal(t, "/srv/kandev", metadata[MetadataKeySSHWorkdirRoot], + "untrusted executor-config keys still fill in when the caller supplied nothing") +} + +func TestIsTrustedExecutorConfigKey(t *testing.T) { + for _, key := range []string{ + MetadataKeySSHHost, MetadataKeySSHHostAlias, MetadataKeySSHPort, MetadataKeySSHUser, + MetadataKeySSHHostFingerprint, MetadataKeySSHIdentitySource, MetadataKeySSHIdentityFile, + MetadataKeySSHProxyJump, + } { + require.True(t, isTrustedExecutorConfigKey(key), "%s steers the connection and must be trusted-only", key) + } + for _, key := range []string{ + MetadataKeySSHWorkdirRoot, MetadataKeySSHShell, MetadataKeyCleanupScript, MetadataKeySetupScript, + } { + require.False(t, isTrustedExecutorConfigKey(key), "%s is per-task config, not connection routing", key) + } +} + +func TestBuildLaunchMetadataProjectsWorktreeAndRepoFields(t *testing.T) { + req := &LaunchRequest{ + RepositoryPath: "/repos/widget", + SetupScript: "make setup", + BaseBranch: "main", + } + + metadata := buildLaunchMetadata(req, "/repos/widget/.git", "wt-1", "kandev/feature") + + require.Equal(t, "/repos/widget/.git", metadata[MetadataKeyMainRepoGitDir]) + require.Equal(t, "wt-1", metadata[MetadataKeyWorktreeID]) + require.Equal(t, "kandev/feature", metadata[MetadataKeyWorktreeBranch]) + require.Equal(t, "/repos/widget", metadata[MetadataKeyRepositoryPath]) + require.Equal(t, "make setup", metadata[MetadataKeySetupScript]) + require.Equal(t, "main", metadata[MetadataKeyBaseBranch]) + require.Equal(t, map[string]string{"": "main"}, metadata[MetadataKeyBaseBranches], + "the single-repo base branch is recorded under the empty key for single-repo trackers") +} + +func TestBuildLaunchMetadataOmitsEmptyOptionalKeys(t *testing.T) { + metadata := buildLaunchMetadata(&LaunchRequest{}, "", "", "") + + for _, key := range []string{ + MetadataKeyMainRepoGitDir, MetadataKeyWorktreeID, MetadataKeyWorktreeBranch, + MetadataKeyRepositoryPath, MetadataKeySetupScript, MetadataKeyBaseBranch, MetadataKeyBaseBranches, + } { + require.NotContains(t, metadata, key, "%s must be absent when the request carries no value", key) + } +} + +func TestCollectRemoteContributionsSingleRepoUsesRootKey(t *testing.T) { + binding := validTestRemoteContribution(7, "contributor/widget") + req := &LaunchRequest{RemoteContribution: &binding} + + bindings, err := collectRemoteContributions(req) + + require.NoError(t, err) + require.Len(t, bindings, 1) + require.Contains(t, bindings, "", "a repo-less launch binds the workspace root") + require.Equal(t, binding.CanonicalURL, bindings[""].CanonicalURL) +} + +func TestCollectRemoteContributionsMultiRepoKeysSiblings(t *testing.T) { + first := validTestRemoteContribution(7, "contributor/widget") + second := validTestRemoteContribution(9, "contributor/gadget") + req := &LaunchRequest{ + Repositories: []RepoLaunchSpec{ + {RepositoryID: "r1", RepoName: "widget", RemoteContribution: &first}, + {RepositoryID: "r2", RepoName: "gadget", BranchSlug: "hotfix", RemoteContribution: &second}, + }, + } + + bindings, err := collectRemoteContributions(req) + + require.NoError(t, err) + require.Len(t, bindings, 2) + require.Equal(t, first.CanonicalURL, bindings[""].CanonicalURL, + "the first repository owns the workspace root") + require.Equal(t, second.CanonicalURL, bindings["gadget-hotfix"].CanonicalURL, + "siblings use the same deterministic key as base-branch projection") +} + +func TestCollectRemoteContributionsRejectsConflictingBindings(t *testing.T) { + first := validTestRemoteContribution(7, "contributor/widget") + second := validTestRemoteContribution(9, "contributor/widget") + req := &LaunchRequest{ + Repositories: []RepoLaunchSpec{ + {RepositoryID: "r1", RepoName: "widget", RemoteContribution: &first}, + // Same key (index > 0 but identical name/slug) with a different URL. + {RepositoryID: "r2", RepoName: "widget", RemoteContribution: &second}, + {RepositoryID: "r3", RepoName: "widget", RemoteContribution: &second}, + }, + } + + _, err := collectRemoteContributions(req) + require.NoError(t, err, "identical sibling URLs are not a conflict") + + third := validTestRemoteContribution(11, "contributor/widget") + req.Repositories[2].RemoteContribution = &third + _, err = collectRemoteContributions(req) + require.ErrorContains(t, err, "multiple remote contributions target workspace repository") +} + +func TestCollectRemoteContributionsPropagatesValidationFailure(t *testing.T) { + invalid := validTestRemoteContribution(7, "contributor/widget") + invalid.State = "closed" + + t.Run("single repo", func(t *testing.T) { + _, err := collectRemoteContributions(&LaunchRequest{RemoteContribution: &invalid}) + require.ErrorContains(t, err, "validate remote contribution") + }) + + t.Run("multi repo names the repository", func(t *testing.T) { + _, err := collectRemoteContributions(&LaunchRequest{ + Repositories: []RepoLaunchSpec{{RepositoryID: "r1", RepoName: "widget", RemoteContribution: &invalid}}, + }) + require.ErrorContains(t, err, `validate remote contribution for repository "widget"`) + }) +} + +func TestCollectRemoteContributionsEmptyInputs(t *testing.T) { + bindings, err := collectRemoteContributions(nil) + require.NoError(t, err) + require.Nil(t, bindings) + + bindings, err = collectRemoteContributions(&LaunchRequest{}) + require.NoError(t, err) + require.Nil(t, bindings) + + bindings, err = collectRemoteContributions(&LaunchRequest{ + Repositories: []RepoLaunchSpec{{RepositoryID: "r1", RepoName: "widget"}}, + }) + require.NoError(t, err) + require.Nil(t, bindings, "repositories without bindings produce no map at all") +} + +// TestBuildEnvPrepareRequestForwardsMultiRepoSpecs pins the per-repo +// RepoSetupScript inheritance: an entry without its own script inherits the +// request-level one, and an entry with one keeps it. +func TestBuildEnvPrepareRequestForwardsMultiRepoSpecs(t *testing.T) { + req := &LaunchRequest{ + TaskID: "task-1", + WorkspaceID: "ws-1", + SessionID: "session-1", + TaskTitle: "Fix the widget", + Metadata: map[string]interface{}{ + MetadataKeyRepoSetupScript: "shared-setup.sh", + MetadataKeyWorktreeBranch: "kandev/fix-widget", + }, + Repositories: []RepoLaunchSpec{ + {RepositoryID: "r1", RepoName: "widget", BaseBranch: "main"}, + {RepositoryID: "r2", RepoName: "gadget", RepoSetupScript: "gadget-setup.sh", PRNumber: 12}, + }, + } + + prep := buildEnvPrepareRequest(req, "/work/task-1", executor.NameStandalone) + + require.Equal(t, "task-1", prep.TaskID) + require.Equal(t, "ws-1", prep.WorkspaceID) + require.Equal(t, "Fix the widget", prep.TaskTitle) + require.Equal(t, executor.NameStandalone, prep.ExecutorType) + require.Equal(t, "/work/task-1", prep.WorkspacePath) + require.Equal(t, "shared-setup.sh", prep.RepoSetupScript) + require.Equal(t, "kandev/fix-widget", prep.WorktreeBranch, + "the worktree branch is read out of launch metadata, not a request field") + + require.Len(t, prep.Repositories, 2) + require.Equal(t, "shared-setup.sh", prep.Repositories[0].RepoSetupScript, + "a repo without its own setup script inherits the request-level one") + require.Equal(t, "main", prep.Repositories[0].BaseBranch) + require.Equal(t, "gadget-setup.sh", prep.Repositories[1].RepoSetupScript, + "a repo with its own setup script keeps it") + require.Equal(t, 12, prep.Repositories[1].PRNumber) +} + +func TestBuildEnvPrepareRequestSingleRepoLeavesRepositoriesEmpty(t *testing.T) { + req := &LaunchRequest{ + TaskID: "task-1", + RepositoryPath: "/repos/widget", + UseWorktree: true, + BaseBranch: "main", + Env: map[string]string{"FOO": "bar"}, + } + + prep := buildEnvPrepareRequest(req, "/work/task-1", executor.NameDocker) + + require.Empty(t, prep.Repositories, "a legacy single-repo launch carries no per-repo spec list") + require.Equal(t, "/repos/widget", prep.RepositoryPath) + require.True(t, prep.UseWorktree) + require.Equal(t, map[string]string{"FOO": "bar"}, prep.Env) + require.Empty(t, prep.RepoSetupScript) +} + +func TestExecutionProfileIDPrefersExplicitExecutionProfile(t *testing.T) { + require.Empty(t, executionProfileID(nil)) + require.Equal(t, "agent-profile", executionProfileID(&LaunchRequest{AgentProfileID: "agent-profile"})) + require.Equal(t, "exec-profile", executionProfileID(&LaunchRequest{ + AgentProfileID: "agent-profile", ExecutionProfileID: "exec-profile", + }), "a concrete execution profile wins over the Office identity") +} + +func TestApplyRouteOverrideToProfileReplacesOnlyExplicitValues(t *testing.T) { + profile := &AgentProfileInfo{AgentName: "claude-acp", Model: "sonnet", Mode: "default"} + + applyRouteOverrideToProfile(profile, &LaunchRequest{RouteOverride: &RouteOverride{Model: "opus"}}) + + require.Equal(t, "claude-acp", profile.AgentName, "an empty ProviderID must not clear the profile agent") + require.Equal(t, "opus", profile.Model) + require.Empty(t, profile.Mode, "routing owns the mode, so it is applied unconditionally") +} + +func TestApplyRouteOverrideToProfileNilInputs(t *testing.T) { + profile := &AgentProfileInfo{AgentName: "claude-acp", Mode: "plan"} + + applyRouteOverrideToProfile(nil, &LaunchRequest{RouteOverride: &RouteOverride{ProviderID: "codex"}}) + applyRouteOverrideToProfile(profile, nil) + applyRouteOverrideToProfile(profile, &LaunchRequest{}) + + require.Equal(t, "claude-acp", profile.AgentName) + require.Equal(t, "plan", profile.Mode, "no override means no mutation at all") +} + +func TestRuntimeEnvFromMetadataAcceptsBothShapes(t *testing.T) { + require.Empty(t, runtimeEnvFromMetadata(nil)) + require.Empty(t, runtimeEnvFromMetadata(map[string]interface{}{})) + + typed := runtimeEnvFromMetadata(map[string]interface{}{ + "runtime_env": map[string]string{"ANTHROPIC_API_KEY": "sk-live", "PATH": "/usr/bin"}, + }) + require.Equal(t, map[string]string{"ANTHROPIC_API_KEY": "sk-live", "PATH": "/usr/bin"}, typed, + "the in-memory shape must reach the agent subprocess verbatim") + + decoded := runtimeEnvFromMetadata(map[string]interface{}{ + "runtime_env": map[string]interface{}{"ANTHROPIC_API_KEY": "sk-live", "PORT": 8080}, + }) + require.Equal(t, map[string]string{"ANTHROPIC_API_KEY": "sk-live"}, decoded, + "a JSON round-tripped map keeps string values and drops non-strings") +} + +func TestGetAttachmentsFromMetadata(t *testing.T) { + exec := &AgentExecution{ID: "exec-1"} + require.Nil(t, getAttachmentsFromMetadata(exec)) + + exec.setMetadataValue("attachments", "not-a-slice") + require.Nil(t, getAttachmentsFromMetadata(exec), "a wrong-typed value must not panic or leak through") + + want := []MessageAttachment{{AttachmentID: "att-1", Type: "image", MimeType: "image/png"}} + exec.setMetadataValue("attachments", want) + require.Equal(t, want, getAttachmentsFromMetadata(exec)) +} + +// TestSetExecutionEnvStoresDefensiveCopy pins the Runtime().Env contract at the +// execution boundary: what SetExecutionEnv stores is exactly what +// configureAndStartAgent later hands to the agentctl child process, and the +// caller's map cannot mutate it after the fact. +func TestSetExecutionEnvStoresDefensiveCopy(t *testing.T) { + mgr := newTestManager(t) + exec := &AgentExecution{ID: "exec-1", SessionID: "session-1"} + require.NoError(t, mgr.executionStore.Add(exec)) + + env := map[string]string{"ANTHROPIC_BASE_URL": "https://proxy.internal", "KANDEV_TASK_ID": "task-1"} + require.NoError(t, mgr.SetExecutionEnv(context.Background(), "exec-1", env)) + env["ANTHROPIC_BASE_URL"] = "https://attacker.example" + + require.Equal(t, + map[string]string{"ANTHROPIC_BASE_URL": "https://proxy.internal", "KANDEV_TASK_ID": "task-1"}, + runtimeEnvFromMetadata(exec.MetadataSnapshot()), + "the stored runtime env must be a copy taken at call time") +} + +func TestSetExecutionEnvAndDescriptionRejectUnknownExecution(t *testing.T) { + mgr := newTestManager(t) + + require.ErrorContains(t, mgr.SetExecutionEnv(context.Background(), "missing", nil), `execution "missing" not found`) + require.ErrorContains(t, mgr.SetExecutionDescription(context.Background(), "missing", "x"), + `execution "missing" not found`) +} + +func TestSetExecutionDescriptionFeedsPassthroughPromptInjection(t *testing.T) { + mgr := newTestManager(t) + exec := &AgentExecution{ID: "exec-1", SessionID: "session-1"} + require.NoError(t, mgr.executionStore.Add(exec)) + + require.NoError(t, mgr.SetExecutionDescription(context.Background(), "exec-1", "Ship the release")) + + require.Equal(t, "Ship the release", getTaskDescriptionFromMetadata(exec), + "the description set on a workspace-only execution is what the prompt path reads back") +} + +func TestPrepareProgressRecorderMergeFillsOnlyBlankSlots(t *testing.T) { + recorder := newPrepareProgressRecorder(nil) + recorder.Callback(0)(PrepareStep{Name: "Validate Docker"}, 0, 2) + + recorder.Merge([]PrepareStep{ + {Name: "Overwrite attempt"}, + {Name: "Create worktree"}, + {Name: "Run setup"}, + }) + + steps := recorder.Steps() + require.Equal(t, 3, recorder.Len()) + require.Equal(t, "Validate Docker", steps[0].Name, "a recorded step must not be overwritten by a merge") + require.Equal(t, "Create worktree", steps[1].Name, "a blank slot is filled from the merged list") + require.Equal(t, "Run setup", steps[2].Name, "steps past the recorded length are appended") +} + +func TestPrepareProgressRecorderCallbackOffsetsIndexes(t *testing.T) { + var gotIndex, gotTotal int + recorder := newPrepareProgressRecorder(func(_ PrepareStep, index, total int) { + gotIndex, gotTotal = index, total + }) + + recorder.Callback(2)(PrepareStep{Name: "Clone repository"}, 1, 3) + + require.Equal(t, 3, gotIndex, "the callback index is offset by the preceding phase's step count") + require.Equal(t, 5, gotTotal) + require.Equal(t, "Clone repository", recorder.Steps()[3].Name) +} + +func TestScratchWorkspacePathRejectsTraversalIDs(t *testing.T) { + mgr := newTestManager(t) + mgr.dataDir = t.TempDir() + + require.Equal(t, filepath.Join(mgr.dataDir, "tasks", "ws-1", "task-1"), + mgr.scratchWorkspacePath(&LaunchRequest{TaskID: "task-1", WorkspaceID: "ws-1"})) + require.Equal(t, filepath.Join(mgr.dataDir, "quick-chat", "session-1"), + mgr.scratchWorkspacePath(&LaunchRequest{SessionID: "session-1", IsEphemeral: true})) + + require.Empty(t, mgr.scratchWorkspacePath(&LaunchRequest{SessionID: "a/../b", IsEphemeral: true})) + require.Empty(t, mgr.scratchWorkspacePath(&LaunchRequest{TaskID: "task-1"}), + "a non-ephemeral scratch workspace needs both a task and a workspace ID") + require.Empty(t, mgr.scratchWorkspacePath(&LaunchRequest{TaskID: "..", WorkspaceID: "ws-1"})) + require.Empty(t, mgr.scratchWorkspacePath(&LaunchRequest{TaskID: "task-1", WorkspaceID: "a/b"})) +} + +func TestResolveWorkspaceFromProvider(t *testing.T) { + ctx := context.Background() + req := &LaunchRequest{TaskID: "task-1", SessionID: "session-1"} + + t.Run("no provider", func(t *testing.T) { + mgr := newTestManager(t) + require.Empty(t, mgr.resolveWorkspaceFromProvider(ctx, req)) + }) + + t.Run("provider returns path", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {SessionID: "session-1", WorkspacePath: "/work/task-1"}, + }} + require.Equal(t, "/work/task-1", mgr.resolveWorkspaceFromProvider(ctx, req)) + }) + + t.Run("provider returns empty path", func(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {SessionID: "session-1"}, + }} + require.Empty(t, mgr.resolveWorkspaceFromProvider(ctx, req)) + }) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_launch_workspace_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_workspace_test.go new file mode 100644 index 0000000000..6e10fc7b81 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_launch_workspace_test.go @@ -0,0 +1,142 @@ +package lifecycle + +import ( + "context" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/task/models" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// TestResolveResumeWorktreePathUsesProviderPath pins the worktree-resume +// branch: the workspace comes from the persisted session, and the main repo's +// git dir is derived from the request so the executor can find the parent repo. +func TestResolveResumeWorktreePathUsesProviderPath(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {SessionID: "session-1", WorkspacePath: "/work/task-1/backend"}, + }} + req := &LaunchRequest{ + TaskID: "task-1", SessionID: "session-1", + RepositoryPath: "/repos/backend", WorktreeID: "wt-1", + } + + ws, gitDir, worktreeID, branch := mgr.resolveResumeWorktreePath(context.Background(), req) + + require.Equal(t, "/work/task-1/backend", ws) + require.Equal(t, filepath.Join("/repos/backend", ".git"), gitDir) + require.Equal(t, "wt-1", worktreeID) + require.Empty(t, branch, "a resume reuses the existing branch rather than naming a new one") +} + +func TestResolveResumeWorktreePathWithoutRepositoryPath(t *testing.T) { + mgr := newTestManager(t) + mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{infos: map[string]*WorkspaceInfo{ + "session-1": {SessionID: "session-1", WorkspacePath: "/work/task-1"}, + }} + + ws, gitDir, _, _ := mgr.resolveResumeWorktreePath(context.Background(), + &LaunchRequest{TaskID: "task-1", SessionID: "session-1"}) + + require.Equal(t, "/work/task-1", ws) + require.Empty(t, gitDir) +} + +func TestResolveResumeWorktreePathUnresolvableWorkspace(t *testing.T) { + mgr := newTestManager(t) + + ws, gitDir, worktreeID, branch := mgr.resolveResumeWorktreePath(context.Background(), + &LaunchRequest{TaskID: "task-1", SessionID: "session-1", WorktreeID: "wt-1"}) + + require.Empty(t, ws) + require.Empty(t, gitDir) + require.Empty(t, worktreeID, "an unresolvable workspace must not leak a worktree ID") + require.Empty(t, branch) +} + +// TestLaunchResolveWorkspacePathSkipsCloneBasedExecutors pins the containment +// rule: a host checkout must never be handed to a clone-based executor. +func TestLaunchResolveWorkspacePathSkipsCloneBasedExecutors(t *testing.T) { + log := newTestRegistryLogger() + registry := NewExecutorRegistry(log) + registry.Register(&capabilityExecutor{ + MockExecutor: MockExecutor{name: "sprites"}, + requiresClone: true, + }) + mgr := NewManager(newTestRegistry(), &MockEventBus{}, registry, nil, nil, nil, + ExecutorFallbackWarn, "", log) + cleanupManagerStopCh(t, mgr) + + ws, gitDir, worktreeID, branch := mgr.launchResolveWorkspacePath(context.Background(), &LaunchRequest{ + ExecutorType: string(models.ExecutorTypeSprites), + WorkspacePath: "/host/checkout", + RepositoryPath: "/repos/backend", + SessionID: "session-1", + }) + + require.Empty(t, ws, "a clone-based executor owns its own filesystem") + require.Empty(t, gitDir) + require.Empty(t, worktreeID) + require.Empty(t, branch) +} + +func TestLaunchResolveWorkspacePathFallsBackToRepositoryPath(t *testing.T) { + mgr := newTestManager(t) + + ws, _, _, _ := mgr.launchResolveWorkspacePath(context.Background(), &LaunchRequest{ + RepositoryPath: "/repos/backend", + SessionID: "session-1", + }) + + require.Equal(t, "/repos/backend", ws, + "a launch with no explicit workspace works directly in the repository") +} + +// TestLaunchResolveWorkspacePathDefersWorktreeCreation pins that a fresh +// worktree launch returns nothing — the WorktreePreparer owns creation. +func TestLaunchResolveWorkspacePathDefersWorktreeCreation(t *testing.T) { + mgr := newTestManager(t) + + ws, _, _, _ := mgr.launchResolveWorkspacePath(context.Background(), &LaunchRequest{ + UseWorktree: true, + RepositoryPath: "/repos/backend", + SessionID: "session-1", + }) + + require.Empty(t, ws, "a first worktree launch has no ACP session and defers to the preparer") +} + +func TestTruncateID(t *testing.T) { + require.Equal(t, "abc", truncateID("abc", 8)) + require.Equal(t, "abcdefgh", truncateID("abcdefgh", 8)) + require.Equal(t, "abcdefgh", truncateID("abcdefghijkl", 8)) +} + +func TestExecutorRunningStatusFromExecution(t *testing.T) { + require.Equal(t, models.ExecutorRunningStatusStarting, executorRunningStatusFromExecution(nil)) + + for status, want := range map[v1.AgentStatus]string{ + v1.AgentStatusRunning: models.ExecutorRunningStatusRunning, + v1.AgentStatusReady: models.ExecutorRunningStatusReady, + v1.AgentStatusFailed: models.ExecutorRunningStatusFailed, + v1.AgentStatusStopped: models.ExecutorRunningStatusStopped, + v1.AgentStatusCompleted: models.ExecutorRunningStatusComplete, + v1.AgentStatusStarting: models.ExecutorRunningStatusStarting, + "": models.ExecutorRunningStatusStarting, + } { + require.Equal(t, want, executorRunningStatusFromExecution(&AgentExecution{Status: status}), + "status %q", status) + } +} + +func TestIsTerminalExecutorRunningStatus(t *testing.T) { + require.True(t, isTerminalExecutorRunningStatus(models.ExecutorRunningStatusFailed)) + require.True(t, isTerminalExecutorRunningStatus(models.ExecutorRunningStatusStopped)) + require.True(t, isTerminalExecutorRunningStatus(models.ExecutorRunningStatusComplete)) + require.False(t, isTerminalExecutorRunningStatus(models.ExecutorRunningStatusRunning), + "a live row must keep its local liveness handle") + require.False(t, isTerminalExecutorRunningStatus(models.ExecutorRunningStatusStarting)) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_launch_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_launch_test.go new file mode 100644 index 0000000000..8ecacc0eb2 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_launch_test.go @@ -0,0 +1,422 @@ +package lifecycle + +import ( + "bytes" + "context" + "path/filepath" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/agent/executor" + "github.com/kandev/kandev/internal/agentctl/server/process" + agentctltypes "github.com/kandev/kandev/internal/agentctl/types" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// runnerBackend is a minimal ExecutorBackend that exposes a real +// InteractiveRunner. The runner is the designed injection seam for the +// passthrough launch paths; a runner with no started processes lets every +// failure and guard branch run without spawning a PTY. +type runnerBackend struct { + MockExecutor + runner *process.InteractiveRunner +} + +func (b *runnerBackend) GetInteractiveRunner() *process.InteractiveRunner { return b.runner } + +// terminalRecorder stands in for the terminal WebSocket writer so the banner +// text the manager emits can be asserted verbatim. +type terminalRecorder struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (w *terminalRecorder) Write(p []byte) (int, error) { + w.mu.Lock() + defer w.mu.Unlock() + return w.buf.Write(p) +} + +func (w *terminalRecorder) Close() error { return nil } + +func (w *terminalRecorder) text() string { + w.mu.Lock() + defer w.mu.Unlock() + return w.buf.String() +} + +// newPassthroughRunnerManager returns a manager whose standalone backend +// exposes a real InteractiveRunner, plus a passthrough-capable execution whose +// workspace points at a directory that exists. +func newPassthroughRunnerManager(t *testing.T) (*Manager, *process.InteractiveRunner, *AgentExecution) { + t.Helper() + mgr, execution, _ := newClaudePassthroughMCPTestManager(t) + + log := newTestRegistryLogger() + runner := process.NewInteractiveRunner(nil, log, 1024) + registry := NewExecutorRegistry(log) + registry.Register(&runnerBackend{ + MockExecutor: MockExecutor{name: executor.NameStandalone}, + runner: runner, + }) + mgr.executorRegistry = registry + require.NoError(t, mgr.executionStore.Add(execution)) + return mgr, runner, execution +} + +// missingWorkspace returns a path that does not exist. Pointing an execution at +// it makes the PTY spawn fail deterministically (the child cannot chdir), +// regardless of which agent binaries happen to be installed on the host, so no +// real agent process is ever started by these tests. +func missingWorkspace(t *testing.T) string { + t.Helper() + return filepath.Join(t.TempDir(), "does-not-exist") +} + +func TestGetInteractiveRunnerResolvesStandaloneBackend(t *testing.T) { + mgr, runner, _ := newPassthroughRunnerManager(t) + + require.Same(t, runner, mgr.GetInteractiveRunner()) +} + +// TestPassthroughRunnerErrorsSurfaceFromRunner pins that a live runner which +// does not know the recorded process ID produces the runner's own error rather +// than a generic "runner unavailable". +func TestPassthroughRunnerErrorsSurfaceFromRunner(t *testing.T) { + ctx := context.Background() + mgr, _, execution := newPassthroughRunnerManager(t) + execution.PassthroughProcessID = "pty-gone" + + require.ErrorContains(t, mgr.WritePassthroughStdin(ctx, execution.SessionID, "\x03"), + "process not found: pty-gone") + require.ErrorContains(t, mgr.ResizePassthroughPTY(ctx, execution.SessionID, 80, 24), + "no process found for session") + + _, err := mgr.GetPassthroughBuffer(ctx, execution.SessionID) + require.ErrorContains(t, err, "passthrough process not found") +} + +func TestResolvePassthroughConfigReturnsAgentConfig(t *testing.T) { + mgr, _, execution := newPassthroughRunnerManager(t) + + pt, err := mgr.ResolvePassthroughConfig(context.Background(), execution.SessionID) + + require.NoError(t, err) + require.NotEmpty(t, pt.SubmitSequence, + "the resolved config must carry the submit sequence callers write to PTY stdin") +} + +func TestResolvePassthroughConfigErrors(t *testing.T) { + mgr, _, execution := newPassthroughRunnerManager(t) + + _, err := mgr.ResolvePassthroughConfig(context.Background(), "session-absent") + require.ErrorContains(t, err, `no execution for session "session-absent"`) + + execution.AgentProfileID = "" + _, err = mgr.ResolvePassthroughConfig(context.Background(), execution.SessionID) + require.ErrorContains(t, err, "failed to get agent config") +} + +// TestStartPassthroughSessionSurfacesLaunchFailure pins that a PTY that cannot +// be spawned is recorded on the execution as an error rather than leaving the +// session looking started. +func TestStartPassthroughSessionSurfacesLaunchFailure(t *testing.T) { + mgr, _, execution := newPassthroughRunnerManager(t) + execution.WorkspacePath = missingWorkspace(t) + + err := mgr.startPassthroughSession(context.Background(), execution, nil) + + require.ErrorContains(t, err, "failed to start passthrough session") + require.Empty(t, execution.PassthroughProcessID, + "a failed launch must not record a process ID the WS handler would then treat as live") + require.Equal(t, v1.AgentStatusFailed, execution.Status) + require.NotEmpty(t, execution.ErrorMessage) +} + +func TestResumePassthroughSessionSurfacesLaunchFailure(t *testing.T) { + mgr, _, execution := newPassthroughRunnerManager(t) + execution.WorkspacePath = missingWorkspace(t) + + err := mgr.ResumePassthroughSession(context.Background(), execution.SessionID) + + require.ErrorContains(t, err, "failed to start passthrough session") + require.Empty(t, execution.PassthroughProcessID) +} + +func TestResumePassthroughSessionUnknownSession(t *testing.T) { + mgr, _, _ := newPassthroughRunnerManager(t) + + err := mgr.ResumePassthroughSession(context.Background(), "session-absent") + + require.ErrorIs(t, err, ErrNoExecutionForSession) +} + +// TestRestartPassthroughProcessClearsProcessIDBeforeStopping pins the ordering +// documented on restartPassthroughProcess: the ID is cleared first so the +// deliberate kill does not trip handlePassthroughStatus's auto-restart. +func TestRestartPassthroughProcessClearsProcessIDBeforeStopping(t *testing.T) { + mgr, _, execution := newPassthroughRunnerManager(t) + execution.WorkspacePath = missingWorkspace(t) + execution.PassthroughProcessID = "pty-old" + execution.PassthroughStartedAt = time.Now() + + err := mgr.restartPassthroughProcess(context.Background(), execution) + + require.ErrorContains(t, err, "failed to restart passthrough session") + require.Empty(t, execution.PassthroughProcessID, + "the old process ID is cleared so the deliberate kill is not auto-restarted") + require.True(t, execution.PassthroughStartedAt.IsZero()) + require.Equal(t, v1.AgentStatusFailed, execution.Status) +} + +func TestRestartPassthroughProcessWithoutRunner(t *testing.T) { + mgr, execution, _ := newClaudePassthroughMCPTestManager(t) + mgr.executorRegistry = nil + require.NoError(t, mgr.executionStore.Add(execution)) + execution.PassthroughProcessID = "pty-old" + + err := mgr.restartPassthroughProcess(context.Background(), execution) + + require.ErrorContains(t, err, "interactive runner not available") + require.Equal(t, "pty-old", execution.PassthroughProcessID, + "without a runner nothing was stopped, so the process ID must be left intact") +} + +// TestNotifyFastFailExitWritesBannerToTerminal pins the exact user-facing copy +// and the values interpolated into it: without them the user has no way to tell +// a bad CLI flag from a crash loop. +func TestNotifyFastFailExitWritesBannerToTerminal(t *testing.T) { + mgr, runner, _ := newPassthroughRunnerManager(t) + recorder := &terminalRecorder{} + runner.TrackSessionWebSocket("session-1", recorder) + + mgr.notifyFastFailExit(runner, "session-1", 350*time.Millisecond, 127, 2*time.Second) + + banner := recorder.text() + require.Contains(t, banner, "Agent exited (code 127) within 2s") + require.Contains(t, banner, "bad CLI flag, missing binary, or auth failure") +} + +func TestNotifyFastFailExitToleratesMissingTerminal(t *testing.T) { + mgr, runner, _ := newPassthroughRunnerManager(t) + + // No session WebSocket registered: the write fails and must be swallowed. + mgr.notifyFastFailExit(runner, "session-gone", time.Second, 1, 2*time.Second) +} + +// TestNotifyFallbackInfrastructureFailureNamesTheStage pins that a kandev-side +// failure gets its own banner naming the stage, rather than the fast-fail copy +// that blames the user's profile. +func TestNotifyFallbackInfrastructureFailureNamesTheStage(t *testing.T) { + mgr, runner, _ := newPassthroughRunnerManager(t) + recorder := &terminalRecorder{} + runner.TrackSessionWebSocket("session-1", recorder) + + mgr.notifyFallbackInfrastructureFailure(runner, "session-1", "start fresh process", + context.DeadlineExceeded) + + banner := recorder.text() + require.Contains(t, banner, "Resume fallback failed: could not start fresh process") + require.Contains(t, banner, context.DeadlineExceeded.Error()) + require.NotContains(t, banner, "bad CLI flag", + "an infrastructure failure must not blame the user's agent profile") +} + +// TestAttemptResumeFallbackAnnouncesAndRelaunchesFresh pins the recovery a user +// sees after `claude -c` fails on a stale conversation: a fresh-start banner +// followed by a relaunch attempt with no resume flag. +func TestAttemptResumeFallbackAnnouncesAndRelaunchesFresh(t *testing.T) { + mgr, runner, execution := newPassthroughRunnerManager(t) + execution.WorkspacePath = missingWorkspace(t) + recorder := &terminalRecorder{} + runner.TrackSessionWebSocket(execution.SessionID, recorder) + + mgr.attemptResumeFallback(execution, runner, execution.SessionID, 1, 400*time.Millisecond) + + banner := recorder.text() + require.Contains(t, banner, "No prior conversation to resume") + require.Contains(t, banner, "Resume fallback failed: could not start fresh process", + "a fallback that cannot start must say so rather than leaving a silent dead terminal") + require.False(t, execution.passthroughLaunchUsedResume) +} + +func TestAttemptResumeFallbackSkipsDuringShutdown(t *testing.T) { + mgr, runner, execution := newPassthroughRunnerManager(t) + mgr.shuttingDown.Store(true) + recorder := &terminalRecorder{} + runner.TrackSessionWebSocket(execution.SessionID, recorder) + + mgr.attemptResumeFallback(execution, runner, execution.SessionID, 1, time.Second) + + require.Empty(t, recorder.text(), "a clean shutdown must not print a recovery banner") +} + +func TestAttemptResumeFallbackSkipsWithoutTerminal(t *testing.T) { + mgr, runner, execution := newPassthroughRunnerManager(t) + + // No session WebSocket: nobody is watching, so no relaunch is attempted. + mgr.attemptResumeFallback(execution, runner, execution.SessionID, 1, time.Second) + + require.Empty(t, execution.PassthroughProcessID) +} + +// TestAttemptPassthroughRestartAnnouncesAndReportsFailure pins the restart +// banner pair the user sees when the agent exits and the relaunch also fails. +func TestAttemptPassthroughRestartAnnouncesAndReportsFailure(t *testing.T) { + mgr, runner, execution := newPassthroughRunnerManager(t) + execution.WorkspacePath = missingWorkspace(t) + recorder := &terminalRecorder{} + runner.TrackSessionWebSocket(execution.SessionID, recorder) + + mgr.attemptPassthroughRestart(execution, runner, execution.SessionID, 1, time.Millisecond) + + banner := recorder.text() + require.Contains(t, banner, "[Agent exited. Restarting...]") + require.Contains(t, banner, "[Restart failed:", + "a failed restart must be reported on the terminal, not only in backend logs") +} + +func TestAttemptPassthroughRestartAbortsDuringShutdown(t *testing.T) { + mgr, runner, execution := newPassthroughRunnerManager(t) + recorder := &terminalRecorder{} + runner.TrackSessionWebSocket(execution.SessionID, recorder) + mgr.shuttingDown.Store(true) + + mgr.attemptPassthroughRestart(execution, runner, execution.SessionID, 1, time.Millisecond) + + require.Contains(t, recorder.text(), "[Agent exited. Restarting...]") + require.NotContains(t, recorder.text(), "[Restart failed:", + "shutdown aborts before the relaunch, so no failure banner is emitted") +} + +func TestAttemptPassthroughRestartAbortsWhenTerminalDisconnects(t *testing.T) { + mgr, runner, execution := newPassthroughRunnerManager(t) + + // No session WebSocket at all: the announce write fails and the + // post-delay check aborts the restart. + mgr.attemptPassthroughRestart(execution, runner, execution.SessionID, 1, time.Millisecond) + + require.Empty(t, execution.PassthroughProcessID) +} + +// TestHandlePassthroughExitFastFailsResumeLaunch drives the exit handler end to +// end for the issue-#2330 shape: a resume launch that exits non-zero quickly +// must take the fallback path, not the plain restart path. +func TestHandlePassthroughExitFastFailsResumeLaunch(t *testing.T) { + mgr, runner, execution := newPassthroughRunnerManager(t) + execution.WorkspacePath = missingWorkspace(t) + recorder := &terminalRecorder{} + runner.TrackSessionWebSocket(execution.SessionID, recorder) + + startedAt := time.Now() + exitCode := 1 + mgr.handlePassthroughExit(execution, &agentctltypes.ProcessStatusUpdate{ + SessionID: execution.SessionID, + ProcessID: "pty-1", + Status: agentctltypes.ProcessStatusExited, + ExitCode: &exitCode, + Timestamp: startedAt.Add(200 * time.Millisecond), + }, startedAt, true) + + require.Contains(t, recorder.text(), "No prior conversation to resume", + "a fast-failed resume must retry once without the resume flag") + require.NotContains(t, recorder.text(), "[Agent exited. Restarting...]", + "the fast-fail short-circuit must skip the plain restart path") +} + +func TestHandlePassthroughExitFastFailsFreshLaunch(t *testing.T) { + mgr, runner, execution := newPassthroughRunnerManager(t) + recorder := &terminalRecorder{} + runner.TrackSessionWebSocket(execution.SessionID, recorder) + + startedAt := time.Now() + exitCode := 127 + mgr.handlePassthroughExit(execution, &agentctltypes.ProcessStatusUpdate{ + SessionID: execution.SessionID, + ProcessID: "pty-1", + ExitCode: &exitCode, + Timestamp: startedAt.Add(100 * time.Millisecond), + }, startedAt, false) + + require.Contains(t, recorder.text(), "Agent exited (code 127)") + require.NotContains(t, recorder.text(), "No prior conversation to resume", + "a launch that never used a resume flag has nothing to fall back from") +} + +// TestHandlePassthroughExitSkipsWithoutTerminal pins the no-subscriber guard. +// The observable is that none of the relaunch machinery ran: building a fresh +// command materializes an MCP config file and records it on the execution, so +// an empty file list proves the handler returned before that work. +func TestHandlePassthroughExitSkipsWithoutTerminal(t *testing.T) { + mgr, _, execution := newPassthroughRunnerManager(t) + + exitCode := 1 + mgr.handlePassthroughExit(execution, &agentctltypes.ProcessStatusUpdate{ + SessionID: execution.SessionID, + ProcessID: "pty-1", + ExitCode: &exitCode, + Timestamp: time.Now(), + }, time.Now(), true) + + require.Empty(t, getPassthroughMCPFiles(execution), + "with nobody watching the terminal, no relaunch command may be built") + require.Empty(t, execution.PassthroughProcessID) +} + +// TestHandlePassthroughExitUsesStatusTimestampNotWallClock pins that the +// measured uptime comes from the exit event, so the handler's own 100ms cleanup +// delay cannot push a genuine fast fail outside the window. +func TestHandlePassthroughExitUsesStatusTimestampNotWallClock(t *testing.T) { + mgr, runner, execution := newPassthroughRunnerManager(t) + recorder := &terminalRecorder{} + runner.TrackSessionWebSocket(execution.SessionID, recorder) + + startedAt := time.Now().Add(-time.Hour) + exitCode := 3 + mgr.handlePassthroughExit(execution, &agentctltypes.ProcessStatusUpdate{ + SessionID: execution.SessionID, + ProcessID: "pty-1", + ExitCode: &exitCode, + // The child really lived 50ms, an hour ago. + Timestamp: startedAt.Add(50 * time.Millisecond), + }, startedAt, false) + + require.True(t, strings.Contains(recorder.text(), "Agent exited (code 3)"), + "uptime must be measured from the exit event, not from time.Now()") +} + +// TestStopPassthroughProcessLeavesExecutionStateToItsCaller pins that a failed +// runner stop is swallowed AND that this helper never edits the execution: +// StopAgentWithReason still needs the process ID to build the ExecutorInstance +// it hands to the backend teardown immediately afterwards. +func TestStopPassthroughProcessLeavesExecutionStateToItsCaller(t *testing.T) { + mgr, _, execution := newPassthroughRunnerManager(t) + backend, err := mgr.executorRegistry.GetBackend(executor.NameStandalone) + require.NoError(t, err) + execution.PassthroughProcessID = "pty-gone" + + // The runner does not know this process; the failure is logged and + // teardown continues rather than aborting the stop. + mgr.stopPassthroughProcess(context.Background(), execution.ID, execution, backend) + + require.Equal(t, "pty-gone", execution.PassthroughProcessID, + "clearing the process ID is the caller's job, not this helper's") +} + +func TestStopPassthroughProcessSkipsNonPassthroughExecutions(t *testing.T) { + mgr, _, execution := newPassthroughRunnerManager(t) + backend, err := mgr.executorRegistry.GetBackend(executor.NameStandalone) + require.NoError(t, err) + + // No passthrough process ID: nothing to stop. + mgr.stopPassthroughProcess(context.Background(), execution.ID, execution, backend) + + // A backend without an interactive runner is also a no-op. + execution.PassthroughProcessID = "pty-1" + mgr.stopPassthroughProcess(context.Background(), execution.ID, execution, + &MockExecutor{name: executor.NameDocker}) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_mcpfiles_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_mcpfiles_test.go new file mode 100644 index 0000000000..7b2fc8aaa6 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_mcpfiles_test.go @@ -0,0 +1,285 @@ +package lifecycle + +import ( + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/agent/mcpconfig" +) + +func TestSafePassthroughMCPConfigName(t *testing.T) { + for _, tc := range []struct{ in, want string }{ + {in: "", want: "session"}, + {in: "abcXYZ019", want: "abcXYZ019"}, + {in: "sess-1_2.json", want: "sess-1_2.json"}, + // Dots are legal in a filename; it is the separator that is neutralised. + {in: "../../etc/passwd", want: ".._.._etc_passwd"}, + {in: "a b\tc", want: "a_b_c"}, + {in: "sess/../id", want: "sess_.._id"}, + } { + require.Equal(t, tc.want, safePassthroughMCPConfigName(tc.in), + "safePassthroughMCPConfigName(%q)", tc.in) + } +} + +func TestPassthroughMCPPathsFallBackToTempWhenNoDataDir(t *testing.T) { + mgr := newTestManager(t) + workspace := t.TempDir() + + paths := mgr.passthroughMCPPaths(&AgentExecution{ + ID: "exec-1", SessionID: "session-1", WorkspacePath: workspace, + }) + + require.Equal(t, filepath.Join(os.TempDir(), "kandev", "passthrough-mcp", "session-1.json"), + paths.TempConfigPath, "an unset data dir falls back to the OS temp dir") + require.Equal(t, workspace, paths.WorkspaceDir) +} + +func TestPassthroughMCPPathsUsesExecutionIDWhenSessionMissing(t *testing.T) { + mgr := newTestManager(t) + mgr.dataDir = t.TempDir() + + paths := mgr.passthroughMCPPaths(&AgentExecution{ID: "exec-1"}) + + require.Equal(t, filepath.Join(mgr.dataDir, "passthrough-mcp", "exec-1.json"), paths.TempConfigPath, + "a session-less execution names its config after the execution") +} + +func TestPassthroughMCPConfigPortPrefersLiveStandalonePort(t *testing.T) { + require.Zero(t, passthroughMCPConfigPort(nil)) + + exec := &AgentExecution{} + exec.setMetadataValue("standalone_port", 45678) + require.Equal(t, 45678, passthroughMCPConfigPort(exec), + "a recovered execution reads the port back out of metadata") + + exec.standalonePort = 45679 + require.Equal(t, 45679, passthroughMCPConfigPort(exec), + "the live port wins over the persisted metadata copy") +} + +// TestMaterializePassthroughFileOverwritesKandevOwnedTempFile pins the +// relaunch path: an existing kandev-owned config (no MergeKey) is overwritten +// and stays tracked for cleanup. +func TestMaterializePassthroughFileOverwritesKandevOwnedTempFile(t *testing.T) { + mgr := newTestManager(t) + dir := t.TempDir() + path := filepath.Join(dir, "config.json") + require.NoError(t, os.WriteFile(path, []byte(`{"stale":true}`), 0o600)) + + owned, err := mgr.materializePassthroughFile(&AgentExecution{ID: "exec-1"}, mcpconfig.PassthroughConfigFile{ + Path: path, + Content: []byte(`{"mcpServers":{"kandev":{}}}`), + }) + + require.NoError(t, err) + require.True(t, owned, "a kandev-owned temp config stays tracked so teardown removes it") + content, err := os.ReadFile(path) + require.NoError(t, err) + require.JSONEq(t, `{"mcpServers":{"kandev":{}}}`, string(content)) +} + +// TestMaterializePassthroughFileSkipsPathEscapingWorkspace pins the symlink +// containment guard: a config path that resolves outside the worktree is +// skipped, not written. +func TestMaterializePassthroughFileSkipsPathEscapingWorkspace(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink creation is not reliably available on Windows CI") + } + mgr := newTestManager(t) + root := t.TempDir() + workspace := filepath.Join(root, "workspace") + outside := filepath.Join(root, "outside") + require.NoError(t, os.MkdirAll(workspace, 0o700)) + require.NoError(t, os.MkdirAll(outside, 0o700)) + require.NoError(t, os.Symlink(outside, filepath.Join(workspace, ".cursor"))) + + target := filepath.Join(workspace, ".cursor", "mcp.json") + owned, err := mgr.materializePassthroughFile( + &AgentExecution{ID: "exec-1", WorkspacePath: workspace}, + mcpconfig.PassthroughConfigFile{Path: target, Content: []byte(`{"mcpServers":{}}`), MergeKey: "mcpServers"}, + ) + + require.NoError(t, err) + require.False(t, owned) + _, statErr := os.Stat(filepath.Join(outside, "mcp.json")) + require.True(t, os.IsNotExist(statErr), + "a symlinked parent escaping the worktree must not receive kandev's config") +} + +func TestMaterializePassthroughFileSurfacesUnreadableParent(t *testing.T) { + mgr := newTestManager(t) + dir := t.TempDir() + // A regular file where a directory is expected: Lstat on the child fails + // with ENOTDIR, which is neither "exists" nor "not exist". + blocker := filepath.Join(dir, "blocker") + require.NoError(t, os.WriteFile(blocker, []byte("x"), 0o600)) + + _, err := mgr.materializePassthroughFile(&AgentExecution{ID: "exec-1"}, mcpconfig.PassthroughConfigFile{ + Path: filepath.Join(blocker, "config.json"), + Content: []byte(`{}`), + }) + + require.Error(t, err, "an unexpected lstat failure must be surfaced, not silently skipped") +} + +func TestWritePassthroughMCPFilesSkipsEmptyPaths(t *testing.T) { + mgr := newTestManager(t) + execution := &AgentExecution{ID: "exec-1"} + + require.NoError(t, mgr.writePassthroughMCPFiles(execution, []mcpconfig.PassthroughConfigFile{ + {Path: "", Content: []byte(`{}`)}, + })) + + require.Empty(t, getPassthroughMCPFiles(execution), + "a strategy entry with no path is skipped and never tracked") +} + +// TestWriteFileNoFollowLeavesConcurrentlyCreatedFileAlone pins the O_EXCL +// contract: a file that appeared between the lstat and the create is left +// untouched and NOT tracked for cleanup. +func TestWriteFileNoFollowLeavesConcurrentlyCreatedFileAlone(t *testing.T) { + mgr := newTestManager(t) + path := filepath.Join(t.TempDir(), "config.json") + require.NoError(t, os.WriteFile(path, []byte("user content"), 0o600)) + + owned, err := mgr.writeFileNoFollow(path, []byte("kandev content")) + + require.NoError(t, err) + require.False(t, owned, "kandev does not own a file it did not create") + content, err := os.ReadFile(path) + require.NoError(t, err) + require.Equal(t, "user content", string(content), "the pre-existing file must not be clobbered") +} + +func TestWriteFileNoFollowSurfacesCreateFailure(t *testing.T) { + mgr := newTestManager(t) + dir := t.TempDir() + blocker := filepath.Join(dir, "blocker") + require.NoError(t, os.WriteFile(blocker, []byte("x"), 0o600)) + + owned, err := mgr.writeFileNoFollow(filepath.Join(blocker, "config.json"), []byte("{}")) + + require.ErrorContains(t, err, "create passthrough MCP config") + require.False(t, owned) +} + +func TestMergePassthroughConfigLeavesUnreadableFileUntouched(t *testing.T) { + mgr := newTestManager(t) + // A directory cannot be read as a file: os.ReadFile fails, and the merge + // must degrade to a warning rather than clobbering anything. + dir := t.TempDir() + + require.NoError(t, mgr.mergePassthroughConfig(mcpconfig.PassthroughConfigFile{ + Path: dir, + Content: []byte(`{"mcpServers":{"kandev":{}}}`), + MergeKey: "mcpServers", + })) +} + +func TestMergePassthroughConfigLeavesMalformedFileUntouched(t *testing.T) { + mgr := newTestManager(t) + path := filepath.Join(t.TempDir(), "mcp.json") + require.NoError(t, os.WriteFile(path, []byte("not json at all"), 0o600)) + + require.NoError(t, mgr.mergePassthroughConfig(mcpconfig.PassthroughConfigFile{ + Path: path, + Content: []byte(`{"mcpServers":{"kandev":{}}}`), + MergeKey: "mcpServers", + }), "a malformed user config is a warning, not a launch failure") + + content, err := os.ReadFile(path) + require.NoError(t, err) + require.Equal(t, "not json at all", string(content), + "a config kandev cannot parse must never be overwritten") +} + +func TestMergePassthroughConfigPreservesUserEntries(t *testing.T) { + mgr := newTestManager(t) + path := filepath.Join(t.TempDir(), "mcp.json") + require.NoError(t, os.WriteFile(path, + []byte(`{"otherKey":"keep me","mcpServers":{"user-server":{"url":"http://user"}}}`), 0o600)) + + require.NoError(t, mgr.mergePassthroughConfig(mcpconfig.PassthroughConfigFile{ + Path: path, + Content: []byte(`{"mcpServers":{"kandev":{"url":"http://localhost:1/mcp"}}}`), + MergeKey: "mcpServers", + })) + + content, err := os.ReadFile(path) + require.NoError(t, err) + require.JSONEq(t, + `{"otherKey":"keep me","mcpServers":{"user-server":{"url":"http://user"},"kandev":{"url":"http://localhost:1/mcp"}}}`, + string(content)) +} + +func TestWorkspacePathEscapesIgnoresPathsOutsideWorkspace(t *testing.T) { + workspace := t.TempDir() + + escapes, err := workspacePathEscapes("", filepath.Join(workspace, "anything.json")) + require.NoError(t, err) + require.False(t, escapes, "an empty workspace dir disables the check") + + escapes, err = workspacePathEscapes(workspace, filepath.Join(t.TempDir(), "kandev-temp.json")) + require.NoError(t, err) + require.False(t, escapes, "kandev's own temp configs are not workspace-relative and are exempt") + + escapes, err = workspacePathEscapes(workspace, filepath.Join(workspace, "nested", "deep", "mcp.json")) + require.NoError(t, err) + require.False(t, escapes, "not-yet-created parents resolve to the workspace itself") +} + +func TestWorkspacePathEscapesSurfacesUnresolvableWorkspace(t *testing.T) { + missing := filepath.Join(t.TempDir(), "never-created") + + _, err := workspacePathEscapes(missing, filepath.Join(missing, "mcp.json")) + + require.ErrorContains(t, err, "resolve workspace dir") +} + +func TestCleanupPassthroughMCPConfigToleratesRemovalFailure(t *testing.T) { + mgr := newTestManager(t) + dir := t.TempDir() + nonEmpty := filepath.Join(dir, "not-a-file") + require.NoError(t, os.MkdirAll(filepath.Join(nonEmpty, "child"), 0o700)) + removable := filepath.Join(dir, "config.json") + require.NoError(t, os.WriteFile(removable, []byte(`{}`), 0o600)) + + execution := &AgentExecution{ID: "exec-1"} + setPassthroughMCPFiles(execution, []string{nonEmpty, removable, filepath.Join(dir, "already-gone.json")}) + setPassthroughMCPEnv(execution, map[string]string{"OPENCODE_CONFIG": removable}) + + mgr.cleanupPassthroughMCPConfig(execution) + + _, err := os.Stat(removable) + require.True(t, os.IsNotExist(err), "a removable tracked file must still be deleted") + require.Empty(t, getPassthroughMCPFiles(execution), "the tracking keys are always cleared") + require.Empty(t, getPassthroughMCPEnv(execution)) +} + +func TestAppendUniqueKeepsFirstOccurrence(t *testing.T) { + list := appendUnique(nil, "a") + list = appendUnique(list, "b") + list = appendUnique(list, "a") + + require.Equal(t, []string{"a", "b"}, list) +} + +func TestGetPassthroughMCPHelpersHandleNilExecution(t *testing.T) { + require.Nil(t, getPassthroughMCPFiles(nil)) + require.Nil(t, getPassthroughMCPEnv(nil)) +} + +func TestSetPassthroughMCPEnvIgnoresEmptyMap(t *testing.T) { + execution := &AgentExecution{ID: "exec-1"} + + setPassthroughMCPEnv(execution, nil) + setPassthroughMCPEnv(execution, map[string]string{}) + + _, exists := execution.metadataValue(metadataKeyPassthroughMCPEnv) + require.False(t, exists, "an empty strategy env must not write a metadata key at all") +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_runtime_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_runtime_test.go new file mode 100644 index 0000000000..3711635fee --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_passthrough_runtime_test.go @@ -0,0 +1,419 @@ +package lifecycle + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/agent/agents" + settingsmodels "github.com/kandev/kandev/internal/agent/settings/models" + agentctltypes "github.com/kandev/kandev/internal/agentctl/types" + "github.com/kandev/kandev/internal/events" + v1 "github.com/kandev/kandev/pkg/api/v1" +) + +// stubCredentialsManager returns canned credential values so buildPassthroughEnv's +// required-env loop can be observed at the boundary it actually writes. +type stubCredentialsManager struct { + values map[string]string + requested []string + mu sync.Mutex +} + +func (s *stubCredentialsManager) GetCredentialValue(_ context.Context, key string) (string, error) { + s.mu.Lock() + s.requested = append(s.requested, key) + s.mu.Unlock() + if value, ok := s.values[key]; ok { + return value, nil + } + return "", errors.New("no such credential") +} + +// TestBuildPassthroughEnvInjectsRequiredCredentialsAndMCPEnv pins the two +// layers buildPassthroughEnv adds on top of the runtime environment: the +// agent's declared required credentials, and the env vars the passthrough MCP +// strategy contributed during command building (e.g. opencode's +// OPENCODE_CONFIG). Both must reach the PTY child process. +func TestBuildPassthroughEnvInjectsRequiredCredentialsAndMCPEnv(t *testing.T) { + mgr := newTestManager(t) + creds := &stubCredentialsManager{values: map[string]string{ + "ANTHROPIC_API_KEY": "sk-live", + }} + mgr.credsMgr = creds + mgr.profileResolver = &mockPassthroughProfileResolver{ + agentName: "claude-acp", + cliPassthrough: true, + envVars: []settingsmodels.ProfileEnvVar{{Key: "PROFILE_ONLY", Value: "profile-value"}}, + } + + execution := &AgentExecution{ + ID: "exec-1", + TaskID: "task-1", + SessionID: "session-1", + AgentProfileID: "profile-1", + OfficeAgentProfileID: "office-1", + } + execution.setRuntimeEnvironment(map[string]string{ + "ANTHROPIC_BASE_URL": "https://proxy.internal", + "HTTPS_PROXY": "http://proxy:3128", + }) + setPassthroughMCPEnv(execution, map[string]string{"OPENCODE_CONFIG": "/tmp/opencode.json"}) + + env := mgr.buildPassthroughEnv(context.Background(), execution, []string{"ANTHROPIC_API_KEY", "MISSING_KEY"}) + + require.Equal(t, "https://proxy.internal", env["ANTHROPIC_BASE_URL"], + "Agent.Runtime().Env must reach the passthrough subprocess") + require.Equal(t, "http://proxy:3128", env["HTTPS_PROXY"]) + require.Equal(t, "task-1", env["KANDEV_TASK_ID"]) + require.Equal(t, "session-1", env["KANDEV_SESSION_ID"]) + require.Equal(t, "office-1", env["KANDEV_AGENT_PROFILE_ID"], + "the Office identity is the agent profile ID exposed to the agent") + require.Equal(t, "profile-1", env["KANDEV_EXECUTION_PROFILE_ID"]) + require.Equal(t, "profile-value", env["PROFILE_ONLY"], "profile env vars are merged in") + require.Equal(t, "sk-live", env["ANTHROPIC_API_KEY"], "required credentials are resolved and injected") + require.NotContains(t, env, "MISSING_KEY", "an unresolvable credential must not be written as an empty value") + require.Equal(t, "/tmp/opencode.json", env["OPENCODE_CONFIG"], + "env contributed by the passthrough MCP strategy must be merged") + + require.Contains(t, creds.requested, "ANTHROPIC_API_KEY") + require.Contains(t, creds.requested, "MISSING_KEY") +} + +func TestMarkPassthroughRunningPublishesOnceAndGuards(t *testing.T) { + mgr := newTestManager(t) + bus, ok := mgr.eventBus.(*MockEventBus) + require.True(t, ok) + + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-acp", SessionID: "session-acp", Status: v1.AgentStatusReady, + })) + exec := &AgentExecution{ + ID: "exec-pty", SessionID: "session-pty", PassthroughProcessID: "pty-1", Status: v1.AgentStatusReady, + } + require.NoError(t, mgr.executionStore.Add(exec)) + + require.ErrorContains(t, mgr.MarkPassthroughRunning("session-absent"), + "no agent execution found for session: session-absent") + require.ErrorContains(t, mgr.MarkPassthroughRunning("session-acp"), + "is not in passthrough mode") + require.Empty(t, bus.PublishedEvents, "guard failures must publish nothing") + + require.NoError(t, mgr.MarkPassthroughRunning("session-pty")) + require.Equal(t, v1.AgentStatusRunning, exec.Status) + require.Len(t, bus.PublishedEvents, 1) + require.Equal(t, events.AgentRunning, bus.PublishedEvents[0].Type) + + require.NoError(t, mgr.MarkPassthroughRunning("session-pty")) + require.Len(t, bus.PublishedEvents, 1, + "an already-running execution must not publish a duplicate AgentRunning event") +} + +func TestPassthroughAccessorsRequirePassthroughSessionAndRunner(t *testing.T) { + ctx := context.Background() + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-acp", SessionID: "session-acp"})) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-pty", SessionID: "session-pty", PassthroughProcessID: "pty-1", + })) + + require.False(t, mgr.IsPassthroughSession(ctx, "session-absent")) + require.False(t, mgr.IsPassthroughSession(ctx, "session-acp")) + require.True(t, mgr.IsPassthroughSession(ctx, "session-pty")) + + t.Run("write stdin", func(t *testing.T) { + require.ErrorContains(t, mgr.WritePassthroughStdin(ctx, "session-absent", "x"), + "no agent execution found for session") + require.ErrorContains(t, mgr.WritePassthroughStdin(ctx, "session-acp", "x"), + "is not in passthrough mode") + require.ErrorContains(t, mgr.WritePassthroughStdin(ctx, "session-pty", "x"), + "interactive runner not available") + }) + + t.Run("resize pty", func(t *testing.T) { + require.ErrorContains(t, mgr.ResizePassthroughPTY(ctx, "session-absent", 80, 24), + "no agent execution found for session") + require.ErrorContains(t, mgr.ResizePassthroughPTY(ctx, "session-acp", 80, 24), + "is not in passthrough mode") + require.ErrorContains(t, mgr.ResizePassthroughPTY(ctx, "session-pty", 80, 24), + "interactive runner not available") + }) + + t.Run("get buffer", func(t *testing.T) { + _, err := mgr.GetPassthroughBuffer(ctx, "session-absent") + require.ErrorContains(t, err, "no agent execution found for session") + _, err = mgr.GetPassthroughBuffer(ctx, "session-acp") + require.ErrorContains(t, err, "is not in passthrough mode") + _, err = mgr.GetPassthroughBuffer(ctx, "session-pty") + require.ErrorContains(t, err, "interactive runner not available") + }) +} + +func TestGetInteractiveRunnerWithoutStandaloneBackend(t *testing.T) { + mgr := newTestManager(t) + require.Nil(t, mgr.GetInteractiveRunner(), "a manager without an executor registry has no runner") + + log := newTestRegistryLogger() + registry := NewExecutorRegistry(log) + registry.Register(&MockExecutor{name: "docker"}) + mgr.executorRegistry = registry + require.Nil(t, mgr.GetInteractiveRunner(), "passthrough requires the standalone backend specifically") +} + +func TestStartPassthroughShellIsNoOpWithoutAgentctl(t *testing.T) { + mgr := newTestManager(t) + // No agentctl client: the shell start must be skipped silently, not panic. + mgr.startPassthroughShell(context.Background(), &AgentExecution{ID: "exec-1"}, "warn") +} + +func TestStartPassthroughShellCallsAgentctlShellStart(t *testing.T) { + var shellStarts atomic.Int32 + var status atomic.Int32 + status.Store(http.StatusOK) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/api/v1/shell/start" { + shellStarts.Add(1) + w.WriteHeader(int(status.Load())) + _, _ = w.Write([]byte(`{"success":true}`)) + return + } + w.WriteHeader(http.StatusNotFound) + })) + t.Cleanup(server.Close) + + mgr := newTestManager(t) + execution := &AgentExecution{ID: "exec-1", agentctl: newTestAgentctlClient(t, server.URL, newTestLogger())} + + mgr.startPassthroughShell(context.Background(), execution, "failed to start shell") + require.Equal(t, int32(1), shellStarts.Load()) + + // A failing shell start is logged, not propagated — the PTY session stays up. + status.Store(http.StatusInternalServerError) + mgr.startPassthroughShell(context.Background(), execution, "failed to start shell") + require.Equal(t, int32(2), shellStarts.Load()) +} + +func TestHandlePassthroughOutputPublishesToEventBus(t *testing.T) { + mgr := newTestManager(t) + bus, ok := mgr.eventBus.(*MockEventBus) + require.True(t, ok) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-1", TaskID: "task-1", SessionID: "session-1", + })) + + mgr.handlePassthroughOutput(nil) + mgr.handlePassthroughOutput(&agentctltypes.ProcessOutput{SessionID: "session-absent"}) + require.Empty(t, bus.PublishedEvents, "nil output and unknown sessions publish nothing") + + mgr.handlePassthroughOutput(&agentctltypes.ProcessOutput{ + SessionID: "session-1", + ProcessID: "pty-1", + Stream: "stdout", + Data: "hello", + Timestamp: time.Now(), + }) + + require.Len(t, bus.PublishedEvents, 1) + require.Equal(t, events.ProcessOutput, bus.PublishedEvents[0].Type) + payload, ok := bus.PublishedEvents[0].Data.(ProcessOutputEventPayload) + require.True(t, ok, "payload type = %T", bus.PublishedEvents[0].Data) + require.Equal(t, "task-1", payload.TaskID) + require.Equal(t, "session-1", payload.SessionID) + require.Equal(t, "pty-1", payload.ProcessID) + require.Equal(t, "stdout", payload.Stream) + require.Equal(t, "hello", payload.Data) +} + +// TestHandlePassthroughStatusSkipsRestartForShellTerminals pins the guard that +// keeps a user shell terminal's exit from restarting the agent PTY. +func TestHandlePassthroughStatusSkipsRestartForShellTerminals(t *testing.T) { + mgr := newTestManager(t) + bus, ok := mgr.eventBus.(*MockEventBus) + require.True(t, ok) + exec := &AgentExecution{ + ID: "exec-1", TaskID: "task-1", SessionID: "session-1", + PassthroughProcessID: "pty-agent", + PassthroughStartedAt: time.Now(), + } + exec.passthroughLaunchUsedResume = true + require.NoError(t, mgr.executionStore.Add(exec)) + + mgr.handlePassthroughStatus(nil) + mgr.handlePassthroughStatus(&agentctltypes.ProcessStatusUpdate{SessionID: "session-absent"}) + require.Empty(t, bus.PublishedEvents) + + exitCode := 1 + mgr.handlePassthroughStatus(&agentctltypes.ProcessStatusUpdate{ + SessionID: "session-1", + ProcessID: "shell-terminal-7", + Status: agentctltypes.ProcessStatusExited, + ExitCode: &exitCode, + Timestamp: time.Now(), + }) + + require.Len(t, bus.PublishedEvents, 1, "the status is still published for the shell terminal") + require.Equal(t, events.ProcessStatus, bus.PublishedEvents[0].Type) + require.False(t, exec.passthroughResumeFailed, + "a user shell terminal's exit must not mark the agent's resume as failed") + require.True(t, exec.passthroughLaunchUsedResume, + "the agent PTY's resume intent must survive an unrelated process exit") +} + +// TestHandlePassthroughStatusFlipsResumeFailedSynchronously pins the +// synchronous flag flip that beats a racing WebSocket reconnect. +func TestHandlePassthroughStatusFlipsResumeFailedSynchronously(t *testing.T) { + mgr := newTestManager(t) + mgr.shuttingDown.Store(true) // Short-circuits the detached restart goroutine. + exec := &AgentExecution{ + ID: "exec-1", TaskID: "task-1", SessionID: "session-1", + PassthroughProcessID: "pty-agent", + PassthroughStartedAt: time.Now(), + isResumedSession: true, + } + exec.passthroughLaunchUsedResume = true + require.NoError(t, mgr.executionStore.Add(exec)) + + exitCode := 1 + mgr.handlePassthroughStatus(&agentctltypes.ProcessStatusUpdate{ + SessionID: "session-1", + ProcessID: "pty-agent", + Status: agentctltypes.ProcessStatusFailed, + ExitCode: &exitCode, + Timestamp: time.Now(), + }) + + require.True(t, exec.passthroughResumeFailed, + "a non-zero exit of a resume launch must set the sticky resume-failed flag before returning") + require.False(t, exec.isResumedSession) + require.False(t, exec.passthroughLaunchUsedResume) +} + +func TestHandlePassthroughStatusKeepsResumeIntentOnCleanExit(t *testing.T) { + mgr := newTestManager(t) + mgr.shuttingDown.Store(true) + exec := &AgentExecution{ + ID: "exec-1", TaskID: "task-1", SessionID: "session-1", + PassthroughProcessID: "pty-agent", + PassthroughStartedAt: time.Now(), + } + exec.passthroughLaunchUsedResume = true + require.NoError(t, mgr.executionStore.Add(exec)) + + zero := 0 + mgr.handlePassthroughStatus(&agentctltypes.ProcessStatusUpdate{ + SessionID: "session-1", + ProcessID: "pty-agent", + Status: agentctltypes.ProcessStatusExited, + ExitCode: &zero, + Timestamp: time.Now(), + }) + + require.False(t, exec.passthroughResumeFailed, + "a clean exit keeps the resume intent so a quit session is auto-restarted as a resume") + require.True(t, exec.passthroughLaunchUsedResume) +} + +func TestPassthroughExitCodeTreatsAbsentCodeAsClean(t *testing.T) { + require.Equal(t, 0, passthroughExitCode(&agentctltypes.ProcessStatusUpdate{})) + + code := 127 + require.Equal(t, 127, passthroughExitCode(&agentctltypes.ProcessStatusUpdate{ExitCode: &code})) +} + +func TestHandlePassthroughTurnCompleteUnknownSession(t *testing.T) { + mgr := newTestManager(t) + bus, ok := mgr.eventBus.(*MockEventBus) + require.True(t, ok) + + mgr.handlePassthroughTurnComplete("session-absent", "pty-1") + + require.Empty(t, bus.PublishedEvents) +} + +// TestAutoInjectMarksRunningBeforeWriting pins the ordering that blocks a +// racing composer submission: the execution must be RUNNING before the first +// chunk lands on the PTY, so checkSessionPromptable rejects a concurrent +// message.add instead of letting it race into the same PTY mid-submit. +func TestAutoInjectMarksRunningBeforeWriting(t *testing.T) { + mgr := newTestManager(t) + execution := newAutoInjectExecution("Ship the release") + execution.Status = v1.AgentStatusReady + require.NoError(t, mgr.executionStore.Add(execution)) + + var statusAtFirstWrite v1.AgentStatus + runner := &statusObservingRunner{onWrite: func() { statusAtFirstWrite = execution.Status }} + + mgr.autoInjectInitialPromptWith(runner, execution, agents.PassthroughConfig{SubmitSequence: "\r"}) + + require.Equal(t, []string{"Ship the release\r"}, runner.writes) + require.Equal(t, v1.AgentStatusRunning, statusAtFirstWrite, + "the execution must already be RUNNING when the first PTY chunk is written") +} + +func TestAutoInjectStopsAfterWriteFailure(t *testing.T) { + mgr := newTestManager(t) + execution := newAutoInjectExecution("Ship the release") + execution.Status = v1.AgentStatusReady + require.NoError(t, mgr.executionStore.Add(execution)) + runner := &fakePassthroughRunner{writeErr: errors.New("process not found")} + + mgr.autoInjectInitialPromptWith(runner, execution, agents.PassthroughConfig{ + SubmitSequence: "\r", + DisableBracketedPaste: true, + SubmitDelay: time.Millisecond, + }) + + require.Len(t, runner.writes, 1, + "a failed write must abort the chunk loop instead of continuing to the submit byte") +} + +func TestAutoInjectSkipsDuringShutdown(t *testing.T) { + mgr := newTestManager(t) + mgr.shuttingDown.Store(true) + runner := &fakePassthroughRunner{} + + mgr.autoInjectInitialPromptWith(runner, newAutoInjectExecution("Ship it"), agents.PassthroughConfig{ + SubmitSequence: "\r", + }) + + require.False(t, runner.writeCalled, "teardown must not race a stdin write into a dying PTY") +} + +// TestAutoInjectInitialPromptWithoutRunnerIsNoOp covers the outer wrapper: with +// no standalone backend there is no interactive runner, and the goroutine +// startPassthroughSession spawns must exit without touching the execution. +func TestAutoInjectInitialPromptWithoutRunnerIsNoOp(t *testing.T) { + mgr := newTestManager(t) + execution := newAutoInjectExecution("Ship it") + execution.Status = v1.AgentStatusReady + require.NoError(t, mgr.executionStore.Add(execution)) + + mgr.autoInjectInitialPrompt(execution, agents.PassthroughConfig{SubmitSequence: "\r"}) + + require.Equal(t, v1.AgentStatusReady, execution.Status, + "without a runner the execution must not be flipped to RUNNING") +} + +// statusObservingRunner records the execution status observed at the moment of +// the first stdin write, which is what the mark-running-first ordering is about. +type statusObservingRunner struct { + onWrite func() + writes []string +} + +func (r *statusObservingRunner) WaitForFirstIdle(context.Context, string) error { return nil } + +func (r *statusObservingRunner) WriteStdin(_ string, data string) error { + if r.onWrite != nil { + r.onWrite() + } + r.writes = append(r.writes, data) + return nil +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_profile_capabilities_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_profile_capabilities_test.go new file mode 100644 index 0000000000..d3e6fe89b0 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_profile_capabilities_test.go @@ -0,0 +1,369 @@ +package lifecycle + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "sync" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/agent/executor" + "github.com/kandev/kandev/internal/agent/mcpconfig" + "github.com/kandev/kandev/internal/task/models" +) + +// capabilityExecutor reports fixed executor capabilities so the manager's +// pass-through accessors can be checked against a known backend. +type capabilityExecutor struct { + MockExecutor + requiresClone bool + preferShell bool +} + +func (e *capabilityExecutor) RequiresCloneURL() bool { return e.requiresClone } +func (e *capabilityExecutor) ShouldApplyPreferredShell() bool { return e.preferShell } + +func TestManagerCapabilitiesDelegateToBackend(t *testing.T) { + log := newTestRegistryLogger() + registry := NewExecutorRegistry(log) + registry.Register(&capabilityExecutor{ + MockExecutor: MockExecutor{name: executor.NameSprites}, + requiresClone: true, + preferShell: false, + }) + registry.Register(&capabilityExecutor{ + MockExecutor: MockExecutor{name: executor.NameStandalone}, + requiresClone: false, + preferShell: true, + }) + mgr := NewManager(newTestRegistry(), &MockEventBus{}, registry, nil, nil, nil, ExecutorFallbackWarn, "", log) + cleanupManagerStopCh(t, mgr) + + require.True(t, mgr.RequiresCloneURL(string(models.ExecutorTypeSprites)), + "a clone-based executor owns its own filesystem and needs a clone URL") + require.False(t, mgr.ShouldApplyPreferredShell(string(models.ExecutorTypeSprites))) + + require.False(t, mgr.RequiresCloneURL(string(models.ExecutorTypeLocal))) + require.True(t, mgr.ShouldApplyPreferredShell(string(models.ExecutorTypeLocal)), + "a host-local executor inherits the user's preferred shell") +} + +func TestManagerCapabilitiesFailClosedForUnknownExecutor(t *testing.T) { + mgr := newTestManager(t) + + require.False(t, mgr.RequiresCloneURL("not-an-executor")) + require.False(t, mgr.ShouldApplyPreferredShell("not-an-executor")) +} + +func TestResolveAgentProfileRequiresResolver(t *testing.T) { + mgr := newTestManager(t) + mgr.profileResolver = nil + + _, err := mgr.ResolveAgentProfile(context.Background(), "profile-1") + + require.ErrorContains(t, err, "profile resolver not configured") +} + +func TestResolveAgentProfileDelegatesToResolver(t *testing.T) { + mgr := newTestManager(t) + + info, err := mgr.ResolveAgentProfile(context.Background(), "profile-1") + + require.NoError(t, err) + require.Equal(t, "profile-1", info.ProfileID) + require.Equal(t, "auggie", info.AgentName) +} + +func TestRuntimeNameOfNilBackend(t *testing.T) { + require.Equal(t, executor.NameUnknown, runtimeName(nil)) + require.Equal(t, executor.NameDocker, runtimeName(&MockExecutor{name: executor.NameDocker})) +} + +func TestResolveProfileSessionConfigAndPolicyReadsProfileOnce(t *testing.T) { + resolver := &countingProfileResolver{info: &AgentProfileInfo{ + ProfileID: "profile-1", + Model: "claude-opus-5", + Mode: "plan", + ConfigOptions: map[string]string{"reasoning": "high"}, + FallbackModel: "claude-sonnet-5", + AutoFallback: true, + }} + mgr := newTestManager(t) + mgr.profileResolver = resolver + + model, mode, options, policy := mgr.resolveProfileSessionConfigAndPolicy(context.Background(), "profile-1") + + require.Equal(t, "claude-opus-5", model) + require.Equal(t, "plan", mode) + require.Equal(t, map[string]string{"reasoning": "high"}, options) + require.Equal(t, "claude-opus-5", policy.Model) + require.Equal(t, "claude-sonnet-5", policy.FallbackModel) + require.True(t, policy.AutoFallback) + require.Equal(t, int32(1), resolver.calls.Load(), + "session start needs config and policy together; a second read would be a wasted DB hit") +} + +func TestResolveProfileSessionConfigAndPolicyDegradesGracefully(t *testing.T) { + ctx := context.Background() + + t.Run("no profile id", func(t *testing.T) { + mgr := newTestManager(t) + model, mode, options, policy := mgr.resolveProfileSessionConfigAndPolicy(ctx, "") + require.Empty(t, model) + require.Empty(t, mode) + require.Nil(t, options) + require.Equal(t, StartModelPolicy{}, policy) + }) + + t.Run("resolver error", func(t *testing.T) { + mgr := newTestManager(t) + mgr.profileResolver = &countingProfileResolver{err: errors.New("profile deleted")} + model, _, _, policy := mgr.resolveProfileSessionConfigAndPolicy(ctx, "profile-1") + require.Empty(t, model) + require.Equal(t, StartModelPolicy{}, policy, + "a missing profile means no start model is configured, not a hard failure") + }) + + t.Run("no resolver wired", func(t *testing.T) { + mgr := newTestManager(t) + mgr.profileResolver = nil + _, _, _, policy := mgr.resolveProfileSessionConfigAndPolicy(ctx, "profile-1") + require.Equal(t, StartModelPolicy{}, policy) + }) +} + +func TestApplyExecutorMcpPolicyOverridesDefault(t *testing.T) { + mgr := newTestManager(t) + base := mcpconfig.DefaultPolicyForRuntime(executor.NameUnknown) + require.False(t, base.AllowStdio, "the unknown-runtime default denies stdio") + + policy, executorID, err := mgr.applyExecutorMcpPolicy("profile-1", "", + map[string]interface{}{ + "executor_id": "executor-7", + "executor_mcp_policy": `{"allow_stdio":true}`, + }, base) + + require.NoError(t, err) + require.Equal(t, "executor-7", executorID, + "the executor identity is read from metadata so warnings name the right executor") + require.True(t, policy.AllowStdio) +} + +func TestApplyExecutorMcpPolicyPassesThroughWithoutOverride(t *testing.T) { + mgr := newTestManager(t) + base := mcpconfig.DefaultPolicyForRuntime(executor.NameDocker) + + policy, executorID, err := mgr.applyExecutorMcpPolicy("profile-1", "executor-1", nil, base) + require.NoError(t, err) + require.Equal(t, base, policy) + require.Equal(t, "executor-1", executorID) + + policy, executorID, err = mgr.applyExecutorMcpPolicy("profile-1", "executor-1", + map[string]interface{}{"unrelated": "value"}, base) + require.NoError(t, err) + require.Equal(t, base, policy) + require.Equal(t, "executor-1", executorID) +} + +func TestApplyExecutorMcpPolicyRejectsMalformedOverride(t *testing.T) { + mgr := newTestManager(t) + base := mcpconfig.DefaultPolicyForRuntime(executor.NameDocker) + + _, _, err := mgr.applyExecutorMcpPolicy("profile-1", "", map[string]interface{}{ + "executor_mcp_policy": "{not json", + }, base) + + require.ErrorContains(t, err, "invalid executor MCP policy") +} + +func TestResolveMcpServersWithParamsShortCircuits(t *testing.T) { + ctx := context.Background() + agentConfig, ok := newTestRegistry().Get("claude-acp") + require.True(t, ok) + + t.Run("no provider", func(t *testing.T) { + mgr := newTestManager(t) + servers, err := mgr.resolveMcpServersWithParams(ctx, "profile-1", nil, agentConfig) + require.NoError(t, err) + require.Nil(t, servers) + }) + + t.Run("no agent config", func(t *testing.T) { + mgr := newTestManager(t) + mgr.mcpProvider = &fakeMcpConfigProvider{} + servers, err := mgr.resolveMcpServersWithParams(ctx, "profile-1", nil, nil) + require.NoError(t, err) + require.Nil(t, servers) + }) + + t.Run("blank profile id", func(t *testing.T) { + mgr := newTestManager(t) + mgr.mcpProvider = &fakeMcpConfigProvider{} + servers, err := mgr.resolveMcpServersWithParams(ctx, " ", nil, agentConfig) + require.NoError(t, err) + require.Nil(t, servers) + }) + + t.Run("disabled config", func(t *testing.T) { + mgr := newTestManager(t) + mgr.mcpProvider = &fakeMcpConfigProvider{config: &mcpconfig.ProfileConfig{ProfileID: "profile-1"}} + servers, err := mgr.resolveMcpServersWithParams(ctx, "profile-1", nil, agentConfig) + require.NoError(t, err) + require.Nil(t, servers, "a profile with MCP disabled contributes no servers") + }) +} + +func TestResolveMcpServersFromExecutionUsesExecutionMetadata(t *testing.T) { + mgr := newTestManager(t) + mgr.mcpProvider = &fakeMcpConfigProvider{config: &mcpconfig.ProfileConfig{ + ProfileID: "profile-1", + Enabled: true, + Servers: map[string]mcpconfig.ServerDef{ + "github": {Type: mcpconfig.ServerTypeStdio, Command: "npx", Args: []string{"-y", "gh"}}, + }, + }} + agentConfig, ok := newTestRegistry().Get("claude-acp") + require.True(t, ok) + + execution := &AgentExecution{ID: "exec-1", AgentProfileID: "profile-1"} + execution.setMetadataValue("executor_mcp_policy", `{"allow_stdio":true}`) + + servers, err := mgr.resolveMcpServers(context.Background(), execution, agentConfig) + + require.NoError(t, err) + require.Len(t, servers, 1) + require.Equal(t, "github", servers[0].Name) + require.Equal(t, "npx", servers[0].Command, + "the executor policy on the execution's own metadata is what unlocks stdio transports") + + nilServers, err := mgr.resolveMcpServers(context.Background(), nil, agentConfig) + require.NoError(t, err) + require.Nil(t, nilServers) +} + +// TestPushBaseBranchesForTaskTargetsOnlyThatTask pins the fan-out: a base +// branch change must reach every live execution of the task and no other. +func TestPushBaseBranchesForTaskTargetsOnlyThatTask(t *testing.T) { + var mu sync.Mutex + var pushes []map[string]string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/workspace/base-branches" { + w.WriteHeader(http.StatusNotFound) + return + } + var body struct { + BaseBranches map[string]string `json:"base_branches"` + } + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + // t.Errorf, not require: this runs on the server's goroutine and + // require's FailNow/Goexit is only valid on the test goroutine. + t.Errorf("decode base-branches request: %v", err) + w.WriteHeader(http.StatusBadRequest) + return + } + mu.Lock() + pushes = append(pushes, body.BaseBranches) + mu.Unlock() + _, _ = w.Write([]byte(`{"success":true}`)) + })) + t.Cleanup(server.Close) + client := newTestAgentctlClient(t, server.URL, newTestLogger()) + + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-1", TaskID: "task-1", SessionID: "session-1", agentctl: client, + })) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-2", TaskID: "task-1", SessionID: "session-2", agentctl: client, + })) + // Same-workspace execution on another task, plus one with no client. + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-3", TaskID: "task-other", SessionID: "session-3", agentctl: client, + })) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-4", TaskID: "task-1", SessionID: "session-4", + })) + + branches := map[string]string{"": "main", "frontend": "develop"} + mgr.PushBaseBranchesForTask(context.Background(), "task-1", branches) + + mu.Lock() + defer mu.Unlock() + require.Len(t, pushes, 2, "only the two client-bearing executions of task-1 are pushed to") + for _, push := range pushes { + require.Equal(t, branches, push) + } +} + +func TestPushBaseBranchesForTaskIgnoresEmptyTaskID(t *testing.T) { + mgr := newTestManager(t) + + // No panic, no work: an empty task ID cannot identify executions. + mgr.PushBaseBranchesForTask(context.Background(), "", map[string]string{"": "main"}) +} + +// TestPushBaseBranchesForTaskIsBestEffort pins that one failing agentctl does +// not stop the push to the task's other executions. +func TestPushBaseBranchesForTaskIsBestEffort(t *testing.T) { + var mu sync.Mutex + calls := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + mu.Lock() + calls++ + mu.Unlock() + w.WriteHeader(http.StatusInternalServerError) + })) + t.Cleanup(server.Close) + client := newTestAgentctlClient(t, server.URL, newTestLogger()) + + mgr := newTestManager(t) + for _, id := range []string{"session-1", "session-2"} { + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-" + id, TaskID: "task-1", SessionID: id, agentctl: client, + })) + } + + mgr.PushBaseBranchesForTask(context.Background(), "task-1", map[string]string{"": "main"}) + + mu.Lock() + defer mu.Unlock() + require.Equal(t, 2, calls, + "a failed push must be logged and the loop continued; the DB remains the source of truth") +} + +func TestGetRecoveredExecutionsProjectsOfficeIdentity(t *testing.T) { + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-1", TaskID: "task-1", SessionID: "session-1", ContainerID: "container-1", + AgentProfileID: "execution-profile", OfficeAgentProfileID: "office-profile", + })) + + recovered := mgr.GetRecoveredExecutions() + + require.Len(t, recovered, 1) + require.Equal(t, "exec-1", recovered[0].ExecutionID) + require.Equal(t, "task-1", recovered[0].TaskID) + require.Equal(t, "session-1", recovered[0].SessionID) + require.Equal(t, "container-1", recovered[0].ContainerID) + require.Equal(t, "office-profile", recovered[0].AgentProfileID, + "the orchestrator syncs on the stable Office identity") + require.Equal(t, "execution-profile", recovered[0].ExecutionProfileID) +} + +func TestExecutionStoreGetByContainerID(t *testing.T) { + store := NewExecutionStore() + require.NoError(t, store.Add(&AgentExecution{ + ID: "exec-1", SessionID: "session-1", ContainerID: "container-1", + })) + + got, ok := store.GetByContainerID("container-1") + require.True(t, ok) + require.Equal(t, "exec-1", got.ID) + + _, ok = store.GetByContainerID("container-absent") + require.False(t, ok) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_remote_status_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_remote_status_test.go new file mode 100644 index 0000000000..4b014bd5cf --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_remote_status_test.go @@ -0,0 +1,338 @@ +package lifecycle + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/agent/executor" + "github.com/kandev/kandev/internal/agentruntime" +) + +// statusProviderExecutor is an ExecutorBackend that also implements +// RemoteStatusProvider, recording the instance the manager projected from the +// execution so the projection itself can be asserted. +type statusProviderExecutor struct { + MockExecutor + status *RemoteStatus + err error + observed []*ExecutorInstance +} + +func (e *statusProviderExecutor) GetRemoteStatus(_ context.Context, instance *ExecutorInstance) (*RemoteStatus, error) { + e.observed = append(e.observed, instance) + return e.status, e.err +} + +func newRemoteStatusManager(t *testing.T, provider ExecutorBackend) *Manager { + t.Helper() + log := newTestRegistryLogger() + registry := NewExecutorRegistry(log) + registry.Register(provider) + mgr := NewManager(newTestRegistry(), &MockEventBus{}, registry, nil, nil, nil, ExecutorFallbackWarn, "", log) + cleanupManagerStopCh(t, mgr) + return mgr +} + +// TestPollOneRemoteStatusProjectsExecutionIntoInstance pins the projection the +// provider is handed: a status poll must carry the runtime identifiers the +// backend needs to find the remote (container, sprite name in metadata), not +// just the session ID. +func TestPollOneRemoteStatusProjectsExecutionIntoInstance(t *testing.T) { + provider := &statusProviderExecutor{ + MockExecutor: MockExecutor{name: executor.NameSprites}, + status: &RemoteStatus{State: "started"}, + } + mgr := newRemoteStatusManager(t, provider) + execution := &AgentExecution{ + ID: "exec-1", + TaskID: "task-1", + SessionID: "session-1", + RuntimeName: agentruntime.RuntimeSprites, + ContainerID: "container-1", + ContainerIP: "10.0.0.2", + WorkspacePath: "/workspace", + } + execution.setMetadataValue(MetadataKeySpriteName, "kandev-abc") + + mgr.pollOneRemoteStatus(context.Background(), execution) + + require.Len(t, provider.observed, 1) + instance := provider.observed[0] + require.Equal(t, "exec-1", instance.InstanceID) + require.Equal(t, "task-1", instance.TaskID) + require.Equal(t, "session-1", instance.SessionID) + require.Equal(t, agentruntime.RuntimeSprites, instance.RuntimeName) + require.Equal(t, "container-1", instance.ContainerID) + require.Equal(t, "10.0.0.2", instance.ContainerIP) + require.Equal(t, "/workspace", instance.WorkspacePath) + require.Equal(t, "kandev-abc", instance.Metadata[MetadataKeySpriteName], + "the remote's own name lives in metadata; without it the provider cannot find the sandbox") + + status, ok := mgr.GetRemoteStatusBySession("session-1") + require.True(t, ok) + require.Equal(t, "started", status.State) + require.Equal(t, agentruntime.RuntimeSprites, status.RuntimeName, + "a provider that omits the runtime name gets the execution's filled in") + require.False(t, status.LastCheckedAt.IsZero(), + "an unstamped status is stamped so the UI can show staleness") +} + +func TestPollOneRemoteStatusStoresProviderError(t *testing.T) { + provider := &statusProviderExecutor{ + MockExecutor: MockExecutor{name: executor.NameSprites}, + err: errors.New("sprite unreachable"), + } + mgr := newRemoteStatusManager(t, provider) + + mgr.pollOneRemoteStatus(context.Background(), &AgentExecution{ + ID: "exec-1", SessionID: "session-1", RuntimeName: agentruntime.RuntimeSprites, + }) + + status, ok := mgr.GetRemoteStatusBySession("session-1") + require.True(t, ok, "a failed poll must still record a status so the UI can show degraded") + require.Equal(t, "sprite unreachable", status.ErrorMessage) + require.Equal(t, agentruntime.RuntimeSprites, status.RuntimeName) +} + +func TestPollOneRemoteStatusSkipsUnsupportedExecutions(t *testing.T) { + provider := &statusProviderExecutor{ + MockExecutor: MockExecutor{name: executor.NameSprites}, + status: &RemoteStatus{State: "started"}, + } + mgr := newRemoteStatusManager(t, provider) + ctx := context.Background() + + mgr.pollOneRemoteStatus(ctx, nil) + mgr.pollOneRemoteStatus(ctx, &AgentExecution{ID: "exec-1"}) // no session + mgr.pollOneRemoteStatus(ctx, &AgentExecution{ + ID: "exec-2", SessionID: "session-2", RuntimeName: agentruntime.RuntimeDocker, + }) // runtime not registered + + require.Empty(t, provider.observed) + _, ok := mgr.GetRemoteStatusBySession("session-2") + require.False(t, ok) +} + +func TestPollOneRemoteStatusSkipsBackendWithoutStatusSupport(t *testing.T) { + mgr := newRemoteStatusManager(t, &MockExecutor{name: executor.NameStandalone}) + + mgr.pollOneRemoteStatus(context.Background(), &AgentExecution{ + ID: "exec-1", SessionID: "session-1", RuntimeName: agentruntime.RuntimeStandalone, + }) + + _, ok := mgr.GetRemoteStatusBySession("session-1") + require.False(t, ok, "a local executor has no remote status to report") +} + +func TestPollOneRemoteStatusIgnoresNilProviderResult(t *testing.T) { + provider := &statusProviderExecutor{MockExecutor: MockExecutor{name: executor.NameSprites}} + mgr := newRemoteStatusManager(t, provider) + + mgr.pollOneRemoteStatus(context.Background(), &AgentExecution{ + ID: "exec-1", SessionID: "session-1", RuntimeName: agentruntime.RuntimeSprites, + }) + + _, ok := mgr.GetRemoteStatusBySession("session-1") + require.False(t, ok, "a provider returning (nil, nil) must not create an empty cache entry") +} + +func TestPollRemoteStatusesCoversEveryTrackedExecution(t *testing.T) { + provider := &statusProviderExecutor{ + MockExecutor: MockExecutor{name: executor.NameSprites}, + status: &RemoteStatus{State: "started"}, + } + mgr := newRemoteStatusManager(t, provider) + for _, id := range []string{"session-1", "session-2"} { + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-" + id, SessionID: id, RuntimeName: agentruntime.RuntimeSprites, + })) + } + + mgr.pollRemoteStatuses(context.Background()) + + require.Len(t, provider.observed, 2) + for _, id := range []string{"session-1", "session-2"} { + _, ok := mgr.GetRemoteStatusBySession(id) + require.True(t, ok, "session %s was not polled", id) + } +} + +func TestGetRemoteStatusBySessionIDRefreshesTrackedExecution(t *testing.T) { + provider := &statusProviderExecutor{ + MockExecutor: MockExecutor{name: executor.NameSprites}, + status: &RemoteStatus{State: "started", LastCheckedAt: time.Now().UTC()}, + } + mgr := newRemoteStatusManager(t, provider) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-1", SessionID: "session-1", RuntimeName: agentruntime.RuntimeSprites, + })) + + status, ok := mgr.GetRemoteStatusBySessionID(context.Background(), "session-1") + + require.True(t, ok) + require.Equal(t, "started", status.State) + require.Len(t, provider.observed, 1, "a tracked session is refreshed opportunistically on read") + + _, ok = mgr.GetRemoteStatusBySessionID(context.Background(), "session-absent") + require.False(t, ok) + require.Len(t, provider.observed, 1, "an untracked session must not trigger a poll") +} + +func TestGetRemoteStatusBySessionReturnsDefensiveCopy(t *testing.T) { + mgr := newRemoteStatusManager(t, &MockExecutor{name: executor.NameStandalone}) + mgr.storeRemoteStatus("session-1", &RemoteStatus{State: "started"}) + + first, ok := mgr.GetRemoteStatusBySession("session-1") + require.True(t, ok) + first.State = "mutated-by-caller" + + second, ok := mgr.GetRemoteStatusBySession("session-1") + require.True(t, ok) + require.Equal(t, "started", second.State, "the cache must hand out copies, not its own entry") +} + +func TestStoreAndClearRemoteStatusIgnoreEmptyInputs(t *testing.T) { + mgr := newRemoteStatusManager(t, &MockExecutor{name: executor.NameStandalone}) + + mgr.storeRemoteStatus("", &RemoteStatus{State: "started"}) + mgr.storeRemoteStatus("session-1", nil) + _, ok := mgr.GetRemoteStatusBySession("session-1") + require.False(t, ok) + + mgr.storeRemoteStatus("session-1", &RemoteStatus{State: "started"}) + mgr.clearRemoteStatus("") + _, ok = mgr.GetRemoteStatusBySession("session-1") + require.True(t, ok, "clearing an empty session ID must not wipe unrelated entries") + + mgr.clearRemoteStatus("session-1") + _, ok = mgr.GetRemoteStatusBySession("session-1") + require.False(t, ok) +} + +// TestPollRemoteStatusForRecordsPopulatesCacheBeforeResume pins the startup +// path: sessions that have no in-memory execution yet still get their remote +// status cached from the persisted executor records. +func TestPollRemoteStatusForRecordsPopulatesCacheBeforeResume(t *testing.T) { + provider := &statusProviderExecutor{ + MockExecutor: MockExecutor{name: executor.NameSprites}, + status: &RemoteStatus{State: "suspended"}, + } + mgr := newRemoteStatusManager(t, provider) + + mgr.PollRemoteStatusForRecords(context.Background(), []RemoteStatusPollRecord{ + {SessionID: "", Runtime: agentruntime.RuntimeSprites}, // skipped + {SessionID: "session-no-runtime"}, // skipped + {SessionID: "session-unregistered", Runtime: agentruntime.RuntimeSSH}, // skipped + { + SessionID: "session-1", + Runtime: agentruntime.RuntimeSprites, + AgentExecutionID: "exec-1", + ContainerID: "container-1", + Metadata: map[string]interface{}{MetadataKeySpriteName: "kandev-abc"}, + }, + }) + + require.Len(t, provider.observed, 1, "only the complete, registered record is polled") + require.Equal(t, "exec-1", provider.observed[0].InstanceID) + require.Equal(t, "container-1", provider.observed[0].ContainerID) + require.Equal(t, "kandev-abc", provider.observed[0].Metadata[MetadataKeySpriteName]) + + status, ok := mgr.GetRemoteStatusBySession("session-1") + require.True(t, ok) + require.Equal(t, "suspended", status.State) + require.Equal(t, agentruntime.RuntimeSprites, status.RuntimeName) + require.False(t, status.LastCheckedAt.IsZero()) +} + +func TestPollRemoteStatusForRecordsStoresProviderError(t *testing.T) { + provider := &statusProviderExecutor{ + MockExecutor: MockExecutor{name: executor.NameSprites}, + err: errors.New("api token expired"), + } + mgr := newRemoteStatusManager(t, provider) + + mgr.PollRemoteStatusForRecords(context.Background(), []RemoteStatusPollRecord{ + {SessionID: "session-1", Runtime: agentruntime.RuntimeSprites}, + }) + + status, ok := mgr.GetRemoteStatusBySession("session-1") + require.True(t, ok) + require.Equal(t, "api token expired", status.ErrorMessage) +} + +func TestPollRemoteStatusForRecordsSkipsNilProviderResultAndNoRegistry(t *testing.T) { + provider := &statusProviderExecutor{MockExecutor: MockExecutor{name: executor.NameSprites}} + mgr := newRemoteStatusManager(t, provider) + + mgr.PollRemoteStatusForRecords(context.Background(), []RemoteStatusPollRecord{ + {SessionID: "session-1", Runtime: agentruntime.RuntimeSprites}, + }) + _, ok := mgr.GetRemoteStatusBySession("session-1") + require.False(t, ok) + + bare := newTestManager(t) + bare.PollRemoteStatusForRecords(context.Background(), []RemoteStatusPollRecord{ + {SessionID: "session-1", Runtime: agentruntime.RuntimeSprites}, + }) + _, ok = bare.GetRemoteStatusBySession("session-1") + require.False(t, ok, "without an executor registry there is nothing to poll") +} + +// TestStopAgentClearsRemoteStatus pins that a stopped session does not keep +// reporting a stale cloud status in the UI. +func TestStopAgentClearsRemoteStatus(t *testing.T) { + provider := &statusProviderExecutor{ + MockExecutor: MockExecutor{name: executor.NameSprites}, + status: &RemoteStatus{State: "started"}, + } + mgr := newRemoteStatusManager(t, provider) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ + ID: "exec-1", SessionID: "session-1", RuntimeName: agentruntime.RuntimeSprites, + })) + mgr.storeRemoteStatus("session-1", &RemoteStatus{State: "started"}) + + require.NoError(t, mgr.StopAgent(context.Background(), "exec-1", false)) + + _, ok := mgr.GetRemoteStatusBySession("session-1") + require.False(t, ok) +} + +func TestManagerEnvironmentRequiresTypedDockerBackend(t *testing.T) { + ctx := context.Background() + log := newTestRegistryLogger() + registry := NewExecutorRegistry(log) + registry.Register(&MockExecutor{name: executor.NameDocker}) + registry.Register(&MockExecutor{name: executor.NameSprites}) + mgr := NewManager(newTestRegistry(), &MockEventBus{}, registry, nil, nil, nil, ExecutorFallbackWarn, "", log) + cleanupManagerStopCh(t, mgr) + + require.NoError(t, mgr.DestroyContainer(ctx, ""), "an empty container ID is a no-op") + require.ErrorContains(t, mgr.DestroyContainer(ctx, "container-1"), "docker backend has unexpected type") + + status, err := mgr.GetContainerLiveStatus(ctx, "") + require.NoError(t, err) + require.Nil(t, status) + _, err = mgr.GetContainerLiveStatus(ctx, "container-1") + require.ErrorContains(t, err, "docker backend has unexpected type") + + require.NoError(t, mgr.DestroySandbox(ctx, "", "exec-1"), "an empty sandbox ID is a no-op") + require.ErrorContains(t, mgr.DestroySandbox(ctx, "sandbox-1", "exec-1"), + "sprites backend has unexpected type") +} + +func TestManagerEnvironmentReportsMissingBackends(t *testing.T) { + ctx := context.Background() + log := newTestRegistryLogger() + mgr := NewManager(newTestRegistry(), &MockEventBus{}, NewExecutorRegistry(log), nil, nil, nil, + ExecutorFallbackWarn, "", log) + cleanupManagerStopCh(t, mgr) + + require.ErrorContains(t, mgr.DestroyContainer(ctx, "container-1"), "docker backend unavailable") + _, err := mgr.GetContainerLiveStatus(ctx, "container-1") + require.ErrorContains(t, err, "docker backend unavailable") + require.ErrorContains(t, mgr.DestroySandbox(ctx, "sandbox-1", "exec-1"), "sprites backend unavailable") +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager_workspace_rescan_guard_test.go b/apps/backend/internal/agent/runtime/lifecycle/manager_workspace_rescan_guard_test.go new file mode 100644 index 0000000000..111b82e7f3 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/manager_workspace_rescan_guard_test.go @@ -0,0 +1,156 @@ +package lifecycle + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "sync" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/kandev/kandev/internal/agent/executor" +) + +type recordedRescan struct { + WorkDir string `json:"work_dir"` + SourceRoots []string `json:"workspace_source_roots"` +} + +// newRescanServer serves a fixed status for the lifetime of the server. The +// status is passed by value rather than by pointer so the handler goroutine +// cannot race a mid-run write: these tests each want one status throughout. +// (fakeAgentctlProcessServer uses atomic.Int32 because it genuinely does flip +// its status mid-test; matching the shape to the requirement keeps the +// difference between the two helpers visible.) +func newRescanServer(t *testing.T, status int) (*[]recordedRescan, string) { + t.Helper() + var mu sync.Mutex + recorded := &[]recordedRescan{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body recordedRescan + _ = json.NewDecoder(r.Body).Decode(&body) + mu.Lock() + *recorded = append(*recorded, body) + mu.Unlock() + w.WriteHeader(status) + _, _ = w.Write([]byte(`{"success":true}`)) + })) + t.Cleanup(server.Close) + return recorded, server.URL +} + +// TestRescanWorkspaceForSessionForwardsWorkDirAndRoots pins what reaches +// agentctl and, critically, that the execution's recorded source roots are +// updated to match — a rescan that succeeded but left the old roots on the +// execution would send stale roots on the next call. +func TestRescanWorkspaceForSessionForwardsWorkDirAndRoots(t *testing.T) { + recorded, url := newRescanServer(t, http.StatusOK) + mgr := newTestManager(t) + execution := &AgentExecution{ + ID: "exec-1", SessionID: "session-1", + WorkspaceSourceRoots: []string{"/work/task-1/backend"}, + agentctl: newTestAgentctlClient(t, url, newTestLogger()), + } + require.NoError(t, mgr.executionStore.Add(execution)) + + newRoots := []string{"/work/task-1/backend", "/work/task-1/frontend"} + require.NoError(t, mgr.RescanWorkspaceForSession( + context.Background(), "session-1", "/work/task-1", newRoots)) + + require.Len(t, *recorded, 1) + require.Equal(t, "/work/task-1", (*recorded)[0].WorkDir) + require.Equal(t, newRoots, (*recorded)[0].SourceRoots) + require.Equal(t, newRoots, execution.WorkspaceSourceRoots, + "a successful rescan commits the new roots to the execution") +} + +// TestRescanWorkspaceForSessionRollsBackRootsOnFailure pins the rollback: a +// failed rescan must not leave the execution claiming roots agentctl never +// accepted. +func TestRescanWorkspaceForSessionRollsBackRootsOnFailure(t *testing.T) { + _, url := newRescanServer(t, http.StatusInternalServerError) + mgr := newTestManager(t) + original := []string{"/work/task-1/backend"} + execution := &AgentExecution{ + ID: "exec-1", SessionID: "session-1", + WorkspaceSourceRoots: original, + agentctl: newTestAgentctlClient(t, url, newTestLogger()), + } + require.NoError(t, mgr.executionStore.Add(execution)) + + err := mgr.RescanWorkspaceForSession(context.Background(), "session-1", "/work/task-1", + []string{"/work/task-1/backend", "/work/task-1/frontend"}) + + require.ErrorContains(t, err, "rescan workspace via agentctl") + require.Equal(t, original, execution.WorkspaceSourceRoots, + "a failed rescan must restore the roots agentctl is actually tracking") +} + +func TestRescanWorkspaceForSessionSkipsWithoutExecutionOrClient(t *testing.T) { + ctx := context.Background() + mgr := newTestManager(t) + + require.ErrorContains(t, mgr.RescanWorkspaceForSession(ctx, "", ""), "session_id is required") + require.NoError(t, mgr.RescanWorkspaceForSession(ctx, "session-absent", ""), + "no execution means agentctl is not running; the next launch rebuilds trackers") + + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-1", SessionID: "session-1"})) + require.NoError(t, mgr.RescanWorkspaceForSession(ctx, "session-1", ""), + "an execution without an agentctl client has nothing to rescan") +} + +func TestOptionalWorkspaceSourceRoots(t *testing.T) { + current := []string{"/work/backend"} + + unchanged := optionalWorkspaceSourceRoots(current, nil) + require.Equal(t, current, unchanged) + require.NotSame(t, ¤t[0], &unchanged[0], + "the caller must get a copy so a later mutation cannot reach the execution") + + replaced := optionalWorkspaceSourceRoots(current, [][]string{{"/work/frontend"}}) + require.Equal(t, []string{"/work/frontend"}, replaced) + + cleared := optionalWorkspaceSourceRoots(current, [][]string{nil}) + require.Empty(t, cleared, "an explicit empty root list clears the tracked roots") +} + +// TestManagerStartRecoversInstancesIntoStore pins the restart path: instances +// a backend reports as still running are re-tracked so the frontend reattaches +// instead of creating a duplicate execution. +func TestManagerStartRecoversInstancesIntoStore(t *testing.T) { + log := newTestRegistryLogger() + registry := NewExecutorRegistry(log) + registry.Register(&MockExecutor{ + name: executor.NameStandalone, + recoverInstances: []*ExecutorInstance{{ + InstanceID: "exec-recovered", + TaskID: "task-1", + SessionID: "session-1", + ContainerID: "container-1", + ContainerIP: "10.0.0.2", + WorkspacePath: "/workspace", + RuntimeName: executor.NameStandalone, + }}, + }) + mgr := NewManager(newTestRegistry(), &MockEventBus{}, registry, nil, nil, nil, + ExecutorFallbackWarn, "", log) + cleanupManagerStopCh(t, mgr) + t.Cleanup(func() { _ = mgr.Stop() }) + + require.NoError(t, mgr.Start(context.Background())) + + execution, ok := mgr.GetExecutionBySessionID("session-1") + require.True(t, ok, "a recovered instance must be re-tracked under its session") + require.Equal(t, "exec-recovered", execution.ID) + require.Equal(t, "container-1", execution.ContainerID) + require.Equal(t, "/workspace", execution.WorkspacePath) +} + +func TestManagerStartWithoutRegistryIsNoOp(t *testing.T) { + mgr := newTestManager(t) + + require.NoError(t, mgr.Start(context.Background())) + require.Empty(t, mgr.ListExecutions()) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/process_runner_session_test.go b/apps/backend/internal/agent/runtime/lifecycle/process_runner_session_test.go new file mode 100644 index 0000000000..d4b0932141 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/process_runner_session_test.go @@ -0,0 +1,317 @@ +package lifecycle + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "github.com/stretchr/testify/require" + + agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" +) + +// fakeAgentctlProcessServer serves the agentctl process endpoints the lifecycle +// manager drives, recording every request so the arguments the manager sent can +// be asserted rather than merely that the call returned nil. +type fakeAgentctlProcessServer struct { + mu sync.Mutex + + startRequests []agentctl.StartProcessRequest + stopIDs []string + listSessions []string + fetchedIDs []string + + startStatus agentctl.ProcessStatus + knownProcess string + sessionProcs []agentctl.ProcessInfo + stopShouldErr bool + healthy bool +} + +func newFakeAgentctlProcessServer(t *testing.T) (*fakeAgentctlProcessServer, *agentctl.Client) { + t.Helper() + f := &fakeAgentctlProcessServer{startStatus: "running", healthy: true} + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + f.mu.Lock() + defer f.mu.Unlock() + switch { + case r.URL.Path == "/health": + if !f.healthy { + w.WriteHeader(http.StatusServiceUnavailable) + return + } + w.WriteHeader(http.StatusOK) + case r.URL.Path == "/api/v1/processes/start": + var req agentctl.StartProcessRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + // t.Errorf, not require: this runs on the server's goroutine and + // require's FailNow/Goexit is only valid on the test goroutine. + t.Errorf("decode start process request: %v", err) + w.WriteHeader(http.StatusBadRequest) + return + } + f.startRequests = append(f.startRequests, req) + _ = json.NewEncoder(w).Encode(map[string]any{ + "process": agentctl.ProcessInfo{ + ID: "proc-1", SessionID: req.SessionID, Command: req.Command, Status: f.startStatus, + }, + }) + case r.URL.Path == "/api/v1/processes/stop": + var req struct { + ProcessID string `json:"process_id"` + } + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + t.Errorf("decode stop process request: %v", err) + w.WriteHeader(http.StatusBadRequest) + return + } + f.stopIDs = append(f.stopIDs, req.ProcessID) + if f.stopShouldErr { + _ = json.NewEncoder(w).Encode(map[string]any{"success": false, "error": "already gone"}) + return + } + _ = json.NewEncoder(w).Encode(map[string]any{"success": true}) + case r.URL.Path == "/api/v1/processes": + f.listSessions = append(f.listSessions, r.URL.Query().Get("session_id")) + _ = json.NewEncoder(w).Encode(f.sessionProcs) + case strings.HasPrefix(r.URL.Path, "/api/v1/processes/"): + id := strings.TrimPrefix(r.URL.Path, "/api/v1/processes/") + f.fetchedIDs = append(f.fetchedIDs, id) + if f.knownProcess == "" || id != f.knownProcess { + w.WriteHeader(http.StatusNotFound) + return + } + _ = json.NewEncoder(w).Encode(agentctl.ProcessInfo{ID: id, Status: "running"}) + default: + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(server.Close) + + return f, newTestAgentctlClient(t, server.URL, newTestLogger()) +} + +func (f *fakeAgentctlProcessServer) snapshotStarts() []agentctl.StartProcessRequest { + f.mu.Lock() + defer f.mu.Unlock() + return append([]agentctl.StartProcessRequest(nil), f.startRequests...) +} + +func (f *fakeAgentctlProcessServer) snapshotStops() []string { + f.mu.Lock() + defer f.mu.Unlock() + return append([]string(nil), f.stopIDs...) +} + +func newProcessRunnerManager(t *testing.T, client *agentctl.Client) (*Manager, *AgentExecution) { + t.Helper() + mgr := newTestManager(t) + execution := &AgentExecution{ + ID: "exec-1", TaskID: "task-1", SessionID: "session-1", agentctl: client, + } + require.NoError(t, mgr.executionStore.Add(execution)) + return mgr, execution +} + +// TestStartProcessForwardsRequestAndManagedGoCache pins what actually reaches +// agentctl, including the managed GOCACHE overlay that the execution carries in +// metadata rather than in the caller's request. +func TestStartProcessForwardsRequestAndManagedGoCache(t *testing.T) { + fake, client := newFakeAgentctlProcessServer(t) + mgr, execution := newProcessRunnerManager(t, client) + execution.setMetadataValue(managedGoCacheMetadataKey, "/cache/go-build") + + info, err := mgr.StartProcess(context.Background(), StartProcessRequest{ + SessionID: "session-1", + Kind: "script", + ScriptName: "setup", + Command: "make setup", + WorkingDir: "/workspace", + Env: map[string]string{"CI": "1"}, + }) + + require.NoError(t, err) + require.Equal(t, "proc-1", info.ID) + + starts := fake.snapshotStarts() + require.Len(t, starts, 1) + require.Equal(t, "session-1", starts[0].SessionID) + require.Equal(t, agentctl.ProcessKind("script"), starts[0].Kind) + require.Equal(t, "setup", starts[0].ScriptName) + require.Equal(t, "make setup", starts[0].Command) + require.Equal(t, "/workspace", starts[0].WorkingDir) + require.Equal(t, "1", starts[0].Env["CI"], "the caller's env must not be dropped") + require.Equal(t, "/cache/go-build", starts[0].Env["GOCACHE"], + "the managed cache is an install-wide setting the caller never sends") +} + +func TestProcessEnvironmentLeavesRequestUntouchedWithoutManagedCache(t *testing.T) { + execution := &AgentExecution{ID: "exec-1"} + requested := map[string]string{"CI": "1"} + + require.Equal(t, requested, processEnvironment(execution, requested)) + + execution.setMetadataValue(managedGoCacheMetadataKey, "/cache/go-build") + merged := processEnvironment(execution, requested) + require.Equal(t, "/cache/go-build", merged["GOCACHE"]) + require.NotContains(t, requested, "GOCACHE", + "the caller's map must not be mutated in place") +} + +func TestProcessRunnerEntryPointsValidateArguments(t *testing.T) { + ctx := context.Background() + mgr := newTestManager(t) + + _, err := mgr.StartProcess(ctx, StartProcessRequest{}) + require.ErrorContains(t, err, "session_id is required") + _, err = mgr.StartProcess(ctx, StartProcessRequest{SessionID: "session-absent"}) + require.ErrorIs(t, err, ErrNoExecutionForSession) + + require.ErrorContains(t, mgr.WaitForAgentctlReadyForSession(ctx, ""), "session_id is required") + require.ErrorIs(t, mgr.WaitForAgentctlReadyForSession(ctx, "session-absent"), ErrNoExecutionForSession) + + require.ErrorContains(t, mgr.StopProcessForSession(ctx, "", "proc-1"), "session_id is required") + require.ErrorContains(t, mgr.StopProcessForSession(ctx, "session-1", ""), "process_id is required") + require.ErrorIs(t, mgr.StopProcessForSession(ctx, "session-absent", "proc-1"), ErrNoExecutionForSession) + + _, err = mgr.ListProcesses(ctx, "") + require.ErrorContains(t, err, "session_id is required") + _, err = mgr.ListProcesses(ctx, "session-absent") + require.ErrorIs(t, err, ErrNoExecutionForSession) + + _, err = mgr.GetProcess(ctx, "", false) + require.ErrorContains(t, err, "process_id is required") + require.ErrorContains(t, mgr.StopProcess(ctx, ""), "process_id is required") +} + +func TestProcessRunnerEntryPointsRequireAgentctlClient(t *testing.T) { + ctx := context.Background() + mgr := newTestManager(t) + require.NoError(t, mgr.executionStore.Add(&AgentExecution{ID: "exec-1", SessionID: "session-1"})) + + _, err := mgr.StartProcess(ctx, StartProcessRequest{SessionID: "session-1"}) + require.ErrorContains(t, err, "agentctl client not available for session session-1") + require.ErrorContains(t, mgr.WaitForAgentctlReadyForSession(ctx, "session-1"), + "agentctl client not available") + require.ErrorContains(t, mgr.StopProcessForSession(ctx, "session-1", "proc-1"), + "agentctl client not available") + _, err = mgr.ListProcesses(ctx, "session-1") + require.ErrorContains(t, err, "agentctl client not available") + + // Executions without a client are skipped by the cross-execution scans. + _, err = mgr.GetProcess(ctx, "proc-1", false) + require.ErrorContains(t, err, "process not found: proc-1") + require.ErrorContains(t, mgr.StopProcess(ctx, "proc-1"), "process not found: proc-1") + require.NoError(t, mgr.StopAllProcesses(ctx)) +} + +func TestWaitForAgentctlReadyForSessionProbesHealth(t *testing.T) { + fake, client := newFakeAgentctlProcessServer(t) + mgr, _ := newProcessRunnerManager(t, client) + + require.NoError(t, mgr.WaitForAgentctlReadyForSession(context.Background(), "session-1")) + + fake.mu.Lock() + fake.healthy = false + fake.mu.Unlock() + ctx, cancel := context.WithCancel(context.Background()) + cancel() + require.Error(t, mgr.WaitForAgentctlReadyForSession(ctx, "session-1")) +} + +func TestStopProcessForSessionForwardsProcessID(t *testing.T) { + fake, client := newFakeAgentctlProcessServer(t) + mgr, _ := newProcessRunnerManager(t, client) + + require.NoError(t, mgr.StopProcessForSession(context.Background(), "session-1", "proc-7")) + + require.Equal(t, []string{"proc-7"}, fake.snapshotStops()) +} + +func TestStopProcessForSessionSurfacesAgentctlFailure(t *testing.T) { + fake, client := newFakeAgentctlProcessServer(t) + fake.stopShouldErr = true + mgr, _ := newProcessRunnerManager(t, client) + + err := mgr.StopProcessForSession(context.Background(), "session-1", "proc-7") + + require.ErrorContains(t, err, "already gone") +} + +// TestStopProcessScansExecutionsForOwner pins the cross-execution scan: the +// caller supplies only a process ID, so the manager must locate the owning +// execution before issuing the stop. +func TestStopProcessScansExecutionsForOwner(t *testing.T) { + fake, client := newFakeAgentctlProcessServer(t) + fake.knownProcess = "proc-7" + mgr, _ := newProcessRunnerManager(t, client) + + require.NoError(t, mgr.StopProcess(context.Background(), "proc-7")) + require.Equal(t, []string{"proc-7"}, fake.snapshotStops()) + + require.ErrorContains(t, mgr.StopProcess(context.Background(), "proc-unknown"), + "process not found: proc-unknown") + require.Equal(t, []string{"proc-7"}, fake.snapshotStops(), + "an unowned process ID must not produce a stop call") +} + +func TestGetProcessScansExecutions(t *testing.T) { + fake, client := newFakeAgentctlProcessServer(t) + fake.knownProcess = "proc-7" + mgr, _ := newProcessRunnerManager(t, client) + + got, err := mgr.GetProcess(context.Background(), "proc-7", true) + require.NoError(t, err) + require.Equal(t, "proc-7", got.ID) + + _, err = mgr.GetProcess(context.Background(), "proc-unknown", false) + require.ErrorContains(t, err, "process not found: proc-unknown") +} + +func TestListProcessesScopesToSession(t *testing.T) { + fake, client := newFakeAgentctlProcessServer(t) + fake.sessionProcs = []agentctl.ProcessInfo{{ID: "proc-1"}, {ID: "proc-2"}} + mgr, _ := newProcessRunnerManager(t, client) + + got, err := mgr.ListProcesses(context.Background(), "session-1") + + require.NoError(t, err) + require.Len(t, got, 2) + fake.mu.Lock() + defer fake.mu.Unlock() + require.Equal(t, []string{"session-1"}, fake.listSessions, + "the list must be scoped to the session, not the whole agentctl instance") +} + +// TestStopAllProcessesStopsEveryListedProcess pins the shutdown sweep: every +// process agentctl reports for the session is stopped, and per-process failures +// are aggregated rather than aborting the sweep. +func TestStopAllProcessesStopsEveryListedProcess(t *testing.T) { + fake, client := newFakeAgentctlProcessServer(t) + fake.sessionProcs = []agentctl.ProcessInfo{{ID: "proc-1"}, {ID: "proc-2"}} + mgr, _ := newProcessRunnerManager(t, client) + + require.NoError(t, mgr.StopAllProcesses(context.Background())) + require.Equal(t, []string{"proc-1", "proc-2"}, fake.snapshotStops()) + + fake.mu.Lock() + fake.stopShouldErr = true + fake.mu.Unlock() + err := mgr.StopAllProcesses(context.Background()) + require.ErrorContains(t, err, "already gone") + require.Len(t, fake.snapshotStops(), 4, + "a failing stop must not abort the sweep over the remaining processes") +} + +func TestIsTerminalProcessStatus(t *testing.T) { + require.True(t, isTerminalProcessStatus("exited")) + require.True(t, isTerminalProcessStatus("failed")) + require.True(t, isTerminalProcessStatus("stopped")) + require.False(t, isTerminalProcessStatus("running")) + require.False(t, isTerminalProcessStatus("")) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/session_history_resume_test.go b/apps/backend/internal/agent/runtime/lifecycle/session_history_resume_test.go new file mode 100644 index 0000000000..5158f28441 --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/session_history_resume_test.go @@ -0,0 +1,153 @@ +package lifecycle + +import ( + "os" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +func newSessionHistoryManagerForTest(t *testing.T) (*SessionHistoryManager, string) { + t.Helper() + dir := t.TempDir() + mgr, err := NewSessionHistoryManager(dir, "", newTestLogger()) + require.NoError(t, err) + return mgr, dir +} + +// TestGenerateResumeContextRendersEveryEntryKind pins the fork-session prompt: +// each recorded entry kind gets its own labelled section, and the user's new +// request is appended after the history so the agent knows what to do next. +func TestGenerateResumeContextRendersEveryEntryKind(t *testing.T) { + mgr, _ := newSessionHistoryManagerForTest(t) + now := time.Now() + for _, entry := range []HistoryEntry{ + {Timestamp: now, Type: "user_message", Content: "add retries"}, + {Timestamp: now, Type: "agent_message", Content: "added them"}, + {Timestamp: now, Type: historyEntryTypeToolCall, ToolName: "edit_file"}, + {Timestamp: now, Type: "tool_result", ToolName: "edit_file", Content: "3 lines changed"}, + {Timestamp: now, Type: "unrecognised_kind", Content: "should be ignored"}, + } { + require.NoError(t, mgr.AppendEntry("session-1", entry)) + } + + prompt, err := mgr.GenerateResumeContext("session-1", "now add tests") + + require.NoError(t, err) + require.Contains(t, prompt, "[USER]: add retries") + require.Contains(t, prompt, "[ASSISTANT]: added them") + require.Contains(t, prompt, "[TOOL CALL: edit_file]") + require.Contains(t, prompt, "[TOOL RESULT: edit_file] 3 lines changed") + require.NotContains(t, prompt, "should be ignored", + "an unknown entry kind must be skipped rather than rendered raw") + require.Contains(t, prompt, "=== CURRENT REQUEST ===\nnow add tests") + require.Contains(t, prompt, "Do not repeat work that was already completed.") + require.Less(t, strings.Index(prompt, "add retries"), strings.Index(prompt, "now add tests"), + "history must precede the new request so the agent reads it as prior context") +} + +func TestGenerateResumeContextReturnsPromptUnchangedWithoutHistory(t *testing.T) { + mgr, _ := newSessionHistoryManagerForTest(t) + + prompt, err := mgr.GenerateResumeContext("session-fresh", "do the thing") + + require.NoError(t, err) + require.Equal(t, "do the thing", prompt, + "a session with no history must not be given an empty RESUME CONTEXT wrapper") +} + +// TestGenerateResumeContextIgnoresContentlessEntries pins the second early +// return: entries exist, but none render, so the raw prompt is used. +func TestGenerateResumeContextIgnoresContentlessEntries(t *testing.T) { + mgr, _ := newSessionHistoryManagerForTest(t) + require.NoError(t, mgr.AppendEntry("session-1", HistoryEntry{ + Timestamp: time.Now(), Type: "session_started", + })) + + prompt, err := mgr.GenerateResumeContext("session-1", "do the thing") + + require.NoError(t, err) + require.Equal(t, "do the thing", prompt) +} + +func TestGenerateResumeContextSurfacesReadFailure(t *testing.T) { + mgr, _ := newSessionHistoryManagerForTest(t) + + prompt, err := mgr.GenerateResumeContext("", "do the thing") + + require.ErrorContains(t, err, "session ID is required") + require.Equal(t, "do the thing", prompt, + "a failed read must still return the caller's prompt so the launch can proceed") +} + +// TestGenerateResumeContextTruncatesLongEntries pins the size guard: an +// unbounded transcript would blow past the agent's context window. +func TestGenerateResumeContextTruncatesLongEntries(t *testing.T) { + mgr, _ := newSessionHistoryManagerForTest(t) + require.NoError(t, mgr.AppendEntry("session-1", HistoryEntry{ + Timestamp: time.Now(), Type: "user_message", Content: strings.Repeat("a", 5000), + })) + require.NoError(t, mgr.AppendEntry("session-1", HistoryEntry{ + Timestamp: time.Now(), Type: "tool_result", ToolName: "read_file", + Content: strings.Repeat("b", 5000), + })) + + prompt, err := mgr.GenerateResumeContext("session-1", "continue") + + require.NoError(t, err) + require.Equal(t, 2, strings.Count(prompt, "... [truncated]")) + require.NotContains(t, prompt, strings.Repeat("a", 2001), + "user messages are capped at 2000 characters") + require.NotContains(t, prompt, strings.Repeat("b", 501), + "tool results are capped at 500 characters — they are the bulkiest entries") +} + +func TestTruncateForContext(t *testing.T) { + require.Equal(t, "short", truncateForContext("short", 10)) + require.Equal(t, "exactly10c", truncateForContext("exactly10c", 10)) + require.Equal(t, "0123456789... [truncated]", truncateForContext("0123456789abc", 10)) +} + +func TestHasHistoryReflectsStoredEntries(t *testing.T) { + mgr, dir := newSessionHistoryManagerForTest(t) + + require.False(t, mgr.HasHistory(""), "an empty session ID has no history") + require.False(t, mgr.HasHistory("session-1")) + + require.NoError(t, os.WriteFile(filepath.Join(dir, "session-empty.jsonl"), nil, 0o600)) + require.False(t, mgr.HasHistory("session-empty"), "a zero-byte file is not history") + + require.NoError(t, mgr.AppendEntry("session-1", HistoryEntry{ + Timestamp: time.Now(), Type: "user_message", Content: "hi", + })) + require.True(t, mgr.HasHistory("session-1")) +} + +func TestDeleteHistoryRemovesFileAndIsIdempotent(t *testing.T) { + mgr, dir := newSessionHistoryManagerForTest(t) + require.NoError(t, mgr.AppendEntry("session-1", HistoryEntry{ + Timestamp: time.Now(), Type: "user_message", Content: "hi", + })) + path := filepath.Join(dir, "session-1.jsonl") + require.FileExists(t, path) + + require.NoError(t, mgr.DeleteHistory("session-1")) + require.NoFileExists(t, path) + + require.NoError(t, mgr.DeleteHistory("session-1"), + "deleting an already-absent history must not error — task cleanup runs more than once") + require.ErrorContains(t, mgr.DeleteHistory(""), "session ID is required") +} + +// TestHistoryFilePathSanitizesSessionID pins the traversal guard: a session ID +// carrying separators must not write outside the history directory. +func TestHistoryFilePathSanitizesSessionID(t *testing.T) { + mgr, dir := newSessionHistoryManagerForTest(t) + + require.Equal(t, filepath.Join(dir, ".._.._etc_passwd.jsonl"), + mgr.historyFilePath("../../etc/passwd")) + require.Equal(t, filepath.Join(dir, "a_b.jsonl"), mgr.historyFilePath(`a\b`)) +} diff --git a/apps/backend/internal/agent/runtime/lifecycle/streams_callbacks_test.go b/apps/backend/internal/agent/runtime/lifecycle/streams_callbacks_test.go new file mode 100644 index 0000000000..f6f92e8e4e --- /dev/null +++ b/apps/backend/internal/agent/runtime/lifecycle/streams_callbacks_test.go @@ -0,0 +1,184 @@ +package lifecycle + +import ( + "context" + "testing" + "time" + + "github.com/stretchr/testify/require" + + agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" +) + +// recordedWorkspaceCallback captures which handler fired, for which execution, +// and with what payload — the three things a dropped wiring hop silently loses. +type recordedWorkspaceCallback struct { + name string + execution *AgentExecution + payload any +} + +func newRecordingStreamManager(t *testing.T, recorded *[]recordedWorkspaceCallback) *StreamManager { + t.Helper() + record := func(name string) func(*AgentExecution, any) { + return func(execution *AgentExecution, payload any) { + *recorded = append(*recorded, recordedWorkspaceCallback{name: name, execution: execution, payload: payload}) + } + } + stopCh := newTestStopCh(t) + sm := NewStreamManager(newTestLogger(), StreamCallbacks{ + OnShellOutput: func(e *AgentExecution, data string) { record("shell_output")(e, data) }, + OnShellExit: func(e *AgentExecution, code int) { record("shell_exit")(e, code) }, + OnGitStatus: func(e *AgentExecution, u *agentctl.GitStatusUpdate) { + record("git_status")(e, u) + }, + OnGitCommit: func(e *AgentExecution, c *agentctl.GitCommitNotification) { + record("git_commit")(e, c) + }, + OnGitReset: func(e *AgentExecution, r *agentctl.GitResetNotification) { + record("git_reset")(e, r) + }, + OnBranchSwitch: func(e *AgentExecution, b *agentctl.GitBranchSwitchNotification) { + record("branch_switch")(e, b) + }, + OnFileChange: func(e *AgentExecution, n *agentctl.FileChangeNotification) { + record("file_change")(e, n) + }, + OnProcessOutput: func(e *AgentExecution, o *agentctl.ProcessOutput) { + record("process_output")(e, o) + }, + OnProcessStatus: func(e *AgentExecution, s *agentctl.ProcessStatusUpdate) { + record("process_status")(e, s) + }, + }, nil, stopCh) + cleanupStreamManager(t, stopCh, sm) + return sm +} + +// TestBuildWorkspaceCallbacksForwardsEveryHopWithItsExecution pins the full +// wiring chain from the agentctl workspace stream to the manager's handlers. +// A dropped hop here is invisible to any test that calls the manager handler +// directly: the stream would simply stop delivering that notification. +func TestBuildWorkspaceCallbacksForwardsEveryHopWithItsExecution(t *testing.T) { + var recorded []recordedWorkspaceCallback + sm := newRecordingStreamManager(t, &recorded) + execution := &AgentExecution{ID: "exec-1", TaskID: "task-1", SessionID: "session-1"} + + callbacks := sm.buildWorkspaceCallbacks(execution) + + status := &agentctl.GitStatusUpdate{Branch: "feature/cover", Timestamp: time.Now()} + commit := &agentctl.GitCommitNotification{CommitSHA: "abc123"} + reset := &agentctl.GitResetNotification{PreviousHead: "head-after"} + branchSwitch := &agentctl.GitBranchSwitchNotification{CurrentBranch: "feature/cover"} + fileChange := &agentctl.FileChangeNotification{Path: "src/main.go"} + output := &agentctl.ProcessOutput{ProcessID: "proc-1", Data: "hello"} + procStatus := &agentctl.ProcessStatusUpdate{ProcessID: "proc-1", Status: "failed"} + + callbacks.OnShellOutput("build ok\n") + callbacks.OnShellExit(137) + callbacks.OnGitStatus(status) + callbacks.OnGitCommit(commit) + callbacks.OnGitReset(reset) + callbacks.OnBranchSwitch(branchSwitch) + callbacks.OnFileChange(fileChange) + callbacks.OnProcessOutput(output) + callbacks.OnProcessStatus(procStatus) + // These two only log; they must not panic or reach a handler. + callbacks.OnConnected() + callbacks.OnError("stream reset by peer") + + require.Len(t, recorded, 9) + wantOrder := []string{ + "shell_output", "shell_exit", "git_status", "git_commit", "git_reset", + "branch_switch", "file_change", "process_output", "process_status", + } + for i, want := range wantOrder { + require.Equal(t, want, recorded[i].name, "hop %d", i) + require.Same(t, execution, recorded[i].execution, + "%s must be attributed to the execution the stream belongs to", want) + } + require.Equal(t, "build ok\n", recorded[0].payload) + require.Equal(t, 137, recorded[1].payload) + require.Same(t, status, recorded[2].payload) + require.Same(t, commit, recorded[3].payload) + require.Same(t, reset, recorded[4].payload) + require.Same(t, branchSwitch, recorded[5].payload) + require.Same(t, fileChange, recorded[6].payload) + require.Same(t, output, recorded[7].payload) + require.Same(t, procStatus, recorded[8].payload) +} + +// TestBuildWorkspaceCallbacksToleratesUnwiredHandlers pins the nil guards: a +// StreamManager built without a given handler must drop the notification, not +// panic and tear down the workspace stream goroutine. +func TestBuildWorkspaceCallbacksToleratesUnwiredHandlers(t *testing.T) { + stopCh := newTestStopCh(t) + sm := NewStreamManager(newTestLogger(), StreamCallbacks{}, nil, stopCh) + cleanupStreamManager(t, stopCh, sm) + + callbacks := sm.buildWorkspaceCallbacks(&AgentExecution{ID: "exec-1"}) + + callbacks.OnShellOutput("data") + callbacks.OnShellExit(0) + callbacks.OnGitStatus(&agentctl.GitStatusUpdate{}) + callbacks.OnGitCommit(&agentctl.GitCommitNotification{}) + callbacks.OnGitReset(&agentctl.GitResetNotification{}) + callbacks.OnBranchSwitch(&agentctl.GitBranchSwitchNotification{}) + callbacks.OnFileChange(&agentctl.FileChangeNotification{}) + callbacks.OnProcessOutput(&agentctl.ProcessOutput{}) + callbacks.OnProcessStatus(&agentctl.ProcessStatusUpdate{}) + callbacks.OnConnected() + callbacks.OnError("boom") +} + +// TestStreamManagerStartRejectsWorkAfterWait pins the drain barrier: once Wait +// has run, no new stream goroutine may be spawned, and a caller waiting on +// `ready` is still released rather than blocked forever. +func TestStreamManagerStartRejectsWorkAfterWait(t *testing.T) { + stopCh := newTestStopCh(t) + sm := NewStreamManager(newTestLogger(), StreamCallbacks{}, nil, stopCh) + // Registered before the Wait below and before any t.Fatal path, so the + // drain still runs if this test fails early. + cleanupStreamManager(t, stopCh, sm) + sm.Wait() + + execution := &AgentExecution{ID: "exec-1", SessionID: "session-1"} + ready := make(chan struct{}) + sm.ConnectWorkspaceStream(execution, ready) + + select { + case <-ready: + case <-time.After(100 * time.Millisecond): + t.Fatal("ready was not closed after a rejected start; callers would block forever") + } + + // ConnectMCPStream has no ready channel — it simply must not spawn. + // The registered cleanup drains; no trailing Wait needed here. + sm.ConnectMCPStream(execution) +} + +func TestStopChannelContextErrReportsFirstClosedSignal(t *testing.T) { + primary := make(chan struct{}) + secondary := make(chan struct{}) + ctx := &stopChannelContext{Context: context.Background(), primary: primary, secondary: secondary} + + require.NoError(t, ctx.Err(), "no signal has fired yet") + + close(secondary) + require.ErrorIs(t, ctx.Err(), context.Canceled, + "the StreamManager's internal drain signal must cancel the stream context") + + close(primary) + require.ErrorIs(t, ctx.Err(), context.Canceled) +} + +func TestStopChannelContextErrFallsBackToParent(t *testing.T) { + parent, cancel := context.WithCancel(context.Background()) + ctx := &stopChannelContext{Context: parent} + + require.NoError(t, ctx.Err()) + + cancel() + require.ErrorIs(t, ctx.Err(), context.Canceled, + "with no stop channels the parent context still governs") +}