fix(docker): route user shell to container instead of host (#638)
This commit is contained in:
@@ -572,6 +572,42 @@ func (m *Manager) IsRemoteSession(ctx context.Context, sessionID string) bool {
|
||||
return info.RuntimeName == string(executor.NameSprites) || info.RuntimeName == string(executor.NameRemoteDocker)
|
||||
}
|
||||
|
||||
// ShouldUseContainerShell checks whether a session's shell should run inside a container/sandbox
|
||||
// (via agentctl) rather than on the host. This is true for Docker, Sprites, and remote executors.
|
||||
// It first checks the in-memory execution store, then falls back to the database.
|
||||
func (m *Manager) ShouldUseContainerShell(ctx context.Context, sessionID string) bool {
|
||||
// Check in-memory execution first (fast path).
|
||||
if execution, exists := m.executionStore.GetBySessionID(sessionID); exists {
|
||||
// Docker and Sprites executors run shells inside the container/sandbox
|
||||
if execution.RuntimeName == string(executor.NameDocker) ||
|
||||
execution.RuntimeName == string(executor.NameSprites) {
|
||||
return true
|
||||
}
|
||||
if execution.Metadata != nil {
|
||||
if isRemote, ok := execution.Metadata[MetadataKeyIsRemote].(bool); ok && isRemote {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// Fall back to database records (post-restart, execution not yet recreated).
|
||||
if m.workspaceInfoProvider == nil {
|
||||
return false
|
||||
}
|
||||
info, err := m.workspaceInfoProvider.GetWorkspaceInfoForSession(ctx, "", sessionID)
|
||||
if err != nil || info == nil {
|
||||
return false
|
||||
}
|
||||
if models.IsContainerizedExecutorType(models.ExecutorType(info.ExecutorType)) {
|
||||
return true
|
||||
}
|
||||
// Backwards compatibility: old records may only have RuntimeName set.
|
||||
return info.RuntimeName == string(executor.NameDocker) ||
|
||||
info.RuntimeName == string(executor.NameSprites) ||
|
||||
info.RuntimeName == string(executor.NameRemoteDocker)
|
||||
}
|
||||
|
||||
// GetAvailableCommandsForSession returns the available slash commands for a session.
|
||||
// Returns nil if the session doesn't exist or has no commands stored.
|
||||
//
|
||||
|
||||
@@ -621,6 +621,114 @@ func TestIsRemoteSession(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestShouldUseContainerShell(t *testing.T) {
|
||||
t.Run("in-memory execution with docker runtime", func(t *testing.T) {
|
||||
store := NewExecutionStore()
|
||||
store.Add(&AgentExecution{
|
||||
ID: "exec-1",
|
||||
SessionID: "session-1",
|
||||
RuntimeName: string(executor.NameDocker),
|
||||
Status: v1.AgentStatusRunning,
|
||||
})
|
||||
mgr := &Manager{executionStore: store}
|
||||
require.True(t, mgr.ShouldUseContainerShell(context.Background(), "session-1"))
|
||||
})
|
||||
|
||||
t.Run("in-memory execution with sprites runtime", func(t *testing.T) {
|
||||
store := NewExecutionStore()
|
||||
store.Add(&AgentExecution{
|
||||
ID: "exec-2",
|
||||
SessionID: "session-2",
|
||||
RuntimeName: string(executor.NameSprites),
|
||||
Status: v1.AgentStatusRunning,
|
||||
})
|
||||
mgr := &Manager{executionStore: store}
|
||||
require.True(t, mgr.ShouldUseContainerShell(context.Background(), "session-2"))
|
||||
})
|
||||
|
||||
t.Run("in-memory execution with is_remote metadata", func(t *testing.T) {
|
||||
store := NewExecutionStore()
|
||||
store.Add(&AgentExecution{
|
||||
ID: "exec-3",
|
||||
SessionID: "session-3",
|
||||
RuntimeName: string(executor.NameStandalone),
|
||||
Status: v1.AgentStatusRunning,
|
||||
Metadata: map[string]interface{}{MetadataKeyIsRemote: true},
|
||||
})
|
||||
mgr := &Manager{executionStore: store}
|
||||
require.True(t, mgr.ShouldUseContainerShell(context.Background(), "session-3"))
|
||||
})
|
||||
|
||||
t.Run("in-memory execution with standalone runtime", func(t *testing.T) {
|
||||
store := NewExecutionStore()
|
||||
store.Add(&AgentExecution{
|
||||
ID: "exec-4",
|
||||
SessionID: "session-4",
|
||||
RuntimeName: string(executor.NameStandalone),
|
||||
Status: v1.AgentStatusRunning,
|
||||
})
|
||||
mgr := &Manager{executionStore: store}
|
||||
require.False(t, mgr.ShouldUseContainerShell(context.Background(), "session-4"))
|
||||
})
|
||||
|
||||
t.Run("not in memory, DB returns local_docker executor type", func(t *testing.T) {
|
||||
store := NewExecutionStore()
|
||||
provider := &mockWorkspaceInfoProvider{
|
||||
infos: map[string]*WorkspaceInfo{
|
||||
"session-5": {ExecutorType: string(models.ExecutorTypeLocalDocker)},
|
||||
},
|
||||
}
|
||||
mgr := &Manager{executionStore: store, workspaceInfoProvider: provider}
|
||||
require.True(t, mgr.ShouldUseContainerShell(context.Background(), "session-5"))
|
||||
})
|
||||
|
||||
t.Run("not in memory, DB returns sprites executor type", func(t *testing.T) {
|
||||
store := NewExecutionStore()
|
||||
provider := &mockWorkspaceInfoProvider{
|
||||
infos: map[string]*WorkspaceInfo{
|
||||
"session-6": {ExecutorType: string(models.ExecutorTypeSprites)},
|
||||
},
|
||||
}
|
||||
mgr := &Manager{executionStore: store, workspaceInfoProvider: provider}
|
||||
require.True(t, mgr.ShouldUseContainerShell(context.Background(), "session-6"))
|
||||
})
|
||||
|
||||
t.Run("not in memory, DB returns docker runtime name", func(t *testing.T) {
|
||||
store := NewExecutionStore()
|
||||
provider := &mockWorkspaceInfoProvider{
|
||||
infos: map[string]*WorkspaceInfo{
|
||||
"session-7": {RuntimeName: string(executor.NameDocker)},
|
||||
},
|
||||
}
|
||||
mgr := &Manager{executionStore: store, workspaceInfoProvider: provider}
|
||||
require.True(t, mgr.ShouldUseContainerShell(context.Background(), "session-7"))
|
||||
})
|
||||
|
||||
t.Run("not in memory, DB returns local type", func(t *testing.T) {
|
||||
store := NewExecutionStore()
|
||||
provider := &mockWorkspaceInfoProvider{
|
||||
infos: map[string]*WorkspaceInfo{
|
||||
"session-8": {ExecutorType: string(models.ExecutorTypeLocal)},
|
||||
},
|
||||
}
|
||||
mgr := &Manager{executionStore: store, workspaceInfoProvider: provider}
|
||||
require.False(t, mgr.ShouldUseContainerShell(context.Background(), "session-8"))
|
||||
})
|
||||
|
||||
t.Run("not in memory, DB error returns false", func(t *testing.T) {
|
||||
store := NewExecutionStore()
|
||||
provider := &mockWorkspaceInfoProvider{err: fmt.Errorf("db error")}
|
||||
mgr := &Manager{executionStore: store, workspaceInfoProvider: provider}
|
||||
require.False(t, mgr.ShouldUseContainerShell(context.Background(), "session-9"))
|
||||
})
|
||||
|
||||
t.Run("nil workspaceInfoProvider returns false", func(t *testing.T) {
|
||||
store := NewExecutionStore()
|
||||
mgr := &Manager{executionStore: store}
|
||||
require.False(t, mgr.ShouldUseContainerShell(context.Background(), "nonexistent"))
|
||||
})
|
||||
}
|
||||
|
||||
func TestFallbackAuthMethods(t *testing.T) {
|
||||
t.Run("claude-acp returns auth login method", func(t *testing.T) {
|
||||
methods := fallbackAuthMethods("claude-acp")
|
||||
|
||||
@@ -19,7 +19,6 @@ import (
|
||||
gorillaws "github.com/gorilla/websocket"
|
||||
"go.uber.org/zap"
|
||||
|
||||
"github.com/kandev/kandev/internal/agent/executor"
|
||||
"github.com/kandev/kandev/internal/agent/lifecycle"
|
||||
"github.com/kandev/kandev/internal/agentctl/server/process"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
@@ -213,24 +212,9 @@ func (h *TerminalHandler) HandleTerminalWS(c *gin.Context) {
|
||||
}
|
||||
|
||||
func (h *TerminalHandler) shouldUseWorkspaceShell(ctx context.Context, sessionID string) bool {
|
||||
// Check in-memory execution first (fast path when execution is already running).
|
||||
execution, exists := h.lifecycleMgr.GetExecutionBySessionID(sessionID)
|
||||
if exists {
|
||||
if execution.RuntimeName == string(executor.NameSprites) {
|
||||
return true
|
||||
}
|
||||
if execution.Metadata != nil {
|
||||
if isRemote, ok := execution.Metadata[lifecycle.MetadataKeyIsRemote].(bool); ok && isRemote {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// Execution not in memory — check DB records. After a backend restart the
|
||||
// execution may not exist yet (resume in progress), but the session's
|
||||
// executor type is persisted in the database.
|
||||
return h.lifecycleMgr.IsRemoteSession(ctx, sessionID)
|
||||
// Delegates to lifecycle manager which checks both in-memory execution
|
||||
// and database records for containerized/remote executors.
|
||||
return h.lifecycleMgr.ShouldUseContainerShell(ctx, sessionID)
|
||||
}
|
||||
|
||||
// waitForRemoteExecution polls the lifecycle manager for the session's execution,
|
||||
|
||||
@@ -397,6 +397,18 @@ func IsRemoteExecutorType(t ExecutorType) bool {
|
||||
}
|
||||
}
|
||||
|
||||
// IsContainerizedExecutorType reports whether the given executor type runs
|
||||
// in a container/sandbox where shells must be executed inside the container
|
||||
// via agentctl, not on the host.
|
||||
func IsContainerizedExecutorType(t ExecutorType) bool {
|
||||
switch t {
|
||||
case ExecutorTypeLocalDocker, ExecutorTypeSprites, ExecutorTypeRemoteDocker, ExecutorTypeMockRemote:
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// IsAlwaysResumableRuntime reports whether the given runtime string represents
|
||||
// an executor that can always be resumed even without an explicit resume token.
|
||||
func IsAlwaysResumableRuntime(runtime string) bool {
|
||||
|
||||
Reference in New Issue
Block a user