feat(workflow): add set_session_mode action and preserve mode across reset (#1188)
This commit is contained in:
@@ -341,6 +341,11 @@ func (a *lifecycleAdapter) SetSessionModelBySessionID(ctx context.Context, sessi
|
||||
return a.mgr.SetSessionModelBySessionID(ctx, sessionID, modelID)
|
||||
}
|
||||
|
||||
// SetSessionModeBySessionID applies a session permission mode via ACP session/set_mode.
|
||||
func (a *lifecycleAdapter) SetSessionModeBySessionID(ctx context.Context, sessionID, modeID string) error {
|
||||
return a.mgr.SetSessionModeBySessionID(ctx, sessionID, modeID)
|
||||
}
|
||||
|
||||
// RespondToPermissionBySessionID sends a response to a permission request for a session
|
||||
func (a *lifecycleAdapter) RespondToPermissionBySessionID(ctx context.Context, sessionID, pendingID, optionID string, cancelled bool) error {
|
||||
return a.mgr.RespondToPermissionBySessionID(sessionID, pendingID, optionID, cancelled)
|
||||
|
||||
@@ -205,6 +205,7 @@ var (
|
||||
string(models.OnEnterEnablePlanMode): true,
|
||||
string(models.OnEnterAutoStartAgent): true,
|
||||
string(models.OnEnterResetAgentContext): true,
|
||||
string(models.OnEnterSetSessionMode): true,
|
||||
string(models.OnEnterClearDecisions): true,
|
||||
string(models.OnEnterQueueRunForEachParticipant): true,
|
||||
string(models.OnEnterQueueRun): true,
|
||||
@@ -231,6 +232,16 @@ func convertEvents(e stepEventsYAML) (models.StepEvents, error) {
|
||||
if !validOnEnter[a.Type] {
|
||||
return events, fmt.Errorf("invalid on_enter action type %q", a.Type)
|
||||
}
|
||||
// set_session_mode must carry a non-empty string mode. Modes are
|
||||
// agent-specific (reported dynamically by the agent), so there is no
|
||||
// global allow-list to check against — but a missing or non-string
|
||||
// value would otherwise be silently dropped at compile time, so reject
|
||||
// it here to fail misconfigured templates loudly.
|
||||
if a.Type == string(models.OnEnterSetSessionMode) {
|
||||
if mode, _ := a.Config["mode"].(string); mode == "" {
|
||||
return events, fmt.Errorf("on_enter set_session_mode requires a non-empty string \"mode\" config")
|
||||
}
|
||||
}
|
||||
events.OnEnter = append(events.OnEnter, models.OnEnterAction{
|
||||
Type: models.OnEnterActionType(a.Type),
|
||||
Config: a.Config,
|
||||
|
||||
@@ -3,6 +3,9 @@ package workflows
|
||||
import (
|
||||
"fmt"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/workflow/models"
|
||||
"gopkg.in/yaml.v3"
|
||||
)
|
||||
|
||||
func TestLoadTemplates_AllValid(t *testing.T) {
|
||||
@@ -26,6 +29,57 @@ func TestLoadTemplates_AllValid(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestConvertEvents_SetSessionMode round-trips a set_session_mode on_enter
|
||||
// action through the YAML loader: the action type passes the allow-list and its
|
||||
// "mode" config survives into the typed StepEvents. See issue #1183.
|
||||
func TestConvertEvents_SetSessionMode(t *testing.T) {
|
||||
const yamlDoc = `
|
||||
on_enter:
|
||||
- type: set_session_mode
|
||||
config:
|
||||
mode: acceptEdits
|
||||
`
|
||||
var e stepEventsYAML
|
||||
if err := yaml.Unmarshal([]byte(yamlDoc), &e); err != nil {
|
||||
t.Fatalf("unmarshal yaml: %v", err)
|
||||
}
|
||||
events, err := convertEvents(e)
|
||||
if err != nil {
|
||||
t.Fatalf("convertEvents returned error: %v", err)
|
||||
}
|
||||
if len(events.OnEnter) != 1 {
|
||||
t.Fatalf("expected 1 on_enter action, got %d", len(events.OnEnter))
|
||||
}
|
||||
if events.OnEnter[0].Type != models.OnEnterSetSessionMode {
|
||||
t.Fatalf("unexpected action type %q", events.OnEnter[0].Type)
|
||||
}
|
||||
if mode, _ := events.OnEnter[0].Config["mode"].(string); mode != "acceptEdits" {
|
||||
t.Fatalf("expected mode=acceptEdits, got %q", mode)
|
||||
}
|
||||
}
|
||||
|
||||
// TestConvertEvents_SetSessionModeRejectsMissingMode verifies the loader fails
|
||||
// fast when a set_session_mode action has no usable "mode" config, rather than
|
||||
// silently dropping it at compile time. See issue #1183.
|
||||
func TestConvertEvents_SetSessionModeRejectsMissingMode(t *testing.T) {
|
||||
cases := map[string]string{
|
||||
"no config": "on_enter:\n - type: set_session_mode\n",
|
||||
"empty mode": "on_enter:\n - type: set_session_mode\n config:\n mode: \"\"\n",
|
||||
"non-string": "on_enter:\n - type: set_session_mode\n config:\n mode: 3\n",
|
||||
}
|
||||
for name, doc := range cases {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
var e stepEventsYAML
|
||||
if err := yaml.Unmarshal([]byte(doc), &e); err != nil {
|
||||
t.Fatalf("unmarshal yaml: %v", err)
|
||||
}
|
||||
if _, err := convertEvents(e); err == nil {
|
||||
t.Fatal("expected convertEvents to reject set_session_mode with no usable mode")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoadTemplates_EachHasStartStep(t *testing.T) {
|
||||
templates, err := LoadTemplates()
|
||||
if err != nil {
|
||||
|
||||
@@ -293,6 +293,52 @@ func (m *Manager) AuthenticateBySessionID(ctx context.Context, sessionID, method
|
||||
return execution.agentctl.Authenticate(ctx, methodID)
|
||||
}
|
||||
|
||||
// reapplySessionModeAfterReset re-applies the active session permission mode
|
||||
// (e.g. auto / accept-edits) to a freshly (re)initialized ACP session so the
|
||||
// user's choice survives a context reset instead of silently reverting to the
|
||||
// agent's default.
|
||||
//
|
||||
// The mode to restore is resolved from the persisted session_mode in the DB
|
||||
// (the authoritative, synchronously-written source — see persistSessionMode and
|
||||
// the set_session_mode action), falling back to prev (the in-memory mode state
|
||||
// captured before the reset) only when no provider is wired or nothing is
|
||||
// persisted. Preferring the DB avoids re-applying a stale in-memory mode when a
|
||||
// set_session_mode action persisted a newer mode in the same on_enter batch
|
||||
// before its agent mode event updated modeState. A nil/empty resolved mode is a
|
||||
// no-op. Addresses issue #1183.
|
||||
func (m *Manager) reapplySessionModeAfterReset(ctx context.Context, execution *AgentExecution, newSessionID string, prev *CachedModeState) {
|
||||
if execution.agentctl == nil {
|
||||
return
|
||||
}
|
||||
fallback := ""
|
||||
var availableModes []streams.SessionModeInfo
|
||||
if prev != nil {
|
||||
fallback = prev.CurrentModeID
|
||||
availableModes = prev.AvailableModes
|
||||
}
|
||||
mode := m.effectiveSessionMode(ctx, execution, fallback)
|
||||
if mode == "" {
|
||||
return
|
||||
}
|
||||
if err := execution.agentctl.SetMode(ctx, newSessionID, mode); err != nil {
|
||||
m.logger.Warn("failed to re-apply session mode after context reset",
|
||||
zap.String("execution_id", execution.ID),
|
||||
zap.String("mode", mode),
|
||||
zap.Error(err))
|
||||
return
|
||||
}
|
||||
// Restore the cache too: the fresh session would otherwise report the agent's
|
||||
// default mode, leaving modeState stale relative to what we just re-applied.
|
||||
execution.SetModeState(&CachedModeState{
|
||||
CurrentModeID: mode,
|
||||
AvailableModes: availableModes,
|
||||
})
|
||||
m.logger.Info("re-applied session mode after context reset",
|
||||
zap.String("execution_id", execution.ID),
|
||||
zap.String("session_id", execution.SessionID),
|
||||
zap.String("mode", mode))
|
||||
}
|
||||
|
||||
// ResetAgentContext resets the agent's conversation context. For ACP agents that support
|
||||
// session reset, this creates a new session on the existing connection (fast, no process restart).
|
||||
// For all other agents, this falls back to RestartAgentProcess (full subprocess restart).
|
||||
@@ -311,6 +357,11 @@ func (m *Manager) ResetAgentContext(ctx context.Context, executionID string) err
|
||||
return fmt.Errorf("execution %q has no agentctl client", executionID)
|
||||
}
|
||||
|
||||
// Capture the active session mode before the reset so it can be re-applied to
|
||||
// the fresh ACP session (issue #1183). The agent's new session starts at its
|
||||
// default mode, so without this the user's chosen mode is lost.
|
||||
prevMode := execution.GetModeState()
|
||||
|
||||
// Resolve agent config and MCP servers for session reset
|
||||
agentConfig, err := m.getAgentConfigForExecution(execution)
|
||||
if err != nil {
|
||||
@@ -355,6 +406,9 @@ func (m *Manager) ResetAgentContext(ctx context.Context, executionID string) err
|
||||
}
|
||||
})
|
||||
|
||||
// Restore the user's session permission mode onto the fresh ACP session.
|
||||
m.reapplySessionModeAfterReset(ctx, execution, newSessionID, prevMode)
|
||||
|
||||
m.logger.Info("agent context reset via session (no process restart)",
|
||||
zap.String("execution_id", executionID),
|
||||
zap.String("session_id", execution.SessionID),
|
||||
@@ -510,6 +564,11 @@ func (m *Manager) RestartAgentProcess(ctx context.Context, executionID string) e
|
||||
zap.String("task_id", execution.TaskID),
|
||||
zap.String("session_id", execution.SessionID))
|
||||
|
||||
// Capture the active session mode before the restart so it can be re-applied to
|
||||
// the fresh ACP session (issue #1183). Capture now, before the streams reconnect
|
||||
// and the restarted agent reports its default mode (which would overwrite the cache).
|
||||
prevMode := execution.GetModeState()
|
||||
|
||||
// Resolve agent config early — needed for both command rebuild and ACP session init
|
||||
agentConfig, err := m.getAgentConfigForExecution(execution)
|
||||
if err != nil {
|
||||
@@ -585,6 +644,9 @@ func (m *Manager) RestartAgentProcess(ctx context.Context, executionID string) e
|
||||
return fmt.Errorf("failed to initialize ACP session after restart: %w", err)
|
||||
}
|
||||
|
||||
// Restore the user's session permission mode onto the fresh ACP session.
|
||||
m.reapplySessionModeAfterReset(ctx, execution, execution.ACPSessionID, prevMode)
|
||||
|
||||
m.logger.Info("agent process restarted with fresh context",
|
||||
zap.String("execution_id", executionID),
|
||||
zap.String("session_id", execution.SessionID),
|
||||
|
||||
@@ -15,6 +15,7 @@ import (
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/kandev/kandev/internal/agent/executor"
|
||||
agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl"
|
||||
"github.com/kandev/kandev/internal/agentctl/server/process"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
@@ -112,6 +113,15 @@ func newRestartMockAgentctlServer(t *testing.T, failStop, failSessionNew bool) *
|
||||
"session_id": "new-session-123",
|
||||
})
|
||||
}
|
||||
case "agent.session.reset":
|
||||
resp, _ = ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{
|
||||
"success": true,
|
||||
"session_id": "reset-session-456",
|
||||
})
|
||||
case "agent.session.set_mode":
|
||||
resp, _ = ws.NewResponse(msg.ID, msg.Action, map[string]interface{}{
|
||||
"success": true,
|
||||
})
|
||||
default:
|
||||
resp, _ = ws.NewError(msg.ID, msg.Action, ws.ErrorCodeUnknownAction, "unknown action", nil)
|
||||
}
|
||||
@@ -343,6 +353,137 @@ func TestManager_RestartAgentProcess_SessionInitFailure(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// --- Session mode preservation across reset (issue #1183) ---
|
||||
|
||||
// TestManager_RestartAgentProcess_ReappliesSessionMode is the regression test for
|
||||
// the full-process-restart path: a non-default session permission mode (auto /
|
||||
// accept-edits) chosen by the user must be re-applied to the fresh ACP session
|
||||
// after restart instead of silently reverting to the agent's default.
|
||||
func TestManager_RestartAgentProcess_ReappliesSessionMode(t *testing.T) {
|
||||
mgr := newTestManager()
|
||||
mock := newRestartMockAgentctlServer(t, false, false)
|
||||
|
||||
client := createTestClient(t, mock.server.URL)
|
||||
t.Cleanup(client.Close)
|
||||
|
||||
exec := &AgentExecution{
|
||||
ID: "exec-mode-restart",
|
||||
TaskID: "task-1",
|
||||
SessionID: "session-1",
|
||||
AgentProfileID: "profile-1",
|
||||
ACPSessionID: "old-session",
|
||||
AgentCommand: "auggie --model test",
|
||||
Status: v1.AgentStatusRunning,
|
||||
WorkspacePath: "/workspace",
|
||||
agentctl: client,
|
||||
promptDoneCh: make(chan PromptCompletionSignal, 1),
|
||||
}
|
||||
exec.SetModeState(&CachedModeState{
|
||||
CurrentModeID: "acceptEdits",
|
||||
AvailableModes: []streams.SessionModeInfo{{ID: "default"}, {ID: "acceptEdits"}},
|
||||
})
|
||||
require.NoError(t, mgr.executionStore.Add(exec))
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
|
||||
defer cancel()
|
||||
|
||||
require.NoError(t, mgr.RestartAgentProcess(ctx, exec.ID))
|
||||
|
||||
require.Contains(t, mock.getWSActions(), "agent.session.set_mode",
|
||||
"restart must re-apply the previously-active session mode to the new session")
|
||||
require.NotNil(t, exec.GetModeState())
|
||||
require.Equal(t, "acceptEdits", exec.GetModeState().CurrentModeID,
|
||||
"cached mode state must reflect the re-applied mode after restart")
|
||||
}
|
||||
|
||||
// TestManager_ResetAgentContext_ReappliesSessionMode is the regression test for
|
||||
// the ACP fast-path (session reset without a process restart): the user's chosen
|
||||
// session permission mode must deterministically survive the reset.
|
||||
func TestManager_ResetAgentContext_ReappliesSessionMode(t *testing.T) {
|
||||
mgr := newTestManager()
|
||||
mock := newRestartMockAgentctlServer(t, false, false)
|
||||
|
||||
client := createTestClient(t, mock.server.URL)
|
||||
t.Cleanup(client.Close)
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
|
||||
defer cancel()
|
||||
|
||||
// The ACP fast-path reuses the existing agent stream, so connect it first.
|
||||
require.NoError(t, client.StreamUpdates(ctx, func(agentctl.AgentEvent) {}, nil, nil))
|
||||
|
||||
exec := &AgentExecution{
|
||||
ID: "exec-mode-reset",
|
||||
TaskID: "task-1",
|
||||
SessionID: "session-1",
|
||||
AgentProfileID: "profile-1",
|
||||
ACPSessionID: "old-session",
|
||||
AgentCommand: "auggie --model test",
|
||||
Status: v1.AgentStatusRunning,
|
||||
WorkspacePath: "/workspace",
|
||||
sessionInitialized: true,
|
||||
agentctl: client,
|
||||
promptDoneCh: make(chan PromptCompletionSignal, 1),
|
||||
}
|
||||
exec.SetModeState(&CachedModeState{
|
||||
CurrentModeID: "acceptEdits",
|
||||
AvailableModes: []streams.SessionModeInfo{{ID: "default"}, {ID: "acceptEdits"}},
|
||||
})
|
||||
require.NoError(t, mgr.executionStore.Add(exec))
|
||||
|
||||
require.NoError(t, mgr.ResetAgentContext(ctx, exec.ID))
|
||||
|
||||
require.Equal(t, "reset-session-456", exec.ACPSessionID,
|
||||
"fast-path reset should create a new ACP session, not restart the process")
|
||||
require.Contains(t, mock.getWSActions(), "agent.session.set_mode",
|
||||
"fast-path reset must re-apply the previously-active session mode")
|
||||
require.NotNil(t, exec.GetModeState())
|
||||
require.Equal(t, "acceptEdits", exec.GetModeState().CurrentModeID)
|
||||
}
|
||||
|
||||
// TestManager_RestartAgentProcess_PrefersPersistedModeOverStaleCache is the
|
||||
// regression for the ordering hazard flagged on #1188: when a set_session_mode
|
||||
// action persists a newer mode to the DB in the same on_enter batch just before
|
||||
// reset_agent_context, the in-memory modeState is still the old value (its agent
|
||||
// mode event is async). The restart must restore the persisted (DB) mode, not the
|
||||
// stale cached one.
|
||||
func TestManager_RestartAgentProcess_PrefersPersistedModeOverStaleCache(t *testing.T) {
|
||||
mgr := newTestManager()
|
||||
mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{
|
||||
infos: map[string]*WorkspaceInfo{
|
||||
"session-1": {SessionID: "session-1", SessionMode: "acceptEdits"},
|
||||
},
|
||||
}
|
||||
mock := newRestartMockAgentctlServer(t, false, false)
|
||||
client := createTestClient(t, mock.server.URL)
|
||||
t.Cleanup(client.Close)
|
||||
|
||||
exec := &AgentExecution{
|
||||
ID: "exec-mode-stale",
|
||||
TaskID: "task-1",
|
||||
SessionID: "session-1",
|
||||
AgentProfileID: "profile-1",
|
||||
ACPSessionID: "old-session",
|
||||
AgentCommand: "auggie --model test",
|
||||
Status: v1.AgentStatusRunning,
|
||||
WorkspacePath: "/workspace",
|
||||
agentctl: client,
|
||||
promptDoneCh: make(chan PromptCompletionSignal, 1),
|
||||
}
|
||||
// Stale in-memory cache: the previous (pre-action) mode.
|
||||
exec.SetModeState(&CachedModeState{CurrentModeID: "default"})
|
||||
require.NoError(t, mgr.executionStore.Add(exec))
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second)
|
||||
defer cancel()
|
||||
require.NoError(t, mgr.RestartAgentProcess(ctx, exec.ID))
|
||||
|
||||
require.Contains(t, mock.getWSActions(), "agent.session.set_mode")
|
||||
require.NotNil(t, exec.GetModeState())
|
||||
require.Equal(t, "acceptEdits", exec.GetModeState().CurrentModeID,
|
||||
"restart must restore the persisted DB mode, not the stale in-memory cache")
|
||||
}
|
||||
|
||||
// --- SetSessionModel passthrough tests ---
|
||||
|
||||
// TestManager_SetSessionModel_Passthrough_PersistsOverride is the regression test for
|
||||
@@ -561,6 +702,41 @@ func TestIsAgentRunningForSession(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// TestEffectiveSessionMode covers the fresh-launch mode propagation for issue
|
||||
// #1183: a persisted session_mode (e.g. from a set_session_mode workflow step)
|
||||
// must override the profile default at ACP session init, while a missing
|
||||
// provider / lookup error / empty session mode falls back to the profile mode.
|
||||
func TestEffectiveSessionMode(t *testing.T) {
|
||||
exec := &AgentExecution{ID: "exec-1", TaskID: "task-1", SessionID: "session-1"}
|
||||
|
||||
t.Run("session mode overrides profile mode", func(t *testing.T) {
|
||||
mgr := newTestManager()
|
||||
mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{
|
||||
infos: map[string]*WorkspaceInfo{"session-1": {SessionID: "session-1", SessionMode: "acceptEdits"}},
|
||||
}
|
||||
require.Equal(t, "acceptEdits", mgr.effectiveSessionMode(context.Background(), exec, "default"))
|
||||
})
|
||||
|
||||
t.Run("falls back to profile mode when no session mode set", func(t *testing.T) {
|
||||
mgr := newTestManager()
|
||||
mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{
|
||||
infos: map[string]*WorkspaceInfo{"session-1": {SessionID: "session-1"}},
|
||||
}
|
||||
require.Equal(t, "default", mgr.effectiveSessionMode(context.Background(), exec, "default"))
|
||||
})
|
||||
|
||||
t.Run("falls back to profile mode on provider error", func(t *testing.T) {
|
||||
mgr := newTestManager()
|
||||
mgr.workspaceInfoProvider = &mockWorkspaceInfoProvider{err: fmt.Errorf("boom")}
|
||||
require.Equal(t, "default", mgr.effectiveSessionMode(context.Background(), exec, "default"))
|
||||
})
|
||||
|
||||
t.Run("falls back to profile mode when no provider wired", func(t *testing.T) {
|
||||
mgr := newTestManager()
|
||||
require.Equal(t, "default", mgr.effectiveSessionMode(context.Background(), exec, "default"))
|
||||
})
|
||||
}
|
||||
|
||||
// --- IsRemoteSession tests ---
|
||||
|
||||
type mockWorkspaceInfoProvider struct {
|
||||
|
||||
@@ -164,5 +164,23 @@ func (m *Manager) resolveProfileModelAndMode(ctx context.Context, profileID stri
|
||||
// MarkReady fires later from handleCompleteEvent — that path is the true turn-end.
|
||||
func (m *Manager) initializeACPSession(ctx context.Context, execution *AgentExecution, agentConfig agents.Agent, taskDescription string, attachments []MessageAttachment, mcpServers []agentctltypes.McpServer) error {
|
||||
profileModel, profileMode := m.resolveProfileModelAndMode(ctx, execution.AgentProfileID)
|
||||
return m.sessionManager.InitializeAndPrompt(ctx, execution, agentConfig, taskDescription, attachments, mcpServers, m.MarkBootReady, profileModel, profileMode)
|
||||
mode := m.effectiveSessionMode(ctx, execution, profileMode)
|
||||
return m.sessionManager.InitializeAndPrompt(ctx, execution, agentConfig, taskDescription, attachments, mcpServers, m.MarkBootReady, profileModel, mode)
|
||||
}
|
||||
|
||||
// effectiveSessionMode prefers a session-level permission mode persisted in the
|
||||
// session metadata (session_mode — declared via the set_session_mode workflow
|
||||
// action or a user toggle) over the agent profile's default mode. This makes a
|
||||
// fresh launch start in the declared mode before the first prompt, rather than
|
||||
// reverting to the profile default. Falls back to profileMode when no provider
|
||||
// is wired, the lookup fails, or no session mode is set. See issue #1183.
|
||||
func (m *Manager) effectiveSessionMode(ctx context.Context, execution *AgentExecution, profileMode string) string {
|
||||
if m.workspaceInfoProvider == nil {
|
||||
return profileMode
|
||||
}
|
||||
info, err := m.workspaceInfoProvider.GetWorkspaceInfoForSession(ctx, execution.TaskID, execution.SessionID)
|
||||
if err != nil || info == nil || info.SessionMode == "" {
|
||||
return profileMode
|
||||
}
|
||||
return info.SessionMode
|
||||
}
|
||||
|
||||
@@ -490,6 +490,11 @@ type WorkspaceInfo struct {
|
||||
AgentProfileID string // Optional - agent profile for the task
|
||||
AgentID string // Agent type ID (e.g., "auggie", "codex") - required for runtime creation
|
||||
ACPSessionID string // Agent's session ID for conversation resumption (from session metadata)
|
||||
// SessionMode is the persisted session permission mode (e.g. "acceptEdits")
|
||||
// from session metadata, declared via the set_session_mode workflow action or
|
||||
// a user toggle. Applied as a mode override at ACP session init so a fresh
|
||||
// launch starts in the declared mode before the first prompt. See issue #1183.
|
||||
SessionMode string
|
||||
|
||||
// Executor-aware fields for correct runtime selection and remote reconnection
|
||||
ExecutorType string // Executor type (e.g., "local_pc", "sprites")
|
||||
|
||||
@@ -485,6 +485,10 @@ func (s *SimulatedAgentManagerClient) SetSessionModelBySessionID(_ context.Conte
|
||||
return fmt.Errorf("not supported")
|
||||
}
|
||||
|
||||
func (s *SimulatedAgentManagerClient) SetSessionModeBySessionID(_ context.Context, _, _ string) error {
|
||||
return fmt.Errorf("not supported")
|
||||
}
|
||||
|
||||
func (s *SimulatedAgentManagerClient) WasSessionInitialized(_ string) bool { return false }
|
||||
func (s *SimulatedAgentManagerClient) IsPassthroughSession(ctx context.Context, sessionID string) bool {
|
||||
return false
|
||||
|
||||
@@ -894,6 +894,14 @@ func (s *Service) handleSessionModeEvent(ctx context.Context, payload *lifecycle
|
||||
if sessionID == "" || s.eventBus == nil {
|
||||
return
|
||||
}
|
||||
// Persist the agent-reported mode to session metadata so the user's chosen
|
||||
// permission mode survives a backend restart / SSR reload, mirroring how the
|
||||
// current model is persisted. Only non-empty modes are stored — an empty
|
||||
// CurrentModeID means the agent left a special mode, with nothing sticky to keep.
|
||||
if mode := payload.Data.CurrentModeID; mode != "" {
|
||||
s.persistSessionMode(ctx, sessionID, mode)
|
||||
}
|
||||
|
||||
eventPayload := lifecycle.SessionModeEventPayload{
|
||||
TaskID: payload.TaskID,
|
||||
SessionID: sessionID,
|
||||
@@ -906,6 +914,21 @@ func (s *Service) handleSessionModeEvent(ctx context.Context, payload *lifecycle
|
||||
_ = s.eventBus.Publish(ctx, subject, bus.NewEvent(events.SessionModeChanged, "orchestrator", eventPayload))
|
||||
}
|
||||
|
||||
// persistSessionMode stores the agent-reported session permission mode in the
|
||||
// session metadata using a targeted json_set, so other metadata keys (plan_mode,
|
||||
// acp_session_id, …) are preserved. See issue #1183.
|
||||
func (s *Service) persistSessionMode(ctx context.Context, sessionID, modeID string) {
|
||||
if s.repo == nil {
|
||||
return
|
||||
}
|
||||
if err := s.repo.SetSessionMetadataKey(ctx, sessionID, models.SessionMetaKeySessionMode, modeID); err != nil {
|
||||
s.logger.Warn("failed to persist session mode to metadata",
|
||||
zap.String("session_id", sessionID),
|
||||
zap.String("mode", modeID),
|
||||
zap.Error(err))
|
||||
}
|
||||
}
|
||||
|
||||
// handleAgentCapabilitiesEvent broadcasts agent_capabilities events to the WebSocket.
|
||||
func (s *Service) handleAgentCapabilitiesEvent(ctx context.Context, payload *lifecycle.AgentStreamEventPayload) {
|
||||
sessionID := payload.SessionID
|
||||
|
||||
@@ -115,6 +115,57 @@ func TestHandleSessionModeEvent(t *testing.T) {
|
||||
|
||||
require.Empty(t, eb.events)
|
||||
})
|
||||
|
||||
// Regression for issue #1183: a non-empty mode is persisted to session
|
||||
// metadata (so it survives backend restart / SSR) without clobbering other
|
||||
// keys such as plan_mode.
|
||||
t.Run("persists non-empty mode without clobbering plan_mode", func(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "t1", "s1", "step1")
|
||||
require.NoError(t, repo.UpdateSessionMetadata(ctx, "s1", map[string]interface{}{"plan_mode": true}))
|
||||
|
||||
eb := &recordingEventBus{}
|
||||
svc := &Service{logger: testLogger(), eventBus: eb, repo: repo}
|
||||
|
||||
svc.handleSessionModeEvent(ctx, &lifecycle.AgentStreamEventPayload{
|
||||
TaskID: "t1",
|
||||
SessionID: "s1",
|
||||
AgentID: "a1",
|
||||
Data: &lifecycle.AgentStreamEventData{CurrentModeID: "acceptEdits"},
|
||||
})
|
||||
|
||||
updated, err := repo.GetTaskSession(ctx, "s1")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "acceptEdits", updated.Metadata[models.SessionMetaKeySessionMode],
|
||||
"session mode must be persisted to metadata")
|
||||
pm, _ := updated.Metadata["plan_mode"].(bool)
|
||||
require.True(t, pm, "plan_mode and other metadata keys must be preserved")
|
||||
})
|
||||
|
||||
// An empty CurrentModeID (agent left a special mode) must not overwrite a
|
||||
// previously-stored sticky mode.
|
||||
t.Run("empty mode does not overwrite stored mode", func(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "t1", "s1", "step1")
|
||||
require.NoError(t, repo.UpdateSessionMetadata(ctx, "s1",
|
||||
map[string]interface{}{models.SessionMetaKeySessionMode: "acceptEdits"}))
|
||||
|
||||
eb := &recordingEventBus{}
|
||||
svc := &Service{logger: testLogger(), eventBus: eb, repo: repo}
|
||||
|
||||
svc.handleSessionModeEvent(ctx, &lifecycle.AgentStreamEventPayload{
|
||||
TaskID: "t1",
|
||||
SessionID: "s1",
|
||||
AgentID: "a1",
|
||||
Data: &lifecycle.AgentStreamEventData{CurrentModeID: ""},
|
||||
})
|
||||
|
||||
updated, err := repo.GetTaskSession(ctx, "s1")
|
||||
require.NoError(t, err)
|
||||
require.Equal(t, "acceptEdits", updated.Metadata[models.SessionMetaKeySessionMode])
|
||||
})
|
||||
}
|
||||
|
||||
// TestToolEventsWakeSessionAndTaskTogether locks in the fix for the
|
||||
|
||||
@@ -174,6 +174,17 @@ type mockAgentManager struct {
|
||||
cancelAgentCalls atomic.Int32
|
||||
cancelAgentBlock chan struct{}
|
||||
cancelAgentEntered chan struct{}
|
||||
|
||||
// set_session_mode tracking (issue #1183). Records (sessionID, modeID) for
|
||||
// every SetSessionModeBySessionID call. setSessionModeErr, when set, is
|
||||
// returned to simulate "no running agent".
|
||||
setSessionModeCalls []sessionModeCall
|
||||
setSessionModeErr error
|
||||
}
|
||||
|
||||
type sessionModeCall struct {
|
||||
SessionID string
|
||||
ModeID string
|
||||
}
|
||||
|
||||
type stopAgentCall struct {
|
||||
@@ -282,6 +293,12 @@ func (m *mockAgentManager) SetExecutionEnv(_ context.Context, _ string, _ map[st
|
||||
func (m *mockAgentManager) SetSessionModelBySessionID(_ context.Context, _, _ string) error {
|
||||
return fmt.Errorf("not supported")
|
||||
}
|
||||
func (m *mockAgentManager) SetSessionModeBySessionID(_ context.Context, sessionID, modeID string) error {
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
m.setSessionModeCalls = append(m.setSessionModeCalls, sessionModeCall{SessionID: sessionID, ModeID: modeID})
|
||||
return m.setSessionModeErr
|
||||
}
|
||||
func (m *mockAgentManager) SetMcpMode(_ context.Context, _ string, _ string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -862,6 +862,9 @@ func (s *Service) processOnEnter(ctx context.Context, taskID string, session *mo
|
||||
if !isPassthrough && hasPlanMode {
|
||||
s.setSessionPlanMode(ctx, session, true)
|
||||
}
|
||||
case wfmodels.OnEnterSetSessionMode:
|
||||
mode, _ := action.Config["mode"].(string)
|
||||
s.applyStepSessionMode(ctx, session, mode, isPassthrough)
|
||||
case wfmodels.OnEnterAutoStartAgent:
|
||||
hasAutoStart = true
|
||||
}
|
||||
@@ -1727,6 +1730,35 @@ func (s *Service) setSessionPlanMode(ctx context.Context, session *models.TaskSe
|
||||
}
|
||||
}
|
||||
|
||||
// applyStepSessionMode applies a workflow-declared session permission mode to a
|
||||
// session entering a step (set_session_mode action, issue #1183). It persists the
|
||||
// mode to metadata (durable + restored on reset) and best-effort applies it to a
|
||||
// running agent via ACP session/set_mode. Passthrough sessions manage their own
|
||||
// mode in the underlying CLI and are skipped, mirroring plan-mode handling.
|
||||
func (s *Service) applyStepSessionMode(ctx context.Context, session *models.TaskSession, mode string, isPassthrough bool) {
|
||||
if mode == "" || isPassthrough {
|
||||
return
|
||||
}
|
||||
// Persist for durability (SSR / backend restart) and so Part 1's reset
|
||||
// re-apply has the right value. Mirror the in-memory struct for callers
|
||||
// that read session.Metadata afterwards.
|
||||
if session.Metadata == nil {
|
||||
session.Metadata = make(map[string]interface{})
|
||||
}
|
||||
session.Metadata[models.SessionMetaKeySessionMode] = mode
|
||||
s.persistSessionMode(ctx, session.ID, mode)
|
||||
|
||||
// Apply live when an agent is running. When none is (e.g. the step also
|
||||
// auto-starts the agent fresh), this is a no-op and the profile default
|
||||
// governs the new session — the declared mode stays persisted.
|
||||
if err := s.agentManager.SetSessionModeBySessionID(ctx, session.ID, mode); err != nil {
|
||||
s.logger.Debug("set_session_mode: could not apply mode to a live agent (persisted for next launch/reset)",
|
||||
zap.String("session_id", session.ID),
|
||||
zap.String("mode", mode),
|
||||
zap.Error(err))
|
||||
}
|
||||
}
|
||||
|
||||
// processTurnCompleteActions processes on_turn_complete actions for a step:
|
||||
// it executes side-effect actions and returns the first eligible transition action.
|
||||
func (s *Service) processTurnCompleteActions(ctx context.Context, session *models.TaskSession, step *wfmodels.WorkflowStep) *wfmodels.OnTurnCompleteAction {
|
||||
|
||||
@@ -298,6 +298,41 @@ func TestProcessOnTurnStart(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
// TestProcessOnEnterSetSessionMode verifies the set_session_mode on_enter action
|
||||
// (issue #1183): it persists the declared mode to metadata and applies it live to
|
||||
// the running agent.
|
||||
func TestProcessOnEnterSetSessionMode(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "t1", "s1", "step1")
|
||||
|
||||
agentMgr := &mockAgentManager{}
|
||||
svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), agentMgr)
|
||||
|
||||
step := &wfmodels.WorkflowStep{
|
||||
ID: "step1", WorkflowID: "wf1", Name: "Implement",
|
||||
Events: wfmodels.StepEvents{
|
||||
OnEnter: []wfmodels.OnEnterAction{
|
||||
{Type: wfmodels.OnEnterSetSessionMode, Config: map[string]interface{}{"mode": "acceptEdits"}},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
session, _ := repo.GetTaskSession(ctx, "s1")
|
||||
svc.processOnEnter(ctx, "t1", session, step, "test task")
|
||||
|
||||
updated, _ := repo.GetTaskSession(ctx, "s1")
|
||||
if got, _ := updated.Metadata[models.SessionMetaKeySessionMode].(string); got != "acceptEdits" {
|
||||
t.Errorf("expected session_mode metadata acceptEdits, got %q", got)
|
||||
}
|
||||
if len(agentMgr.setSessionModeCalls) != 1 {
|
||||
t.Fatalf("expected 1 live set-mode call, got %d", len(agentMgr.setSessionModeCalls))
|
||||
}
|
||||
if c := agentMgr.setSessionModeCalls[0]; c.SessionID != "s1" || c.ModeID != "acceptEdits" {
|
||||
t.Errorf("unexpected live set-mode call: %+v", c)
|
||||
}
|
||||
}
|
||||
|
||||
func TestProcessOnEnter(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
|
||||
@@ -139,6 +139,11 @@ type AgentManagerClient interface {
|
||||
// Returns an error if the agent doesn't support in-place model switching.
|
||||
SetSessionModelBySessionID(ctx context.Context, sessionID, modelID string) error
|
||||
|
||||
// SetSessionModeBySessionID applies a session permission mode (e.g. "default",
|
||||
// "acceptEdits") to the running agent via ACP session/set_mode. Returns an
|
||||
// error when no agent is running for the session. See issue #1183.
|
||||
SetSessionModeBySessionID(ctx context.Context, sessionID, modeID string) error
|
||||
|
||||
// WasSessionInitialized reports whether the given execution completed session initialization.
|
||||
// Used to distinguish launch-phase failures from normal prompt failures.
|
||||
WasSessionInitialized(executionID string) bool
|
||||
|
||||
@@ -116,6 +116,10 @@ func (m *mockAgentManager) SetSessionModelBySessionID(ctx context.Context, sessi
|
||||
return fmt.Errorf("not supported")
|
||||
}
|
||||
|
||||
func (m *mockAgentManager) SetSessionModeBySessionID(ctx context.Context, sessionID, modeID string) error {
|
||||
return fmt.Errorf("not supported")
|
||||
}
|
||||
|
||||
func (m *mockAgentManager) IsAgentRunningForSession(ctx context.Context, sessionID string) bool {
|
||||
m.isAgentRunningForSessionCallArgs = append(m.isAgentRunningForSessionCallArgs, sessionID)
|
||||
if m.isAgentRunningForSessionFunc != nil {
|
||||
|
||||
@@ -91,6 +91,9 @@ func (m *mockAgentManager) ResetAgentContext(ctx context.Context, agentExecution
|
||||
func (m *mockAgentManager) SetSessionModelBySessionID(_ context.Context, _, _ string) error {
|
||||
return errors.New("not supported")
|
||||
}
|
||||
func (m *mockAgentManager) SetSessionModeBySessionID(_ context.Context, _, _ string) error {
|
||||
return errors.New("not supported")
|
||||
}
|
||||
|
||||
func (m *mockAgentManager) IsAgentRunningForSession(ctx context.Context, sessionID string) bool {
|
||||
return false
|
||||
|
||||
@@ -24,6 +24,7 @@ func buildWorkflowCallbacks(svc *Service) engine.MapRegistry {
|
||||
engine.ActionResetAgentContext: &resetAgentContextCallback{svc: svc},
|
||||
engine.ActionAutoStartAgent: &autoStartAgentCallback{svc: svc},
|
||||
engine.ActionSetWorkflowData: &setWorkflowDataCallback{},
|
||||
engine.ActionSetSessionMode: &setSessionModeCallback{svc: svc},
|
||||
}
|
||||
if svc.engineRunQueue != nil {
|
||||
r[engine.ActionQueueRun] = engine.QueueRunCallback{
|
||||
@@ -109,6 +110,30 @@ func (c *disablePlanModeCallback) Execute(ctx context.Context, in engine.ActionI
|
||||
return engine.ActionResult{}, nil
|
||||
}
|
||||
|
||||
// setSessionModeCallback applies a workflow-declared session permission mode
|
||||
// (e.g. "acceptEdits") when entering a step. See issue #1183.
|
||||
type setSessionModeCallback struct {
|
||||
svc *Service
|
||||
}
|
||||
|
||||
func (c *setSessionModeCallback) Execute(ctx context.Context, in engine.ActionInput) (engine.ActionResult, error) {
|
||||
// Skip before any DB lookup: passthrough sessions manage their own mode in
|
||||
// the CLI, and an action with no mode is a no-op. Guarding here keeps a
|
||||
// skipped action from failing on a session-load error, and mirrors the
|
||||
// enable/disable plan-mode callbacks.
|
||||
if in.Action.SetSessionMode == nil || in.State.IsPassthrough {
|
||||
return engine.ActionResult{}, nil
|
||||
}
|
||||
session, err := c.svc.repo.GetTaskSession(ctx, in.State.SessionID)
|
||||
if err != nil {
|
||||
return engine.ActionResult{}, fmt.Errorf("load session for set session mode: %w", err)
|
||||
}
|
||||
// Passthrough is already excluded above, so pass false explicitly; the
|
||||
// isPassthrough parameter exists for the legacy processOnEnter call site.
|
||||
c.svc.applyStepSessionMode(ctx, session, in.Action.SetSessionMode.Mode, false)
|
||||
return engine.ActionResult{}, nil
|
||||
}
|
||||
|
||||
// resetAgentContextCallback restarts the agent subprocess with a fresh ACP session.
|
||||
type resetAgentContextCallback struct {
|
||||
svc *Service
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
package orchestrator
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
"github.com/kandev/kandev/internal/workflow/engine"
|
||||
)
|
||||
|
||||
// TestSetSessionModeCallback_Execute covers the engine callback wired for the
|
||||
// set_session_mode action (issue #1183): for a non-passthrough session it
|
||||
// persists the declared mode to metadata and applies it live to the agent;
|
||||
// passthrough sessions and actions with no payload are no-ops.
|
||||
func TestSetSessionModeCallback_Execute(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
newInput := func(passthrough bool, mode string) engine.ActionInput {
|
||||
in := engine.ActionInput{
|
||||
Trigger: engine.TriggerOnEnter,
|
||||
State: engine.MachineState{TaskID: "t1", SessionID: "s1", IsPassthrough: passthrough},
|
||||
Action: engine.Action{Kind: engine.ActionSetSessionMode},
|
||||
}
|
||||
if mode != "" {
|
||||
in.Action.SetSessionMode = &engine.SetSessionModeAction{Mode: mode}
|
||||
}
|
||||
return in
|
||||
}
|
||||
|
||||
t.Run("applies and persists for a non-passthrough session", func(t *testing.T) {
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "t1", "s1", "step1")
|
||||
agentMgr := &mockAgentManager{}
|
||||
svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), agentMgr)
|
||||
|
||||
if _, err := (&setSessionModeCallback{svc: svc}).Execute(ctx, newInput(false, "acceptEdits")); err != nil {
|
||||
t.Fatalf("Execute returned error: %v", err)
|
||||
}
|
||||
|
||||
updated, _ := repo.GetTaskSession(ctx, "s1")
|
||||
if got, _ := updated.Metadata[models.SessionMetaKeySessionMode].(string); got != "acceptEdits" {
|
||||
t.Errorf("expected persisted session_mode acceptEdits, got %q", got)
|
||||
}
|
||||
if len(agentMgr.setSessionModeCalls) != 1 || agentMgr.setSessionModeCalls[0].ModeID != "acceptEdits" {
|
||||
t.Errorf("expected one live set-mode call for acceptEdits, got %+v", agentMgr.setSessionModeCalls)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("skips passthrough sessions", func(t *testing.T) {
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "t1", "s1", "step1")
|
||||
agentMgr := &mockAgentManager{isPassthrough: true}
|
||||
svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), agentMgr)
|
||||
|
||||
if _, err := (&setSessionModeCallback{svc: svc}).Execute(ctx, newInput(true, "acceptEdits")); err != nil {
|
||||
t.Fatalf("Execute returned error: %v", err)
|
||||
}
|
||||
|
||||
updated, _ := repo.GetTaskSession(ctx, "s1")
|
||||
if _, ok := updated.Metadata[models.SessionMetaKeySessionMode]; ok {
|
||||
t.Error("passthrough session must not persist session_mode")
|
||||
}
|
||||
if len(agentMgr.setSessionModeCalls) != 0 {
|
||||
t.Errorf("passthrough session must not apply mode live, got %+v", agentMgr.setSessionModeCalls)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("no-op when action carries no mode", func(t *testing.T) {
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "t1", "s1", "step1")
|
||||
agentMgr := &mockAgentManager{}
|
||||
svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), agentMgr)
|
||||
|
||||
if _, err := (&setSessionModeCallback{svc: svc}).Execute(ctx, newInput(false, "")); err != nil {
|
||||
t.Fatalf("Execute returned error: %v", err)
|
||||
}
|
||||
if len(agentMgr.setSessionModeCalls) != 0 {
|
||||
t.Errorf("expected no live set-mode call when mode is empty, got %+v", agentMgr.setSessionModeCalls)
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -71,6 +71,11 @@ const (
|
||||
SessionCreatedByWorkflowSwitch = "workflow_switch"
|
||||
)
|
||||
|
||||
// SessionMetaKeySessionMode records the agent's last-known session permission
|
||||
// mode (auto / default / accept-edits, etc.) so it survives a backend restart or
|
||||
// SSR reload, alongside the in-memory re-apply on context reset. See issue #1183.
|
||||
const SessionMetaKeySessionMode = "session_mode"
|
||||
|
||||
// Task origin values for the Origin field.
|
||||
const (
|
||||
TaskOriginManual = "manual"
|
||||
|
||||
@@ -373,12 +373,15 @@ func (s *Service) GetWorkspaceInfoForSession(ctx context.Context, taskID, sessio
|
||||
}
|
||||
}
|
||||
|
||||
// Get ACP session ID from metadata
|
||||
var acpSessionID string
|
||||
// Get ACP session ID and persisted session permission mode from metadata.
|
||||
var acpSessionID, sessionMode string
|
||||
if session.Metadata != nil {
|
||||
if id, ok := session.Metadata["acp_session_id"].(string); ok {
|
||||
acpSessionID = id
|
||||
}
|
||||
if mode, ok := session.Metadata[models.SessionMetaKeySessionMode].(string); ok {
|
||||
sessionMode = mode
|
||||
}
|
||||
}
|
||||
|
||||
info := &lifecycle.WorkspaceInfo{
|
||||
@@ -389,6 +392,7 @@ func (s *Service) GetWorkspaceInfoForSession(ctx context.Context, taskID, sessio
|
||||
AgentProfileID: session.AgentProfileID,
|
||||
AgentID: agentID,
|
||||
ACPSessionID: acpSessionID,
|
||||
SessionMode: sessionMode,
|
||||
}
|
||||
|
||||
var taskEnv *models.TaskEnvironment
|
||||
|
||||
@@ -78,6 +78,37 @@ func TestGetWorkspaceInfoForSession_BasicFields(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestGetWorkspaceInfoForSession_IncludesSessionMode verifies the persisted
|
||||
// session permission mode is surfaced on WorkspaceInfo so the lifecycle can apply
|
||||
// it as a mode override on a fresh launch. See issue #1183.
|
||||
func TestGetWorkspaceInfoForSession_IncludesSessionMode(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
|
||||
setupTestTask(t, repo)
|
||||
now := time.Now().UTC()
|
||||
|
||||
session := &models.TaskSession{
|
||||
ID: "session-1",
|
||||
TaskID: "task-123",
|
||||
State: models.TaskSessionStateRunning,
|
||||
Metadata: map[string]interface{}{models.SessionMetaKeySessionMode: "acceptEdits"},
|
||||
StartedAt: now,
|
||||
UpdatedAt: now,
|
||||
}
|
||||
if err := repo.CreateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("failed to create session: %v", err)
|
||||
}
|
||||
|
||||
info, err := svc.GetWorkspaceInfoForSession(ctx, "task-123", "session-1")
|
||||
if err != nil {
|
||||
t.Fatalf("GetWorkspaceInfoForSession returned error: %v", err)
|
||||
}
|
||||
if info.SessionMode != "acceptEdits" {
|
||||
t.Errorf("expected SessionMode 'acceptEdits', got %q", info.SessionMode)
|
||||
}
|
||||
}
|
||||
|
||||
func TestGetWorkspaceInfoForSession_InfersTaskID(t *testing.T) {
|
||||
svc, _, repo := createTestService(t)
|
||||
ctx := context.Background()
|
||||
|
||||
@@ -4,6 +4,8 @@ import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
wfmodels "github.com/kandev/kandev/internal/workflow/models"
|
||||
)
|
||||
|
||||
type fakeStore struct {
|
||||
@@ -81,6 +83,57 @@ func (c *fakeCallback) Execute(_ context.Context, _ ActionInput) (ActionResult,
|
||||
return c.result, nil
|
||||
}
|
||||
|
||||
// recordingCallback captures the ActionInput it was invoked with so tests can
|
||||
// assert the engine dispatched the right typed action to the callback.
|
||||
type recordingCallback struct {
|
||||
calls []ActionInput
|
||||
}
|
||||
|
||||
func (c *recordingCallback) Execute(_ context.Context, in ActionInput) (ActionResult, error) {
|
||||
c.calls = append(c.calls, in)
|
||||
return ActionResult{}, nil
|
||||
}
|
||||
|
||||
// TestHandleTrigger_SetSessionMode_InvokesCallback exercises the full engine path
|
||||
// for the set_session_mode action (issue #1183): a step compiled from a
|
||||
// set_session_mode on_enter action dispatches to the registered callback with the
|
||||
// typed mode carried through. This is the engine-level end-to-end of the feature.
|
||||
func TestHandleTrigger_SetSessionMode_InvokesCallback(t *testing.T) {
|
||||
compiled := CompileStep(&wfmodels.WorkflowStep{
|
||||
ID: "step-1", WorkflowID: "wf1",
|
||||
Events: wfmodels.StepEvents{
|
||||
OnEnter: []wfmodels.OnEnterAction{
|
||||
{Type: wfmodels.OnEnterSetSessionMode, Config: map[string]any{"mode": "acceptEdits"}},
|
||||
},
|
||||
},
|
||||
})
|
||||
store := &fakeStore{
|
||||
state: MachineState{TaskID: "t1", SessionID: "s1", WorkflowID: "wf1", CurrentStepID: "step-1"},
|
||||
stepsByID: map[string]StepSpec{"step-1": compiled},
|
||||
applied: map[string]bool{},
|
||||
}
|
||||
|
||||
cb := &recordingCallback{}
|
||||
eng := New(store, MapRegistry{ActionSetSessionMode: cb})
|
||||
|
||||
if _, err := eng.HandleTrigger(context.Background(), HandleInput{
|
||||
TaskID: "t1", SessionID: "s1", Trigger: TriggerOnEnter,
|
||||
}); err != nil {
|
||||
t.Fatalf("unexpected error: %v", err)
|
||||
}
|
||||
|
||||
if len(cb.calls) != 1 {
|
||||
t.Fatalf("expected set_session_mode callback to fire once, got %d", len(cb.calls))
|
||||
}
|
||||
got := cb.calls[0].Action
|
||||
if got.Kind != ActionSetSessionMode {
|
||||
t.Fatalf("unexpected action kind dispatched: %s", got.Kind)
|
||||
}
|
||||
if got.SetSessionMode == nil || got.SetSessionMode.Mode != "acceptEdits" {
|
||||
t.Fatalf("expected dispatched mode acceptEdits, got %+v", got.SetSessionMode)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleTrigger_FirstTransitionWins(t *testing.T) {
|
||||
store := &fakeStore{
|
||||
state: MachineState{TaskID: "t1", SessionID: "s1", WorkflowID: "wf1", CurrentStepID: "step-1"},
|
||||
|
||||
@@ -42,6 +42,7 @@ const (
|
||||
ActionAutoStartAgent ActionKind = "auto_start_agent"
|
||||
ActionResetAgentContext ActionKind = "reset_agent_context"
|
||||
ActionSetWorkflowData ActionKind = "set_workflow_data"
|
||||
ActionSetSessionMode ActionKind = "set_session_mode"
|
||||
|
||||
// New Phase 2 action kinds (ADR-0004). Defined and exposed via callbacks
|
||||
// that intentionally return ErrActionNotYetWired — they will be wired
|
||||
@@ -81,6 +82,7 @@ type Action struct {
|
||||
MoveToStep *MoveToStepAction
|
||||
AutoStartAgent *AutoStartAgentAction
|
||||
SetWorkflowData *SetWorkflowDataAction
|
||||
SetSessionMode *SetSessionModeAction
|
||||
QueueRun *QueueRunAction
|
||||
ClearDecisions *ClearDecisionsAction
|
||||
QueueRunForEachParticipant *QueueRunForEachParticipantAction
|
||||
@@ -131,6 +133,13 @@ type SetWorkflowDataAction struct {
|
||||
Value any
|
||||
}
|
||||
|
||||
// SetSessionModeAction declares the agent session permission mode to apply when
|
||||
// the step is entered (e.g. "default", "acceptEdits"). Mode is read from the
|
||||
// action config's "mode" key by CompileStep. See issue #1183.
|
||||
type SetSessionModeAction struct {
|
||||
Mode string
|
||||
}
|
||||
|
||||
// QueueRunAction represents the Phase 2 "queue a run on a target task/agent"
|
||||
// action. The action is declared but its callback is not yet wired into the
|
||||
// engine; see PlaceholderQueueRunCallback.
|
||||
@@ -282,6 +291,12 @@ func compileOnEnter(step *wfmodels.WorkflowStep) []Action {
|
||||
actions = append(actions, Action{Kind: ActionAutoStartAgent, AutoStartAgent: &AutoStartAgentAction{QueueIfBusy: true}})
|
||||
case wfmodels.OnEnterResetAgentContext:
|
||||
actions = append(actions, Action{Kind: ActionResetAgentContext})
|
||||
case wfmodels.OnEnterSetSessionMode:
|
||||
mode := readSessionMode(action.Config)
|
||||
if mode == "" {
|
||||
continue // skip set_session_mode actions with no target mode
|
||||
}
|
||||
actions = append(actions, Action{Kind: ActionSetSessionMode, SetSessionMode: &SetSessionModeAction{Mode: mode}})
|
||||
case wfmodels.OnEnterClearDecisions:
|
||||
actions = append(actions, Action{
|
||||
Kind: ActionClearDecisions,
|
||||
@@ -421,6 +436,16 @@ func readQueueRunForEachParticipantConfig(config map[string]any) *QueueRunForEac
|
||||
}
|
||||
}
|
||||
|
||||
// readSessionMode reads the target mode for a set_session_mode action from its
|
||||
// config map. Returns "" when the config is missing or the mode key is unset.
|
||||
func readSessionMode(config map[string]any) string {
|
||||
if config == nil {
|
||||
return ""
|
||||
}
|
||||
mode, _ := config["mode"].(string)
|
||||
return mode
|
||||
}
|
||||
|
||||
func readStepID(config map[string]any) (string, error) {
|
||||
if config == nil {
|
||||
return "", fmt.Errorf("missing move_to_step config")
|
||||
|
||||
@@ -46,6 +46,48 @@ func TestCompileStep_CompilesLegacyActionsToTypedActions(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCompileStep_SetSessionMode(t *testing.T) {
|
||||
t.Run("compiles set_session_mode with mode config", func(t *testing.T) {
|
||||
step := &wfmodels.WorkflowStep{
|
||||
ID: "s1",
|
||||
WorkflowID: "wf1",
|
||||
Name: "Implement",
|
||||
Events: wfmodels.StepEvents{
|
||||
OnEnter: []wfmodels.OnEnterAction{
|
||||
{Type: wfmodels.OnEnterSetSessionMode, Config: map[string]any{"mode": "acceptEdits"}},
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
spec := CompileStep(step)
|
||||
actions := spec.Events[TriggerOnEnter]
|
||||
if len(actions) != 1 {
|
||||
t.Fatalf("expected 1 on_enter action, got %d", len(actions))
|
||||
}
|
||||
if actions[0].Kind != ActionSetSessionMode {
|
||||
t.Fatalf("unexpected action kind: %s", actions[0].Kind)
|
||||
}
|
||||
if actions[0].SetSessionMode == nil || actions[0].SetSessionMode.Mode != "acceptEdits" {
|
||||
t.Fatalf("expected compiled set_session_mode mode=acceptEdits, got %+v", actions[0].SetSessionMode)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("skips set_session_mode with no mode", func(t *testing.T) {
|
||||
step := &wfmodels.WorkflowStep{
|
||||
ID: "s1",
|
||||
Events: wfmodels.StepEvents{
|
||||
OnEnter: []wfmodels.OnEnterAction{
|
||||
{Type: wfmodels.OnEnterSetSessionMode},
|
||||
},
|
||||
},
|
||||
}
|
||||
spec := CompileStep(step)
|
||||
if len(spec.Events[TriggerOnEnter]) != 0 {
|
||||
t.Fatalf("expected set_session_mode with no mode to be skipped, got %d actions", len(spec.Events[TriggerOnEnter]))
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestCompileStep_RequiresApproval(t *testing.T) {
|
||||
step := &wfmodels.WorkflowStep{
|
||||
ID: "s1",
|
||||
|
||||
@@ -130,6 +130,9 @@ func (e *WorkflowExport) Validate() error {
|
||||
return fmt.Errorf("workflow %d: duplicate step position %d", i, step.Position)
|
||||
}
|
||||
positions[step.Position] = true
|
||||
if err := validateOnEnterActions(step); err != nil {
|
||||
return fmt.Errorf("workflow %d step %d: %w", i, j, err)
|
||||
}
|
||||
}
|
||||
// Validate that move_to_step references point to valid positions.
|
||||
if err := validateStepPositionRefs(wf.Steps, positions); err != nil {
|
||||
@@ -139,6 +142,23 @@ func (e *WorkflowExport) Validate() error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// validateOnEnterActions rejects malformed on_enter actions in a portable step.
|
||||
// Currently this guards set_session_mode, which requires a non-empty string
|
||||
// "mode" config — without it the action is silently dropped at compile time, so
|
||||
// an import would "succeed" with an inert action. See issue #1183. This mirrors
|
||||
// the embedded-YAML loader's allow-list check.
|
||||
func validateOnEnterActions(step StepPortable) error {
|
||||
for _, a := range step.Events.OnEnter {
|
||||
if a.Type != OnEnterSetSessionMode {
|
||||
continue
|
||||
}
|
||||
if mode, _ := a.Config["mode"].(string); mode == "" {
|
||||
return fmt.Errorf("step %q on_enter: set_session_mode requires a non-empty string \"mode\" config", step.Name)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func validateStepPositionRefs(steps []StepPortable, validPositions map[int]bool) error {
|
||||
for _, step := range steps {
|
||||
for _, a := range step.Events.OnTurnStart {
|
||||
|
||||
@@ -137,6 +137,34 @@ func TestValidate(t *testing.T) {
|
||||
assert.NoError(t, e.Validate())
|
||||
})
|
||||
|
||||
t.Run("set_session_mode with mode passes", func(t *testing.T) {
|
||||
e := validExport()
|
||||
e.Workflows[0].Steps[0].Events = StepEvents{
|
||||
OnEnter: []OnEnterAction{
|
||||
{Type: OnEnterSetSessionMode, Config: map[string]any{"mode": "acceptEdits"}},
|
||||
},
|
||||
}
|
||||
assert.NoError(t, e.Validate())
|
||||
})
|
||||
|
||||
t.Run("set_session_mode without mode fails", func(t *testing.T) {
|
||||
e := validExport()
|
||||
e.Workflows[0].Steps[0].Events = StepEvents{
|
||||
OnEnter: []OnEnterAction{{Type: OnEnterSetSessionMode}},
|
||||
}
|
||||
assert.ErrorContains(t, e.Validate(), "set_session_mode requires a non-empty string")
|
||||
})
|
||||
|
||||
t.Run("set_session_mode with non-string mode fails", func(t *testing.T) {
|
||||
e := validExport()
|
||||
e.Workflows[0].Steps[0].Events = StepEvents{
|
||||
OnEnter: []OnEnterAction{
|
||||
{Type: OnEnterSetSessionMode, Config: map[string]any{"mode": 3}},
|
||||
},
|
||||
}
|
||||
assert.ErrorContains(t, e.Validate(), "set_session_mode requires a non-empty string")
|
||||
})
|
||||
|
||||
t.Run("invalid move_to_step position ref fails", func(t *testing.T) {
|
||||
e := validExport()
|
||||
e.Workflows[0].Steps[0].Events = StepEvents{
|
||||
|
||||
@@ -12,6 +12,10 @@ const (
|
||||
OnEnterEnablePlanMode OnEnterActionType = "enable_plan_mode"
|
||||
OnEnterAutoStartAgent OnEnterActionType = "auto_start_agent"
|
||||
OnEnterResetAgentContext OnEnterActionType = "reset_agent_context"
|
||||
// OnEnterSetSessionMode declares the agent's session permission mode (e.g.
|
||||
// "default", "acceptEdits") for a step on entry. The target mode is carried
|
||||
// in the action Config under the "mode" key. See issue #1183.
|
||||
OnEnterSetSessionMode OnEnterActionType = "set_session_mode"
|
||||
|
||||
// Phase 2 (ADR-0004) — generic actions are also permitted on on_enter
|
||||
// so review/approval steps can clear decisions and fan out runs to
|
||||
|
||||
@@ -166,7 +166,7 @@ import/export:
|
||||
|
||||
| Trigger | Allowed action `type`s | `config` |
|
||||
|---------|------------------------|----------|
|
||||
| `on_enter` | `enable_plan_mode`, `auto_start_agent`, `reset_agent_context`, `clear_decisions`, `queue_run`, `queue_run_for_each_participant` | first four take no config; `queue_run` / `queue_run_for_each_participant` use the same config keys as the office triggers (see [Office triggers](#office--phase-2-triggers-intended-format--see-caveat)) |
|
||||
| `on_enter` | `enable_plan_mode`, `auto_start_agent`, `reset_agent_context`, `set_session_mode`, `clear_decisions`, `queue_run`, `queue_run_for_each_participant` | the first three take no config; `set_session_mode` takes `mode` (the agent permission mode to apply, e.g. `acceptEdits`); `queue_run` / `queue_run_for_each_participant` use the same config keys as the office triggers (see [Office triggers](#office--phase-2-triggers-intended-format--see-caveat)) |
|
||||
| `on_turn_start` | `move_to_next`, `move_to_previous`, `move_to_step` | `move_to_step` needs `step_position` |
|
||||
| `on_turn_complete` | `move_to_next`, `move_to_previous`, `move_to_step`, `disable_plan_mode` | `move_to_step` needs `step_position` |
|
||||
| `on_exit` | `disable_plan_mode` | — |
|
||||
|
||||
Reference in New Issue
Block a user