fix: restore completed task conversation follow-ups (#3564)

This commit is contained in:
Carlos Florêncio
2026-09-10 15:01:37 +01:00
committed by GitHub
parent 3973825f66
commit 272ddfca8a
136 changed files with 5015 additions and 460 deletions
@@ -11,10 +11,10 @@ WORKFLOW TOOLS:
- update_workflow_kandev: Update a workflow. Required: workflow_id. Optional: name, description.
- delete_workflow_kandev: Delete a workflow and all its steps (destructive). Required: workflow_id.
- import_workflow_kandev: Import one or more workflows into a workspace from a portable document (the kandev_workflow YAML/JSON export envelope). Workflows whose name already exists are skipped. Required: workspace_id, document. Returns the created and skipped workflow names.
- export_workflow_kandev: Export one workflow as a portable version 1 kandev_workflow JSON document. Required: workflow_id. Pass the returned JSON unchanged as document to import_workflow_kandev.
- export_workflow_kandev: Export one workflow as a portable version 2 kandev_workflow JSON document. Required: workflow_id. Pass the returned JSON unchanged as document to import_workflow_kandev. Version 1 documents remain accepted for compatibility.
- list_workflow_steps_kandev: List workflow steps (columns) in a workflow. Required: workflow_id.
- create_workflow_step_kandev: Create a new workflow step. Required: workflow_id, name. Optional: position, color, prompt, is_start_step, allow_manual_move, show_in_command_panel, auto_advance_requires_signal, cancel_triggers_turn_complete, events.
- update_workflow_step_kandev: Update a workflow step. Required: step_id. Optional: name, color, prompt, is_start_step, allow_manual_move, show_in_command_panel, auto_archive_after_hours, auto_advance_requires_signal, cancel_triggers_turn_complete, events.
- create_workflow_step_kandev: Create a new workflow step. Required: workflow_id, name. Optional: position, color, prompt, is_start_step, allow_manual_move, show_in_command_panel, complete_task_on_enter, auto_advance_requires_signal, cancel_triggers_turn_complete, events.
- update_workflow_step_kandev: Update a workflow step. Required: step_id. Optional: name, color, prompt, is_start_step, allow_manual_move, show_in_command_panel, auto_archive_after_hours, complete_task_on_enter, auto_advance_requires_signal, cancel_triggers_turn_complete, events.
- cancel_triggers_turn_complete: When true, an explicit user cancellation runs the step's normal on_turn_complete actions after the cancelled turn settles. It does not apply to internal or agent cancellations.
- delete_workflow_step_kandev: Delete a workflow step (destructive). Required: step_id.
- reorder_workflow_steps_kandev: Reorder workflow steps. Required: workflow_id, step_ids (ordered array of step IDs).
@@ -75,4 +75,5 @@ steps:
name: Approved
position: 3
color: bg-green-500
complete_task_on_enter: true
allow_manual_move: true
@@ -349,4 +349,5 @@ steps:
name: Done
position: 7
color: bg-green-500
complete_task_on_enter: true
allow_manual_move: true
+1
View File
@@ -54,6 +54,7 @@ steps:
name: Done
position: 3
color: bg-green-500
complete_task_on_enter: true
allow_manual_move: true
events:
on_turn_start:
+2
View File
@@ -39,6 +39,7 @@ type stepDefYAML struct {
ProfileSessionEndPolicy taskmodels.WorkflowProfileSessionEndPolicy `yaml:"profile_session_end_policy,omitempty"`
AutoAdvanceRequiresSignal bool `yaml:"auto_advance_requires_signal,omitempty"`
CancelTriggersTurnComplete bool `yaml:"cancel_triggers_turn_complete,omitempty"`
CompleteTaskOnEnter bool `yaml:"complete_task_on_enter,omitempty"`
Events stepEventsYAML `yaml:"events,omitempty"`
}
@@ -183,6 +184,7 @@ func convertStep(s stepDefYAML) (models.StepDefinition, error) {
ProfileSessionEndPolicy: taskmodels.NormalizeWorkflowProfileSessionEndPolicy(string(s.ProfileSessionEndPolicy)),
AutoAdvanceRequiresSignal: s.AutoAdvanceRequiresSignal,
CancelTriggersTurnComplete: s.CancelTriggersTurnComplete,
CompleteTaskOnEnter: s.CompleteTaskOnEnter,
StageType: stage,
}, nil
}
@@ -61,6 +61,32 @@ func TestLoadTemplates_CancelTriggersTurnCompleteDefaults(t *testing.T) {
}
}
func TestLoadTemplates_CompletionPolicyUsesExplicitFinalStepSetting(t *testing.T) {
templates, err := LoadTemplates()
if err != nil {
t.Fatalf("LoadTemplates() returned error: %v", err)
}
for _, template := range templates {
for _, step := range template.Steps {
want := template.ID != "improve-kandev" && template.ID != "report-kandev-issue" &&
step.Position == lastTemplatePosition(template.Steps)
if step.CompleteTaskOnEnter != want {
t.Errorf("template %q step %q completion policy = %t, want %t", template.ID, step.Name, step.CompleteTaskOnEnter, want)
}
}
}
}
func lastTemplatePosition(steps []models.StepDefinition) int {
last := 0
for i, step := range steps {
if i == 0 || step.Position > last {
last = step.Position
}
}
return last
}
func TestLoadTemplates_AllValid(t *testing.T) {
templates, err := LoadTemplates()
if err != nil {
@@ -144,6 +144,7 @@ steps:
position: 4
color: bg-green-500
stage_type: custom
complete_task_on_enter: true
allow_manual_move: true
show_in_command_panel: true
events:
@@ -117,4 +117,5 @@ steps:
name: Done
position: 3
color: bg-green-500
complete_task_on_enter: true
allow_manual_move: true
@@ -169,4 +169,5 @@ steps:
name: Done
position: 2
color: bg-green-500
complete_task_on_enter: true
allow_manual_move: true
@@ -27,5 +27,6 @@ steps:
name: Done
position: 1
color: bg-green-500
complete_task_on_enter: true
show_in_command_panel: false
allow_manual_move: false
@@ -1,8 +1,10 @@
package handlers
import (
"bytes"
"context"
"encoding/json"
"fmt"
"github.com/kandev/kandev/internal/events"
"github.com/kandev/kandev/internal/task/service"
@@ -162,6 +164,7 @@ func (h *Handlers) handleCreateWorkflowStep(ctx context.Context, msg *ws.Message
ShowInCommandPanel *bool `json:"show_in_command_panel"`
AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal"`
CancelTriggersTurnComplete *bool `json:"cancel_triggers_turn_complete"`
CompleteTaskOnEnter json.RawMessage `json:"complete_task_on_enter"`
WIPLimit *int `json:"wip_limit"`
PullFromStepID *string `json:"pull_from_step_id"`
Events *wfmodels.StepEvents `json:"events"`
@@ -169,6 +172,10 @@ func (h *Handlers) handleCreateWorkflowStep(ctx context.Context, msg *ws.Message
if err := json.Unmarshal(msg.Payload, &req); err != nil {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil)
}
completeTaskOnEnter, err := optionalCompleteTaskOnEnter(req.CompleteTaskOnEnter)
if err != nil {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil)
}
if req.WorkflowID == "" {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "workflow_id is required", nil)
}
@@ -189,6 +196,7 @@ func (h *Handlers) handleCreateWorkflowStep(ctx context.Context, msg *ws.Message
ShowInCommandPanel: req.ShowInCommandPanel,
AutoAdvanceRequiresSignal: req.AutoAdvanceRequiresSignal,
CancelTriggersTurnComplete: req.CancelTriggersTurnComplete,
CompleteTaskOnEnter: completeTaskOnEnter,
WIPLimit: req.WIPLimit,
PullFromStepID: req.PullFromStepID,
Events: req.Events,
@@ -207,6 +215,20 @@ func (h *Handlers) handleCreateWorkflowStep(ctx context.Context, msg *ws.Message
return ws.NewResponse(msg.ID, msg.Action, resp)
}
func optionalCompleteTaskOnEnter(raw json.RawMessage) (*bool, error) {
if len(raw) == 0 {
return nil, nil
}
if bytes.Equal(bytes.TrimSpace(raw), []byte("null")) {
return nil, fmt.Errorf("complete_task_on_enter must be a boolean")
}
var value bool
if err := json.Unmarshal(raw, &value); err != nil {
return nil, err
}
return &value, nil
}
func (h *Handlers) handleUpdateWorkflowStep(ctx context.Context, msg *ws.Message) (*ws.Message, error) {
var req struct {
StepID string `json:"step_id"`
@@ -222,6 +244,7 @@ func (h *Handlers) handleUpdateWorkflowStep(ctx context.Context, msg *ws.Message
AutoArchiveAfterHours *int `json:"auto_archive_after_hours"`
AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal"`
CancelTriggersTurnComplete *bool `json:"cancel_triggers_turn_complete"`
CompleteTaskOnEnter json.RawMessage `json:"complete_task_on_enter"`
WIPLimit *int `json:"wip_limit"`
PullFromStepID *string `json:"pull_from_step_id"`
Events *wfmodels.StepEvents `json:"events"`
@@ -229,6 +252,10 @@ func (h *Handlers) handleUpdateWorkflowStep(ctx context.Context, msg *ws.Message
if err := json.Unmarshal(msg.Payload, &req); err != nil {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil)
}
completeTaskOnEnter, err := optionalCompleteTaskOnEnter(req.CompleteTaskOnEnter)
if err != nil {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeBadRequest, "Invalid payload: "+err.Error(), nil)
}
if req.StepID == "" {
return ws.NewError(msg.ID, msg.Action, ws.ErrorCodeValidation, "step_id is required", nil)
}
@@ -247,6 +274,7 @@ func (h *Handlers) handleUpdateWorkflowStep(ctx context.Context, msg *ws.Message
AutoArchiveAfterHours: req.AutoArchiveAfterHours,
AutoAdvanceRequiresSignal: req.AutoAdvanceRequiresSignal,
CancelTriggersTurnComplete: req.CancelTriggersTurnComplete,
CompleteTaskOnEnter: completeTaskOnEnter,
WIPLimit: req.WIPLimit,
PullFromStepID: req.PullFromStepID,
Events: req.Events,
@@ -216,6 +216,23 @@ func TestHandleCreateWorkflowStep_PersistsCancelTriggersTurnComplete(t *testing.
assert.True(t, steps[0].CancelTriggersTurnComplete)
}
func TestHandleCreateWorkflowStep_RejectsNullCompletionPolicy(t *testing.T) {
h, _, repo := setupImportHandlers(t)
ctx := context.Background()
h.workflowCtrl = workflowctrl.NewController(h.workflowSvc)
resp, err := h.handleCreateWorkflowStep(ctx, makeWSMessage(t, ws.ActionMCPCreateWorkflowStep, map[string]interface{}{
"workflow_id": "wf-test",
"name": "Null completion",
"complete_task_on_enter": nil,
}))
require.NoError(t, err)
assertWSError(t, resp, ws.ErrorCodeBadRequest)
steps, err := repo.ListStepsByWorkflow(ctx, "wf-test")
require.NoError(t, err)
require.Empty(t, steps)
}
func TestHandleUpdateWorkflowStep_PublishesDemotedStartStep(t *testing.T) {
h, _, repo := setupImportHandlers(t)
ctx := context.Background()
@@ -316,6 +333,29 @@ func TestHandleUpdateWorkflowStep_PersistsCancelTriggersTurnCompleteFalse(t *tes
assert.False(t, step.CancelTriggersTurnComplete)
}
func TestHandleUpdateWorkflowStep_RejectsNullCompletionPolicyWithoutMutation(t *testing.T) {
h, _, repo := setupImportHandlers(t)
ctx := context.Background()
h.workflowCtrl = workflowctrl.NewController(h.workflowSvc)
require.NoError(t, repo.CreateStep(ctx, &wfmodels.WorkflowStep{
ID: "completed-step",
WorkflowID: "wf-test",
Name: "Done",
Position: 0,
CompleteTaskOnEnter: true,
}))
resp, err := h.handleUpdateWorkflowStep(ctx, makeWSMessage(t, ws.ActionMCPUpdateWorkflowStep, map[string]interface{}{
"step_id": "completed-step",
"complete_task_on_enter": nil,
}))
require.NoError(t, err)
assertWSError(t, resp, ws.ErrorCodeBadRequest)
step, err := repo.GetStep(ctx, "completed-step")
require.NoError(t, err)
require.True(t, step.CompleteTaskOnEnter)
}
func TestHandleListWorkflowSteps_IncludesAutoAdvanceRequiresSignal(t *testing.T) {
h, _, repo := setupImportHandlers(t)
ctx := context.Background()
@@ -3198,6 +3198,15 @@ func (h *Handlers) dispatchTaskMessage(ctx context.Context, taskID string, sessi
switch session.State {
case models.TaskSessionStateFailed, models.TaskSessionStateCancelled:
return taskMessageDispatchResult{}, terminalSessionDispatchError(session)
case models.TaskSessionStateCompleted:
if !pinnedTarget {
return taskMessageDispatchResult{}, terminalSessionDispatchError(session)
}
resumed, err := h.resumeCompletedTaskMessageSession(ctx, taskID, session.ID)
if err != nil {
return taskMessageDispatchResult{}, err
}
return h.dispatchPreparedTaskMessage(ctx, taskID, resumed, prompt, metadata)
case models.TaskSessionStateRunning, models.TaskSessionStateStarting:
if interruptIfBusy {
@@ -3249,6 +3258,33 @@ func (h *Handlers) dispatchTaskMessage(ctx context.Context, taskID string, sessi
}
}
// resumeCompletedTaskMessageSession admits a pinned completed target through
// the same guarded resume path as the user-facing recovery action. The session
// is reloaded before the message is recorded so the prompt cannot be attached
// to a different session or to the pre-resume terminal snapshot.
func (h *Handlers) resumeCompletedTaskMessageSession(ctx context.Context, taskID, sessionID string) (*models.TaskSession, error) {
_, err := h.sessionLauncher.LaunchSession(ctx, &orchestrator.LaunchSessionRequest{
TaskID: taskID,
SessionID: sessionID,
Intent: orchestrator.IntentResume,
AllowCompletedSessionResume: true,
})
if err != nil {
return nil, fmt.Errorf("failed to resume completed session: %w", err)
}
resumed, err := h.taskSvc.GetTaskSession(ctx, sessionID)
if err != nil {
return nil, fmt.Errorf("failed to reload completed session after resume: %w", err)
}
if resumed == nil || resumed.TaskID != taskID {
return nil, errors.New("completed session was not available after resume")
}
if resumed.State == models.TaskSessionStateCompleted {
return nil, errors.New("completed session did not become ready for a follow-up message")
}
return resumed, nil
}
func (h *Handlers) dispatchPreparedTaskMessage(ctx context.Context, taskID string, session *models.TaskSession, prompt string, metadata map[string]interface{}) (taskMessageDispatchResult, error) {
switch session.State {
case models.TaskSessionStateFailed, models.TaskSessionStateCancelled:
@@ -98,6 +98,13 @@ func (h *Handlers) resolveMessageTargetSession(
// business rewriting as a side effect.
return primary, true, nil
}
completed, completedErr := h.newestCompletedSession(ctx, taskID)
if completedErr != nil {
return nil, false, wsError(msg.ID, msg.Action, ws.ErrorCodeInternalError, "failed to list completed sessions for task: "+completedErr.Error())
}
if completed != nil {
return completed, true, nil
}
return nil, false, wsError(msg.ID, msg.Action, ws.ErrorCodeNotFound,
"target task exists but has no active session — use spawn_session_kandev to start one")
}
@@ -118,6 +125,23 @@ func (h *Handlers) newestLiveSession(ctx context.Context, taskID string) (*model
return nil, nil
}
// newestCompletedSession returns the most recently started completed session
// on a task. It is used only when the task has no primary and no live session;
// a completed primary remains a dedicated terminal target so FAILED/CANCELLED
// dispatch behavior is not changed.
func (h *Handlers) newestCompletedSession(ctx context.Context, taskID string) (*models.TaskSession, error) {
sessions, err := h.taskSvc.ListTaskSessions(ctx, taskID)
if err != nil {
return nil, err
}
for _, session := range sessions {
if session != nil && session.State == models.TaskSessionStateCompleted {
return session, nil
}
}
return nil, nil
}
// resolveExplicitTargetSession loads and validates a caller-named target
// session for message_task. Only genuine no-row lookups are NotFound;
// transient DB errors surface as internal so callers keep retrying instead of
@@ -306,6 +306,34 @@ func TestHandleMessageTask_CompletedSiblingDoesNotShadowTheLiveFallback(t *testi
assert.Equal(t, 0, orch.queue.GetStatus(context.Background(), retired.ID).Count)
}
// A task with no primary and no live session can still address its newest
// completed conversation. The resolver pins that historical choice so the
// dispatcher can perform explicit completed-session recovery on the same row.
func TestResolveMessageTargetSession_NoPrimaryUsesNewestCompletedSession(t *testing.T) {
svc, repo := newTestTaskService(t)
ctx := context.Background()
require.NoError(t, repo.CreateWorkspace(ctx, &models.Workspace{ID: "ws-completed-fallback", Name: "Test"}))
require.NoError(t, repo.CreateWorkflow(ctx, &models.Workflow{ID: "wf-completed-fallback", WorkspaceID: "ws-completed-fallback", Name: "Board"}))
targetResult, err := svc.CreateTask(ctx, &service.CreateTaskRequest{
WorkspaceID: "ws-completed-fallback", WorkflowID: "wf-completed-fallback", Title: "Target",
})
require.NoError(t, err)
older := addSession(t, repo, targetResult.Task.ID, "sess-completed-older", models.TaskSessionStateCompleted, 1)
newer := addSession(t, repo, targetResult.Task.ID, "sess-completed-newer", models.TaskSessionStateCompleted, 2)
require.NotEqual(t, older.ID, newer.ID)
h, _ := newMessageTaskHandler(t, svc, repo)
msg := makeWSMessage(t, ws.ActionMCPMessageTask, map[string]interface{}{
"task_id": targetResult.Task.ID,
"prompt": "continue the conversation",
})
session, pinned, errResp := h.resolveMessageTargetSession(ctx, msg, targetResult.Task.ID, "")
require.Nil(t, errResp)
require.NotNil(t, session)
assert.Equal(t, newer.ID, session.ID)
assert.True(t, pinned)
}
// indexOfSession reports where a session lands in the resolver's walk order.
func indexOfSession(t *testing.T, sessions []*models.TaskSession, sessionID string) int {
t.Helper()
@@ -17,6 +17,7 @@ import (
promptservice "github.com/kandev/kandev/internal/prompts/service"
"github.com/kandev/kandev/internal/steptelemetry"
"github.com/kandev/kandev/internal/task/models"
sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite"
"github.com/kandev/kandev/internal/task/service"
v1 "github.com/kandev/kandev/pkg/api/v1"
ws "github.com/kandev/kandev/pkg/websocket"
@@ -41,6 +42,7 @@ type fakeOrchestrator struct {
interruptCalls []interruptCall
launchCalls []*orchestrator.LaunchSessionRequest
launchErr error
launchFunc func(context.Context, *orchestrator.LaunchSessionRequest) (*orchestrator.LaunchSessionResponse, error)
launchResponseProfileID string
renameCalls []renameCall
renameErr error
@@ -105,13 +107,18 @@ type renameCall struct {
sessionID, name string
}
func (f *fakeOrchestrator) LaunchSession(_ context.Context, req *orchestrator.LaunchSessionRequest) (*orchestrator.LaunchSessionResponse, error) {
func (f *fakeOrchestrator) LaunchSession(ctx context.Context, req *orchestrator.LaunchSessionRequest) (*orchestrator.LaunchSessionResponse, error) {
f.mu.Lock()
defer f.mu.Unlock()
f.launchCalls = append(f.launchCalls, req)
launchFunc := f.launchFunc
if f.launchErr != nil {
f.mu.Unlock()
return nil, f.launchErr
}
f.mu.Unlock()
if launchFunc != nil {
return launchFunc(ctx, req)
}
response := &orchestrator.LaunchSessionResponse{
Success: true,
TaskID: req.TaskID,
@@ -126,6 +133,34 @@ func (f *fakeOrchestrator) LaunchSession(_ context.Context, req *orchestrator.La
return response, nil
}
func completedSessionLaunchFunc(
t *testing.T,
repo *sqliterepo.Repository,
taskID, sessionID string,
) func(context.Context, *orchestrator.LaunchSessionRequest) (*orchestrator.LaunchSessionResponse, error) {
t.Helper()
return func(ctx context.Context, req *orchestrator.LaunchSessionRequest) (*orchestrator.LaunchSessionResponse, error) {
require.Equal(t, orchestrator.IntentResume, req.Intent)
require.True(t, req.AllowCompletedSessionResume)
require.Equal(t, taskID, req.TaskID)
require.Equal(t, sessionID, req.SessionID)
require.NoError(t, repo.UpdateTaskSessionState(ctx, sessionID, models.TaskSessionStateWaitingForInput, ""))
require.NoError(t, repo.SetSessionMetadataKey(ctx, sessionID, models.SessionMetaKeyCompletionFollowUp, true))
require.NoError(t, repo.UpsertExecutorRunning(ctx, &models.ExecutorRunning{
ID: "exec-row-" + sessionID,
SessionID: sessionID,
TaskID: taskID,
Status: "running",
Resumable: true,
AgentExecutionID: "exec-" + sessionID,
}))
return &orchestrator.LaunchSessionResponse{
Success: true, TaskID: taskID, SessionID: sessionID,
State: string(models.TaskSessionStateWaitingForInput),
}, nil
}
}
func (f *fakeOrchestrator) PromptTask(_ context.Context, taskID, sessionID, prompt, _ string, _ bool, _ []v1.MessageAttachment, dispatchOnly bool) (*orchestrator.PromptResult, error) {
f.mu.Lock()
defer f.mu.Unlock()
@@ -576,6 +611,57 @@ func TestHandleMessageTask_QueuesPromptWhenOnTurnStartQueuesTask(t *testing.T) {
assert.Contains(t, status.Entries[0].Content, "wait for admission")
}
func TestHandleMessageTask_CompletedSessionResumesBeforePersistingPrompt(t *testing.T) {
svc, repo := newTestTaskService(t)
sender, target, session := seedTaskWithSession(t, svc, repo, models.TaskSessionStateCompleted)
ctx := context.Background()
require.NoError(t, repo.UpdateTaskState(ctx, target.ID, v1.TaskStateCompleted))
h, orch := newMessageTaskHandler(t, svc, repo)
orch.launchFunc = func(ctx context.Context, req *orchestrator.LaunchSessionRequest) (*orchestrator.LaunchSessionResponse, error) {
require.Equal(t, orchestrator.IntentResume, req.Intent)
require.True(t, req.AllowCompletedSessionResume)
require.Equal(t, target.ID, req.TaskID)
require.Equal(t, session.ID, req.SessionID)
require.NoError(t, repo.UpdateTaskSessionState(ctx, session.ID, models.TaskSessionStateWaitingForInput, ""))
require.NoError(t, repo.UpsertExecutorRunning(ctx, &models.ExecutorRunning{
ID: "exec-row-" + session.ID,
SessionID: session.ID,
TaskID: target.ID,
Status: "running",
Resumable: true,
AgentExecutionID: "exec-" + session.ID,
}))
return &orchestrator.LaunchSessionResponse{
Success: true, TaskID: target.ID, SessionID: session.ID,
State: string(models.TaskSessionStateWaitingForInput),
}, nil
}
msg := makeWSMessage(t, ws.ActionMCPMessageTask,
senderPayload(target.ID, "continue the conversation", sender.ID))
resp, err := h.handleMessageTask(ctx, msg)
require.NoError(t, err)
require.Equal(t, ws.MessageTypeResponse, resp.Type)
var out map[string]interface{}
require.NoError(t, json.Unmarshal(resp.Payload, &out))
assert.Equal(t, session.ID, out["session_id"])
assert.Equal(t, taskMessageStatusSent, out[stopTaskStatusKey])
require.Len(t, orch.launchCalls, 1)
require.Len(t, orch.promptCalls, 1)
assert.Equal(t, session.ID, orch.promptCalls[0].sessionID)
assert.Equal(t, 0, orch.resumeCalls, "completed follow-up must use the explicit launch permission")
messages, err := svc.ListMessages(ctx, session.ID)
require.NoError(t, err)
require.Len(t, messages, 1)
assert.Contains(t, messages[0].Content, "continue the conversation")
reloadedTask, err := svc.GetTask(ctx, target.ID)
require.NoError(t, err)
assert.Equal(t, v1.TaskStateCompleted, reloadedTask.State, "conversation follow-up must not reopen task work")
}
func TestHandleMessageTask_SessionIDWrongTask_Rejected(t *testing.T) {
svc, repo := newTestTaskService(t)
sender, target, sess := seedTaskWithSession(t, svc, repo, models.TaskSessionStateRunning)
@@ -1181,8 +1267,10 @@ func TestHandleMessageTask_CompletedSessionWithoutSwitch_PromptsSameSession(t *t
ctx := context.Background()
svc, repo := newTestTaskService(t)
sender, target, sess := seedTaskWithSession(t, svc, repo, models.TaskSessionStateCompleted)
require.NoError(t, repo.UpdateTaskState(ctx, target.ID, v1.TaskStateCompleted))
h, orch := newMessageTaskHandler(t, svc, repo)
orch.launchFunc = completedSessionLaunchFunc(t, repo, target.ID, sess.ID)
msg := makeWSMessage(t, ws.ActionMCPMessageTask, senderPayload(target.ID, "follow up completed", sender.ID))
resp, err := h.handleMessageTask(ctx, msg)
@@ -1205,27 +1293,16 @@ func TestHandleMessageTask_CompletedSessionWithoutSwitch_PromptsSameSession(t *t
assert.Equal(t, sender.ID, messages[0].Metadata["sender_task_id"])
}
func TestHandleMessageTask_CompletedSession_UsesSessionSelectedByTurnStart(t *testing.T) {
func TestHandleMessageTask_CompletedSession_DoesNotReplayTurnStart(t *testing.T) {
ctx := context.Background()
svc, repo := newTestTaskService(t)
sender, target, sess := seedTaskWithSession(t, svc, repo, models.TaskSessionStateCompleted)
replacement := &models.TaskSession{
ID: "sess-2",
TaskID: target.ID,
AgentProfileID: "agent-profile-2",
State: models.TaskSessionStateWaitingForInput,
IsPrimary: false,
}
require.NoError(t, repo.CreateTaskSession(ctx, replacement))
require.NoError(t, repo.UpdateTaskState(ctx, target.ID, v1.TaskStateCompleted))
h, orch := newMessageTaskHandler(t, svc, repo)
orch.onTurnStart = func(ctx context.Context, _, _ string) error {
oldSession, err := svc.GetTaskSession(ctx, sess.ID)
require.NoError(t, err)
oldSession.IsPrimary = false
require.NoError(t, repo.UpdateTaskSession(ctx, oldSession))
require.NoError(t, repo.SetSessionPrimary(ctx, replacement.ID))
orch.launchFunc = completedSessionLaunchFunc(t, repo, target.ID, sess.ID)
orch.onTurnStart = func(context.Context, string, string) error {
t.Fatal("completed follow-up must resume before on_turn_start")
return nil
}
@@ -1237,21 +1314,18 @@ func TestHandleMessageTask_CompletedSession_UsesSessionSelectedByTurnStart(t *te
var payload map[string]interface{}
require.NoError(t, json.Unmarshal(resp.Payload, &payload))
assert.Equal(t, "started", payload["status"])
assert.Equal(t, replacement.ID, payload["session_id"])
assert.Equal(t, "sent", payload["status"])
assert.Equal(t, sess.ID, payload["session_id"])
require.Len(t, orch.startCreatedCalls, 1)
assert.Equal(t, replacement.ID, orch.startCreatedCalls[0].sessionID)
assert.Empty(t, orch.promptCalls)
assert.Empty(t, orch.startCreatedCalls)
require.Len(t, orch.promptCalls, 1)
assert.Equal(t, sess.ID, orch.promptCalls[0].sessionID)
oldMessages, err := svc.ListMessages(ctx, sess.ID)
require.NoError(t, err)
assert.Empty(t, oldMessages)
newMessages, err := svc.ListMessages(ctx, replacement.ID)
require.NoError(t, err)
require.Len(t, newMessages, 1)
assert.Contains(t, newMessages[0].Content, "handoff from completed")
assert.Equal(t, sender.ID, newMessages[0].Metadata["sender_task_id"])
require.Len(t, oldMessages, 1)
assert.Contains(t, oldMessages[0].Content, "handoff from completed")
assert.Equal(t, sender.ID, oldMessages[0].Metadata["sender_task_id"])
}
func TestHandleMessageTask_WaitingForInputCompletedWithoutPrimarySwitchRejects(t *testing.T) {
@@ -63,7 +63,7 @@ func (s *Server) registerConfigWorkflowTools() {
)
s.mcpServer.AddTool(
mcp.NewTool("import_workflow_kandev",
mcp.WithDescription("Import one or more workflows into a workspace from a portable document. The document is the same YAML/JSON envelope produced by the workflow export (type: kandev_workflow, version: 1) and may contain multiple workflows. Workflows whose name already exists in the workspace are skipped. Returns the names that were created and skipped."),
mcp.WithDescription("Import one or more workflows into a workspace from a portable document. The document is the same YAML/JSON envelope produced by the workflow export (type: kandev_workflow, version: 2; version 1 is accepted) and may contain multiple workflows. Workflows whose name already exists in the workspace are skipped. Returns the names that were created and skipped."),
mcp.WithString("workspace_id", mcp.Required(), mcp.Description("The workspace ID to import the workflows into")),
mcp.WithString(documentArg, mcp.Required(), mcp.Description("The portable workflow document as a YAML or JSON string (a kandev_workflow export envelope). Includes the workflows and their steps.")),
),
@@ -71,7 +71,7 @@ func (s *Server) registerConfigWorkflowTools() {
)
s.mcpServer.AddTool(
mcp.NewTool("export_workflow_kandev",
mcp.WithDescription("Export one workflow as a portable version 1 kandev_workflow JSON document. The result contains one workflow and its steps without instance IDs or timestamps. Pass the JSON text unchanged as document to import_workflow_kandev (the import document limit is 1 MiB)."),
mcp.WithDescription("Export one workflow as a portable version 2 kandev_workflow JSON document. The result contains one workflow and its steps without instance IDs or timestamps. Pass the JSON text unchanged as document to import_workflow_kandev (the import document limit is 1 MiB)."),
mcp.WithReadOnlyHintAnnotation(true),
mcp.WithDestructiveHintAnnotation(false),
mcp.WithIdempotentHintAnnotation(true),
@@ -107,6 +107,7 @@ func (s *Server) registerConfigWorkflowStepTools() {
mcp.WithBoolean("show_in_command_panel", mcp.Description("Show this step in the command panel")),
mcp.WithBoolean("auto_advance_requires_signal", mcp.Description("Require step_complete_kandev before on_turn_complete auto-advance transitions run")),
mcp.WithBoolean("cancel_triggers_turn_complete", mcp.Description("Run on_turn_complete actions when an explicit user cancellation occurs")),
mcp.WithBoolean("complete_task_on_enter", mcp.Description("Complete the task when it enters this final workflow step")),
mcp.WithNumber("wip_limit", mcp.Description("Work-in-progress limit for this step. 0 means unlimited.")),
mcp.WithString("pull_from_step_id", mcp.Description("Optional feeder workflow step ID to pull from when capacity opens.")),
mcp.WithObject("events", mcp.Description("Event-driven actions. Keys: on_enter, on_exit, on_turn_start, on_turn_complete. Each is an array of {type, config} objects.")),
@@ -129,6 +130,7 @@ func (s *Server) registerConfigWorkflowStepTools() {
mcp.WithNumber("auto_archive_after_hours", mcp.Description("Auto-archive tasks after N hours in this step (0 to disable)")),
mcp.WithBoolean("auto_advance_requires_signal", mcp.Description("Require step_complete_kandev before on_turn_complete auto-advance transitions run")),
mcp.WithBoolean("cancel_triggers_turn_complete", mcp.Description("Run on_turn_complete actions when an explicit user cancellation occurs")),
mcp.WithBoolean("complete_task_on_enter", mcp.Description("Complete the task when it enters this final workflow step")),
mcp.WithNumber("wip_limit", mcp.Description("Work-in-progress limit for this step. 0 means unlimited.")),
mcp.WithString("pull_from_step_id", mcp.Description("Optional feeder workflow step ID to pull from when capacity opens.")),
mcp.WithObject("events", mcp.Description("Event-driven actions. Keys: on_enter, on_exit, on_turn_start, on_turn_complete.")),
@@ -496,7 +498,7 @@ func (s *Server) createWorkflowStepHandler() server.ToolHandlerFunc {
payload["prompt"] = prompt
}
args := req.GetArguments()
for _, key := range []string{"position", "is_start_step", "allow_manual_move", "show_in_command_panel", "agent_profile_id", "profile_session_start_policy", "profile_session_end_policy", "auto_advance_requires_signal", "cancel_triggers_turn_complete", "wip_limit", "pull_from_step_id", "events"} {
for _, key := range []string{"position", "is_start_step", "allow_manual_move", "show_in_command_panel", "agent_profile_id", "profile_session_start_policy", "profile_session_end_policy", "auto_advance_requires_signal", "cancel_triggers_turn_complete", "complete_task_on_enter", "wip_limit", "pull_from_step_id", "events"} {
if args[key] != nil {
payload[key] = args[key]
}
@@ -522,7 +524,7 @@ func (s *Server) updateWorkflowStepHandler() server.ToolHandlerFunc {
payload["prompt"] = prompt
}
args := req.GetArguments()
for _, key := range []string{"is_start_step", "allow_manual_move", "show_in_command_panel", "agent_profile_id", "profile_session_start_policy", "profile_session_end_policy", "auto_archive_after_hours", "auto_advance_requires_signal", "cancel_triggers_turn_complete", "wip_limit", "pull_from_step_id", "events"} {
for _, key := range []string{"is_start_step", "allow_manual_move", "show_in_command_panel", "agent_profile_id", "profile_session_start_policy", "profile_session_end_policy", "auto_archive_after_hours", "auto_advance_requires_signal", "cancel_triggers_turn_complete", "complete_task_on_enter", "wip_limit", "pull_from_step_id", "events"} {
if args[key] != nil {
payload[key] = args[key]
}
@@ -234,6 +234,9 @@ func ciAutomationSessionCanReceivePrompt(session *models.TaskSession) bool {
if session == nil {
return false
}
if models.IsCompletionFollowUpSession(session.Metadata) {
return false
}
switch session.State {
case models.TaskSessionStateCreated,
models.TaskSessionStateStarting,
@@ -0,0 +1,360 @@
package orchestrator
import (
"context"
"sync"
"sync/atomic"
"testing"
"time"
"github.com/kandev/kandev/internal/orchestrator/executor"
"github.com/kandev/kandev/internal/orchestrator/watcher"
"github.com/kandev/kandev/internal/task/models"
v1 "github.com/kandev/kandev/pkg/api/v1"
"github.com/stretchr/testify/require"
)
// TestCompletedSessionResumePreservesConversation covers the explicit recovery
// path for a completed session. Recovery must relaunch the existing provider
// conversation in place instead of creating a sibling or silently losing the
// provider resume identity.
func TestCompletedSessionResumePreservesConversation(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
taskRepo := newMockTaskRepo()
var captured *executor.LaunchAgentRequest
started := false
agentMgr := &sessionUpdatingAgentManager{
mockAgentManager: &mockAgentManager{
launchAgentFunc: func(_ context.Context, req *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
captured = req
return &executor.LaunchAgentResponse{
AgentExecutionID: "exec-completed-follow-up",
Status: v1.AgentStatusStarting,
}, nil
},
},
repo: repo,
sessionID: "session-completed-follow-up",
taskID: "task-completed-follow-up",
onStartCalled: &started,
}
svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr)
svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{})
seedTaskAndSession(
t,
repo,
"task-completed-follow-up",
"session-completed-follow-up",
models.TaskSessionStateCompleted,
)
now := time.Now().UTC()
require.NoError(t, repo.CreateExecutor(ctx, &models.Executor{
ID: "executor-completed-follow-up",
Name: "Worktree",
Type: models.ExecutorTypeWorktree,
Status: models.ExecutorStatusActive,
Resumable: true,
}))
require.NoError(t, repo.CreateRepository(ctx, &models.Repository{
ID: "repo-completed-follow-up",
WorkspaceID: "ws1",
Name: "backend",
SourceType: "local",
LocalPath: t.TempDir(),
DefaultBranch: "main",
CreatedAt: now,
UpdatedAt: now,
}))
require.NoError(t, repo.CreateTaskRepository(ctx, &models.TaskRepository{
ID: "task-repo-completed-follow-up",
TaskID: "task-completed-follow-up",
RepositoryID: "repo-completed-follow-up",
BaseBranch: "main",
Position: 0,
CreatedAt: now,
UpdatedAt: now,
}))
session, err := repo.GetTaskSession(ctx, "session-completed-follow-up")
require.NoError(t, err)
session.AgentProfileID = "profile-completed-follow-up"
session.ExecutorID = "executor-completed-follow-up"
session.RepositoryID = "repo-completed-follow-up"
session.BaseBranch = "main"
require.NoError(t, repo.UpdateTaskSession(ctx, session))
require.NoError(t, repo.UpsertExecutorRunning(ctx, &models.ExecutorRunning{
ID: "running-completed-follow-up",
SessionID: "session-completed-follow-up",
TaskID: "task-completed-follow-up",
AgentExecutionID: "exec-before-completed-follow-up",
ResumeToken: "acp-session-completed-follow-up",
Resumable: true,
CreatedAt: now,
UpdatedAt: now,
}))
response, err := svc.RecoverSession(
ctx,
"task-completed-follow-up",
"session-completed-follow-up",
"resume",
)
require.NoError(t, err)
require.NotNil(t, response)
require.Equal(t, "task-completed-follow-up", response.TaskID)
require.Equal(t, "session-completed-follow-up", response.SessionID)
require.NotNil(t, captured)
require.Equal(t, "session-completed-follow-up", captured.SessionID)
require.Equal(t, "acp-session-completed-follow-up", captured.ACPSessionID)
require.True(t, started)
reloaded, err := repo.GetTaskSession(ctx, "session-completed-follow-up")
require.NoError(t, err)
require.Equal(t, models.TaskSessionStateWaitingForInput, reloaded.State)
require.Equal(t, session.ID, reloaded.ID)
require.NotNil(t, reloaded.Metadata)
require.Equal(t, true, reloaded.Metadata["completion_follow_up"])
running, err := repo.GetExecutorRunningBySessionID(ctx, "session-completed-follow-up")
require.NoError(t, err)
require.Equal(t, "acp-session-completed-follow-up", running.ResumeToken)
sessions, err := repo.ListTaskSessions(ctx, "task-completed-follow-up")
require.NoError(t, err)
require.Len(t, sessions, 1)
}
func TestCompletedSessionFollowUpOwnership(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedTaskAndSession(t, repo, "task-completed-ownership", "session-follow-up", models.TaskSessionStateCompleted)
followUp, err := repo.GetTaskSession(ctx, "session-follow-up")
require.NoError(t, err)
followUp.AgentProfileID = "profile-ownership"
followUp.UpdatedAt = time.Now().UTC().Add(2 * time.Minute)
require.NoError(t, repo.UpdateTaskSession(ctx, followUp))
require.NoError(t, repo.SetSessionMetadataKey(
ctx,
"session-follow-up",
models.SessionMetaKeyCompletionFollowUp,
true,
))
require.NoError(t, repo.CreateTaskSession(ctx, &models.TaskSession{
ID: "session-reusable",
TaskID: "task-completed-ownership",
AgentProfileID: "profile-ownership",
State: models.TaskSessionStateWaitingForInput,
StartedAt: time.Now().UTC(),
UpdatedAt: time.Now().UTC().Add(time.Minute),
}))
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
reusable, err := svc.findReusableSessionForProfile(
ctx,
"task-completed-ownership",
"profile-ownership",
"",
)
require.NoError(t, err)
require.NotNil(t, reusable)
require.Equal(t, "session-reusable", reusable.ID)
}
func TestCompletedSessionFollowUpTurnReturnsToWaiting(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedTaskAndSession(t, repo, "task-completed-ready", "session-completed-ready", models.TaskSessionStateRunning)
require.NoError(t, repo.UpdateTaskState(ctx, "task-completed-ready", v1.TaskStateCompleted))
session, err := repo.GetTaskSession(ctx, "session-completed-ready")
require.NoError(t, err)
require.NoError(t, repo.SetSessionMetadataKey(
ctx,
"session-completed-ready",
models.SessionMetaKeyCompletionFollowUp,
true,
))
require.Equal(t, models.TaskSessionStateRunning, session.State)
storedSession, err := repo.GetTaskSession(ctx, "session-completed-ready")
require.NoError(t, err)
require.True(t, models.IsCompletionFollowUpSession(storedSession.Metadata))
svc := createTestServiceWithAgent(
repo,
newMockStepGetter(),
newMockTaskRepo(),
&mockAgentManager{isAgentRunning: true},
)
svc.handleAgentReady(ctx, watcher.AgentEventData{
TaskID: "task-completed-ready",
SessionID: "session-completed-ready",
})
reloaded, err := repo.GetTaskSession(ctx, "session-completed-ready")
require.NoError(t, err)
require.Equal(t, models.TaskSessionStateWaitingForInput, reloaded.State)
require.True(t, models.IsCompletionFollowUpSession(reloaded.Metadata))
task, err := repo.GetTask(ctx, "task-completed-ready")
require.NoError(t, err)
require.Equal(t, v1.TaskStateCompleted, task.State)
}
func TestCompletedSessionResumeRaces(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
taskRepo := newMockTaskRepo()
startEntered := make(chan struct{})
startRelease := make(chan struct{})
startFinished := make(chan struct{})
var startOnce sync.Once
var launchCalls atomic.Int32
started := false
agentMgr := &blockingCompletedResumeAgentManager{
sessionUpdatingAgentManager: &sessionUpdatingAgentManager{
mockAgentManager: &mockAgentManager{
isAgentRunning: true,
launchAgentFunc: func(_ context.Context, req *executor.LaunchAgentRequest) (*executor.LaunchAgentResponse, error) {
launchCalls.Add(1)
return &executor.LaunchAgentResponse{
AgentExecutionID: "exec-completed-race",
Status: v1.AgentStatusStarting,
}, nil
},
},
repo: repo,
sessionID: "session-completed-race",
taskID: "task-completed-race",
onStartCalled: &started,
},
startEntered: startEntered,
startRelease: startRelease,
startFinished: startFinished,
startOnce: &startOnce,
finishOnce: &sync.Once{},
}
svc := createTestServiceWithAgent(repo, newMockStepGetter(), taskRepo, agentMgr)
svc.executor = executor.NewExecutor(agentMgr, repo, testLogger(), executor.ExecutorConfig{})
seedTaskAndSession(
t,
repo,
"task-completed-race",
"session-completed-race",
models.TaskSessionStateCompleted,
)
now := time.Now().UTC()
require.NoError(t, repo.CreateExecutor(ctx, &models.Executor{
ID: "executor-completed-race",
Name: "Worktree",
Type: models.ExecutorTypeWorktree,
Status: models.ExecutorStatusActive,
Resumable: true,
}))
require.NoError(t, repo.CreateRepository(ctx, &models.Repository{
ID: "repo-completed-race",
WorkspaceID: "ws1",
Name: "backend",
SourceType: "local",
LocalPath: t.TempDir(),
DefaultBranch: "main",
CreatedAt: now,
UpdatedAt: now,
}))
require.NoError(t, repo.CreateTaskRepository(ctx, &models.TaskRepository{
ID: "task-repo-completed-race",
TaskID: "task-completed-race",
RepositoryID: "repo-completed-race",
BaseBranch: "main",
Position: 0,
CreatedAt: now,
UpdatedAt: now,
}))
session, err := repo.GetTaskSession(ctx, "session-completed-race")
require.NoError(t, err)
session.AgentProfileID = "profile-completed-race"
session.ExecutorID = "executor-completed-race"
session.RepositoryID = "repo-completed-race"
session.BaseBranch = "main"
require.NoError(t, repo.UpdateTaskSession(ctx, session))
require.NoError(t, repo.UpsertExecutorRunning(ctx, &models.ExecutorRunning{
ID: "running-completed-race",
SessionID: "session-completed-race",
TaskID: "task-completed-race",
AgentExecutionID: "exec-before-completed-race",
ResumeToken: "acp-session-completed-race",
Resumable: true,
CreatedAt: now,
UpdatedAt: now,
}))
type recoveryResult struct {
response *LaunchSessionResponse
err error
}
firstResult := make(chan recoveryResult, 1)
go func() {
response, recoverErr := svc.RecoverSession(ctx, "task-completed-race", "session-completed-race", "resume")
firstResult <- recoveryResult{response: response, err: recoverErr}
}()
select {
case <-startEntered:
case <-time.After(5 * time.Second):
t.Fatal("timed out waiting for the first completed-session resume")
}
secondResult := make(chan recoveryResult, 1)
go func() {
response, recoverErr := svc.RecoverSession(ctx, "task-completed-race", "session-completed-race", "resume")
secondResult <- recoveryResult{response: response, err: recoverErr}
}()
close(startRelease)
first := <-firstResult
require.NoError(t, first.err)
require.NotNil(t, first.response)
select {
case result := <-secondResult:
if result.err == nil {
require.NotNil(t, result.response)
require.Equal(t, "session-completed-race", result.response.SessionID)
}
case <-time.After(5 * time.Second):
t.Fatal("timed out waiting for the duplicate completed-session resume")
}
select {
case <-startFinished:
case <-time.After(5 * time.Second):
t.Fatal("timed out waiting for the resumed agent process")
}
require.Equal(t, int32(1), launchCalls.Load())
require.True(t, started)
reloaded, err := repo.GetTaskSession(ctx, "session-completed-race")
require.NoError(t, err)
require.Equal(t, models.TaskSessionStateWaitingForInput, reloaded.State)
sessions, err := repo.ListTaskSessions(ctx, "task-completed-race")
require.NoError(t, err)
require.Len(t, sessions, 1)
}
type blockingCompletedResumeAgentManager struct {
*sessionUpdatingAgentManager
startEntered chan struct{}
startRelease <-chan struct{}
startFinished chan struct{}
startOnce *sync.Once
finishOnce *sync.Once
}
func (m *blockingCompletedResumeAgentManager) StartAgentProcess(ctx context.Context, agentExecutionID string) error {
m.startOnce.Do(func() { close(m.startEntered) })
<-m.startRelease
err := m.sessionUpdatingAgentManager.StartAgentProcess(ctx, agentExecutionID)
m.finishOnce.Do(func() { close(m.startFinished) })
return err
}
@@ -0,0 +1,73 @@
package orchestrator
import (
"context"
"testing"
"github.com/kandev/kandev/internal/orchestrator/watcher"
"github.com/kandev/kandev/internal/task/models"
v1 "github.com/kandev/kandev/pkg/api/v1"
"github.com/stretchr/testify/require"
)
func TestCompletedTaskFollowUpAdmissionIsConversationalOnly(t *testing.T) {
for _, child := range []bool{false, true} {
t.Run(map[bool]string{false: "root", true: "child"}[child], func(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
steps, ids := buildWorkflowFromJSON(t, developmentWorkflowJSON)
steps.steps[ids["Done"]].CompleteTaskOnEnter = true
seedSession(t, repo, "task", "session", ids["New Step"])
setSessionExecID(t, repo, "session", "exec")
setSessionState(t, ctx, repo, "session", models.TaskSessionStateRunning)
if child {
task, err := repo.GetTask(ctx, "task")
require.NoError(t, err)
task.ParentID = "parent"
require.NoError(t, repo.UpdateTask(ctx, task))
}
agentMgr := &mockAgentManager{repoForExecutionLookup: repo, isAgentRunning: true}
svc := createEngineService(t, repo, steps, agentMgr)
session, err := repo.GetTaskSession(ctx, "session")
require.NoError(t, err)
require.True(t, svc.processOnTurnCompleteViaEngine(ctx, "task", session))
task, err := repo.GetTask(ctx, "task")
require.NoError(t, err)
require.Equal(t, v1.TaskStateCompleted, task.State)
require.Equal(t, ids["Done"], task.WorkflowStepID)
require.Equal(t, models.TaskSessionStateWaitingForInput, mustGetSession(t, repo, "session").State)
result, err := svc.ProcessOnTurnStart(ctx, "task", "session")
require.NoError(t, err)
require.False(t, result.Queued)
followUp, err := repo.GetTaskSession(ctx, "session")
require.NoError(t, err)
require.True(t, models.IsCompletionFollowUpSession(followUp.Metadata))
svc.setSessionRunning(ctx, "task", "session")
task, err = repo.GetTask(ctx, "task")
require.NoError(t, err)
require.Equal(t, v1.TaskStateCompleted, task.State)
svc.handleAgentReady(ctx, watcher.AgentEventData{
TaskID: "task",
SessionID: "session",
AgentExecutionID: "exec",
})
require.Equal(t, models.TaskSessionStateWaitingForInput, mustGetSession(t, repo, "session").State)
task, err = repo.GetTask(ctx, "task")
require.NoError(t, err)
require.Equal(t, v1.TaskStateCompleted, task.State)
require.Equal(t, ids["Done"], task.WorkflowStepID)
})
}
}
func mustGetSession(t *testing.T, repo sessionExecutorStore, sessionID string) *models.TaskSession {
t.Helper()
session, err := repo.GetTaskSession(context.Background(), sessionID)
require.NoError(t, err)
return session
}
@@ -916,20 +916,28 @@ func (s *Service) handleAgentReady(ctx context.Context, data watcher.AgentEventD
return
}
// Check for workflow transition based on session's current step.
// Uses the engine when available; falls back to legacy evaluation.
// The ViaEngine method handles setSessionWaitingForInput internally when no transition occurs.
transitioned := s.processOnTurnCompleteViaEngine(ctx, data.TaskID, session)
completionFollowUp := models.IsCompletionFollowUpSession(session.Metadata)
if completionFollowUp {
// A completed task's explicit follow-up turn is conversational only. It
// must become promptable again without re-evaluating the final step's
// workflow actions or changing the task's completed state.
s.setSessionWaitingForInput(ctx, data.TaskID, data.SessionID, session)
} else {
// Check for workflow transition based on session's current step.
// Uses the engine when available; falls back to legacy evaluation.
// The ViaEngine method handles setSessionWaitingForInput internally when no transition occurs.
transitioned := s.processOnTurnCompleteViaEngine(ctx, data.TaskID, session)
// When a workflow transition occurred (e.g. Work → Review), the new step's
// on_enter actions handle the next prompt (auto_start_agent launches a goroutine).
// Skip the queued-message check to avoid racing with that auto-start goroutine —
// both would try to call PromptTask and the loser's queued message would be lost.
if transitioned {
s.logger.Debug("workflow transition occurred, skipping queued message check",
zap.String("task_id", data.TaskID),
zap.String("session_id", data.SessionID))
return
// When a workflow transition occurred (e.g. Work → Review), the new step's
// on_enter actions handle the next prompt (auto_start_agent launches a goroutine).
// Skip the queued-message check to avoid racing with that auto-start goroutine —
// both would try to call PromptTask and the loser's queued message would be lost.
if transitioned {
s.logger.Debug("workflow transition occurred, skipping queued message check",
zap.String("task_id", data.TaskID),
zap.String("session_id", data.SessionID))
return
}
}
// Passthrough sessions: deliver queued messages via PTY stdin instead of ACP.
@@ -1811,17 +1819,22 @@ func (s *Service) finishAgentCompleted(
session *models.TaskSession,
guard *lockedCancelInFlightGuard,
) {
completionFollowUp := models.IsCompletionFollowUpSession(session.Metadata)
// A successful, still-live completion clears retry state and scheduler
// ownership only after the guarded terminal/rotation checks above.
s.resetTransientRetry(data.SessionID)
s.scheduler.HandleTaskCompleted(data.TaskID, true)
s.scheduler.RemoveTask(data.TaskID)
if !completionFollowUp {
s.scheduler.HandleTaskCompleted(data.TaskID, true)
s.scheduler.RemoveTask(data.TaskID)
}
// The agent finished a turn, so any stored failure no longer describes the
// session. `session` was read above, so the guard costs nothing.
s.clearRecoveredAgentError(context.WithoutCancel(ctx), data.TaskID, session)
s.reconcileCIAutoFixTurnBeforeCompletion(ctx, data.TaskID, data.SessionID, "")
if !completionFollowUp {
s.reconcileCIAutoFixTurnBeforeCompletion(ctx, data.TaskID, data.SessionID, "")
}
s.completeTurnForSession(context.WithoutCancel(ctx), data.SessionID)
if s.sessionHasPendingClarification(ctx, data.SessionID) {
@@ -1836,9 +1849,10 @@ func (s *Service) finishAgentCompleted(
return
}
transitioned := !s.drainQueuedBeforeWorkflowTransition(ctx, data.TaskID, data.SessionID, session) &&
transitioned := !completionFollowUp &&
!s.drainQueuedBeforeWorkflowTransition(ctx, data.TaskID, data.SessionID, session) &&
s.processOnTurnCompleteViaEngine(ctx, data.TaskID, session)
s.finishAgentCompletedTurn(ctx, data, session, transitioned, guard)
s.finishAgentCompletedTurn(ctx, data, session, transitioned, completionFollowUp, guard)
}
// finishAgentCompletedTurn runs the settle steps after
@@ -1852,6 +1866,7 @@ func (s *Service) finishAgentCompletedTurn(
data watcher.AgentEventData,
session *models.TaskSession,
transitioned bool,
completionFollowUp bool,
guard *lockedCancelInFlightGuard,
) {
// Agent-exit path: processOnTurnCompleteViaEngine handles normal
@@ -1888,7 +1903,9 @@ func (s *Service) finishAgentCompletedTurn(
// Finalize the automation run: mark status=succeeded so the automation's
// concurrency slot is released. The worktree stays.
s.finalizeAutomationRun(ctx, data.TaskID, true, "")
if !completionFollowUp {
s.finalizeAutomationRun(ctx, data.TaskID, true, "")
}
// Settle point: turn has been completed and the session state has been
// reconciled. reclaimIdleSession is the synchronous equivalent of the
@@ -2336,13 +2353,19 @@ func (s *Service) handleRecoverableFailureLocked(ctx context.Context, data watch
// run after the guard is released. Deletion uses the same guard, so an active
// error event cannot publish after the deleted-session inactive event.
func (s *Service) handleRecoverableFailureLockedState(ctx context.Context, data watcher.AgentEventData) func() {
completionFollowUp := false
if session, err := s.repo.GetTaskSession(ctx, data.SessionID); err == nil && session != nil {
completionFollowUp = models.IsCompletionFollowUpSession(session.Metadata)
}
s.logger.Warn("handling recoverable agent failure",
zap.String("task_id", data.TaskID),
zap.String("session_id", data.SessionID),
zap.String("error", data.ErrorMessage))
// Complete the current turn.
s.reconcileCIAutoFixTurnBeforeCompletion(ctx, data.TaskID, data.SessionID, "")
if !completionFollowUp {
s.reconcileCIAutoFixTurnBeforeCompletion(ctx, data.TaskID, data.SessionID, "")
}
s.completeTurnForSession(ctx, data.SessionID)
s.persistLastAgentError(ctx, data)
@@ -2383,7 +2406,7 @@ func (s *Service) handleRecoverableFailureLockedState(ctx context.Context, data
// the session's metadata bag until it's silently cleared on resume.
// Office sessions go FAILED, not WAITING_FOR_INPUT, and must not
// advance the step here.
if nextState == models.TaskSessionStateWaitingForInput && data.SessionID != "" {
if !completionFollowUp && nextState == models.TaskSessionStateWaitingForInput && data.SessionID != "" {
session, err := s.repo.GetTaskSession(ctx, data.SessionID)
if err != nil {
s.logger.Warn("failed to reload session for step-completion reconciliation; "+
@@ -2404,6 +2427,9 @@ func (s *Service) handleRecoverableFailureLockedState(ctx context.Context, data
// guard, so dispatching while it is held would deadlock. Panic recovery is
// the recovered wrapper's job, not this one's — routes R2-R5 reach this
// closure with no recover above them on the stack.
if completionFollowUp {
return func() {}
}
return func() {
s.dispatchKanbanAgentErrorTriggerRecovered(context.WithoutCancel(ctx), data)
}
@@ -147,7 +147,6 @@ func (s *Service) readyChildCompletionRows(ctx context.Context, parentID string)
zap.Error(err))
return nil, false
}
s.annotateTerminalChildSteps(ctx, rows)
if len(rows) == 0 || !allChildrenTerminal(rows) {
return nil, false
}
@@ -260,31 +259,13 @@ func (s *Service) markChildCompletionApplied(ctx context.Context, parentID, oper
func allChildrenTerminal(rows []models.ChildCompletionRow) bool {
for _, row := range rows {
if !models.IsTerminalTaskState(row.State) && !row.TerminalWorkflowStep {
if !models.IsTerminalTaskState(row.State) {
return false
}
}
return true
}
func (s *Service) annotateTerminalChildSteps(ctx context.Context, rows []models.ChildCompletionRow) {
if s.workflowStepGetter == nil {
return
}
cache := make(map[string]bool)
for i := range rows {
if models.IsTerminalTaskState(rows[i].State) || rows[i].WorkflowStepID == "" {
continue
}
terminal, ok := cache[rows[i].WorkflowStepID]
if !ok {
terminal = s.workflowStepIsTerminal(ctx, rows[i].WorkflowStepID)
cache[rows[i].WorkflowStepID] = terminal
}
rows[i].TerminalWorkflowStep = terminal
}
}
func (s *Service) workflowStepIsTerminal(ctx context.Context, workflowStepID string) bool {
if s.workflowStepGetter == nil || workflowStepID == "" {
return false
@@ -316,9 +297,6 @@ func childCompletionPayload(rows []models.ChildCompletionRow) engine.OnChildrenC
}
func childCompletionStatus(row models.ChildCompletionRow) string {
if row.TerminalWorkflowStep && !models.IsTerminalTaskState(row.State) {
return string(v1.TaskStateCompleted)
}
return string(row.State)
}
@@ -333,12 +311,6 @@ func childCompletionOperationID(parentID string, rows []models.ChildCompletionRo
b.WriteString(":")
b.WriteString(row.WorkflowStepID)
b.WriteString(":")
if row.TerminalWorkflowStep {
b.WriteString("terminal")
} else {
b.WriteString("active")
}
b.WriteString(":")
b.WriteString(row.UpdatedAt.UTC().Format(time.RFC3339Nano))
}
sum := sha256.Sum256([]byte(b.String()))
@@ -109,7 +109,7 @@ func TestProcessOnChildrenCompleted_TransitionsParentWhenAllActiveChildrenTermin
}
}
func TestProcessOnChildrenCompleted_TreatsTerminalStepAsCompleted(t *testing.T) {
func TestProcessOnChildrenCompleted_UsesCommittedTaskState(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedSession(t, repo, "parent", "parent-session", "step_wait")
@@ -152,7 +152,7 @@ func TestProcessOnChildrenCompleted_TreatsTerminalStepAsCompleted(t *testing.T)
now := time.Now().UTC()
requireCreateTask(t, repo, &models.Task{
ID: "child-terminal-step", WorkspaceID: "ws1", WorkflowID: "wf-child", WorkflowStepID: "child_done",
Title: "Child in Done", State: v1.TaskStateReview, ParentID: "parent",
Title: "Child in Done", State: v1.TaskStateCompleted, ParentID: "parent",
CreatedAt: now, UpdatedAt: now,
})
@@ -200,10 +200,11 @@ func TestHandleTaskMovedToTerminalStepProcessesParentChildrenCompleted(t *testin
Position: 0,
}
stepGetter.steps["child_done"] = &wfmodels.WorkflowStep{
ID: "child_done",
WorkflowID: "wf-child",
Name: "Done",
Position: 1,
ID: "child_done",
WorkflowID: "wf-child",
Name: "Done",
Position: 1,
CompleteTaskOnEnter: true,
}
agentMgr := &mockAgentManager{repoForExecutionLookup: repo}
@@ -85,6 +85,7 @@ func TestPendingMove_ReviewToInProgress_OneTransitionOnly(t *testing.T) {
func TestPendingMove_OutOfTerminalStepReopensCompletedTask(t *testing.T) {
sc := buildPendingMoveScenario(t)
sc.stepGetter.steps[stepReviewedID].Name = "Done"
sc.stepGetter.steps[stepReviewedID].CompleteTaskOnEnter = true
task, err := sc.repo.GetTask(sc.ctx, "task-1")
if err != nil {
@@ -63,14 +63,14 @@ func (m *reclaimTrackingAgentManager) callsSnapshot() []string {
return append([]string(nil), m.calls...)
}
// TestSubtaskTerminalCollapse_ReclaimsProviderRuntime is the canonical
// TestSubtaskTerminalSettle_ReclaimsProviderRuntime is the canonical
// end-to-end wiring test: a child task whose last agent message did not
// request input collapses to COMPLETED inside
// request input remains WAITING_FOR_INPUT inside
// setSessionWaitingForInputIfRequested. With no live agent process and no
// active turn, reclaimIdleSession must fire on this synchronous settle
// point, the executor row must flip to status=stopped with LocalPID=0,
// and the resume_token/worktree_path must remain intact.
func TestSubtaskTerminalCollapse_ReclaimsProviderRuntime(t *testing.T) {
func TestSubtaskTerminalSettle_ReclaimsProviderRuntime(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
now := time.Now().UTC()
@@ -116,13 +116,13 @@ func TestSubtaskTerminalCollapse_ReclaimsProviderRuntime(t *testing.T) {
svc.handleAgentCompleted(ctx, watcherAgentCompletedData("child-task", "s-child", "exec-child"))
waitForStopCall(t, inner)
// 1. Session collapsed to COMPLETED (existing guard behavior).
// 1. Session remains promptable after a successful child completion.
updated, err := repo.GetTaskSession(ctx, "s-child")
if err != nil {
t.Fatalf("load session: %v", err)
}
if updated.State != models.TaskSessionStateCompleted {
t.Fatalf("subtask terminal must collapse to COMPLETED, got %q", updated.State)
if updated.State != models.TaskSessionStateWaitingForInput {
t.Fatalf("successful subtask must remain WAITING_FOR_INPUT, got %q", updated.State)
}
// 2. reclaim fired exactly once for this session.
@@ -1860,12 +1860,22 @@ func (s *Service) deleteCompletedExecutionIfExpired(key string, expiresAt time.T
func allowsSessionStartingRecovery(
nextState, expectedState, currentState models.TaskSessionState,
promoteTask bool,
) bool {
return allowsSessionStartingRecoveryWithPermission(
nextState, expectedState, currentState, promoteTask, false,
)
}
func allowsSessionStartingRecoveryWithPermission(
nextState, expectedState, currentState models.TaskSessionState,
promoteTask, allowCompletedResume bool,
) bool {
return !promoteTask &&
nextState == models.TaskSessionStateStarting &&
currentState == expectedState &&
(expectedState == models.TaskSessionStateFailed ||
expectedState == models.TaskSessionStateCancelled)
expectedState == models.TaskSessionStateCancelled ||
(allowCompletedResume && expectedState == models.TaskSessionStateCompleted))
}
func (s *Service) setSessionStarting(
@@ -1874,6 +1884,18 @@ func (s *Service) setSessionStarting(
session *models.TaskSession,
expectedState models.TaskSessionState,
promoteTask bool,
) error {
return s.setSessionStartingWithOptions(
ctx, taskID, session, expectedState, promoteTask, false,
)
}
func (s *Service) setSessionStartingWithOptions(
ctx context.Context,
taskID string,
session *models.TaskSession,
expectedState models.TaskSessionState,
promoteTask, allowCompletedResume bool,
) error {
if session == nil {
return nil
@@ -1890,8 +1912,8 @@ func (s *Service) setSessionStarting(
if err != nil {
return err
}
allowedTerminalRecovery := allowsSessionStartingRecovery(
session.State, expectedState, current.State, promoteTask,
allowedTerminalRecovery := allowsSessionStartingRecoveryWithPermission(
session.State, expectedState, current.State, promoteTask, allowCompletedResume,
)
if isTerminalSessionState(current.State) && !allowedTerminalRecovery {
return &executor.SessionStateSupersededError{SessionID: session.ID, State: current.State}
@@ -2090,21 +2112,9 @@ func (s *Service) setSessionWaitingForInputWithHook(
}
// Child terminal receipts use the task's parent relationship and durable
// clarification projection to decide whether a session can collapse to
// COMPLETED. Root-task sibling sessions keep the original WAITING affordance.
// setSessionWaitingForInputIfRequested is the terminal-receipt variant
// of setSessionWaitingForInput. It only applies to subtasks (task.ParentID
// non-empty) because the symptom it guards — "child WAITING_FOR_INPUT
// after the agent's last turn had no active clarification" — only
// arises for child tasks whose task or workflow step is terminal, not to
// surface a UI prompt. Sibling sessions on a root task keep the original
// affordance so a finishing session on a multi-session task still flips to
// WAITING_FOR_INPUT.
//
// When a terminal task has no input request, collapse the session to
// COMPLETED so the child row does not leak in a stuck active state. A clean
// turn on a non-terminal child is not enough evidence for that collapse, so
// it remains WAITING_FOR_INPUT.
// clarification projection to preserve existing failure/cancellation cleanup.
// Successful completion is independent from conversation access, so a
// completed child remains WAITING_FOR_INPUT and can receive a follow-up.
func (s *Service) setSessionWaitingForInputIfRequested(
ctx context.Context,
taskID, sessionID string,
@@ -2145,9 +2155,9 @@ func (s *Service) setSessionWaitingForInputIfRequestedWithHook(
s.setSessionWaitingForInputWithHook(ctx, taskID, sessionID, onChanged, preloadedSession...)
return
}
taskTerminal := models.IsTerminalTaskState(task.State) || s.workflowStepIsTerminal(ctx, task.WorkflowStepID)
if len(activeClarifications) == 0 && taskTerminal {
s.logger.Debug("subtask terminal: skipping WAITING write; collapsing session to COMPLETED",
failedOrCancelled := task.State == v1.TaskStateFailed || task.State == v1.TaskStateCancelled
if len(activeClarifications) == 0 && failedOrCancelled {
s.logger.Debug("subtask failed or cancelled: skipping WAITING write; collapsing session to COMPLETED",
zap.String("task_id", taskID),
zap.String("session_id", sessionID))
s.updateTaskSessionState(ctx, taskID, sessionID, models.TaskSessionStateCompleted, "", false)
@@ -2493,10 +2503,16 @@ func (s *Service) reconcileTaskStateForRuntimeLocked(
if state == v1.TaskStateInProgress && task != nil && task.IsFromOffice {
return nil
}
if state == v1.TaskStateInProgress && task != nil && task.State == v1.TaskStateCompleted {
return nil
}
session, err := s.repo.GetTaskSession(ctx, sessionID)
if err != nil {
return err
}
if state == v1.TaskStateInProgress && models.IsCompletionFollowUpSession(session.Metadata) {
return nil
}
if !runtimeSessionOwnsTaskState(session, state) {
return nil
}
@@ -12,20 +12,14 @@ import (
v1 "github.com/kandev/kandev/pkg/api/v1"
)
// TestHandleAgentCompleted_SubtaskWithoutRequestsInputCollapsesToCompleted
// pins the symptom reported in the v0.88 total-control fix plan:
// "child in a terminal state without requests_input must NOT write
// WAITING_FOR_INPUT". handleAgentCompleted's existing line 1238 writes
// WAITING unconditionally for non-transitioned terminal receipts.
// setSessionWaitingForInputIfRequested (this commit's guard) refuses
// the WAITING write for child tasks and instead collapses the session
// to COMPLETED, so a child agent's clean exit does not leave the
// session in a stuck RUNNING/RUNNING-equivalent state.
// TestHandleAgentCompleted_SubtaskWithoutRequestsInputRemainsPromptable
// protects completed task conversation access. A successful child can finish
// its task while its session remains an ordinary promptable conversation.
//
// The seed task is a subtask (ParentID="parent") in a terminal task state
// and the agent's last message has requests_input=false; the orchestrator
// must therefore finish the session to COMPLETED, not WAITING.
func TestHandleAgentCompleted_SubtaskWithoutRequestsInputCollapsesToCompleted(t *testing.T) {
// must therefore finish the session in WAITING_FOR_INPUT.
func TestHandleAgentCompleted_SubtaskWithoutRequestsInputRemainsPromptable(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
now := time.Now().UTC()
@@ -40,8 +34,8 @@ func TestHandleAgentCompleted_SubtaskWithoutRequestsInputCollapsesToCompleted(t
}
seedExecutorRunning(t, repo, "s-child", "child-task", "exec-child")
// No messages: requests_input must be false, so the guard refuses
// the WAITING write and collapses the session to COMPLETED.
// No messages: requests_input is false, but this is still a promptable
// successful conversation.
taskRepo := newMockTaskRepo()
agentMgr := &mockAgentManager{repoForExecutionLookup: repo}
@@ -54,8 +48,8 @@ func TestHandleAgentCompleted_SubtaskWithoutRequestsInputCollapsesToCompleted(t
if err != nil {
t.Fatalf("load session after subtask terminal: %v", err)
}
if updated.State != models.TaskSessionStateCompleted {
t.Fatalf("subtask terminal without requests_input must collapse to COMPLETED, got %q", updated.State)
if updated.State != models.TaskSessionStateWaitingForInput {
t.Fatalf("successful subtask without requests_input must remain WAITING_FOR_INPUT, got %q", updated.State)
}
}
@@ -95,7 +89,7 @@ func TestHandleAgentCompleted_NonTerminalSubtaskWithoutRequestsInputWritesWaitin
}
}
func TestHandleAgentCompleted_SubtaskIgnoresResolvedClarification(t *testing.T) {
func TestHandleAgentCompleted_SubtaskWithResolvedClarificationRemainsPromptable(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
now := time.Now().UTC()
@@ -132,8 +126,8 @@ func TestHandleAgentCompleted_SubtaskIgnoresResolvedClarification(t *testing.T)
if err != nil {
t.Fatalf("load session after resolved clarification: %v", err)
}
if updated.State != models.TaskSessionStateCompleted {
t.Fatalf("resolved clarification must not keep a terminal child waiting, got %q", updated.State)
if updated.State != models.TaskSessionStateWaitingForInput {
t.Fatalf("resolved clarification must leave a completed child promptable, got %q", updated.State)
}
}
@@ -382,12 +382,25 @@ func (s *Service) ProcessOnTurnStart(ctx context.Context, taskID, sessionID stri
State: session.State,
}
}
task, err := s.repo.GetTask(ctx, taskID)
if err != nil {
return ProcessOnTurnStartResult{}, fmt.Errorf("load task for on_turn_start: %w", err)
}
if task != nil && task.State == v1.TaskStateCompleted && !models.IsCompletionFollowUpSession(session.Metadata) {
if err := s.repo.SetSessionMetadataKey(ctx, session.ID, models.SessionMetaKeyCompletionFollowUp, true); err != nil {
return ProcessOnTurnStartResult{}, fmt.Errorf("mark completed task follow-up: %w", err)
}
if session.Metadata == nil {
session.Metadata = make(map[string]interface{})
}
session.Metadata[models.SessionMetaKeyCompletionFollowUp] = true
}
// ADR 0015 — a fresh user message before the pending signal's
// transition has fired cancels the signal (re-open semantics). The
// user is continuing the conversation; this step is no longer "done".
s.clearPendingStepSignal(ctx, session)
s.processOnTurnStartViaEngine(ctx, taskID, session)
task, err := s.repo.GetTask(ctx, taskID)
task, err = s.repo.GetTask(ctx, taskID)
if err != nil {
// Do not let a read race turn an unknown admission state into an
// immediate prompt. The caller can retry after reconciliation.
@@ -2411,6 +2424,13 @@ func (s *Service) tagSessionAsWorkflowSwitched(ctx context.Context, sessionID st
s.logger.Warn("failed to persist workflow-switch tag",
zap.String("session_id", sessionID), zap.Error(err))
}
// A workflow step explicitly taking ownership of this session is the only
// path that clears conversational-only follow-up ownership. Ordinary sends,
// page activation, and automatic profile lookup must leave the marker set.
if err := s.repo.SetSessionMetadataKey(ctx, sessionID, models.SessionMetaKeyCompletionFollowUp, nil); err != nil {
s.logger.Warn("failed to clear completed-conversation follow-up marker",
zap.String("session_id", sessionID), zap.Error(err))
}
}
// switchSessionForStep activates a session for the new agent profile.
@@ -2537,6 +2557,9 @@ func (s *Service) findReusableSessionForProfile(ctx context.Context, taskID, pro
if sess.AgentProfileID != profileID {
continue
}
if models.IsCompletionFollowUpSession(sess.Metadata) {
continue
}
if isTerminalSessionState(sess.State) {
continue
}
@@ -3160,9 +3183,7 @@ func (s *Service) prepareWorkflowStepSession(
ctx = withWorkflowMetaCache(ctx)
effectiveProfile := s.resolveStepAgentProfile(ctx, step)
if effectiveProfile == "" || effectiveProfile == session.AgentProfileID {
if effectiveProfile != "" {
s.tagSessionAsWorkflowSwitched(ctx, session.ID)
}
s.tagSessionAsWorkflowSwitched(ctx, session.ID)
if !session.IsPrimary {
if err := s.SetPrimarySession(ctx, session.ID); err != nil {
s.logger.Warn("failed to preserve session as primary for workflow step",
@@ -5947,6 +5968,9 @@ func (s *Service) processOnTurnCompleteViaEngineWithCause(
session *models.TaskSession,
cause turnCompletionCause,
) bool {
if session == nil || models.IsCompletionFollowUpSession(session.Metadata) {
return false
}
task, err := s.repo.GetTask(ctx, taskID)
if err != nil {
s.logger.Warn("failed to load task for on_turn_complete",
@@ -6530,6 +6554,9 @@ func (s *Service) launchProcessOnEnter(
// actions. Falls back to the legacy method when the engine is not initialized.
// Returns true if a step transition occurred.
func (s *Service) processOnTurnStartViaEngine(ctx context.Context, taskID string, session *models.TaskSession) bool {
if session == nil || models.IsCompletionFollowUpSession(session.Metadata) {
return false
}
task, err := s.repo.GetTask(ctx, taskID)
if err != nil {
s.logger.Warn("failed to load task for on_turn_start",
@@ -528,6 +528,36 @@ func TestPrepareWorkflowStepSession_PreservesMatchingProfileSession(t *testing.T
}
}
func TestPrepareWorkflowStepSession_ClearsCompletionFollowUpWithoutProfile(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
if err := repo.SetSessionMetadataKey(ctx, "s1", models.SessionMetaKeyCompletionFollowUp, true); err != nil {
t.Fatalf("mark completion follow-up: %v", err)
}
session, err := repo.GetTaskSession(ctx, "s1")
if err != nil {
t.Fatalf("get session: %v", err)
}
step := &wfmodels.WorkflowStep{ID: "step1", WorkflowID: "wf1"}
svc := createTestService(repo, newMockStepGetter(), newMockTaskRepo())
if _, switched, err := svc.prepareWorkflowStepSession(ctx, "t1", session, step, nil); err != nil {
t.Fatalf("prepareWorkflowStepSession returned error: %v", err)
} else if switched {
t.Fatal("step without a profile must not switch sessions")
}
updated, err := repo.GetTaskSession(ctx, session.ID)
if err != nil {
t.Fatalf("reload session: %v", err)
}
if models.IsCompletionFollowUpSession(updated.Metadata) {
t.Fatal("explicit workflow step entry retained completion follow-up ownership")
}
}
func TestSwitchWorkflowDispatcherRoutesOnEnterToDestinationProfileSession(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
@@ -98,6 +98,18 @@ type executorStore interface {
GetTaskPlan(ctx context.Context, taskID string) (*models.TaskPlan, error)
}
// sessionMetadataKeyStateSetter is an optional repository capability. Legacy
// test stores can keep their existing metadata API, while the SQL repository
// can guard recovery markers against a concurrent stop or archive.
type sessionMetadataKeyStateSetter interface {
SetSessionMetadataKeyIfState(
ctx context.Context,
sessionID, key string,
value interface{},
expectedState models.TaskSessionState,
) (bool, error)
}
// officeTaskSessionCreator lets repositories make Office-session origin
// selection part of the insert transaction. Test and legacy stores can omit
// it; the executor keeps a per-task fallback lock for those implementations.
@@ -750,6 +762,19 @@ type SessionStartingFunc func(
promoteTask bool,
) error
// SessionStartingWithOptionsFunc is the extended STARTING callback used by
// explicit completed-conversation recovery. It keeps the legacy callback
// shape available to lightweight executors and tests while carrying the
// narrow permission needed for the guarded completed-state transition.
type SessionStartingWithOptionsFunc func(
ctx context.Context,
taskID string,
session *models.TaskSession,
expectedState models.TaskSessionState,
promoteTask bool,
allowCompletedResume bool,
) error
// ExecutionCleanupClaimFunc atomically claims forced cleanup for one exact
// session execution. It returns true when the executor owns cleanup and false
// when another teardown path already owns that execution.
@@ -864,6 +889,9 @@ type Executor struct {
// the orchestrator so launch/resume/model-switch transitions serialize with
// runtime task-state reconciliation.
onSessionStarting SessionStartingFunc
// Extended STARTING callback for explicit completed-session recovery. When
// present, it takes precedence over the legacy callback above.
onSessionStartingWithOptions SessionStartingWithOptionsFunc
// Callback for exact-execution forced cleanup arbitration. Set by the
// orchestrator so coordinator graceful stop and launch cleanup cannot both
@@ -1159,6 +1187,12 @@ func (e *Executor) SetOnSessionStarting(fn SessionStartingFunc) {
e.onSessionStarting = fn
}
// SetOnSessionStartingWithOptions sets the extended STARTING callback used by
// explicit completed-session recovery.
func (e *Executor) SetOnSessionStartingWithOptions(fn SessionStartingWithOptionsFunc) {
e.onSessionStartingWithOptions = fn
}
// SetOnExecutionCleanupClaim sets the exact-execution forced cleanup arbiter.
func (e *Executor) SetOnExecutionCleanupClaim(fn ExecutionCleanupClaimFunc) {
e.onExecutionCleanupClaim = fn
@@ -589,12 +589,22 @@ func isStopTerminalSessionState(state models.TaskSessionState) bool {
func allowsSessionStartingRecovery(
nextState, expectedState, currentState models.TaskSessionState,
promoteTask bool,
) bool {
return allowsSessionStartingRecoveryWithPermission(
nextState, expectedState, currentState, promoteTask, false,
)
}
func allowsSessionStartingRecoveryWithPermission(
nextState, expectedState, currentState models.TaskSessionState,
promoteTask, allowCompletedResume bool,
) bool {
return !promoteTask &&
nextState == models.TaskSessionStateStarting &&
currentState == expectedState &&
(expectedState == models.TaskSessionStateFailed ||
expectedState == models.TaskSessionStateCancelled)
expectedState == models.TaskSessionStateCancelled ||
(allowCompletedResume && expectedState == models.TaskSessionStateCompleted))
}
// updateSessionStarting persists a full session-row STARTING transition, using
@@ -607,6 +617,23 @@ func (e *Executor) updateSessionStarting(
expectedState models.TaskSessionState,
promoteTask bool,
) error {
return e.updateSessionStartingWithOptions(
ctx, taskID, session, expectedState, promoteTask, false,
)
}
func (e *Executor) updateSessionStartingWithOptions(
ctx context.Context,
taskID string,
session *models.TaskSession,
expectedState models.TaskSessionState,
promoteTask, allowCompletedResume bool,
) error {
if e.onSessionStartingWithOptions != nil {
return e.onSessionStartingWithOptions(
ctx, taskID, session, expectedState, promoteTask, allowCompletedResume,
)
}
if e.onSessionStarting != nil {
return e.onSessionStarting(ctx, taskID, session, expectedState, promoteTask)
}
@@ -620,6 +647,11 @@ func (e *Executor) updateSessionStarting(
allowedTerminalRecovery := allowsSessionStartingRecovery(
session.State, expectedState, current.State, promoteTask,
)
if allowCompletedResume {
allowedTerminalRecovery = allowsSessionStartingRecoveryWithPermission(
session.State, expectedState, current.State, promoteTask, true,
)
}
if isStopTerminalSessionState(current.State) && !allowedTerminalRecovery {
return &SessionStateSupersededError{SessionID: session.ID, State: current.State}
}
@@ -744,6 +744,10 @@ func buildPrepareResultMetadata(result *lifecycle.EnvPrepareResult) map[string]i
// original worktree branch and reports when it is unrecoverable.
type ResumeOptions struct {
AllowBranchReplacement bool
// AllowCompletedSessionResume is granted only by an explicit user recovery
// or a pinned follow-up dispatch. It does not change the global terminal
// session predicate or permit implicit resume paths.
AllowCompletedSessionResume bool
}
// ResumeSession restarts an existing task session using its stored worktree.
@@ -775,7 +779,7 @@ func (e *Executor) resumeSession(
resumeSnapshot.Metadata = cloneMetadata(session.Metadata)
session = &resumeSnapshot
}
task, unlock, err := e.validateAndLockResume(ctx, session)
task, unlock, err := e.validateAndLockResume(ctx, session, options)
if err != nil {
return nil, err
}
@@ -783,7 +787,9 @@ func (e *Executor) resumeSession(
resumeInitialState := session.State
previousCredentialSnapshot := captureResumeCredentialSnapshot(session)
wasTerminalResume := isTerminalSessionState(resumeInitialState)
completedResume := options.AllowCompletedSessionResume &&
resumeInitialState == models.TaskSessionStateCompleted
wasTerminalResume := isTerminalSessionState(resumeInitialState) || completedResume
// Force-cleanup any stale in-memory execution / agentctl state for terminal-state
// sessions. Their agent process is dead by definition, so "already running" signals
// from the execution store or agentctl's "starting" status are stale and would
@@ -800,10 +806,12 @@ func (e *Executor) resumeSession(
var beforeCredentialLease func() error
if startAgent {
beforeCredentialLease = func() error {
if persistErr := e.persistResumeState(ctx, task.ID, session, true); persistErr != nil {
// The rollback path must remain armed if persistence fails after the
// session has entered STARTING.
resumeStatePersisted = true
if persistErr := e.persistResumeStateWithOptions(ctx, task.ID, session, true, options); persistErr != nil {
return persistErr
}
resumeStatePersisted = true
return nil
}
}
@@ -923,7 +931,13 @@ func (e *Executor) resumeSession(
}
if startAgent {
e.startAgentProcessOnResume(ctx, task.ID, session, resp.AgentExecutionID)
e.startAgentProcessOnResumeWithTaskPromotion(
ctx,
task.ID,
session,
resp.AgentExecutionID,
!completedResume,
)
}
return execution, nil
@@ -1030,9 +1044,13 @@ func (e *Executor) rollbackResumeStateAfterFailure(
// validateAndLockResume validates the session is resumable, acquires the per-session lock,
// and loads the associated task. Returns the task, an unlock function, and any error.
// The caller must call unlock() when the critical section is complete.
func (e *Executor) validateAndLockResume(ctx context.Context, session *models.TaskSession) (*v1.Task, func(), error) {
if session == nil {
return nil, func() {}, ErrExecutionNotFound
func (e *Executor) validateAndLockResume(
ctx context.Context,
session *models.TaskSession,
options ResumeOptions,
) (*v1.Task, func(), error) {
if err := validateResumeSession(session, options); err != nil {
return nil, func() {}, err
}
requestedState := session.State
@@ -1043,32 +1061,11 @@ func (e *Executor) validateAndLockResume(ctx context.Context, session *models.Ta
sessionLock.Lock()
unlock := func() { sessionLock.Unlock() }
taskModel, err := e.repo.GetTask(ctx, session.TaskID)
task, err := e.loadResumeTask(ctx, session)
if err != nil {
unlock()
e.logger.Error("failed to load task for session resume",
zap.String("task_id", session.TaskID),
zap.String("session_id", session.ID),
zap.Error(err))
return nil, func() {}, err
}
if taskModel.ArchivedAt != nil {
unlock()
return nil, func() {}, ErrTaskArchived
}
task := taskModel.ToAPI()
if task == nil {
unlock()
return nil, func() {}, ErrExecutionNotFound
}
if session.AgentProfileID == "" {
unlock()
e.logger.Error("task session has no agent_profile_id configured",
zap.String("task_id", session.TaskID),
zap.String("session_id", session.ID))
return nil, func() {}, ErrNoAgentProfileID
}
// Re-read session state after acquiring the lock. The caller fetched the
// session before the lock, so on concurrent resumes the state may be stale —
@@ -1078,38 +1075,97 @@ func (e *Executor) validateAndLockResume(ctx context.Context, session *models.Ta
// registered, launching a duplicate. If the re-read fails, abort rather than
// proceeding with uncertain state — silently falling back to the stale state
// would reintroduce the exact race this re-read prevents.
fresh, fetchErr := e.repo.GetTaskSession(ctx, session.ID)
if fetchErr != nil {
if err := e.refreshResumeSessionState(ctx, session, requestedState); err != nil {
unlock()
e.logger.Warn("failed to re-read session state inside lock; aborting resume to avoid duplicate agent",
zap.String("session_id", session.ID),
zap.Error(fetchErr))
return nil, func() {}, fetchErr
}
if fresh != nil {
if isTerminalSessionState(fresh.State) && fresh.State != requestedState {
unlock()
return nil, func() {}, &SessionStateSupersededError{
SessionID: session.ID,
State: fresh.State,
}
}
session.State = fresh.State
return nil, func() {}, err
}
// Skip the "already running" rejection for terminal-state sessions — the agent
// process is dead by definition, and ResumeSession will force-cleanup stale
// state before the relaunch.
if !isTerminalSessionState(session.State) {
if existing, ok := e.GetExecutionBySession(session.ID); ok && existing != nil {
unlock()
return nil, func() {}, ErrExecutionAlreadyRunning
}
if err := e.rejectRunningResume(session, options); err != nil {
unlock()
return nil, func() {}, err
}
return task, unlock, nil
}
func validateResumeSession(session *models.TaskSession, options ResumeOptions) error {
if session == nil {
return ErrExecutionNotFound
}
if session.State == models.TaskSessionStateCompleted && !options.AllowCompletedSessionResume {
return &SessionStateSupersededError{
SessionID: session.ID,
State: session.State,
}
}
return nil
}
func (e *Executor) loadResumeTask(ctx context.Context, session *models.TaskSession) (*v1.Task, error) {
taskModel, err := e.repo.GetTask(ctx, session.TaskID)
if err != nil {
e.logger.Error("failed to load task for session resume",
zap.String("task_id", session.TaskID),
zap.String("session_id", session.ID),
zap.Error(err))
return nil, err
}
if taskModel.ArchivedAt != nil {
return nil, ErrTaskArchived
}
task := taskModel.ToAPI()
if task == nil {
return nil, ErrExecutionNotFound
}
if session.AgentProfileID == "" {
e.logger.Error("task session has no agent_profile_id configured",
zap.String("task_id", session.TaskID),
zap.String("session_id", session.ID))
return nil, ErrNoAgentProfileID
}
return task, nil
}
func (e *Executor) refreshResumeSessionState(
ctx context.Context,
session *models.TaskSession,
requestedState models.TaskSessionState,
) error {
fresh, err := e.repo.GetTaskSession(ctx, session.ID)
if err != nil {
e.logger.Warn("failed to re-read session state inside lock; aborting resume to avoid duplicate agent",
zap.String("session_id", session.ID),
zap.Error(err))
return err
}
if fresh == nil {
return nil
}
if isTerminalSessionState(fresh.State) && fresh.State != requestedState {
return &SessionStateSupersededError{
SessionID: session.ID,
State: fresh.State,
}
}
session.State = fresh.State
return nil
}
func (e *Executor) rejectRunningResume(session *models.TaskSession, options ResumeOptions) error {
completedResume := options.AllowCompletedSessionResume &&
session.State == models.TaskSessionStateCompleted
if isTerminalSessionState(session.State) || completedResume {
return nil
}
if existing, ok := e.GetExecutionBySession(session.ID); ok && existing != nil {
return ErrExecutionAlreadyRunning
}
return nil
}
// buildResumeRequest constructs the LaunchAgentRequest for a session resume, resolving executor config,
// repository details, worktree settings, and ACP resume token.
// Returns the request, repository ID, executor config, existing ExecutorRunning record (may be nil), and error.
@@ -1507,14 +1563,26 @@ func (e *Executor) applyRunningRecordToResumeRequest(
) *models.ExecutorRunning {
if running == nil {
// Archive cleanup tears down the executors_running row entirely, so an
// archive-cancelled session reaches this point with running == nil —
// exactly the shape GetTaskSessionStatus's resumeReasonArchiveCancelledResumable
// auto-resumes once the task is unarchived. There is no resume token or
// running record to fall back on here, but the launch still must not
// replay task.Description as a fresh prompt — see the else-if branch
// below for the same guard on the running-record path.
if startAgent && isArchiveCancelledResumeSession(session) {
req.TaskDescription = ""
// archive-cancelled session reaches this point with running == nil. The
// session metadata mirrors the provider conversation identity so an
// explicit completed follow-up can still restore the same conversation
// after runtime cleanup removed the operational row.
noAutoPromptState := session.State == models.TaskSessionStateWaitingForInput ||
isArchiveCancelledResumeSession(session) ||
session.State == models.TaskSessionStateCompleted
if startAgent && noAutoPromptState {
if token := persistedSessionResumeToken(session); token != "" {
req.ACPSessionID = token
req.TaskDescription = ""
e.logger.Info("found persisted session resume token after runtime cleanup",
zap.String("task_id", task.ID),
zap.String("session_id", session.ID),
zap.Bool("has_resume_token", true))
} else {
// A missing token must still never turn recovery into an automatic
// task-description prompt.
req.TaskDescription = ""
}
}
return nil
}
@@ -1547,11 +1615,12 @@ func (e *Executor) applyRunningRecordToResumeRequest(
zap.String("task_id", task.ID),
zap.String("session_id", session.ID),
zap.Bool("has_resume_token", running.ResumeToken != ""))
} else if startAgent && (session.State == models.TaskSessionStateWaitingForInput || isArchiveCancelledResumeSession(session)) {
} else if startAgent && (session.State == models.TaskSessionStateWaitingForInput ||
isArchiveCancelledResumeSession(session) || session.State == models.TaskSessionStateCompleted) {
// Fresh-start resume (no resume token): don't auto-prompt with the task
// description. Also covers an archive-cancelled session whose running
// record survived cleanup but carries no token — same auto-resume shape
// as the running==nil branch above.
// description. Also covers completed and archive-cancelled sessions whose
// running record survived cleanup but carries no token — the same
// auto-resume shape as the running==nil branch above.
req.TaskDescription = ""
e.logger.Info("fresh-start resume, clearing task description to avoid auto-prompt",
zap.String("task_id", task.ID),
@@ -1561,6 +1630,34 @@ func (e *Executor) applyRunningRecordToResumeRequest(
return running
}
// persistedSessionResumeToken returns the provider conversation identity that
// survives executors_running cleanup. The dynamic route projection is checked
// first; the generic ACP metadata mirror is the fallback for ordinary sessions.
func persistedSessionResumeToken(session *models.TaskSession) string {
if session == nil {
return ""
}
if session.DownstreamACPSessionID != "" {
return session.DownstreamACPSessionID
}
if session.Metadata == nil {
return ""
}
acp, ok := session.Metadata["acp"]
if !ok {
return ""
}
switch value := acp.(type) {
case map[string]interface{}:
token, _ := value["session_id"].(string)
return token
case map[string]string:
return value["session_id"]
default:
return ""
}
}
// applyResumeRepoConfig resolves repository details and applies them to req.
// Returns the resolved repositoryID. existingEnv is the task_environments row
// resolved upstream in buildResumeRequest; it is passed in so we can derive
@@ -1815,16 +1912,41 @@ func resolveResumeTaskDirName(existingEnv *models.TaskEnvironment, task *v1.Task
// lifecycle.persistExecutorRunning. The orchestrator's remaining
// responsibility is the session-row state machine (STARTING / CompletedAt-clear).
func (e *Executor) persistResumeState(ctx context.Context, taskID string, session *models.TaskSession, startAgent bool) error {
return e.persistResumeStateWithOptions(ctx, taskID, session, startAgent, ResumeOptions{})
}
func (e *Executor) persistResumeStateWithOptions(
ctx context.Context,
taskID string,
session *models.TaskSession,
startAgent bool,
options ResumeOptions,
) error {
expectedState := session.State
completedResume := startAgent && options.AllowCompletedSessionResume &&
expectedState == models.TaskSessionStateCompleted
session.ErrorMessage = ""
if startAgent {
session.State = models.TaskSessionStateStarting
session.CompletedAt = nil
if completedResume {
if session.Metadata == nil {
session.Metadata = make(map[string]interface{})
}
session.Metadata[models.SessionMetaKeyCompletionFollowUp] = true
}
}
var updateErr error
if startAgent {
updateErr = e.updateSessionStarting(ctx, taskID, session, expectedState, false)
updateErr = e.updateSessionStartingWithOptions(
ctx,
taskID,
session,
expectedState,
false,
completedResume,
)
} else {
updateErr = e.persistSessionFullRowIfCurrentState(ctx, session, expectedState)
}
@@ -1835,9 +1957,41 @@ func (e *Executor) persistResumeState(ctx context.Context, taskID string, sessio
zap.Error(updateErr))
return updateErr
}
if completedResume {
return e.persistCompletionFollowUpMarker(ctx, session.ID)
}
return nil
}
func (e *Executor) persistCompletionFollowUpMarker(ctx context.Context, sessionID string) error {
setter, ok := e.repo.(sessionMetadataKeyStateSetter)
if !ok {
return e.repo.SetSessionMetadataKey(
ctx,
sessionID,
models.SessionMetaKeyCompletionFollowUp,
true,
)
}
changed, err := setter.SetSessionMetadataKeyIfState(
ctx,
sessionID,
models.SessionMetaKeyCompletionFollowUp,
true,
models.TaskSessionStateStarting,
)
if err != nil {
return err
}
if changed {
return nil
}
return &SessionStateSupersededError{
SessionID: sessionID,
State: models.TaskSessionStateStarting,
}
}
// prNumberFromMetadata extracts a GitHub PR number from a task_repository's
// metadata bag. Stored as JSON, so on retrieval the value can decode as either
// float64 (default for json.Unmarshal into interface{}) or int. Returns 0 when
@@ -1871,12 +2025,24 @@ func prNumberFromMetadata(metadata map[string]interface{}) int {
// Task state is managed by workflow triggers and stream handlers elsewhere; this callback
// just logs successful process start.
func (e *Executor) startAgentProcessOnResume(ctx context.Context, taskID string, session *models.TaskSession, agentExecutionID string) {
e.startAgentProcessOnResumeWithTaskPromotion(ctx, taskID, session, agentExecutionID, true)
}
func (e *Executor) startAgentProcessOnResumeWithTaskPromotion(
ctx context.Context,
taskID string,
session *models.TaskSession,
agentExecutionID string,
promoteTask bool,
) {
e.runAgentProcessAsync(ctx, taskID, session.ID, agentExecutionID, func(updCtx context.Context) {
if updateErr := e.writeTaskInProgressForRuntime(updCtx, taskID, session.ID); updateErr != nil {
e.logger.Warn("failed to update task state to IN_PROGRESS after resume start",
zap.String("task_id", taskID),
zap.String("session_id", session.ID),
zap.Error(updateErr))
if promoteTask {
if updateErr := e.writeTaskInProgressForRuntime(updCtx, taskID, session.ID); updateErr != nil {
e.logger.Warn("failed to update task state to IN_PROGRESS after resume start",
zap.String("task_id", taskID),
zap.String("session_id", session.ID),
zap.Error(updateErr))
}
}
e.logger.Debug("agent resumed successfully",
zap.String("task_id", taskID),
@@ -983,6 +983,45 @@ func TestResumeSession_ArchiveCancelledWithoutRunningRow_ClearsTaskDescription(t
}
}
func TestApplyRunningRecordToResumeRequest_FailedSessionKeepsTaskDescription(t *testing.T) {
repo := newMockRepository()
exec := newTestExecutor(t, &mockAgentManager{}, repo)
req := &LaunchAgentRequest{TaskDescription: "recover the failed task"}
task := &v1.Task{ID: "task-1"}
session := &models.TaskSession{
ID: "sess-1",
State: models.TaskSessionStateFailed,
DownstreamACPSessionID: "previous-conversation",
}
exec.applyRunningRecordToResumeRequest(req, task, session, true, nil)
if req.TaskDescription != "recover the failed task" {
t.Fatalf("failed-session TaskDescription = %q, want original prompt", req.TaskDescription)
}
if req.ACPSessionID != "" {
t.Fatalf("failed-session ACP session ID = %q, want empty", req.ACPSessionID)
}
}
func TestApplyRunningRecordToResumeRequest_CompletedTokenlessRunningRowClearsTaskDescription(t *testing.T) {
repo := newMockRepository()
exec := newTestExecutor(t, &mockAgentManager{}, repo)
req := &LaunchAgentRequest{TaskDescription: "recover the completed task"}
task := &v1.Task{ID: "task-1"}
session := &models.TaskSession{
ID: "sess-1",
State: models.TaskSessionStateCompleted,
}
running := &models.ExecutorRunning{SessionID: "sess-1", TaskID: "task-1"}
exec.applyRunningRecordToResumeRequest(req, task, session, true, running)
if req.TaskDescription != "" {
t.Fatalf("completed-session TaskDescription = %q, want empty", req.TaskDescription)
}
}
// TestResumeSession_UserCancelledWithoutRunningRow_KeepsTaskDescription is
// the scoping counterpart to the archive-cancelled test above: a session the
// user explicitly stopped (not an archive side effect) must not have its
@@ -1640,6 +1640,18 @@ func NewService(
) error {
return s.setSessionStarting(ctx, taskID, session, expectedState, promoteTask)
})
exec.SetOnSessionStartingWithOptions(func(
ctx context.Context,
taskID string,
session *models.TaskSession,
expectedState models.TaskSessionState,
promoteTask bool,
allowCompletedResume bool,
) error {
return s.setSessionStartingWithOptions(
ctx, taskID, session, expectedState, promoteTask, allowCompletedResume,
)
})
exec.SetOnExecutionCleanupClaim(s.claimForcedExecutionCleanup)
exec.SetOnExecutionStopOwnerRegistration(s.RegisterExecutionStopOwner)
exec.SetOnTaskReviewStateReconcile(func(ctx context.Context, taskID, completedSessionID string) {
@@ -82,6 +82,11 @@ type LaunchSessionRequest struct {
// AllowBranchReplacement is set only by RecoverSession for the explicit
// resume_new_branch action. Clients cannot grant this permission directly.
AllowBranchReplacement bool `json:"-"`
// AllowCompletedSessionResume is set only by explicit recovery or a pinned
// follow-up dispatcher. It is intentionally not part of the wire request:
// ordinary launch, ensure, and startup recovery paths must keep completed
// sessions terminal.
AllowCompletedSessionResume bool `json:"-"`
}
// SpawnOrigin describes the agent session that spawned a new sibling session.
@@ -350,7 +355,8 @@ func (s *Service) launchStartCreated(ctx context.Context, req *LaunchSessionRequ
// launchResume resumes a stopped session.
func (s *Service) launchResume(ctx context.Context, req *LaunchSessionRequest) (*LaunchSessionResponse, error) {
execution, err := s.ResumeTaskSessionWithOptions(ctx, req.TaskID, req.SessionID, executor.ResumeOptions{
AllowBranchReplacement: req.AllowBranchReplacement,
AllowBranchReplacement: req.AllowBranchReplacement,
AllowCompletedSessionResume: req.AllowCompletedSessionResume,
})
if err != nil {
return nil, err
@@ -451,10 +457,11 @@ func (s *Service) RecoverSession(ctx context.Context, taskID, sessionID, action
}
resp, err := s.LaunchSession(ctx, &LaunchSessionRequest{
TaskID: taskID,
SessionID: sessionID,
Intent: IntentResume,
AllowBranchReplacement: action == "resume_new_branch",
TaskID: taskID,
SessionID: sessionID,
Intent: IntentResume,
AllowBranchReplacement: action == "resume_new_branch",
AllowCompletedSessionResume: action == "resume",
})
if err != nil {
return nil, normalizeRecoverSessionError(err)
@@ -130,13 +130,14 @@ func TestTaskPRLaunchErrorStampIsStableAndIncludesState(t *testing.T) {
func TestWorkflowHasValidTerminalFinalStep(t *testing.T) {
getter := newMockStepGetter()
getter.steps["review"] = &wfmodels.WorkflowStep{ID: "review", WorkflowID: "workflow", Position: 1, Name: "Review"}
getter.steps["done"] = &wfmodels.WorkflowStep{ID: "done", WorkflowID: "workflow", Position: 2, Name: "Done"}
getter.steps["done"] = &wfmodels.WorkflowStep{ID: "done", WorkflowID: "workflow", Position: 2, Name: "Done", CompleteTaskOnEnter: true}
if !workflowHasValidTerminalFinalStep(t.Context(), getter, "review") {
t.Fatal("final Done step should enable mark_review_done")
}
getter.steps["done"].Name = "Archive"
getter.steps["done"].CompleteTaskOnEnter = false
if workflowHasValidTerminalFinalStep(t.Context(), getter, "review") {
t.Fatal("non-terminal final step should not enable mark_review_done")
}
@@ -159,7 +160,7 @@ func TestShouldSkipTerminalPRAutoStartPersistsAndClearsStampedError(t *testing.T
}
steps := newMockStepGetter()
steps.steps["review"] = &wfmodels.WorkflowStep{ID: "review", WorkflowID: "wf1", Position: 1, Name: "Review"}
steps.steps["done"] = &wfmodels.WorkflowStep{ID: "done", WorkflowID: "wf1", Position: 2, Name: "Done"}
steps.steps["done"] = &wfmodels.WorkflowStep{ID: "done", WorkflowID: "wf1", Position: 2, Name: "Done", CompleteTaskOnEnter: true}
svc := createTestService(repo, steps, newMockTaskRepo())
svc.SetGitHubService(&mockGitHubService{taskPRs: []*github.TaskPR{
{TaskID: "task-pr-gate", RepositoryID: "repo-1", PRNumber: 42, State: githubPRStateMerged},
@@ -125,7 +125,7 @@ func recoveryFixtureService(t *testing.T, repo *sqliterepo.Repository, fake *tas
t.Helper()
steps := newMockStepGetter()
steps.steps["review-recovery"] = &wfmodels.WorkflowStep{ID: "review-recovery", WorkflowID: "wf-recovery", Position: 0, Name: "Review"}
steps.steps["done-recovery"] = &wfmodels.WorkflowStep{ID: "done-recovery", WorkflowID: "wf-recovery", Position: 1, Name: "Done"}
steps.steps["done-recovery"] = &wfmodels.WorkflowStep{ID: "done-recovery", WorkflowID: "wf-recovery", Position: 1, Name: "Done", CompleteTaskOnEnter: true}
svc := createTestService(repo, steps, newMockTaskRepo())
svc.taskLaunchRecoveryRepo = repo
svc.taskLaunchRecoveryTasks = fake
@@ -2301,26 +2301,33 @@ func (s *Service) ResumeTaskSessionWithOptions(
if session.TaskID != taskID {
return nil, fmt.Errorf("task session does not belong to task")
}
allowCompletedResume := options.AllowCompletedSessionResume &&
session.State == models.TaskSessionStateCompleted
// The completed-session permission is valid only for the exact completed
// row admitted by the caller. Do not let an option intended for that state
// alter the ordinary FAILED/CANCELLED recovery paths.
options.AllowCompletedSessionResume = allowCompletedResume
// Completed sessions remain closed to every implicit resume path. Check this
// before looking for an executor row because cleanup commonly removes that
// row, and the caller should receive the terminal-state rejection rather
// than an incidental "no executor record" error.
if session.State == models.TaskSessionStateCompleted && !allowCompletedResume {
return nil, fmt.Errorf("session is completed and cannot be resumed; create a new session instead")
}
running, err := s.repo.GetExecutorRunningBySessionID(ctx, sessionID)
if (err != nil || running == nil) &&
session.State != models.TaskSessionStateCancelled &&
session.State != models.TaskSessionStateFailed {
session.State != models.TaskSessionStateFailed &&
!allowCompletedResume {
// Executor record is required for non-terminal sessions. For cancelled/failed sessions
// the record may already have been cleaned up before the user clicked Resume — allow it.
// Explicit completed follow-up has the same cleanup shape and is admitted
// only through the narrow permission above.
return nil, fmt.Errorf("session is not resumable: no executor record")
}
s.noteMissingWorktreesBeforeResume(sessionID, session)
branchRecoveryBefore := s.captureBranchRecoveryBeforeResume(ctx, taskID, sessionID, options)
// Completed sessions cannot be restarted — they require a new session.
// Failed and cancelled sessions keep the resume token so the relaunched
// agent continues the previous conversation (via ACP session/load for
// native-resume agents, or --resume on CLI). Users who want a fresh start
// after a failure can invoke RecoverSession with action="fresh_start".
if session.State == models.TaskSessionStateCompleted {
return nil, fmt.Errorf("session is completed and cannot be resumed; create a new session instead")
}
isOfficeTask, err := s.lookupOfficeTask(ctx, taskID)
if err != nil {
return nil, fmt.Errorf("failed to determine office task status: %w", err)
@@ -1050,10 +1050,11 @@ func cancelCompletionStepGetter(enabled, signalRequired bool) *mockStepGetter {
}}},
}
getter.steps["step3"] = &wfmodels.WorkflowStep{
ID: "step3",
WorkflowID: "wf1",
Name: "Done",
Position: 2,
ID: "step3",
WorkflowID: "wf1",
Name: "Done",
Position: 2,
CompleteTaskOnEnter: true,
}
return getter
}
@@ -1383,6 +1384,7 @@ func TestCancelAgent_TerminalTransitionSkipsIntermediateReview(t *testing.T) {
seedSession(t, repo, taskID, sessionID, "step1")
steps := cancelCompletionStepGetter(true, false)
steps.steps["step2"].Name = "Done"
steps.steps["step2"].CompleteTaskOnEnter = true
delete(steps.steps, "step3")
taskRepo := newMockTaskRepo()
@@ -280,6 +280,9 @@ func buildWorkflowFromJSON(t *testing.T, jsonStr string) (*mockStepGetter, map[s
if err := json.Unmarshal([]byte(jsonStr), &export); err != nil {
t.Fatalf("failed to unmarshal workflow JSON: %v", err)
}
if err := export.NormalizeCompletionPolicy(); err != nil {
t.Fatalf("failed to normalize workflow completion policy: %v", err)
}
if len(export.Workflows) == 0 {
t.Fatal("workflow JSON contains no workflows")
}
@@ -297,7 +300,7 @@ func buildWorkflowFromJSON(t *testing.T, jsonStr string) (*mockStepGetter, map[s
events := wfmodels.ConvertPositionToStepID(sp.Events, posToID)
sg.steps[id] = &wfmodels.WorkflowStep{
ID: id, WorkflowID: "wf1", Name: sp.Name, Position: sp.Position,
Prompt: sp.Prompt, Events: events,
Prompt: sp.Prompt, Events: events, CompleteTaskOnEnter: sp.CompleteTaskOnEnter,
}
nameToID[sp.Name] = id
}
@@ -28,6 +28,7 @@ func FromWorkflowStep(step *wfmodels.WorkflowStep) WorkflowStepDTO {
StageType: string(step.StageType),
AutoAdvanceRequiresSignal: step.AutoAdvanceRequiresSignal,
CancelTriggersTurnComplete: step.CancelTriggersTurnComplete,
CompleteTaskOnEnter: step.CompleteTaskOnEnter,
}
if hasStepEvents(step.Events) {
events := &StepEventsDTO{}
+1
View File
@@ -1180,6 +1180,7 @@ type WorkflowStepDTO struct {
StageType string `json:"stage_type,omitempty"`
AutoAdvanceRequiresSignal bool `json:"auto_advance_requires_signal"`
CancelTriggersTurnComplete bool `json:"cancel_triggers_turn_complete"`
CompleteTaskOnEnter bool `json:"complete_task_on_enter"`
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
}
+17 -6
View File
@@ -269,6 +269,10 @@ func HasAutoStartOnCreateIntent(metadata map[string]interface{}) bool {
const (
SessionMetaKeyCreatedBy = "created_by"
SessionCreatedByWorkflowSwitch = "workflow_switch"
// SessionMetaKeyCompletionFollowUp marks a completed conversation that was
// explicitly resumed for conversational follow-up. It is not workflow
// ownership and must not be treated as a task-state transition.
SessionMetaKeyCompletionFollowUp = "completion_follow_up"
// SessionMetaKeyWorkflowProfileSwitchStopIntent identifies the transient
// coordination record used to suppress the lifecycle event caused by a
// parked workflow profile switch.
@@ -285,6 +289,14 @@ const (
SessionMetaKeyRecoveryResolvedAt = "recovery_resolved_at"
)
// IsCompletionFollowUpSession reports whether a session was explicitly
// reopened only to continue its completed conversation. The marker remains
// meaningful after the session settles back to WAITING_FOR_INPUT.
func IsCompletionFollowUpSession(metadata map[string]interface{}) bool {
followUp, ok := metadata[SessionMetaKeyCompletionFollowUp].(bool)
return ok && followUp
}
// WorkflowProfileSwitchStopIntent binds a deliberate parked-session stop to
// one exact runtime execution. Stamp is compared before the metadata value is
// marked consumed, so a delayed event cannot consume a newer switch intent.
@@ -983,12 +995,11 @@ type OfficeDecisionWaitCursor struct {
// ChildCompletionRow is the compact active-child projection used to decide
// whether a parent task's on_children_completed trigger is ready to fire.
type ChildCompletionRow struct {
ID string `json:"id" db:"id"`
State v1.TaskState `json:"state" db:"state"`
Title string `json:"title" db:"title"`
WorkflowStepID string `json:"workflow_step_id" db:"workflow_step_id"`
TerminalWorkflowStep bool `json:"terminal_workflow_step"` // computed by annotateTerminalChildSteps, not a DB column
UpdatedAt time.Time `json:"updated_at" db:"updated_at"`
ID string `json:"id" db:"id"`
State v1.TaskState `json:"state" db:"state"`
Title string `json:"title" db:"title"`
WorkflowStepID string `json:"workflow_step_id" db:"workflow_step_id"`
UpdatedAt time.Time `json:"updated_at" db:"updated_at"`
}
// HasStartWhenUnblockedIntent reports whether a task's deferred launch intent
@@ -95,6 +95,19 @@ func TestIsOriginalTaskSessionUsesImmutableOriginMarker(t *testing.T) {
}))
}
func TestIsCompletionFollowUpSessionRequiresTrueBoolean(t *testing.T) {
require.True(t, IsCompletionFollowUpSession(map[string]interface{}{
SessionMetaKeyCompletionFollowUp: true,
}))
require.False(t, IsCompletionFollowUpSession(map[string]interface{}{
SessionMetaKeyCompletionFollowUp: false,
}))
require.False(t, IsCompletionFollowUpSession(map[string]interface{}{
SessionMetaKeyCompletionFollowUp: "true",
}))
require.False(t, IsCompletionFollowUpSession(nil))
}
func TestLoadSessionACPConfigBaselinePreservesEmptyJSONValues(t *testing.T) {
baseline, ok := LoadSessionACPConfigBaseline(map[string]interface{}{
SessionMetaKeyACPConfigBaseline: map[string]interface{}{
@@ -307,6 +307,7 @@ func (r *Repository) runMigrations() error {
// migrations are idempotent and preserve the false default for legacy rows.
r.migrate.Apply("workflow_steps.auto_advance_requires_signal", `ALTER TABLE workflow_steps ADD COLUMN auto_advance_requires_signal INTEGER NOT NULL DEFAULT 0`)
r.migrate.Apply("workflow_steps.cancel_triggers_turn_complete", `ALTER TABLE workflow_steps ADD COLUMN cancel_triggers_turn_complete INTEGER NOT NULL DEFAULT 0`)
_ = r.migrate.Apply("workflow_steps.complete_task_on_enter", `ALTER TABLE workflow_steps ADD COLUMN complete_task_on_enter INTEGER NOT NULL DEFAULT 0`)
r.migrate.Apply("workflow_steps.profile_session_start_policy", `ALTER TABLE workflow_steps ADD COLUMN profile_session_start_policy TEXT NOT NULL DEFAULT 'reuse'`)
r.migrate.Apply("workflow_steps.profile_session_end_policy", `ALTER TABLE workflow_steps ADD COLUMN profile_session_end_policy TEXT NOT NULL DEFAULT 'complete'`)
@@ -238,6 +238,7 @@ func (r *Repository) ensureRunnerProjectionTables() error {
stage_type TEXT NOT NULL DEFAULT 'custom',
auto_advance_requires_signal INTEGER NOT NULL DEFAULT 0,
cancel_triggers_turn_complete INTEGER NOT NULL DEFAULT 0,
complete_task_on_enter INTEGER NOT NULL DEFAULT 0,
created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP
)`); err != nil {
@@ -184,6 +184,34 @@ func TestCreateWorkspaceWithKanban_CancelTriggersTurnCompleteDefaults(t *testing
}
}
func TestCreateWorkspaceWithKanban_PersistsCompletionAndWIPFields(t *testing.T) {
repo := newRepoForBuiltinWorkflowTests(t)
ctx := context.Background()
workflow, err := repo.CreateWorkspaceWithKanban(ctx, &taskmodels.Workspace{ID: "ws-kanban-completion", Name: "Kanban Completion"})
if err != nil {
t.Fatalf("create Kanban workspace: %v", err)
}
var wipLimit, completeTaskOnEnter int
var pullFromStepID string
if err := repo.db.QueryRowContext(ctx, repo.db.Rebind(`
SELECT wip_limit, pull_from_step_id, complete_task_on_enter
FROM workflow_steps
WHERE workflow_id = ? AND name = 'Done'
`), workflow.ID).Scan(&wipLimit, &pullFromStepID, &completeTaskOnEnter); err != nil {
t.Fatalf("query Kanban completion fields: %v", err)
}
if wipLimit != 0 {
t.Fatalf("Done WIP limit = %d, want 0", wipLimit)
}
if pullFromStepID != "" {
t.Fatalf("Done pull-from step = %q, want empty", pullFromStepID)
}
if completeTaskOnEnter != 1 {
t.Fatalf("Done complete-task-on-enter = %d, want 1", completeTaskOnEnter)
}
}
func TestEnsureOfficeDefaultWorkflow_TriggersJSONShape(t *testing.T) {
repo := newRepoForBuiltinWorkflowTests(t)
ctx := context.Background()
@@ -473,14 +473,14 @@ func (r *Repository) insertTemplateStep(
allow_manual_move, is_start_step, show_in_command_panel,
auto_archive_after_hours, agent_profile_id, profile_session_start_policy, profile_session_end_policy, stage_type,
auto_advance_requires_signal, cancel_triggers_turn_complete,
created_at, updated_at
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
complete_task_on_enter, created_at, updated_at
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
`),
idMap[def.ID], workflowID, def.Name, def.Position, def.Color, def.Prompt,
string(eventsJSON), dialect.BoolToInt(def.AllowManualMove),
dialect.BoolToInt(def.IsStartStep), dialect.BoolToInt(def.ShowInCommandPanel),
def.AutoArchiveAfterHours, def.AgentProfileID, models.NormalizeWorkflowProfileSessionStartPolicy(string(def.ProfileSessionStartPolicy)), models.NormalizeWorkflowProfileSessionEndPolicy(string(def.ProfileSessionEndPolicy)), string(stage),
dialect.BoolToInt(def.AutoAdvanceRequiresSignal), dialect.BoolToInt(def.CancelTriggersTurnComplete),
dialect.BoolToInt(def.AutoAdvanceRequiresSignal), dialect.BoolToInt(def.CancelTriggersTurnComplete), dialect.BoolToInt(def.CompleteTaskOnEnter),
now, now,
)
if err != nil {
@@ -691,6 +691,7 @@ func TestRunnerProjectionWorkflowStepColumnsReplayMigration(t *testing.T) {
}{
{name: "auto_advance_requires_signal", sql: `ALTER TABLE workflow_steps DROP COLUMN auto_advance_requires_signal`},
{name: "cancel_triggers_turn_complete", sql: `ALTER TABLE workflow_steps DROP COLUMN cancel_triggers_turn_complete`},
{name: "complete_task_on_enter", sql: `ALTER TABLE workflow_steps DROP COLUMN complete_task_on_enter`},
}
for _, column := range legacyColumns {
if _, err := repo.db.Exec(column.sql); err != nil {
@@ -713,12 +714,12 @@ func TestRunnerProjectionWorkflowStepColumnsReplayMigration(t *testing.T) {
t.Fatalf("workflow_steps.%s column count = %d, want 1", column, count)
}
}
var autoAdvance, cancelComplete int
if err := repo.db.QueryRow(`SELECT auto_advance_requires_signal, cancel_triggers_turn_complete FROM workflow_steps WHERE id = 'legacy-projection-step'`).Scan(&autoAdvance, &cancelComplete); err != nil {
var autoAdvance, cancelComplete, completeTask int
if err := repo.db.QueryRow(`SELECT auto_advance_requires_signal, cancel_triggers_turn_complete, complete_task_on_enter FROM workflow_steps WHERE id = 'legacy-projection-step'`).Scan(&autoAdvance, &cancelComplete, &completeTask); err != nil {
t.Fatalf("read migrated workflow step: %v", err)
}
if autoAdvance != 0 || cancelComplete != 0 {
t.Fatalf("legacy workflow step defaults = (%d, %d), want (0, 0)", autoAdvance, cancelComplete)
if autoAdvance != 0 || cancelComplete != 0 || completeTask != 0 {
t.Fatalf("legacy workflow step defaults = (%d, %d, %d), want (0, 0, 0)", autoAdvance, cancelComplete, completeTask)
}
}
@@ -1863,6 +1863,34 @@ func (r *Repository) SetSessionMetadataKey(ctx context.Context, sessionID, key s
return nil
}
// SetSessionMetadataKeyIfState atomically sets one metadata key only while the
// session remains in expectedState. Runtime recovery uses this to keep a
// follow-up marker from being attached to a session that was stopped between
// its guarded state transition and metadata persistence.
func (r *Repository) SetSessionMetadataKeyIfState(
ctx context.Context,
sessionID, key string,
value interface{},
expectedState models.TaskSessionState,
) (bool, error) {
valueJSON, err := json.Marshal(value)
if err != nil {
return false, fmt.Errorf("failed to serialize metadata value: %w", err)
}
query := metadataKeyUpdateQuery("task_sessions", r.db.DriverName()) + " AND state = ?"
args := metadataKeyUpdateArgs(r.db.DriverName(), key, string(valueJSON), r.nowUTC(), sessionID)
args = append(args, string(expectedState))
result, err := r.db.ExecContext(ctx, r.db.Rebind(query), args...)
if err != nil {
return false, err
}
rows, err := result.RowsAffected()
if err != nil {
return false, err
}
return rows > 0, nil
}
// UpdateSessionContextWindow stores a context-window sample and atomically
// increments the session's inferred compaction count when the new used-token
// value is lower than the previous persisted sample.
@@ -810,6 +810,45 @@ func TestSetSessionMetadataKeyIfAbsentSQLiteIsWriteOnce(t *testing.T) {
}
}
func TestSetSessionMetadataKeyIfStateGuardsTheSessionState(t *testing.T) {
repo := newRepoForSessionTests(t)
ctx := context.Background()
seedForMsgTest(t, repo, "task-session-marker", "session-session-marker", "turn-session-marker")
require.NoError(t, repo.SetSessionMetadataKey(ctx, "session-session-marker", "keep", "value"))
changed, err := repo.SetSessionMetadataKeyIfState(
ctx,
"session-session-marker",
"completion_follow_up",
true,
models.TaskSessionStateCreated,
)
require.NoError(t, err)
require.True(t, changed)
require.NoError(t, repo.UpdateTaskSessionState(
ctx,
"session-session-marker",
models.TaskSessionStateCancelled,
"stopped",
))
changed, err = repo.SetSessionMetadataKeyIfState(
ctx,
"session-session-marker",
"completion_follow_up",
false,
models.TaskSessionStateCreated,
)
require.NoError(t, err)
require.False(t, changed)
stored, err := repo.GetTaskSession(ctx, "session-session-marker")
require.NoError(t, err)
require.Equal(t, models.TaskSessionStateCancelled, stored.State)
require.Equal(t, true, stored.Metadata["completion_follow_up"])
require.Equal(t, "value", stored.Metadata["keep"])
}
func TestSetSessionMetadataKeyIfAbsentOrDifferentStepSQLiteReplacesOnlyStaleStep(t *testing.T) {
repo := newRepoForSessionTests(t)
seedForMsgTest(t, repo, "task-step-claim", "session-step-claim", "turn-step-claim")
@@ -3346,6 +3346,9 @@ func (r *Repository) tryUpdateTaskStateIfSessionState(
if err != nil {
return "", false, false, err
}
if oldState == v1.TaskStateCompleted && state == v1.TaskStateInProgress {
return oldState, false, false, nil
}
if archivedAt.Valid || currentSessionState != expectedSessionState ||
(requirePrimary && !currentSessionIsPrimary) {
return oldState, false, false, nil
@@ -77,6 +77,40 @@ func TestUpdateTaskStateIfSessionState_SkipsArchivedTask(t *testing.T) {
}
}
func TestUpdateTaskStateIfSessionState_DoesNotReopenCompletedTask(t *testing.T) {
repo := newRepoForHealTests(t)
ctx := context.Background()
insertTask(t, repo.db, "task-session-completed")
insertSession(t, repo, "session-completed", "task-session-completed", string(models.TaskSessionStateStarting))
if err := repo.UpdateTaskState(ctx, "task-session-completed", v1.TaskStateCompleted); err != nil {
t.Fatalf("seed COMPLETED: %v", err)
}
oldState, updated, err := repo.UpdateTaskStateIfSessionState(
ctx,
"task-session-completed",
"session-completed",
models.TaskSessionStateStarting,
v1.TaskStateInProgress,
)
if err != nil {
t.Fatalf("UpdateTaskStateIfSessionState: %v", err)
}
if updated {
t.Fatal("completed task was reopened by a runtime state writer")
}
if oldState != v1.TaskStateCompleted {
t.Fatalf("old state = %q, want %q", oldState, v1.TaskStateCompleted)
}
task, err := repo.GetTask(ctx, "task-session-completed")
if err != nil {
t.Fatalf("get task: %v", err)
}
if task.State != v1.TaskStateCompleted {
t.Fatalf("task state = %q, want %q", task.State, v1.TaskStateCompleted)
}
}
func TestUpdateTaskStateIfSessionState_UpdatesMatchingNonPrimarySession(t *testing.T) {
repo := newRepoForHealTests(t)
ctx := context.Background()
@@ -84,12 +84,12 @@ func (r *Repository) insertTemplateSteps(ctx context.Context, tx *sqlx.Tx, workf
allow_manual_move, is_start_step, show_in_command_panel,
auto_archive_after_hours, agent_profile_id, profile_session_start_policy, profile_session_end_policy, stage_type,
auto_advance_requires_signal, cancel_triggers_turn_complete, wip_limit, pull_from_step_id,
created_at, updated_at
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
complete_task_on_enter, created_at, updated_at
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
`), idMap[stepDef.ID], workflowID, stepDef.Name, stepDef.Position, stepDef.Color, stepDef.Prompt, string(events),
dialect.BoolToInt(stepDef.AllowManualMove), dialect.BoolToInt(stepDef.IsStartStep), dialect.BoolToInt(stepDef.ShowInCommandPanel),
stepDef.AutoArchiveAfterHours, stepDef.AgentProfileID, models.NormalizeWorkflowProfileSessionStartPolicy(string(stepDef.ProfileSessionStartPolicy)), models.NormalizeWorkflowProfileSessionEndPolicy(string(stepDef.ProfileSessionEndPolicy)), normalizeBootstrapStageType(stepDef.StageType),
dialect.BoolToInt(stepDef.AutoAdvanceRequiresSignal), dialect.BoolToInt(stepDef.CancelTriggersTurnComplete), stepDef.WIPLimit, wfmodels.RemapStepID(stepDef.PullFromStepID, idMap), now, now); err != nil {
dialect.BoolToInt(stepDef.AutoAdvanceRequiresSignal), dialect.BoolToInt(stepDef.CancelTriggersTurnComplete), stepDef.WIPLimit, wfmodels.RemapStepID(stepDef.PullFromStepID, idMap), dialect.BoolToInt(stepDef.CompleteTaskOnEnter), now, now); err != nil {
return fmt.Errorf("create Kanban step %q: %w", stepDef.Name, err)
}
}
@@ -192,7 +192,7 @@ func TestService_MoveTaskToTerminalStepCompletesTask(t *testing.T) {
seedMoveSteps(svc)
getter := svc.workflowStepGetter.(*fakeWorkflowStepGetter)
getter.steps["step-done"] = &wfmodels.WorkflowStep{
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2,
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2, CompleteTaskOnEnter: true,
}
createMoveTask(t, ctx, repo, "task-terminal", "wf-source", "step-source", nil)
eventBus.ClearEvents()
@@ -232,7 +232,7 @@ func TestService_MoveTaskToTerminalStepPreservesTerminalFailureStates(t *testing
seedMoveSteps(svc)
getter := svc.workflowStepGetter.(*fakeWorkflowStepGetter)
getter.steps["step-done"] = &wfmodels.WorkflowStep{
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2,
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2, CompleteTaskOnEnter: true,
}
createMoveTask(t, ctx, repo, "task-terminal-"+tc.name, "wf-source", "step-source", nil)
task, err := repo.GetTask(ctx, "task-terminal-"+tc.name)
@@ -268,7 +268,7 @@ func TestService_MoveTaskRecoveryCompletesFailedTaskAtTerminalStep(t *testing.T)
seedMoveSteps(svc)
getter := svc.workflowStepGetter.(*fakeWorkflowStepGetter)
getter.steps["step-done"] = &wfmodels.WorkflowStep{
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2,
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2, CompleteTaskOnEnter: true,
}
createMoveTask(t, ctx, repo, "task-recovery", "wf-source", "step-source", nil)
task, err := repo.GetTask(ctx, "task-recovery")
@@ -304,7 +304,7 @@ func TestService_MoveTaskRecoveryIsIdempotentAtTerminalStep(t *testing.T) {
seedMoveSteps(svc)
getter := svc.workflowStepGetter.(*fakeWorkflowStepGetter)
getter.steps["step-done"] = &wfmodels.WorkflowStep{
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2,
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2, CompleteTaskOnEnter: true,
}
createMoveTask(t, ctx, repo, "task-recovery-idempotent", "wf-source", "step-done", nil)
@@ -349,7 +349,7 @@ func TestService_MoveTaskOutOfTerminalStepReopensTask(t *testing.T) {
seedMoveSteps(svc)
getter := svc.workflowStepGetter.(*fakeWorkflowStepGetter)
getter.steps["step-done"] = &wfmodels.WorkflowStep{
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2,
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2, CompleteTaskOnEnter: true,
}
createMoveTask(t, ctx, repo, "task-reopened", "wf-source", "step-done", nil)
task, err := repo.GetTask(ctx, "task-reopened")
@@ -385,7 +385,7 @@ func TestService_ApproveSessionToTerminalStepCompletesTask(t *testing.T) {
seedMoveSteps(svc)
getter := svc.workflowStepGetter.(*fakeWorkflowStepGetter)
getter.steps["step-done"] = &wfmodels.WorkflowStep{
ID: "step-done", WorkflowID: "wf-source", Name: "Approved", Position: 2,
ID: "step-done", WorkflowID: "wf-source", Name: "Approved", Position: 2, CompleteTaskOnEnter: true,
}
createMoveTask(t, ctx, repo, "task-approved", "wf-source", "step-review-target", nil)
createMoveSession(t, ctx, repo, "session-approved", "task-approved", models.TaskSessionStateWaitingForInput, models.ReviewStatusPending)
@@ -795,7 +795,7 @@ func TestService_BulkMoveTasksToTerminalStepCompletesTasks(t *testing.T) {
seedMoveSteps(svc)
getter := svc.workflowStepGetter.(*fakeWorkflowStepGetter)
getter.steps["step-done"] = &wfmodels.WorkflowStep{
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2,
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2, CompleteTaskOnEnter: true,
}
createMoveTask(t, ctx, repo, "task-bulk-terminal", "wf-source", "step-source", nil)
eventBus.ClearEvents()
@@ -832,7 +832,7 @@ func TestService_BulkMoveTasksToTerminalStepPreservesTerminalFailureStates(t *te
seedMoveSteps(svc)
getter := svc.workflowStepGetter.(*fakeWorkflowStepGetter)
getter.steps["step-done"] = &wfmodels.WorkflowStep{
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2,
ID: "step-done", WorkflowID: "wf-source", Name: "Done", Position: 2, CompleteTaskOnEnter: true,
}
createMoveTask(t, ctx, repo, "task-bulk-terminal-"+tc.name, "wf-source", "step-source", nil)
task, err := repo.GetTask(ctx, "task-bulk-terminal-"+tc.name)
@@ -1,7 +1,9 @@
package controller
import (
"bytes"
"context"
"encoding/json"
"fmt"
"strings"
@@ -136,12 +138,40 @@ type CreateStepRequest struct {
ShowInCommandPanel *bool `json:"show_in_command_panel,omitempty"`
AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal,omitempty"`
CancelTriggersTurnComplete *bool `json:"cancel_triggers_turn_complete,omitempty"`
CompleteTaskOnEnter *bool `json:"complete_task_on_enter,omitempty"`
ProfileSessionStartPolicy *string `json:"profile_session_start_policy,omitempty"`
ProfileSessionEndPolicy *string `json:"profile_session_end_policy,omitempty"`
WIPLimit *int `json:"wip_limit,omitempty"`
PullFromStepID *string `json:"pull_from_step_id,omitempty"`
}
func rejectNullCompleteTaskOnEnter(data []byte) error {
var fields map[string]json.RawMessage
if err := json.Unmarshal(data, &fields); err != nil {
return err
}
if raw, ok := fields["complete_task_on_enter"]; ok && bytes.Equal(bytes.TrimSpace(raw), []byte("null")) {
return fmt.Errorf("complete_task_on_enter must be a boolean")
}
return nil
}
// UnmarshalJSON keeps omission distinct from an explicit null. The REST
// boundary uses nil to mean the documented default for create and no-op for
// update, so null must be rejected instead of silently taking either path.
func (r *CreateStepRequest) UnmarshalJSON(data []byte) error {
if err := rejectNullCompleteTaskOnEnter(data); err != nil {
return err
}
type requestAlias CreateStepRequest
var decoded requestAlias
if err := json.Unmarshal(data, &decoded); err != nil {
return err
}
*r = CreateStepRequest(decoded)
return nil
}
// CreateStep creates a new workflow step.
func (c *Controller) CreateStep(ctx context.Context, req CreateStepRequest) (*GetStepResponse, error) {
// The new step names its workflow in the body; authorize that workflow
@@ -189,6 +219,9 @@ func (c *Controller) CreateStep(ctx context.Context, req CreateStepRequest) (*Ge
if req.CancelTriggersTurnComplete != nil {
step.CancelTriggersTurnComplete = *req.CancelTriggersTurnComplete
}
if req.CompleteTaskOnEnter != nil {
step.CompleteTaskOnEnter = *req.CompleteTaskOnEnter
}
if req.WIPLimit != nil {
if *req.WIPLimit < 0 {
return nil, fmt.Errorf("wip_limit must be non-negative")
@@ -224,12 +257,27 @@ type UpdateStepRequest struct {
AgentProfileID *string `json:"agent_profile_id,omitempty"`
AutoAdvanceRequiresSignal *bool `json:"auto_advance_requires_signal,omitempty"`
CancelTriggersTurnComplete *bool `json:"cancel_triggers_turn_complete,omitempty"`
CompleteTaskOnEnter *bool `json:"complete_task_on_enter,omitempty"`
ProfileSessionStartPolicy *string `json:"profile_session_start_policy,omitempty"`
ProfileSessionEndPolicy *string `json:"profile_session_end_policy,omitempty"`
WIPLimit *int `json:"wip_limit,omitempty"`
PullFromStepID *string `json:"pull_from_step_id,omitempty"`
}
// UnmarshalJSON rejects null while preserving omission semantics for updates.
func (r *UpdateStepRequest) UnmarshalJSON(data []byte) error {
if err := rejectNullCompleteTaskOnEnter(data); err != nil {
return err
}
type requestAlias UpdateStepRequest
var decoded requestAlias
if err := json.Unmarshal(data, &decoded); err != nil {
return err
}
*r = UpdateStepRequest(decoded)
return nil
}
// UpdateStep updates an existing workflow step.
func (c *Controller) UpdateStep(ctx context.Context, req UpdateStepRequest) (*GetStepResponse, error) {
if err := c.svc.AuthorizeStep(ctx, req.ID); err != nil {
@@ -287,6 +335,9 @@ func (c *Controller) UpdateStep(ctx context.Context, req UpdateStepRequest) (*Ge
if req.CancelTriggersTurnComplete != nil {
step.CancelTriggersTurnComplete = *req.CancelTriggersTurnComplete
}
if req.CompleteTaskOnEnter != nil {
step.CompleteTaskOnEnter = *req.CompleteTaskOnEnter
}
if req.WIPLimit != nil {
if *req.WIPLimit < 0 {
return nil, fmt.Errorf("wip_limit must be non-negative")
@@ -212,6 +212,41 @@ func TestStepMutationValidationRejections(t *testing.T) {
}
})
t.Run("create rejects explicit null completion policy", func(t *testing.T) {
h := setupStepRouter(t)
rec := recordStepEvents(t, h.eventBus)
resp := doRaw(t, h, http.MethodPost, "/api/v1/workflow/steps", `{"workflow_id":"workflow-1","name":"Null completion","complete_task_on_enter":null}`)
if resp.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want 400; body = %s", resp.Code, resp.Body.String())
}
if len(rec.events) != 0 {
t.Fatalf("rejected create published %v", rec.subjects())
}
})
t.Run("update rejects explicit null completion policy without mutation", func(t *testing.T) {
h := setupStepRouter(t)
step := createStepViaHTTP(t, h.router, map[string]interface{}{
"workflow_id": "workflow-1", "name": "Done", "position": 0,
"complete_task_on_enter": true,
})
rec := recordStepEvents(t, h.eventBus)
resp := doRaw(t, h, http.MethodPut, "/api/v1/workflow/steps/"+step.ID, `{"complete_task_on_enter":null}`)
if resp.Code != http.StatusBadRequest {
t.Fatalf("status = %d, want 400; body = %s", resp.Code, resp.Body.String())
}
if len(rec.events) != 0 {
t.Fatalf("rejected update published %v", rec.subjects())
}
stored, err := h.service.GetStep(context.Background(), step.ID)
if err != nil {
t.Fatalf("reload step: %v", err)
}
if !stored.CompleteTaskOnEnter {
t.Fatal("explicit null update changed the saved completion policy")
}
})
t.Run("create without workflow_id or name", func(t *testing.T) {
h := setupStepRouter(t)
rec := recordStepEvents(t, h.eventBus)
+132 -20
View File
@@ -1,16 +1,20 @@
package models
import (
"bytes"
"encoding/json"
"fmt"
"maps"
"math"
taskmodels "github.com/kandev/kandev/internal/task/models"
"gopkg.in/yaml.v3"
)
const (
ExportVersion = 1
ExportType = "kandev_workflow"
LegacyExportVersion = 1
ExportVersion = 2
ExportType = "kandev_workflow"
)
// WorkflowExport is the portable format for sharing workflows.
@@ -49,22 +53,92 @@ type WorkflowPortable struct {
// StepPortable is a workflow step without instance-specific fields.
type StepPortable struct {
Name string `json:"name" yaml:"name"`
Position int `json:"position" yaml:"position"`
Color string `json:"color" yaml:"color"`
Prompt string `json:"prompt,omitempty" yaml:"prompt,omitempty"`
Events StepEvents `json:"events" yaml:"events"`
IsStartStep bool `json:"is_start_step" yaml:"is_start_step"`
ShowInCommandPanel bool `json:"show_in_command_panel" yaml:"show_in_command_panel"`
AllowManualMove bool `json:"allow_manual_move" yaml:"allow_manual_move"`
AutoArchiveAfterHours int `json:"auto_archive_after_hours,omitempty" yaml:"auto_archive_after_hours,omitempty"`
AgentProfile *AgentProfilePortable `json:"agent_profile,omitempty" yaml:"agent_profile,omitempty"`
ProfileSessionStartPolicy taskmodels.WorkflowProfileSessionStartPolicy `json:"profile_session_start_policy,omitempty" yaml:"profile_session_start_policy,omitempty"`
ProfileSessionEndPolicy taskmodels.WorkflowProfileSessionEndPolicy `json:"profile_session_end_policy,omitempty" yaml:"profile_session_end_policy,omitempty"`
AutoAdvanceRequiresSignal bool `json:"auto_advance_requires_signal" yaml:"auto_advance_requires_signal"`
CancelTriggersTurnComplete bool `json:"cancel_triggers_turn_complete" yaml:"cancel_triggers_turn_complete"`
WIPLimit int `json:"wip_limit,omitempty" yaml:"wip_limit,omitempty"`
PullFromStepPosition *int `json:"pull_from_step_position,omitempty" yaml:"pull_from_step_position,omitempty"`
Name string `json:"name" yaml:"name"`
Position int `json:"position" yaml:"position"`
Color string `json:"color" yaml:"color"`
Prompt string `json:"prompt,omitempty" yaml:"prompt,omitempty"`
Events StepEvents `json:"events" yaml:"events"`
IsStartStep bool `json:"is_start_step" yaml:"is_start_step"`
ShowInCommandPanel bool `json:"show_in_command_panel" yaml:"show_in_command_panel"`
AllowManualMove bool `json:"allow_manual_move" yaml:"allow_manual_move"`
AutoArchiveAfterHours int `json:"auto_archive_after_hours,omitempty" yaml:"auto_archive_after_hours,omitempty"`
AgentProfile *AgentProfilePortable `json:"agent_profile,omitempty" yaml:"agent_profile,omitempty"`
ProfileSessionStartPolicy taskmodels.WorkflowProfileSessionStartPolicy `json:"profile_session_start_policy,omitempty" yaml:"profile_session_start_policy,omitempty"`
ProfileSessionEndPolicy taskmodels.WorkflowProfileSessionEndPolicy `json:"profile_session_end_policy,omitempty" yaml:"profile_session_end_policy,omitempty"`
AutoAdvanceRequiresSignal bool `json:"auto_advance_requires_signal" yaml:"auto_advance_requires_signal"`
CancelTriggersTurnComplete bool `json:"cancel_triggers_turn_complete" yaml:"cancel_triggers_turn_complete"`
CompleteTaskOnEnter bool `json:"complete_task_on_enter" yaml:"complete_task_on_enter"`
WIPLimit int `json:"wip_limit,omitempty" yaml:"wip_limit,omitempty"`
PullFromStepPosition *int `json:"pull_from_step_position,omitempty" yaml:"pull_from_step_position,omitempty"`
completionTaskOnEnterDecoded bool `json:"-" yaml:"-"`
completionTaskOnEnterPresent bool `json:"-" yaml:"-"`
}
// UnmarshalJSON records whether a decoded document explicitly supplied the
// version-2 completion field. The domain field remains a bool so callers that
// build exports in Go keep the same ergonomic API.
func (s *StepPortable) UnmarshalJSON(data []byte) error {
type plainStepPortable StepPortable
var fields map[string]json.RawMessage
if err := json.Unmarshal(data, &fields); err != nil {
return err
}
if raw, ok := fields["complete_task_on_enter"]; ok && bytes.Equal(bytes.TrimSpace(raw), []byte("null")) {
return fmt.Errorf("complete_task_on_enter must be a boolean, not null")
}
var decoded plainStepPortable
if err := json.Unmarshal(data, &decoded); err != nil {
return err
}
*s = StepPortable(decoded)
_, s.completionTaskOnEnterPresent = fields["complete_task_on_enter"]
s.completionTaskOnEnterDecoded = true
return nil
}
// UnmarshalYAML records whether a decoded document explicitly supplied the
// version-2 completion field.
func (s *StepPortable) UnmarshalYAML(node *yaml.Node) error {
type plainStepPortable StepPortable
if node.Kind == yaml.MappingNode {
for i := 0; i+1 < len(node.Content); i += 2 {
if node.Content[i].Value != "complete_task_on_enter" {
continue
}
valueNode := node.Content[i+1]
if valueNode.Tag == "!!null" || valueNode.ShortTag() == "!!null" {
return fmt.Errorf("complete_task_on_enter must be a boolean, not null")
}
}
}
var decoded plainStepPortable
if err := node.Decode(&decoded); err != nil {
return err
}
*s = StepPortable(decoded)
present := false
if node.Kind == yaml.MappingNode {
for i := 0; i+1 < len(node.Content); i += 2 {
if node.Content[i].Value == "complete_task_on_enter" {
present = true
break
}
}
}
s.completionTaskOnEnterPresent = present
s.completionTaskOnEnterDecoded = true
return nil
}
func (s StepPortable) completionTaskOnEnterIsExplicit() bool {
if !s.completionTaskOnEnterDecoded {
return true
}
return s.completionTaskOnEnterPresent
}
func (s *StepPortable) markCompletionTaskOnEnterExplicit() {
s.completionTaskOnEnterPresent = true
}
// BuildWorkflowExport builds a portable WorkflowExport from domain models.
@@ -105,6 +179,7 @@ func buildWorkflowPortable(wf *taskmodels.Workflow, steps []*WorkflowStep, resol
ProfileSessionEndPolicy: taskmodels.NormalizeWorkflowProfileSessionEndPolicy(string(s.ProfileSessionEndPolicy)),
AutoAdvanceRequiresSignal: s.AutoAdvanceRequiresSignal,
CancelTriggersTurnComplete: s.CancelTriggersTurnComplete,
CompleteTaskOnEnter: s.CompleteTaskOnEnter,
WIPLimit: s.WIPLimit,
}
if pos, ok := idToPos[s.PullFromStepID]; ok {
@@ -130,12 +205,15 @@ func buildWorkflowPortable(wf *taskmodels.Workflow, steps []*WorkflowStep, resol
// Validate checks that the export data is well-formed.
func (e *WorkflowExport) Validate() error {
if e.Version != ExportVersion {
return fmt.Errorf("unsupported export version: %d (expected %d)", e.Version, ExportVersion)
if e.Version != LegacyExportVersion && e.Version != ExportVersion {
return fmt.Errorf("unsupported export version: %d (expected %d or %d)", e.Version, LegacyExportVersion, ExportVersion)
}
if e.Type != ExportType {
return fmt.Errorf("unsupported export type: %q (expected %q)", e.Type, ExportType)
}
if err := e.NormalizeCompletionPolicy(); err != nil {
return err
}
if len(e.Workflows) == 0 {
return fmt.Errorf("export contains no workflows")
}
@@ -152,6 +230,9 @@ func (e *WorkflowExport) Validate() error {
return fmt.Errorf("workflow %d: duplicate step position %d", i, step.Position)
}
positions[step.Position] = true
if e.Version == ExportVersion && !step.completionTaskOnEnterIsExplicit() {
return fmt.Errorf("workflow %d step %d: complete_task_on_enter must be explicitly set", i, j)
}
if err := validateOnEnterActions(step); err != nil {
return fmt.Errorf("workflow %d step %d: %w", i, j, err)
}
@@ -170,6 +251,37 @@ func (e *WorkflowExport) Validate() error {
return nil
}
// NormalizeCompletionPolicy converts version-1 exports at the import/sync
// boundary. Version 1 had no explicit completion field, so only its final
// step inherits the legacy terminal-name convention. Version-2 values are
// never inferred or changed.
func (e *WorkflowExport) NormalizeCompletionPolicy() error {
if e.Version != LegacyExportVersion {
return nil
}
for workflowIndex := range e.Workflows {
workflow := &e.Workflows[workflowIndex]
if len(workflow.Steps) == 0 {
continue
}
maxPosition := workflow.Steps[0].Position
for _, step := range workflow.Steps[1:] {
if step.Position > maxPosition {
maxPosition = step.Position
}
}
for stepIndex := range workflow.Steps {
step := &workflow.Steps[stepIndex]
if step.completionTaskOnEnterDecoded && step.completionTaskOnEnterPresent {
continue
}
step.CompleteTaskOnEnter = step.Position == maxPosition && IsTerminalStepName(step.Name)
step.markCompletionTaskOnEnterExplicit()
}
}
return nil
}
func validatePullSourceRefs(steps []StepPortable, validPositions map[int]bool) error {
pullSources := make(map[int]int, len(steps))
for _, step := range steps {
@@ -1,6 +1,7 @@
package models
import (
"encoding/json"
"reflect"
"strings"
"testing"
@@ -639,6 +640,76 @@ func TestCancelTriggersTurnCompleteYAMLExportIncludesFalse(t *testing.T) {
}
}
func TestCompletionPolicyPortableContract(t *testing.T) {
wf := &taskmodels.Workflow{ID: "wf-completion", Name: "Completion"}
export := BuildWorkflowExport([]*taskmodels.Workflow{wf}, map[string][]*WorkflowStep{"wf-completion": {
{ID: "work", Name: "Work", Position: 0},
{ID: "done", Name: "Done", Position: 1, CompleteTaskOnEnter: true},
}}, nil)
require.Equal(t, ExportVersion, export.Version)
payload, err := json.Marshal(export)
require.NoError(t, err)
var fields map[string]any
require.NoError(t, json.Unmarshal(payload, &fields))
steps := fields["workflows"].([]any)[0].(map[string]any)["steps"].([]any)
for i, raw := range steps {
if _, ok := raw.(map[string]any)["complete_task_on_enter"]; !ok {
t.Fatalf("exported step %d omitted complete_task_on_enter", i)
}
}
var decoded WorkflowExport
require.NoError(t, json.Unmarshal([]byte(`{
"version": 1,
"type": "kandev_workflow",
"workflows": [{"name":"Legacy","steps":[
{"name":"Work","position":0,"color":"blue"},
{"name":" dOnE ","position":1,"color":"green"}
]}]
}`), &decoded))
require.NoError(t, decoded.Validate())
assert.False(t, decoded.Workflows[0].Steps[0].CompleteTaskOnEnter)
assert.True(t, decoded.Workflows[0].Steps[1].CompleteTaskOnEnter)
var missing WorkflowExport
require.NoError(t, json.Unmarshal([]byte(`{
"version": 2,
"type": "kandev_workflow",
"workflows": [{"name":"Current","steps":[{"name":"Work","position":0,"color":"blue"}]}]
}`), &missing))
assert.ErrorContains(t, missing.Validate(), "must be explicitly set")
}
func TestCompletionPolicyPortableContractRejectsNull(t *testing.T) {
t.Run("json", func(t *testing.T) {
var decoded WorkflowExport
err := json.Unmarshal([]byte(`{
"version": 2,
"type": "kandev_workflow",
"workflows": [{"name":"Current","steps":[
{"name":"Work","position":0,"color":"blue","complete_task_on_enter":null}
]}]
}`), &decoded)
assert.ErrorContains(t, err, "complete_task_on_enter")
})
t.Run("yaml", func(t *testing.T) {
var decoded WorkflowExport
err := yaml.Unmarshal([]byte(`
version: 2
type: kandev_workflow
workflows:
- name: Current
steps:
- name: Work
position: 0
color: blue
complete_task_on_enter: null
`), &decoded)
assert.ErrorContains(t, err, "complete_task_on_enter")
})
}
func TestPullFromStepPositionToID(t *testing.T) {
position := 0
step := StepPortable{
@@ -211,6 +211,8 @@ type StepDefinition struct {
// CancelTriggersTurnComplete allows an explicit user cancellation to run
// this step's ordinary on_turn_complete actions.
CancelTriggersTurnComplete bool `json:"cancel_triggers_turn_complete,omitempty" yaml:"cancel_triggers_turn_complete,omitempty"`
// CompleteTaskOnEnter marks the final step as completing its task on entry.
CompleteTaskOnEnter bool `json:"complete_task_on_enter" yaml:"complete_task_on_enter"`
}
// WorkflowStep represents a step in a workflow
@@ -244,9 +246,11 @@ type WorkflowStep struct {
AutoAdvanceRequiresSignal bool `json:"auto_advance_requires_signal"`
// CancelTriggersTurnComplete allows an explicit user cancellation to run
// this step's ordinary on_turn_complete actions.
CancelTriggersTurnComplete bool `json:"cancel_triggers_turn_complete"`
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
CancelTriggersTurnComplete bool `json:"cancel_triggers_turn_complete"`
// CompleteTaskOnEnter marks the final step as completing its task on entry.
CompleteTaskOnEnter bool `json:"complete_task_on_enter"`
CreatedAt time.Time `json:"created_at"`
UpdatedAt time.Time `json:"updated_at"`
}
// HasOnEnterAction checks if the step has a specific on_enter action type.
@@ -2,13 +2,13 @@ package models
import "strings"
// IsTerminalStep reports whether a workflow step represents finished work by
// the existing board convention: a final column named Done/Complete/Approved.
// IsTerminalStep reports whether a workflow step is configured to complete its
// task and has no successor in the committed workflow order.
func IsTerminalStep(step, nextStep *WorkflowStep) bool {
if step == nil || nextStep != nil {
return false
}
return IsTerminalStepName(step.Name)
return step.CompleteTaskOnEnter
}
// IsTerminalStepName recognizes the existing built-in terminal column names.
@@ -42,25 +42,32 @@ func TestIsTerminalStep(t *testing.T) {
}
})
t.Run("matching final step", func(t *testing.T) {
step := &WorkflowStep{Name: "Done"}
t.Run("checked final step", func(t *testing.T) {
step := &WorkflowStep{Name: "Release", CompleteTaskOnEnter: true}
if !IsTerminalStep(step, nil) {
t.Fatalf("final Done step was not terminal")
t.Fatalf("checked final step was not terminal")
}
})
t.Run("matching non-final step", func(t *testing.T) {
t.Run("unchecked final step", func(t *testing.T) {
step := &WorkflowStep{Name: "Done"}
if IsTerminalStep(step, nil) {
t.Fatalf("unchecked final step reported terminal")
}
})
t.Run("checked non-final step", func(t *testing.T) {
step := &WorkflowStep{Name: "Release", CompleteTaskOnEnter: true}
nextStep := &WorkflowStep{Name: "Archive"}
if IsTerminalStep(step, nextStep) {
t.Fatalf("non-final Done step reported terminal")
t.Fatalf("checked non-final step reported terminal")
}
})
t.Run("non-matching final step", func(t *testing.T) {
step := &WorkflowStep{Name: "Work"}
if IsTerminalStep(step, nil) {
t.Fatalf("final Work step reported terminal")
t.Run("checked arbitrary final step", func(t *testing.T) {
step := &WorkflowStep{Name: "Ready for launch", CompleteTaskOnEnter: true}
if !IsTerminalStep(step, nil) {
t.Fatalf("checked arbitrary final step reported non-terminal")
}
})
}
@@ -0,0 +1,203 @@
package repository
import (
"database/sql"
"encoding/json"
"fmt"
"github.com/jmoiron/sqlx"
"github.com/kandev/kandev/internal/workflow/models"
)
const completionPolicyBackfillMetaKey = "workflow_complete_task_on_enter_backfill_v1"
// backfillCompletionPolicy converts the legacy name-based terminal convention
// once, before system templates and default workflow steps are seeded. The
// marker and all data changes are committed together so a failed startup can
// safely retry the migration.
func (r *Repository) backfillCompletionPolicy() error {
if _, err := r.db.Exec(`
CREATE TABLE IF NOT EXISTS kandev_meta (
key TEXT PRIMARY KEY,
value TEXT NOT NULL DEFAULT ''
)`); err != nil {
return fmt.Errorf("create metadata table: %w", err)
}
tx, err := r.db.Beginx()
if err != nil {
return fmt.Errorf("begin completion policy backfill: %w", err)
}
defer func() { _ = tx.Rollback() }()
var marker string
err = tx.QueryRowx(tx.Rebind(`
SELECT value
FROM kandev_meta
WHERE key = ?
`), completionPolicyBackfillMetaKey).Scan(&marker)
if err == nil {
if err := tx.Commit(); err != nil {
return fmt.Errorf("commit completed completion policy backfill: %w", err)
}
return nil
}
if err != sql.ErrNoRows {
return fmt.Errorf("read completion policy backfill marker: %w", err)
}
if _, err := tx.Exec(tx.Rebind(`
UPDATE workflow_steps
SET complete_task_on_enter = 1
WHERE complete_task_on_enter = 0
AND LOWER(TRIM(workflow_steps.name)) IN ('done', 'complete', 'completed', 'approved')
AND NOT EXISTS (
SELECT 1
FROM workflow_steps AS successor
WHERE successor.workflow_id = workflow_steps.workflow_id
AND successor.position > workflow_steps.position
)
`)); err != nil {
return fmt.Errorf("backfill workflow step completion policy: %w", err)
}
if err := r.backfillTemplateCompletionPolicy(tx); err != nil {
return err
}
if _, err := tx.Exec(tx.Rebind(`
INSERT INTO kandev_meta (key, value)
VALUES (?, ?)
ON CONFLICT(key) DO UPDATE SET value = excluded.value
`), completionPolicyBackfillMetaKey, "completed"); err != nil {
return fmt.Errorf("write completion policy backfill marker: %w", err)
}
if err := tx.Commit(); err != nil {
return fmt.Errorf("commit completion policy backfill: %w", err)
}
return nil
}
func (r *Repository) backfillTemplateCompletionPolicy(tx *sqlx.Tx) error {
rows, err := tx.Queryx(tx.Rebind(`
SELECT id, steps
FROM workflow_templates
`))
if err != nil {
return fmt.Errorf("read workflow template steps: %w", err)
}
defer func() { _ = rows.Close() }()
type templateRow struct {
id string
stepsJSON string
}
templates := make([]templateRow, 0)
for rows.Next() {
var id, stepsJSON string
if err := rows.Scan(&id, &stepsJSON); err != nil {
return fmt.Errorf("scan workflow template %q: %w", id, err)
}
templates = append(templates, templateRow{id: id, stepsJSON: stepsJSON})
}
if err := rows.Err(); err != nil {
return fmt.Errorf("read workflow template steps: %w", err)
}
if err := rows.Close(); err != nil {
return fmt.Errorf("close workflow template steps: %w", err)
}
for _, template := range templates {
normalized, changed, err := normalizeTemplateCompletionPolicy([]byte(template.stepsJSON))
if err != nil {
return fmt.Errorf("normalize workflow template %q: %w", template.id, err)
}
if !changed {
continue
}
if _, err := tx.Exec(tx.Rebind(`
UPDATE workflow_templates
SET steps = ?
WHERE id = ?
`), string(normalized), template.id); err != nil {
return fmt.Errorf("update workflow template %q: %w", template.id, err)
}
}
return nil
}
func normalizeTemplateCompletionPolicy(data []byte) ([]byte, bool, error) {
var steps []map[string]json.RawMessage
if err := json.Unmarshal(data, &steps); err != nil {
return nil, false, err
}
if len(steps) == 0 {
return data, false, nil
}
positions := make([]int, len(steps))
maxPosition := 0
for i, step := range steps {
position, err := templateStepPosition(step)
if err != nil {
return nil, false, err
}
positions[i] = position
if i == 0 || position > maxPosition {
maxPosition = position
}
}
changed := false
for i, step := range steps {
if _, exists := step["complete_task_on_enter"]; exists {
continue
}
name, err := templateStepName(step)
if err != nil {
return nil, false, err
}
enabled := positions[i] == maxPosition && models.IsTerminalStepName(name)
encoded, err := json.Marshal(enabled)
if err != nil {
return nil, false, err
}
step["complete_task_on_enter"] = encoded
changed = true
}
if !changed {
return data, false, nil
}
normalized, err := json.Marshal(steps)
if err != nil {
return nil, false, err
}
return normalized, true, nil
}
func templateStepPosition(step map[string]json.RawMessage) (int, error) {
raw, ok := step["position"]
if !ok {
return 0, fmt.Errorf("step is missing position")
}
var position int
if err := json.Unmarshal(raw, &position); err != nil {
return 0, fmt.Errorf("decode step position: %w", err)
}
return position, nil
}
func templateStepName(step map[string]json.RawMessage) (string, error) {
raw, ok := step["name"]
if !ok {
return "", fmt.Errorf("step is missing name")
}
var name string
if err := json.Unmarshal(raw, &name); err != nil {
return "", fmt.Errorf("decode step name: %w", err)
}
return name, nil
}
@@ -0,0 +1,301 @@
package repository
import (
"context"
"database/sql"
"encoding/json"
"testing"
"time"
"github.com/jmoiron/sqlx"
_ "github.com/mattn/go-sqlite3"
"github.com/kandev/kandev/internal/workflow/models"
)
func TestCompletionPolicyMigration(t *testing.T) {
db, err := sql.Open("sqlite3", ":memory:?_foreign_keys=on")
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
db.SetMaxOpenConns(1)
database := sqlx.NewDb(db, "sqlite3")
t.Cleanup(func() { _ = database.Close() })
_, err = database.Exec(`
CREATE TABLE workflows (
id TEXT PRIMARY KEY,
workspace_id TEXT NOT NULL DEFAULT '',
workflow_template_id TEXT DEFAULT '',
name TEXT NOT NULL,
description TEXT DEFAULT '',
created_at TIMESTAMP NOT NULL,
updated_at TIMESTAMP NOT NULL
);
CREATE TABLE task_sessions (id TEXT PRIMARY KEY);
CREATE TABLE workflow_templates (
id TEXT PRIMARY KEY,
name TEXT NOT NULL,
description TEXT,
is_system INTEGER DEFAULT 0,
steps TEXT NOT NULL,
created_at TIMESTAMP NOT NULL,
updated_at TIMESTAMP NOT NULL
);
INSERT INTO workflows (id, name, created_at, updated_at)
VALUES ('workflow-completion-migration', 'Legacy', datetime('now'), datetime('now'));
INSERT INTO workflow_templates (id, name, description, steps, created_at, updated_at)
VALUES ('legacy-template', 'Legacy template', '', '[
{"id":"template-work","name":"Work","position":0,"custom":"preserve"},
{"id":"template-done","name":" dOnE ","position":1}
]', datetime('now'), datetime('now'));
CREATE TABLE workflow_steps (
id TEXT PRIMARY KEY,
workflow_id TEXT NOT NULL,
name TEXT NOT NULL,
position INTEGER NOT NULL,
color TEXT,
prompt TEXT,
events TEXT,
allow_manual_move INTEGER DEFAULT 1,
is_start_step INTEGER DEFAULT 0,
show_in_command_panel INTEGER DEFAULT 1,
auto_archive_after_hours INTEGER DEFAULT 0,
created_at TIMESTAMP NOT NULL,
updated_at TIMESTAMP NOT NULL,
FOREIGN KEY (workflow_id) REFERENCES workflows(id) ON DELETE CASCADE
);
INSERT INTO workflow_steps (id, workflow_id, name, position, created_at, updated_at)
VALUES
('legacy-work', 'workflow-completion-migration', 'Work', 0, datetime('now'), datetime('now')),
('legacy-done', 'workflow-completion-migration', ' dOnE ', 1, datetime('now'), datetime('now'));
`)
if err != nil {
t.Fatalf("seed legacy schema: %v", err)
}
repo, err := NewWithDB(database, database, nil)
if err != nil {
t.Fatalf("initialize workflow repository: %v", err)
}
ctx := context.Background()
work, err := repo.GetStep(ctx, "legacy-work")
if err != nil {
t.Fatalf("get legacy work step: %v", err)
}
done, err := repo.GetStep(ctx, "legacy-done")
if err != nil {
t.Fatalf("get legacy done step: %v", err)
}
if work.CompleteTaskOnEnter {
t.Fatal("non-terminal legacy step was enabled")
}
if !done.CompleteTaskOnEnter {
t.Fatal("legacy final Done step was not enabled")
}
template, err := repo.GetTemplate(ctx, "legacy-template")
if err != nil {
t.Fatalf("get migrated template: %v", err)
}
if template.Steps[0].CompleteTaskOnEnter {
t.Fatal("non-final template step was enabled")
}
if !template.Steps[1].CompleteTaskOnEnter {
t.Fatal("legacy final template step was not enabled")
}
var stepsJSON string
if err := database.Get(&stepsJSON, `SELECT steps FROM workflow_templates WHERE id = 'legacy-template'`); err != nil {
t.Fatalf("read migrated template JSON: %v", err)
}
var rawSteps []map[string]json.RawMessage
if err := json.Unmarshal([]byte(stepsJSON), &rawSteps); err != nil {
t.Fatalf("decode migrated template JSON: %v", err)
}
var custom string
if err := json.Unmarshal(rawSteps[0]["custom"], &custom); err != nil {
t.Fatalf("decode preserved template field: %v", err)
}
if custom != "preserve" {
t.Fatalf("preserved template field = %q, want preserve", custom)
}
for i, rawStep := range rawSteps {
if _, ok := rawStep["complete_task_on_enter"]; !ok {
t.Fatalf("template step %d is missing completion policy", i)
}
}
}
func TestCompletionPolicyReplayPreservesDisabled(t *testing.T) {
repo, database := setupTestRepoWithDB(t)
ctx := context.Background()
step := &models.WorkflowStep{
WorkflowID: "wf-test",
Name: "Done",
Position: 0,
CompleteTaskOnEnter: true,
}
if err := repo.CreateStep(ctx, step); err != nil {
t.Fatalf("create completion step: %v", err)
}
step.CompleteTaskOnEnter = false
if err := repo.UpdateStep(ctx, step); err != nil {
t.Fatalf("disable completion step: %v", err)
}
if err := repo.backfillCompletionPolicy(); err != nil {
t.Fatalf("replay completion policy backfill: %v", err)
}
got, err := repo.GetStep(ctx, step.ID)
if err != nil {
t.Fatalf("reload disabled completion step: %v", err)
}
if got.CompleteTaskOnEnter {
t.Fatal("replayed backfill overwrote an explicit false")
}
var marker string
if err := database.Get(&marker, database.Rebind(`SELECT value FROM kandev_meta WHERE key = ?`), completionPolicyBackfillMetaKey); err != nil {
t.Fatalf("read completion policy marker: %v", err)
}
if marker == "" {
t.Fatal("completion policy marker is empty")
}
}
func TestBackfillTemplateCompletionPolicyUpdatesMultipleTemplates(t *testing.T) {
repo, database := setupTestRepoWithDB(t)
ctx := context.Background()
now := time.Now().UTC()
for _, template := range []struct {
id string
steps string
}{
{id: "template-one", steps: `[{"name":"Work","position":0},{"name":"Done","position":1}]`},
{id: "template-two", steps: `[{"name":"Queue","position":0},{"name":"Approved","position":1}]`},
} {
_, err := database.ExecContext(ctx, `
INSERT INTO workflow_templates (id, name, description, is_system, steps, created_at, updated_at)
VALUES (?, ?, '', 0, ?, ?, ?)
`, template.id, template.id, template.steps, now, now)
if err != nil {
t.Fatalf("insert %s: %v", template.id, err)
}
}
tx, err := database.Beginx()
if err != nil {
t.Fatalf("begin template backfill: %v", err)
}
if err := repo.backfillTemplateCompletionPolicy(tx); err != nil {
_ = tx.Rollback()
t.Fatalf("backfill template completion policy: %v", err)
}
if err := tx.Commit(); err != nil {
t.Fatalf("commit template backfill: %v", err)
}
for _, templateID := range []string{"template-one", "template-two"} {
var stepsJSON string
if err := database.Get(&stepsJSON, database.Rebind(`SELECT steps FROM workflow_templates WHERE id = ?`), templateID); err != nil {
t.Fatalf("read %s: %v", templateID, err)
}
var steps []map[string]json.RawMessage
if err := json.Unmarshal([]byte(stepsJSON), &steps); err != nil {
t.Fatalf("decode %s: %v", templateID, err)
}
if len(steps) != 2 {
t.Fatalf("%s step count = %d, want 2", templateID, len(steps))
}
var enabled bool
if err := json.Unmarshal(steps[1]["complete_task_on_enter"], &enabled); err != nil {
t.Fatalf("decode %s completion policy: %v", templateID, err)
}
if !enabled {
t.Fatalf("%s final step completion policy = false, want true", templateID)
}
}
}
func TestCompletionPolicyMigrationRollsBackAndRetries(t *testing.T) {
db, err := sql.Open("sqlite3", ":memory:?_foreign_keys=on")
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
db.SetMaxOpenConns(1)
database := sqlx.NewDb(db, "sqlite3")
t.Cleanup(func() { _ = database.Close() })
_, err = database.Exec(`
CREATE TABLE workflows (
id TEXT PRIMARY KEY,
workspace_id TEXT NOT NULL DEFAULT '',
workflow_template_id TEXT DEFAULT '',
name TEXT NOT NULL,
description TEXT DEFAULT '',
created_at TIMESTAMP NOT NULL,
updated_at TIMESTAMP NOT NULL
);
CREATE TABLE task_sessions (id TEXT PRIMARY KEY);
CREATE TABLE kandev_meta (key TEXT PRIMARY KEY, value TEXT NOT NULL DEFAULT '');
INSERT INTO workflows (id, name, created_at, updated_at)
VALUES ('workflow-completion-retry', 'Legacy', datetime('now'), datetime('now'));
CREATE TABLE workflow_steps (
id TEXT PRIMARY KEY,
workflow_id TEXT NOT NULL,
name TEXT NOT NULL,
position INTEGER NOT NULL,
color TEXT,
prompt TEXT,
events TEXT,
allow_manual_move INTEGER DEFAULT 1,
is_start_step INTEGER DEFAULT 0,
show_in_command_panel INTEGER DEFAULT 1,
auto_archive_after_hours INTEGER DEFAULT 0,
created_at TIMESTAMP NOT NULL,
updated_at TIMESTAMP NOT NULL,
FOREIGN KEY (workflow_id) REFERENCES workflows(id) ON DELETE CASCADE
);
INSERT INTO workflow_steps (id, workflow_id, name, position, created_at, updated_at)
VALUES ('legacy-retry-done', 'workflow-completion-retry', 'Done', 0, datetime('now'), datetime('now'));
CREATE TRIGGER fail_completion_policy_marker
BEFORE INSERT ON kandev_meta
WHEN NEW.key = 'workflow_complete_task_on_enter_backfill_v1'
BEGIN
SELECT RAISE(ABORT, 'injected completion policy failure');
END;
`)
if err != nil {
t.Fatalf("seed retry schema: %v", err)
}
if _, err := NewWithDB(database, database, nil); err == nil {
t.Fatal("completion policy migration succeeded despite injected failure")
}
var enabled int
if err := database.Get(&enabled, `SELECT complete_task_on_enter FROM workflow_steps WHERE id = 'legacy-retry-done'`); err != nil {
t.Fatalf("read rolled-back completion policy: %v", err)
}
if enabled != 0 {
t.Fatalf("rolled-back completion policy = %d, want 0", enabled)
}
var marker string
if err := database.Get(&marker, database.Rebind(`SELECT value FROM kandev_meta WHERE key = ?`), completionPolicyBackfillMetaKey); err != sql.ErrNoRows {
t.Fatalf("rolled-back marker error = %v, want sql.ErrNoRows", err)
}
if _, err := database.Exec(`DROP TRIGGER fail_completion_policy_marker`); err != nil {
t.Fatalf("remove injected failure: %v", err)
}
repo, err := NewWithDB(database, database, nil)
if err != nil {
t.Fatalf("retry completion policy migration: %v", err)
}
step, err := repo.GetStep(context.Background(), "legacy-retry-done")
if err != nil {
t.Fatalf("read retried completion policy: %v", err)
}
if !step.CompleteTaskOnEnter {
t.Fatal("retried completion policy was not enabled")
}
if err := database.Get(&marker, database.Rebind(`SELECT value FROM kandev_meta WHERE key = ?`), completionPolicyBackfillMetaKey); err != nil {
t.Fatalf("read retried marker: %v", err)
}
}
@@ -78,6 +78,7 @@ func (r *Repository) initSchema() error {
pull_from_step_id TEXT NOT NULL DEFAULT '',
auto_advance_requires_signal INTEGER NOT NULL DEFAULT 0,
cancel_triggers_turn_complete INTEGER NOT NULL DEFAULT 0,
complete_task_on_enter INTEGER NOT NULL DEFAULT 0,
created_at TIMESTAMP NOT NULL,
updated_at TIMESTAMP NOT NULL,
FOREIGN KEY (workflow_id) REFERENCES workflows(id) ON DELETE CASCADE
@@ -129,6 +130,7 @@ func (r *Repository) initSchema() error {
// template supplies its own true values; this migration must never backfill
// existing workflows.
r.migrate.Apply("workflow_steps.cancel_triggers_turn_complete", `ALTER TABLE workflow_steps ADD COLUMN cancel_triggers_turn_complete INTEGER NOT NULL DEFAULT 0`)
_ = r.migrate.Apply("workflow_steps.complete_task_on_enter", `ALTER TABLE workflow_steps ADD COLUMN complete_task_on_enter INTEGER NOT NULL DEFAULT 0`)
r.migrate.Apply("workflow_steps.wip_limit", `ALTER TABLE workflow_steps ADD COLUMN wip_limit INTEGER NOT NULL DEFAULT 0`)
r.migrate.Apply("workflow_steps.pull_from_step_id", `ALTER TABLE workflow_steps ADD COLUMN pull_from_step_id TEXT NOT NULL DEFAULT ''`)
r.migrate.Apply("idx_workflow_steps_pull_from", `
@@ -142,6 +144,9 @@ func (r *Repository) initSchema() error {
if err := r.initPhase2Schema(); err != nil {
return err
}
if err := r.backfillCompletionPolicy(); err != nil {
return fmt.Errorf("backfill task completion policy: %w", err)
}
// Seed system templates
if err := r.seedSystemTemplates(); err != nil {
@@ -245,15 +250,18 @@ func (r *Repository) seedDefaultWorkflowSteps() error {
if _, err := r.db.Exec(r.db.Rebind(`
INSERT INTO workflow_steps (
id, workflow_id, name, position, color,
prompt, events, allow_manual_move, is_start_step, show_in_command_panel, agent_profile_id, profile_session_start_policy, profile_session_end_policy, wip_limit, pull_from_step_id, auto_advance_requires_signal, cancel_triggers_turn_complete, created_at, updated_at
prompt, events, allow_manual_move, is_start_step, show_in_command_panel,
agent_profile_id, profile_session_start_policy, profile_session_end_policy,
wip_limit, pull_from_step_id, auto_advance_requires_signal,
cancel_triggers_turn_complete, complete_task_on_enter, created_at, updated_at
) VALUES (
?, ?, ?, ?, ?, ?, ?, ?, ?,
?, ?, ?, ?, ?, ?, ?, ?, ?, ?,
?, ?, ?, ?, ?, ?, ?, ?, ?, ?
)
`),
idMap[stepDef.ID], workflowID, stepDef.Name, stepDef.Position, stepDef.Color,
stepDef.Prompt, string(eventsJSON), dialect.BoolToInt(stepDef.AllowManualMove),
dialect.BoolToInt(stepDef.IsStartStep), dialect.BoolToInt(stepDef.ShowInCommandPanel), stepDef.AgentProfileID, taskmodels.NormalizeWorkflowProfileSessionStartPolicy(string(stepDef.ProfileSessionStartPolicy)), taskmodels.NormalizeWorkflowProfileSessionEndPolicy(string(stepDef.ProfileSessionEndPolicy)), stepDef.WIPLimit, models.RemapStepID(stepDef.PullFromStepID, idMap), dialect.BoolToInt(stepDef.AutoAdvanceRequiresSignal), dialect.BoolToInt(stepDef.CancelTriggersTurnComplete), now, now,
dialect.BoolToInt(stepDef.IsStartStep), dialect.BoolToInt(stepDef.ShowInCommandPanel), stepDef.AgentProfileID, taskmodels.NormalizeWorkflowProfileSessionStartPolicy(string(stepDef.ProfileSessionStartPolicy)), taskmodels.NormalizeWorkflowProfileSessionEndPolicy(string(stepDef.ProfileSessionEndPolicy)), stepDef.WIPLimit, models.RemapStepID(stepDef.PullFromStepID, idMap), dialect.BoolToInt(stepDef.AutoAdvanceRequiresSignal), dialect.BoolToInt(stepDef.CancelTriggersTurnComplete), dialect.BoolToInt(stepDef.CompleteTaskOnEnter), now, now,
); err != nil {
return err
}
@@ -638,13 +646,13 @@ func (r *Repository) CreateStepWithDemotedStartSteps(ctx context.Context, step *
}
_, err = tx.ExecContext(ctx, tx.Rebind(`
INSERT INTO workflow_steps (
id, workflow_id, name, position, color,
prompt, events, allow_manual_move, is_start_step, show_in_command_panel, auto_archive_after_hours, agent_profile_id, profile_session_start_policy, profile_session_end_policy, stage_type, auto_advance_requires_signal, cancel_triggers_turn_complete, wip_limit, pull_from_step_id, created_at, updated_at
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
INSERT INTO workflow_steps (
id, workflow_id, name, position, color,
prompt, events, allow_manual_move, is_start_step, show_in_command_panel, auto_archive_after_hours, agent_profile_id, profile_session_start_policy, profile_session_end_policy, stage_type, auto_advance_requires_signal, cancel_triggers_turn_complete, complete_task_on_enter, wip_limit, pull_from_step_id, created_at, updated_at
) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
`), step.ID, step.WorkflowID, step.Name, step.Position, step.Color,
step.Prompt, string(eventsJSON), dialect.BoolToInt(step.AllowManualMove),
dialect.BoolToInt(step.IsStartStep), dialect.BoolToInt(step.ShowInCommandPanel), step.AutoArchiveAfterHours, step.AgentProfileID, taskmodels.NormalizeWorkflowProfileSessionStartPolicy(string(step.ProfileSessionStartPolicy)), taskmodels.NormalizeWorkflowProfileSessionEndPolicy(string(step.ProfileSessionEndPolicy)), normalizeStageType(step.StageType), dialect.BoolToInt(step.AutoAdvanceRequiresSignal), dialect.BoolToInt(step.CancelTriggersTurnComplete), step.WIPLimit, step.PullFromStepID, step.CreatedAt, step.UpdatedAt)
dialect.BoolToInt(step.IsStartStep), dialect.BoolToInt(step.ShowInCommandPanel), step.AutoArchiveAfterHours, step.AgentProfileID, taskmodels.NormalizeWorkflowProfileSessionStartPolicy(string(step.ProfileSessionStartPolicy)), taskmodels.NormalizeWorkflowProfileSessionEndPolicy(string(step.ProfileSessionEndPolicy)), normalizeStageType(step.StageType), dialect.BoolToInt(step.AutoAdvanceRequiresSignal), dialect.BoolToInt(step.CancelTriggersTurnComplete), dialect.BoolToInt(step.CompleteTaskOnEnter), step.WIPLimit, step.PullFromStepID, step.CreatedAt, step.UpdatedAt)
if err != nil {
return nil, err
}
@@ -671,12 +679,12 @@ func (r *Repository) scanStep(row interface {
Scan(dest ...interface{}) error
}) (*models.WorkflowStep, error) {
step := &models.WorkflowStep{}
var allowManualMove, isStartStep, showInCommandPanel, autoAdvanceRequiresSignal, cancelTriggersTurnComplete int
var allowManualMove, isStartStep, showInCommandPanel, autoAdvanceRequiresSignal, cancelTriggersTurnComplete, completeTaskOnEnter int
var autoArchiveAfterHours sql.NullInt64
var color, prompt, eventsJSON, agentProfileID, profileSessionStartPolicy, profileSessionEndPolicy, stageType, pullFromStepID sql.NullString
err := row.Scan(&step.ID, &step.WorkflowID, &step.Name, &step.Position, &color,
&prompt, &eventsJSON, &allowManualMove, &isStartStep, &showInCommandPanel, &autoArchiveAfterHours, &agentProfileID, &profileSessionStartPolicy, &profileSessionEndPolicy, &stageType, &autoAdvanceRequiresSignal, &cancelTriggersTurnComplete, &step.WIPLimit, &pullFromStepID, &step.CreatedAt, &step.UpdatedAt)
&prompt, &eventsJSON, &allowManualMove, &isStartStep, &showInCommandPanel, &autoArchiveAfterHours, &agentProfileID, &profileSessionStartPolicy, &profileSessionEndPolicy, &stageType, &autoAdvanceRequiresSignal, &cancelTriggersTurnComplete, &completeTaskOnEnter, &step.WIPLimit, &pullFromStepID, &step.CreatedAt, &step.UpdatedAt)
if err != nil {
return nil, err
@@ -687,6 +695,7 @@ func (r *Repository) scanStep(row interface {
step.ShowInCommandPanel = showInCommandPanel == 1
step.AutoAdvanceRequiresSignal = autoAdvanceRequiresSignal == 1
step.CancelTriggersTurnComplete = cancelTriggersTurnComplete == 1
step.CompleteTaskOnEnter = completeTaskOnEnter == 1
if autoArchiveAfterHours.Valid {
step.AutoArchiveAfterHours = int(autoArchiveAfterHours.Int64)
}
@@ -718,7 +727,7 @@ func (r *Repository) scanStep(row interface {
return step, nil
}
const stepSelectColumns = `id, workflow_id, name, position, color, prompt, events, allow_manual_move, is_start_step, show_in_command_panel, auto_archive_after_hours, agent_profile_id, profile_session_start_policy, profile_session_end_policy, stage_type, auto_advance_requires_signal, cancel_triggers_turn_complete, wip_limit, pull_from_step_id, created_at, updated_at`
const stepSelectColumns = `id, workflow_id, name, position, color, prompt, events, allow_manual_move, is_start_step, show_in_command_panel, auto_archive_after_hours, agent_profile_id, profile_session_start_policy, profile_session_end_policy, stage_type, auto_advance_requires_signal, cancel_triggers_turn_complete, complete_task_on_enter, wip_limit, pull_from_step_id, created_at, updated_at`
// GetStep retrieves a workflow step by ID.
func (r *Repository) GetStep(ctx context.Context, id string) (*models.WorkflowStep, error) {
@@ -773,11 +782,11 @@ func (r *Repository) UpdateStepWithDemotedStartSteps(ctx context.Context, step *
UPDATE workflow_steps SET
name = ?, position = ?, color = ?,
prompt = ?, events = ?,
allow_manual_move = ?, is_start_step = ?, show_in_command_panel = ?, auto_archive_after_hours = ?, agent_profile_id = ?, profile_session_start_policy = ?, profile_session_end_policy = ?, stage_type = ?, auto_advance_requires_signal = ?, cancel_triggers_turn_complete = ?, wip_limit = ?, pull_from_step_id = ?, updated_at = ?
allow_manual_move = ?, is_start_step = ?, show_in_command_panel = ?, auto_archive_after_hours = ?, agent_profile_id = ?, profile_session_start_policy = ?, profile_session_end_policy = ?, stage_type = ?, auto_advance_requires_signal = ?, cancel_triggers_turn_complete = ?, complete_task_on_enter = ?, wip_limit = ?, pull_from_step_id = ?, updated_at = ?
WHERE id = ?
`), step.Name, step.Position, step.Color,
step.Prompt, string(eventsJSON),
dialect.BoolToInt(step.AllowManualMove), dialect.BoolToInt(step.IsStartStep), dialect.BoolToInt(step.ShowInCommandPanel), step.AutoArchiveAfterHours, step.AgentProfileID, taskmodels.NormalizeWorkflowProfileSessionStartPolicy(string(step.ProfileSessionStartPolicy)), taskmodels.NormalizeWorkflowProfileSessionEndPolicy(string(step.ProfileSessionEndPolicy)), normalizeStageType(step.StageType), dialect.BoolToInt(step.AutoAdvanceRequiresSignal), dialect.BoolToInt(step.CancelTriggersTurnComplete), step.WIPLimit, step.PullFromStepID, step.UpdatedAt, step.ID)
dialect.BoolToInt(step.AllowManualMove), dialect.BoolToInt(step.IsStartStep), dialect.BoolToInt(step.ShowInCommandPanel), step.AutoArchiveAfterHours, step.AgentProfileID, taskmodels.NormalizeWorkflowProfileSessionStartPolicy(string(step.ProfileSessionStartPolicy)), taskmodels.NormalizeWorkflowProfileSessionEndPolicy(string(step.ProfileSessionEndPolicy)), normalizeStageType(step.StageType), dialect.BoolToInt(step.AutoAdvanceRequiresSignal), dialect.BoolToInt(step.CancelTriggersTurnComplete), dialect.BoolToInt(step.CompleteTaskOnEnter), step.WIPLimit, step.PullFromStepID, step.UpdatedAt, step.ID)
if err != nil {
return nil, err
}
@@ -987,7 +996,7 @@ func (r *Repository) ListStepsByWorkflow(ctx context.Context, workflowID string)
func (r *Repository) ListStepsByWorkspaceID(ctx context.Context, workspaceID string) ([]*models.WorkflowStep, error) {
rows, err := r.ro.QueryContext(ctx, r.ro.Rebind(`
SELECT ws.id, ws.workflow_id, ws.name, ws.position, ws.color, ws.prompt, ws.events,
ws.allow_manual_move, ws.is_start_step, ws.show_in_command_panel, ws.auto_archive_after_hours, ws.agent_profile_id, ws.profile_session_start_policy, ws.profile_session_end_policy, ws.stage_type, ws.auto_advance_requires_signal, ws.cancel_triggers_turn_complete, ws.wip_limit, ws.pull_from_step_id, ws.created_at, ws.updated_at
ws.allow_manual_move, ws.is_start_step, ws.show_in_command_panel, ws.auto_archive_after_hours, ws.agent_profile_id, ws.profile_session_start_policy, ws.profile_session_end_policy, ws.stage_type, ws.auto_advance_requires_signal, ws.cancel_triggers_turn_complete, ws.complete_task_on_enter, ws.wip_limit, ws.pull_from_step_id, ws.created_at, ws.updated_at
FROM workflow_steps ws
JOIN workflows w ON ws.workflow_id = w.id
WHERE w.workspace_id = ?
@@ -392,6 +392,7 @@ func (s *Service) CreateStepsFromTemplate(ctx context.Context, workflowID, templ
ProfileSessionEndPolicy: taskmodels.NormalizeWorkflowProfileSessionEndPolicy(string(stepDef.ProfileSessionEndPolicy)),
AutoAdvanceRequiresSignal: stepDef.AutoAdvanceRequiresSignal,
CancelTriggersTurnComplete: stepDef.CancelTriggersTurnComplete,
CompleteTaskOnEnter: stepDef.CompleteTaskOnEnter,
WIPLimit: stepDef.WIPLimit,
PullFromStepID: models.RemapStepID(stepDef.PullFromStepID, idMap),
StageType: stepDef.StageType,
@@ -734,6 +735,9 @@ func (s *Service) ImportWorkflows(ctx context.Context, workspaceID string, expor
if err := s.AuthorizeWorkspace(ctx, workspaceID); err != nil {
return nil, err
}
if err := export.NormalizeCompletionPolicy(); err != nil {
return nil, fmt.Errorf("invalid export data: %w", err)
}
if err := export.Validate(); err != nil {
return nil, fmt.Errorf("invalid export data: %w", err)
}
@@ -879,6 +883,7 @@ func (s *Service) stepFromPortableWithMatcher(workflowID string, sp models.StepP
ProfileSessionEndPolicy: taskmodels.NormalizeWorkflowProfileSessionEndPolicy(string(sp.ProfileSessionEndPolicy)),
AutoAdvanceRequiresSignal: sp.AutoAdvanceRequiresSignal,
CancelTriggersTurnComplete: sp.CancelTriggersTurnComplete,
CompleteTaskOnEnter: sp.CompleteTaskOnEnter,
WIPLimit: sp.WIPLimit,
PullFromStepID: sp.PullFromStepID(posToID),
}
@@ -367,6 +367,7 @@ func stepMatchesDefinition(existing, desired *models.WorkflowStep) bool {
existing.PullFromStepID == desired.PullFromStepID &&
existing.AutoAdvanceRequiresSignal == desired.AutoAdvanceRequiresSignal &&
existing.CancelTriggersTurnComplete == desired.CancelTriggersTurnComplete &&
existing.CompleteTaskOnEnter == desired.CompleteTaskOnEnter &&
eventsEqual(existing.Events, desired.Events)
}
@@ -382,6 +382,37 @@ func TestApplySyncedWorkflows_PreservesCancelTriggersTurnComplete(t *testing.T)
assert.False(t, steps[0].CancelTriggersTurnComplete)
}
func TestApplySyncedWorkflows_PreservesCompleteTaskOnEnterFalse(t *testing.T) {
svc, _, _ := setupSyncService(t)
ctx := context.Background()
first := models.WorkflowPortable{
Name: "Completion Flow",
Steps: []models.StepPortable{
{Name: "Work", Position: 0, IsStartStep: true, CompleteTaskOnEnter: false},
{Name: "Done", Position: 1, CompleteTaskOnEnter: true},
},
}
result, err := svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{{Path: "flows/completion.yml", Export: exportOf(first)}})
require.NoError(t, err)
require.Equal(t, []string{"Completion Flow"}, result.Created)
steps, err := svc.ListStepsByWorkflow(ctx, "imported-Completion Flow")
require.NoError(t, err)
require.Len(t, steps, 2)
assert.False(t, steps[0].CompleteTaskOnEnter)
assert.True(t, steps[1].CompleteTaskOnEnter)
updated := first
updated.Steps = append([]models.StepPortable(nil), first.Steps...)
updated.Steps[1].CompleteTaskOnEnter = false
result, err = svc.ApplySyncedWorkflows(ctx, "ws-1", []SyncFileExport{{Path: "flows/completion.yml", Export: exportOf(updated)}})
require.NoError(t, err)
require.Equal(t, []string{"Completion Flow"}, result.Updated)
steps, err = svc.ListStepsByWorkflow(ctx, "imported-Completion Flow")
require.NoError(t, err)
assert.False(t, steps[1].CompleteTaskOnEnter)
}
func TestApplySyncedWorkflows_SecondApplyIsNoOp(t *testing.T) {
svc, _, _ := setupSyncService(t)
ctx := context.Background()
@@ -84,6 +84,7 @@ func payload(step *models.WorkflowStep) map[string]interface{} {
"stage_type": string(step.StageType),
"auto_advance_requires_signal": step.AutoAdvanceRequiresSignal,
"cancel_triggers_turn_complete": step.CancelTriggersTurnComplete,
"complete_task_on_enter": step.CompleteTaskOnEnter,
"created_at": step.CreatedAt,
"updated_at": step.UpdatedAt,
}
@@ -57,6 +57,7 @@ func fullStep() *models.WorkflowStep {
AutoAdvanceRequiresSignal: true,
CancelTriggersTurnComplete: true,
CompleteTaskOnEnter: true,
CreatedAt: created,
UpdatedAt: updated,
}
@@ -104,6 +105,7 @@ func TestPublishCarriesEveryStepField(t *testing.T) {
"stage_type": string(step.StageType),
"auto_advance_requires_signal": step.AutoAdvanceRequiresSignal,
"cancel_triggers_turn_complete": step.CancelTriggersTurnComplete,
"complete_task_on_enter": step.CompleteTaskOnEnter,
"created_at": step.CreatedAt,
"updated_at": step.UpdatedAt,
}
+70
View File
@@ -266,6 +266,76 @@ describe("workflow step cancellation fields", () => {
});
});
describe("workflow step completion fields", () => {
beforeEach(() => {
vi.stubGlobal(
"fetch",
vi.fn(async () =>
Response.json({
id: "step-1",
workflow_id: "wf-1",
name: REVIEW_STEP_NAME,
position: 1,
color: STEP_COLOR,
complete_task_on_enter: true,
created_at: "",
updated_at: "",
}),
),
);
});
afterEach(() => {
vi.unstubAllGlobals();
vi.restoreAllMocks();
});
it("preserves the completion policy returned from the step API", async () => {
vi.stubGlobal(
"fetch",
vi.fn(async () =>
Response.json({
steps: [
{
id: "step-1",
workflow_id: "wf-1",
name: REVIEW_STEP_NAME,
position: 1,
color: STEP_COLOR,
complete_task_on_enter: true,
created_at: "",
updated_at: "",
},
],
}),
),
);
const result = await listWorkflowStepsAction("wf-1");
expect(result.steps[0].complete_task_on_enter).toBe(true);
});
it("sends explicit false when creating and updating completion policy", async () => {
await createWorkflowStepAction({
workflow_id: "wf-1",
name: REVIEW_STEP_NAME,
position: 1,
color: STEP_COLOR,
complete_task_on_enter: false,
});
await updateWorkflowStepAction("step-1", { complete_task_on_enter: false });
const fetchMock = fetch as unknown as ReturnType<typeof vi.fn>;
expect(JSON.parse(fetchMock.mock.calls[0][1].body as string)).toMatchObject({
complete_task_on_enter: false,
});
expect(JSON.parse(fetchMock.mock.calls[1][1].body as string)).toMatchObject({
complete_task_on_enter: false,
});
});
});
describe("workflow step stage types", () => {
beforeEach(() => {
vi.stubGlobal(
+6
View File
@@ -361,6 +361,7 @@ type BackendTemplateStep = {
agent_profile_id?: StepDefinition["agent_profile_id"];
profile_session_start_policy?: WorkflowStep["profile_session_start_policy"];
profile_session_end_policy?: WorkflowStep["profile_session_end_policy"];
complete_task_on_enter?: boolean;
auto_advance_requires_signal?: boolean;
cancel_triggers_turn_complete?: boolean;
wip_limit?: number;
@@ -418,6 +419,7 @@ type BackendWorkflowStep = {
agent_profile_id?: string;
profile_session_start_policy?: WorkflowStep["profile_session_start_policy"];
profile_session_end_policy?: WorkflowStep["profile_session_end_policy"];
complete_task_on_enter?: boolean;
auto_advance_requires_signal?: boolean;
cancel_triggers_turn_complete?: boolean;
wip_limit?: number;
@@ -446,6 +448,7 @@ const transformWorkflowStep = (step: BackendWorkflowStep): WorkflowStep => ({
profile_session_end_policy: normalizeWorkflowProfileSessionEndPolicy(
step.profile_session_end_policy,
),
complete_task_on_enter: step.complete_task_on_enter,
auto_advance_requires_signal: step.auto_advance_requires_signal,
cancel_triggers_turn_complete: step.cancel_triggers_turn_complete,
wip_limit: step.wip_limit ?? 0,
@@ -488,6 +491,7 @@ export async function createWorkflowStepAction(payload: {
color: string;
prompt?: string;
events?: StepEvents;
complete_task_on_enter?: boolean;
is_start_step?: boolean;
show_in_command_panel?: boolean;
agent_profile_id?: string;
@@ -514,6 +518,7 @@ export async function createWorkflowStepAction(payload: {
wip_limit: payload.wip_limit ?? 0,
pull_from_step_id: payload.pull_from_step_id ?? "",
stage_type: payload.stage_type,
complete_task_on_enter: payload.complete_task_on_enter ?? false,
cancel_triggers_turn_complete: payload.cancel_triggers_turn_complete ?? false,
profile_session_start_policy: payload.profile_session_start_policy,
profile_session_end_policy: payload.profile_session_end_policy,
@@ -548,6 +553,7 @@ export async function updateWorkflowStepAction(
| "stage_type"
| "profile_session_start_policy"
| "profile_session_end_policy"
| "complete_task_on_enter"
>
>,
): Promise<WorkflowStep> {
@@ -13,6 +13,7 @@ const template = {
name: "Template Step",
position: 0,
color: "bg-blue-500",
complete_task_on_enter: true,
cancel_triggers_turn_complete: true,
},
],
@@ -117,6 +118,7 @@ describe("useWorkflowCreation", () => {
expect(result.current.initialStepsByWorkflowId.get(workflow.id)?.[0]).toMatchObject({
name: "Template Step",
color: "bg-blue-500",
complete_task_on_enter: true,
cancel_triggers_turn_complete: true,
});
});
@@ -76,6 +76,7 @@ function toDraftStep(
profile_session_end_policy: normalizeWorkflowProfileSessionEndPolicy(
definition.profile_session_end_policy,
),
complete_task_on_enter: definition.complete_task_on_enter ?? false,
auto_advance_requires_signal: definition.auto_advance_requires_signal,
cancel_triggers_turn_complete: definition.cancel_triggers_turn_complete,
wip_limit: definition.wip_limit,
@@ -49,6 +49,7 @@ function sourceSteps(): WorkflowStep[] {
agent_profile_id: "step-agent",
profile_session_start_policy: "reuse",
profile_session_end_policy: "complete",
complete_task_on_enter: false,
auto_advance_requires_signal: true,
cancel_triggers_turn_complete: true,
wip_limit: 2,
@@ -70,6 +71,7 @@ function sourceSteps(): WorkflowStep[] {
show_in_command_panel: false,
auto_archive_after_hours: 0,
agent_profile_id: "",
complete_task_on_enter: true,
auto_advance_requires_signal: false,
cancel_triggers_turn_complete: false,
wip_limit: 0,
@@ -157,6 +159,7 @@ describe("createWorkflowDuplication", () => {
agent_profile_id: "step-agent",
profile_session_start_policy: "reuse",
profile_session_end_policy: "complete",
complete_task_on_enter: false,
auto_advance_requires_signal: true,
cancel_triggers_turn_complete: true,
wip_limit: 2,
@@ -165,6 +168,7 @@ describe("createWorkflowDuplication", () => {
updated_at: "",
workflow_id: result.workflow.id,
});
expect(copiedDone.complete_task_on_enter).toBe(true);
expect(copiedDone.pull_from_step_id).toBe(copiedReview.id);
expect(copiedReview.events?.on_turn_complete).toEqual([
{
@@ -85,6 +85,7 @@ function cloneWorkflowStep(
profile_session_end_policy: normalizeWorkflowProfileSessionEndPolicy(
source.profile_session_end_policy,
),
complete_task_on_enter: source.complete_task_on_enter,
auto_advance_requires_signal: source.auto_advance_requires_signal,
cancel_triggers_turn_complete: source.cancel_triggers_turn_complete,
wip_limit: source.wip_limit,
@@ -12,7 +12,7 @@ import { WorkflowExportDialog } from "@/components/settings/workflow-export-dial
import type { WorkflowTemplate } from "@/lib/types/http";
const YAML_PLACEHOLDER =
"version: 1\ntype: kandev_workflow\nworkflows:\n - name: My Workflow\n steps: [...]";
"version: 2\ntype: kandev_workflow\nworkflows:\n - name: My Workflow\n steps: [...]";
type ImportWorkflowsDialogProps = {
open: boolean;
@@ -13,6 +13,8 @@ import { useToast } from "@/components/toast-provider";
import { useSerializedMutationQueue } from "./use-serialized-mutation-queue";
import type { WorkflowMutationGuardController } from "./workflow-mutation-guard";
import { applyWorkflowStepUpdates } from "./workflow-step-mutations";
import { areStepDraftsEqual, stepUpdatePayload } from "./workflow-step-equality";
export { areStepDraftsEqual } from "./workflow-step-equality";
import {
createWorkflowAction,
createWorkflowStepAction,
@@ -94,6 +96,7 @@ function createDraftStep(workflow: Workflow, position: number): WorkflowStep {
...NEW_STEP_DEFAULTS,
position,
allow_manual_move: true,
complete_task_on_enter: false,
created_at: "",
updated_at: "",
};
@@ -323,6 +326,7 @@ async function createMissingSteps(
position: step.position,
color: step.color,
stage_type: step.stage_type ?? "custom",
complete_task_on_enter: step.complete_task_on_enter ?? false,
cancel_triggers_turn_complete: step.cancel_triggers_turn_complete ?? false,
agent_profile_id: step.agent_profile_id,
profile_session_start_policy: normalizeWorkflowProfileSessionStartPolicy(
@@ -402,46 +406,6 @@ function remapStepReferences<T>(value: T, mappings: Map<string, string>): T {
return Object.fromEntries(mapped) as T;
}
function stepUpdatePayload(step: WorkflowStep): Partial<WorkflowStep> {
return {
name: step.name,
position: step.position,
color: step.color,
stage_type: step.stage_type ?? "custom",
prompt: step.prompt ?? "",
events: step.events ?? {},
allow_manual_move: step.allow_manual_move ?? true,
is_start_step: step.is_start_step ?? false,
show_in_command_panel: step.show_in_command_panel ?? false,
auto_archive_after_hours: step.auto_archive_after_hours ?? 0,
agent_profile_id: step.agent_profile_id ?? "",
profile_session_start_policy: normalizeWorkflowProfileSessionStartPolicy(
step.profile_session_start_policy,
),
profile_session_end_policy: normalizeWorkflowProfileSessionEndPolicy(
step.profile_session_end_policy,
),
auto_advance_requires_signal: step.auto_advance_requires_signal ?? false,
cancel_triggers_turn_complete: step.cancel_triggers_turn_complete ?? false,
wip_limit: step.wip_limit ?? 0,
pull_from_step_id: step.pull_from_step_id ?? "",
};
}
export function areStepDraftsEqual(left: WorkflowStep[], right: WorkflowStep[]): boolean;
export function areStepDraftsEqual(left: WorkflowStep, right: WorkflowStep): boolean;
export function areStepDraftsEqual(
left: WorkflowStep[] | WorkflowStep,
right: WorkflowStep[] | WorkflowStep,
): boolean {
if (Array.isArray(left) && Array.isArray(right)) {
if (left.length !== right.length) return false;
return left.every((step, index) => areStepDraftsEqual(step, right[index]));
}
if (Array.isArray(left) || Array.isArray(right)) return false;
return JSON.stringify(stepUpdatePayload(left)) === JSON.stringify(stepUpdatePayload(right));
}
type WorkflowDeleteHandlersParams = {
workflow: Workflow;
/** See `WorkflowStepActionsParams.readOnly` — defense-in-depth for synced workflows. */
@@ -1,5 +1,5 @@
import type { Workflow, WorkflowStep } from "@/lib/types/http";
import { areStepDraftsEqual } from "./workflow-card-actions";
import { areStepDraftsEqual } from "./workflow-step-equality";
function valuesEqual(left: unknown, right: unknown): boolean {
if (Object.is(left, right)) return true;
@@ -1,11 +1,21 @@
"use client";
import { IconInfoCircle } from "@tabler/icons-react";
import type { ReactNode } from "react";
import { useState, type ReactNode } from "react";
import { useTranslation } from "react-i18next";
import {
Drawer,
DrawerContent,
DrawerDescription,
DrawerHeader,
DrawerTitle,
DrawerTrigger,
} from "@kandev/ui/drawer";
import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@kandev/ui/tooltip";
import type { WorkflowStep } from "@/lib/types/http";
import type { ScriptPlaceholder } from "@/components/settings/profile-edit/script-editor-completions";
import { useTouchDrawer } from "@/hooks/use-compact-task-chrome";
import { cn } from "@/lib/utils";
/**
* Monaco completion entries for the step-prompt editor. `key` is the
@@ -28,26 +38,52 @@ export function HelpTip({
text,
testId,
ariaLabel,
mobileTouchTarget = false,
}: {
text: ReactNode;
testId?: string;
ariaLabel?: string;
mobileTouchTarget?: boolean;
}) {
const { t } = useTranslation();
const label = ariaLabel ?? t("workflows:moreInformation");
const touchDrawer = useTouchDrawer();
const usesTouchDrawer = mobileTouchTarget && touchDrawer;
const [drawerOpen, setDrawerOpen] = useState(false);
const button = (
<button
type="button"
className={cn(
"inline-flex h-5 w-5 shrink-0 cursor-pointer items-center justify-center rounded-sm text-muted-foreground/50 hover:text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring",
mobileTouchTarget && "h-11 w-11 md:h-5 md:w-5",
)}
aria-label={label}
aria-haspopup={usesTouchDrawer ? "dialog" : undefined}
aria-expanded={usesTouchDrawer ? drawerOpen : undefined}
data-testid={testId}
>
<IconInfoCircle className="h-3.5 w-3.5" aria-hidden="true" />
</button>
);
if (usesTouchDrawer) {
return (
<Drawer open={drawerOpen} onOpenChange={setDrawerOpen}>
<DrawerTrigger asChild>{button}</DrawerTrigger>
<DrawerContent>
<DrawerHeader>
<DrawerTitle>{label}</DrawerTitle>
<DrawerDescription>{text}</DrawerDescription>
</DrawerHeader>
</DrawerContent>
</Drawer>
);
}
return (
<TooltipProvider>
<Tooltip>
<TooltipTrigger asChild>
<button
type="button"
className="inline-flex h-5 w-5 shrink-0 cursor-help items-center justify-center rounded-sm text-muted-foreground/50 hover:text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring"
aria-label={label}
data-testid={testId}
>
<IconInfoCircle className="h-3.5 w-3.5" />
</button>
</TooltipTrigger>
<TooltipTrigger asChild>{button}</TooltipTrigger>
<TooltipContent className="max-w-xs">{text}</TooltipContent>
</Tooltip>
</TooltipProvider>
@@ -19,6 +19,7 @@ import {
} from "./workflow-pipeline-editor-helpers";
import {
useStepActions,
CompleteTaskOnEnterToggle,
TurnStartSelect,
TurnCompleteSelect,
ChildrenCompletedSelect,
@@ -220,10 +221,11 @@ type StepBehaviorSectionProps = {
steps: WorkflowStep[];
onUpdate: (updates: Partial<WorkflowStep>) => void;
toggleOnEnterAction: (type: string) => void;
isFinalStep: boolean;
readOnly: boolean;
};
type StepToggleRowsProps = Omit<StepBehaviorSectionProps, "steps">;
type StepToggleRowsProps = Omit<StepBehaviorSectionProps, "steps" | "isFinalStep">;
// The six on-enter / board-behavior toggles. Split out of StepBehaviorSection
// so that component stays under the 100-line function cap.
@@ -325,6 +327,7 @@ function StepBehaviorSection({
steps,
onUpdate,
toggleOnEnterAction,
isFinalStep,
readOnly,
}: StepBehaviorSectionProps) {
return (
@@ -338,6 +341,13 @@ function StepBehaviorSection({
readOnly={readOnly}
/>
</div>
<CompleteTaskOnEnterToggle
step={step}
savedStep={savedStep}
onUpdate={onUpdate}
readOnly={readOnly}
isFinalStep={isFinalStep}
/>
<StepWipControls
step={step}
savedStep={savedStep}
@@ -475,6 +485,7 @@ export function StepConfigPanel({
}, 500);
const actions = useStepActions({ step, onUpdate });
const isFinalStep = steps[steps.length - 1]?.id === step.id;
return (
<div
@@ -501,6 +512,7 @@ export function StepConfigPanel({
steps={steps}
onUpdate={onUpdate}
toggleOnEnterAction={actions.toggleOnEnterAction}
isFinalStep={isFinalStep}
readOnly={readOnly}
/>
<SessionConfigEditor
@@ -2,13 +2,25 @@ import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest";
import type { WorkflowStep } from "@/lib/types/http";
import { workflowId as toWorkflowId } from "@/lib/types/ids";
import { TurnCompleteSelect } from "./workflow-pipeline-editor-step-actions";
import {
CompleteTaskOnEnterToggle,
TurnCompleteSelect,
} from "./workflow-pipeline-editor-step-actions";
afterEach(cleanup);
const STEP_ID = "step-1";
const CANCEL_COMPLETION_CHECKBOX_TEST_ID = `${STEP_ID}-cancel-completion-checkbox`;
const CANCEL_COMPLETION_HELP_TEST_ID = `${STEP_ID}-cancel-completion-help`;
const COMPLETE_TASK_CHECKBOX_TEST_ID = `${STEP_ID}-complete-task-on-enter-checkbox`;
const COMPLETE_TASK_HELP_TEST_ID = `${STEP_ID}-complete-task-on-enter-help`;
const CHECKED_ARIA_VALUE = "true";
const UNCHECKED_ARIA_VALUE = "false";
const ARIA_CHECKED_ATTRIBUTE = "aria-checked";
function step(overrides: Partial<WorkflowStep> = {}): WorkflowStep {
return {
id: "step-1",
id: STEP_ID,
workflow_id: toWorkflowId("workflow-1"),
name: "In Progress",
position: 0,
@@ -40,10 +52,10 @@ function renderTurnComplete(current: WorkflowStep, readOnly = false) {
describe("TurnCompleteSelect cancel completion policy", () => {
it("renders the policy disabled by default and updates it when checked", () => {
const { onUpdate } = renderTurnComplete(step());
const checkbox = screen.getByTestId("step-1-cancel-completion-checkbox");
const checkbox = screen.getByTestId(CANCEL_COMPLETION_CHECKBOX_TEST_ID);
expect(checkbox.getAttribute("aria-checked")).toBe("false");
expect(screen.getByTestId("step-1-cancel-completion-help").getAttribute("aria-label")).toBe(
expect(checkbox.getAttribute(ARIA_CHECKED_ATTRIBUTE)).toBe(UNCHECKED_ARIA_VALUE);
expect(screen.getByTestId(CANCEL_COMPLETION_HELP_TEST_ID).getAttribute("aria-label")).toBe(
"More information",
);
expect(screen.queryByText(/Applies only when a user explicitly cancels a turn\./)).toBeNull();
@@ -53,9 +65,9 @@ describe("TurnCompleteSelect cancel completion policy", () => {
it("preserves the persisted value and disables it in read-only mode", () => {
renderTurnComplete(step({ cancel_triggers_turn_complete: true }), true);
const checkbox = screen.getByTestId("step-1-cancel-completion-checkbox");
const checkbox = screen.getByTestId(CANCEL_COMPLETION_CHECKBOX_TEST_ID);
expect(checkbox.getAttribute("aria-checked")).toBe("true");
expect(checkbox.getAttribute(ARIA_CHECKED_ATTRIBUTE)).toBe(CHECKED_ARIA_VALUE);
expect((checkbox as HTMLButtonElement).disabled).toBe(true);
});
@@ -64,6 +76,49 @@ describe("TurnCompleteSelect cancel completion policy", () => {
step({ events: { on_turn_complete: [] }, cancel_triggers_turn_complete: true }),
);
expect(screen.queryByTestId("step-1-cancel-completion-checkbox")).toBeNull();
expect(screen.queryByTestId(CANCEL_COMPLETION_CHECKBOX_TEST_ID)).toBeNull();
});
});
describe("CompleteTaskOnEnterToggle", () => {
it("renders only for the final step and keeps help separate from the checkbox", () => {
const onUpdate = vi.fn();
render(
<CompleteTaskOnEnterToggle
step={step({ complete_task_on_enter: true })}
savedStep={step({ complete_task_on_enter: true })}
onUpdate={onUpdate}
readOnly={false}
isFinalStep
/>,
);
const checkbox = screen.getByTestId(COMPLETE_TASK_CHECKBOX_TEST_ID);
const help = screen.getByTestId(COMPLETE_TASK_HELP_TEST_ID);
expect(checkbox.getAttribute(ARIA_CHECKED_ATTRIBUTE)).toBe(CHECKED_ARIA_VALUE);
expect(help).not.toBe(checkbox);
expect(help.className).toContain("h-11");
fireEvent.click(help);
expect(checkbox.getAttribute(ARIA_CHECKED_ATTRIBUTE)).toBe(CHECKED_ARIA_VALUE);
expect(onUpdate).not.toHaveBeenCalled();
fireEvent.click(checkbox);
expect(onUpdate).toHaveBeenCalledWith({ complete_task_on_enter: false });
});
it("does not expose the control for a non-final step", () => {
render(
<CompleteTaskOnEnterToggle
step={step({ complete_task_on_enter: true })}
savedStep={step({ complete_task_on_enter: true })}
onUpdate={vi.fn()}
readOnly={false}
isFinalStep={false}
/>,
);
expect(screen.queryByTestId(COMPLETE_TASK_CHECKBOX_TEST_ID)).toBeNull();
expect(screen.queryByTestId(COMPLETE_TASK_HELP_TEST_ID)).toBeNull();
});
});
@@ -112,6 +112,62 @@ type StepSelectProps = {
readOnly: boolean;
};
type CompleteTaskOnEnterToggleProps = {
step: WorkflowStep;
savedStep?: WorkflowStep;
onUpdate: (updates: Partial<WorkflowStep>) => void;
readOnly: boolean;
isFinalStep: boolean;
};
export function CompleteTaskOnEnterToggle({
step,
savedStep,
onUpdate,
readOnly,
isFinalStep,
}: CompleteTaskOnEnterToggleProps) {
const { t } = useTranslation();
if (!isFinalStep) return null;
const checkboxId = `${step.id}-complete-task-on-enter`;
return (
<div
className="flex flex-wrap items-center gap-2 pt-1"
data-testid={`${step.id}-complete-task-on-enter-row`}
>
<Checkbox
id={checkboxId}
data-testid={`${step.id}-complete-task-on-enter-checkbox`}
checked={step.complete_task_on_enter === true}
onCheckedChange={(checked) => {
if (readOnly) return;
onUpdate({ complete_task_on_enter: checked === true });
}}
disabled={readOnly}
data-settings-dirty={isWorkflowStepValueDirty(
step,
savedStep,
(item) => item.complete_task_on_enter ?? false,
)}
/>
<Label
htmlFor={checkboxId}
data-testid={`${step.id}-complete-task-on-enter-label`}
className="flex min-h-11 min-w-0 cursor-pointer items-center text-sm md:min-h-0"
>
{t("workflows:completeTaskOnEnter")}
</Label>
<HelpTip
testId={`${step.id}-complete-task-on-enter-help`}
ariaLabel={t("workflows:completeTaskOnEnterHelpAria")}
text={t("workflows:completeTaskOnEnterHelp")}
mobileTouchTarget
/>
</div>
);
}
export function TurnStartSelect({
step,
savedStep,
@@ -0,0 +1,48 @@
import {
normalizeWorkflowProfileSessionEndPolicy,
normalizeWorkflowProfileSessionStartPolicy,
type WorkflowStep,
} from "@/lib/types/http";
function stepUpdatePayload(step: WorkflowStep): Partial<WorkflowStep> {
return {
name: step.name,
position: step.position,
color: step.color,
stage_type: step.stage_type ?? "custom",
prompt: step.prompt ?? "",
events: step.events ?? {},
allow_manual_move: step.allow_manual_move ?? true,
is_start_step: step.is_start_step ?? false,
show_in_command_panel: step.show_in_command_panel ?? false,
auto_archive_after_hours: step.auto_archive_after_hours ?? 0,
agent_profile_id: step.agent_profile_id ?? "",
profile_session_start_policy: normalizeWorkflowProfileSessionStartPolicy(
step.profile_session_start_policy,
),
profile_session_end_policy: normalizeWorkflowProfileSessionEndPolicy(
step.profile_session_end_policy,
),
complete_task_on_enter: step.complete_task_on_enter ?? false,
auto_advance_requires_signal: step.auto_advance_requires_signal ?? false,
cancel_triggers_turn_complete: step.cancel_triggers_turn_complete ?? false,
wip_limit: step.wip_limit ?? 0,
pull_from_step_id: step.pull_from_step_id ?? "",
};
}
export function areStepDraftsEqual(left: WorkflowStep[], right: WorkflowStep[]): boolean;
export function areStepDraftsEqual(left: WorkflowStep, right: WorkflowStep): boolean;
export function areStepDraftsEqual(
left: WorkflowStep[] | WorkflowStep,
right: WorkflowStep[] | WorkflowStep,
): boolean {
if (Array.isArray(left) && Array.isArray(right)) {
if (left.length !== right.length) return false;
return left.every((step, index) => areStepDraftsEqual(step, right[index]));
}
if (Array.isArray(left) || Array.isArray(right)) return false;
return JSON.stringify(stepUpdatePayload(left)) === JSON.stringify(stepUpdatePayload(right));
}
export { stepUpdatePayload };
@@ -25,6 +25,7 @@ export function newWorkflowStep(workflow: Workflow, position: number, id: string
...NEW_STEP_DEFAULTS,
position,
allow_manual_move: true,
complete_task_on_enter: false,
created_at: "",
updated_at: "",
};
@@ -59,9 +59,12 @@ vi.mock("react-i18next", () => ({
import { SessionStoppedBanner } from "./session-stopped-banner";
const SESSION_RECOVER_ACTION = "session.recover";
const TASK_ID = "task-1";
const SESSION_ID = "session-1";
const baseProps: Omit<SessionStoppedBannerProps, "mode" | "onShowDialog" | "showDialog"> = {
taskId: "task-1",
sessionId: "session-1",
taskId: TASK_ID,
sessionId: SESSION_ID,
workspaceId: "workspace-1",
};
const RESUME_BUTTON_TEST_ID = "recovery-resume-button";
@@ -93,19 +96,27 @@ beforeEach(() => {
afterEach(cleanup);
describe("SessionStoppedBanner basics", () => {
it("offers only New Agent for a completed session and opens the new-session dialog", async () => {
it("offers Resume and New Agent for a completed session", async () => {
render(<BannerHarness mode="completed" />);
expect(screen.getByTestId("completed-session-banner")).toBeTruthy();
expect(screen.getByText("This session is complete.")).toBeTruthy();
expect(screen.getByRole("button", { name: "New Agent" })).toBeTruthy();
expect(screen.queryByTestId(RESUME_BUTTON_TEST_ID)).toBeNull();
expect(screen.getByTestId(RESUME_BUTTON_TEST_ID)).toBeTruthy();
expect(screen.queryByTestId(FRESH_BUTTON_TEST_ID)).toBeNull();
fireEvent.click(screen.getByTestId(RESUME_BUTTON_TEST_ID));
await waitFor(() =>
expect(mocks.request).toHaveBeenCalledWith(
SESSION_RECOVER_ACTION,
{ task_id: TASK_ID, session_id: SESSION_ID, action: "resume" },
30000,
),
);
fireEvent.click(screen.getByRole("button", { name: "New Agent" }));
expect(await screen.findByTestId("new-session-dialog")).toBeTruthy();
expect(mocks.request).not.toHaveBeenCalled();
});
it("disables New Agent when no task is available", () => {
@@ -136,8 +147,8 @@ describe("SessionStoppedBanner basics", () => {
fireEvent.click(screen.getByTestId(RESUME_BUTTON_TEST_ID));
await waitFor(() =>
expect(mocks.request).toHaveBeenCalledWith(
"session.recover",
{ task_id: "task-1", session_id: "session-1", action: "resume" },
SESSION_RECOVER_ACTION,
{ task_id: TASK_ID, session_id: SESSION_ID, action: "resume" },
30000,
),
);
@@ -145,8 +156,8 @@ describe("SessionStoppedBanner basics", () => {
fireEvent.click(screen.getByTestId(FRESH_BUTTON_TEST_ID));
await waitFor(() =>
expect(mocks.request).toHaveBeenCalledWith(
"session.recover",
{ task_id: "task-1", session_id: "session-1", action: "fresh_start" },
SESSION_RECOVER_ACTION,
{ task_id: TASK_ID, session_id: SESSION_ID, action: "fresh_start" },
30000,
),
);
@@ -206,8 +217,8 @@ describe("SessionStoppedBanner recovery failures", () => {
await waitFor(() =>
expect(mocks.request).toHaveBeenLastCalledWith(
"session.recover",
{ task_id: "task-1", session_id: "session-1", action: "resume_new_branch" },
SESSION_RECOVER_ACTION,
{ task_id: TASK_ID, session_id: SESSION_ID, action: "resume_new_branch" },
30000,
),
);
@@ -95,6 +95,16 @@ function StoppedRecoveryFeedback({
);
}
function useSessionProfileExists(sessionId: string | null): boolean {
return useAppStore((s) => {
if (!sessionId) return false;
const agentProfileId = s.taskSessions.items[sessionId]?.agent_profile_id;
return (
!!agentProfileId && s.agentProfiles.items.some((p: { id: string }) => p.id === agentProfileId)
);
});
}
function RecoverableSessionActions({
onShowDialog,
taskId,
@@ -131,13 +141,7 @@ function RecoverableSessionActions({
handleNewBranch,
} = recoveryActions ?? localRecoveryActions;
const profileExists = useAppStore((s) => {
if (!sessionId) return false;
const agentProfileId = s.taskSessions.items[sessionId]?.agent_profile_id;
return (
!!agentProfileId && s.agentProfiles.items.some((p: { id: string }) => p.id === agentProfileId)
);
});
const profileExists = useSessionProfileExists(sessionId);
const handleResume = useCallback(() => {
if (taskId && sessionId) void handleRecover("resume");
@@ -176,6 +180,94 @@ function RecoverableSessionActions({
);
}
function CompletedSessionActions({
onShowDialog,
taskId,
sessionId,
workspaceId,
recoveryActions,
}: Pick<
SessionStoppedBannerProps,
"onShowDialog" | "taskId" | "sessionId" | "workspaceId" | "recoveryActions"
>) {
const { t } = useTranslation();
const localRecoveryActions = useSessionRecoveryActions({
taskId: taskId ?? "",
sessionId: sessionId ?? "",
});
const {
busyAction,
recoveryError,
branchDetails,
recoveryNotice,
handleRecover,
handleRestore,
handleRetry,
handleNewBranch,
} = recoveryActions ?? localRecoveryActions;
const profileExists = useSessionProfileExists(sessionId);
const handleResume = useCallback(() => {
if (taskId && sessionId) void handleRecover("resume");
}, [handleRecover, sessionId, taskId]);
return (
<div className="flex w-full flex-col gap-2 sm:w-auto">
<StoppedRecoveryFeedback
workspaceId={workspaceId}
recoveryError={recoveryError}
recoveryNotice={recoveryNotice}
branchDetails={branchDetails}
busyAction={busyAction}
onRetry={handleRetry}
onRestore={() => void handleRestore()}
onNewBranch={handleNewBranch}
/>
<div className="flex w-full flex-col gap-2 sm:w-auto sm:flex-row">
{sessionId && taskId && (
<Tooltip>
<TooltipTrigger asChild>
<span
className="inline-flex w-full sm:w-auto"
data-testid="completed-session-resume-wrapper"
tabIndex={busyAction !== null || !profileExists ? 0 : -1}
>
<Button
variant="default"
size="sm"
data-testid="recovery-resume-button"
className="min-h-11 w-full shrink-0 gap-1.5 cursor-pointer sm:w-auto"
onClick={handleResume}
disabled={busyAction !== null || !profileExists}
>
<IconPlayerPlay className="h-3.5 w-3.5" />
{busyAction === "resume" ? t("task:resuming") : t("task:resume")}
</Button>
</span>
</TooltipTrigger>
{!profileExists && (
<TooltipContent>{t("task:agentProfileNoLongerExists")}</TooltipContent>
)}
</Tooltip>
)}
<Button
variant="outline"
size="sm"
className="min-h-11 w-full shrink-0 gap-1.5 cursor-pointer sm:w-auto"
onClick={() => {
if (taskId) onShowDialog(true);
}}
disabled={busyAction !== null || !taskId}
data-testid="completed-session-new-agent-button"
>
<IconPlus className="h-3.5 w-3.5" />
{t("task:newAgent")}
</Button>
</div>
</div>
);
}
function RecoverableSessionButtons({
taskId,
sessionId,
@@ -204,6 +296,7 @@ function RecoverableSessionButtons({
<span
className="inline-flex w-full sm:w-auto"
data-testid="failed-session-resume-wrapper"
tabIndex={busyAction !== null || !profileExists ? 0 : -1}
>
<Button
variant="default"
@@ -278,19 +371,13 @@ export function SessionStoppedBanner({
</div>
{isCompleted ? (
<Button
variant="default"
size="sm"
data-testid="completed-session-new-agent-button"
className="min-h-11 w-full shrink-0 gap-1.5 cursor-pointer sm:w-auto"
onClick={() => {
if (taskId) onShowDialog(true);
}}
disabled={!taskId}
>
<IconPlus className="h-3.5 w-3.5" />
{t("task:newAgent")}
</Button>
<CompletedSessionActions
onShowDialog={onShowDialog}
taskId={taskId}
sessionId={sessionId}
workspaceId={workspaceId}
recoveryActions={recoveryActions}
/>
) : (
<RecoverableSessionActions
onShowDialog={onShowDialog}
+5
View File
@@ -821,6 +821,7 @@ export class ApiClient {
agent_profile_id?: string;
profile_session_start_policy?: WorkflowProfileSessionStartPolicy;
profile_session_end_policy?: WorkflowProfileSessionEndPolicy;
complete_task_on_enter?: boolean;
events?: {
on_enter?: Array<{ type: string; config?: Record<string, unknown> }>;
on_turn_start?: Array<{ type: string; config?: Record<string, unknown> }>;
@@ -840,6 +841,9 @@ export class ApiClient {
...(opts?.profile_session_end_policy
? { profile_session_end_policy: opts.profile_session_end_policy }
: {}),
...(opts?.complete_task_on_enter != null
? { complete_task_on_enter: opts.complete_task_on_enter }
: {}),
...(opts?.events != null ? { events: opts.events } : {}),
});
}
@@ -1271,6 +1275,7 @@ export class ApiClient {
wip_limit?: number;
pull_from_step_id?: string | null;
cancel_triggers_turn_complete?: boolean;
complete_task_on_enter?: boolean;
stage_type?: "work" | "review" | "approval" | "custom";
profile_session_start_policy?: WorkflowProfileSessionStartPolicy;
profile_session_end_policy?: WorkflowProfileSessionEndPolicy;
+5
View File
@@ -699,6 +699,11 @@ export class SessionPage {
return this.completedSessionBanner().getByTestId("completed-session-new-agent-button");
}
/** "Resume" action shown for an explicitly completed conversation. */
completedSessionResumeButton(): Locator {
return this.completedSessionBanner().getByTestId("recovery-resume-button");
}
/** "Cancel" button shown on the yellow transient-retry (529 Overloaded) card. */
recoveryCancelRetryButton(): Locator {
return this.page.getByTestId("recovery-cancel-retry-button");
@@ -95,6 +95,35 @@ export class WorkflowSettingsPage {
return card.locator(".group.relative").filter({ hasText: stepName });
}
completeTaskOnEnterCheckbox(card: Locator, stepId: string): Locator {
return card.getByTestId(`${stepId}-complete-task-on-enter-checkbox`);
}
completeTaskOnEnterHelp(card: Locator, stepId: string): Locator {
return card.getByTestId(`${stepId}-complete-task-on-enter-help`);
}
async reorderStep(card: Locator, fromName: string, toName: string): Promise<void> {
const source = this.stepNodeByName(card, fromName).locator("button").first();
const target = this.stepNodeByName(card, toName);
const sourceBox = await source.boundingBox();
const targetBox = await target.boundingBox();
if (!sourceBox || !targetBox) throw new Error(`Cannot drag ${fromName} to ${toName}`);
const sourcePoint = {
x: sourceBox.x + sourceBox.width / 2,
y: sourceBox.y + sourceBox.height / 2,
};
const targetPoint = {
x: targetBox.x + targetBox.width / 2,
y: targetBox.y + targetBox.height / 2,
};
await this.page.mouse.move(sourcePoint.x, sourcePoint.y);
await this.page.mouse.down();
await this.page.mouse.move(sourcePoint.x + 12, sourcePoint.y, { steps: 2 });
await this.page.mouse.move(targetPoint.x, targetPoint.y, { steps: 8 });
await this.page.mouse.up();
}
/** A replay-cycle diagnostic rendered inside a workflow card or guard dialog. */
cycleDiagnostic(container: Locator, autoStartStepId: string): Locator {
return container.getByTestId(`workflow-cycle-diagnostic-${autoStartStepId}`);
@@ -0,0 +1,116 @@
import { test, expect } from "../../fixtures/test-base";
import type { SeedData } from "../../fixtures/test-base";
import type { ApiClient } from "../../helpers/api-client";
import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions";
import { waitForSessionDone } from "../../helpers/session";
import { SessionPage } from "../../pages/session-page";
async function seedCompletedConversation(apiClient: ApiClient, seedData: SeedData) {
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
`Completed conversation ${Date.now()}`,
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [seedData.repositoryId],
},
);
if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id");
await waitForSessionDone(apiClient, task.id, task.session_id, "Waiting for initial conversation");
await apiClient.seedTaskSession(task.id, {
state: "COMPLETED",
sessionId: task.session_id,
agentProfileId: seedData.agentProfileId,
repositoryId: seedData.repositoryId,
completedAt: new Date().toISOString(),
});
await apiClient.updateTaskState(task.id, "COMPLETED");
return task;
}
test.describe("Completed conversation resume", () => {
test("resumes the selected completed conversation and sends a follow-up in place", async ({
testPage,
apiClient,
seedData,
}) => {
test.setTimeout(180_000);
const task = await seedCompletedConversation(apiClient, seedData);
const before = await apiClient.listTaskSessions(task.id);
const primary = before.sessions.find((session) => session.is_primary);
expect(primary?.id).toBe(task.session_id);
await testPage.goto(`/t/${task.id}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
const banner = session.completedSessionBanner();
await expect(banner).toBeVisible({ timeout: 30_000 });
await expect(session.completedSessionResumeButton()).toBeVisible();
await expect(session.completedSessionNewAgentButton()).toBeVisible();
// Opening historical work is passive. Reloading must not launch a new
// execution or replace the selected session.
await testPage.reload();
await session.waitForLoad();
await expect(session.completedSessionBanner()).toBeVisible({ timeout: 30_000 });
const afterReload = await apiClient.listTaskSessions(task.id);
expect(afterReload.sessions).toHaveLength(before.sessions.length);
expect(afterReload.sessions.find((item) => item.is_primary)?.id).toBe(task.session_id);
await session.completedSessionResumeButton().click();
await expect(session.completedSessionBanner()).toHaveCount(0, { timeout: 60_000 });
await expect
.poll(
async () => {
const current = await apiClient.listTaskSessions(task.id);
return current.sessions.find((item) => item.id === task.session_id)?.state ?? "MISSING";
},
{ timeout: 60_000, message: "Waiting for the resumed conversation to become idle" },
)
.toBe("WAITING_FOR_INPUT");
await expect(session.activeChat().locator(".tiptap.ProseMirror:visible").first()).toBeEditable({
timeout: 60_000,
});
const afterResume = await apiClient.listTaskSessions(task.id);
expect(afterResume.sessions).toHaveLength(before.sessions.length);
expect(afterResume.sessions.find((item) => item.id === task.session_id)?.state).toBe(
"WAITING_FOR_INPUT",
);
await session.sendMessage("/e2e:simple-message");
await session.expectChatResponseVisible("simple mock response", 1, { timeout: 60_000 });
await expect
.poll(
async () => {
const current = await apiClient.listTaskSessions(task.id);
return current.sessions.find((item) => item.id === task.session_id)?.state ?? "MISSING";
},
{ timeout: 60_000, message: "Waiting for the follow-up turn to become idle" },
)
.toBe("WAITING_FOR_INPUT");
await expect(session.activeChat().locator(".tiptap.ProseMirror:visible").first()).toBeEditable({
timeout: 60_000,
});
const afterFollowUp = await apiClient.listTaskSessions(task.id);
const taskAfterFollowUp = await apiClient.getTask(task.id);
expect(afterFollowUp.sessions).toHaveLength(before.sessions.length);
expect(afterFollowUp.sessions.find((item) => item.id === task.session_id)?.is_primary).toBe(
true,
);
expect(taskAfterFollowUp.primary_session_id).toBe(task.session_id);
expect(taskAfterFollowUp.state).toBe("COMPLETED");
const messages = await apiClient.listSessionMessages(task.session_id);
expect(messages.messages.filter((message) => message.author_type === "user")).toHaveLength(2);
expect(
messages.messages.filter(
(message) =>
message.author_type === "agent" && message.content.includes("simple mock response"),
),
).toHaveLength(2);
await assertNoDocumentHorizontalOverflow(testPage, "completed conversation resume");
});
});
@@ -0,0 +1,85 @@
import { test, expect } from "../../fixtures/test-base";
import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions";
import { waitForSessionDone } from "../../helpers/session";
import { SessionPage } from "../../pages/session-page";
test.describe("Completed conversation resume on mobile", () => {
test("resumes in place with touch-sized actions and no overflow", async ({
testPage,
apiClient,
seedData,
}) => {
test.setTimeout(180_000);
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
`Mobile completed conversation ${Date.now()}`,
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: seedData.workflowId,
workflow_step_id: seedData.startStepId,
repository_ids: [seedData.repositoryId],
},
);
if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id");
await waitForSessionDone(
apiClient,
task.id,
task.session_id,
"Waiting for initial conversation",
);
await apiClient.seedTaskSession(task.id, {
state: "COMPLETED",
sessionId: task.session_id,
agentProfileId: seedData.agentProfileId,
repositoryId: seedData.repositoryId,
completedAt: new Date().toISOString(),
});
await apiClient.updateTaskState(task.id, "COMPLETED");
const before = await apiClient.listTaskSessions(task.id);
await testPage.goto(`/t/${task.id}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
await expect(session.completedSessionBanner()).toBeVisible({ timeout: 30_000 });
const resume = session.completedSessionResumeButton();
const newAgent = session.completedSessionNewAgentButton();
await expect(resume).toBeVisible();
await expect(newAgent).toBeVisible();
const viewportWidth = await testPage.evaluate(() => window.innerWidth);
for (const control of [resume, newAgent]) {
const box = await control.boundingBox();
expect(box).not.toBeNull();
expect(box!.height).toBeGreaterThanOrEqual(44);
expect(box!.x).toBeGreaterThanOrEqual(0);
expect(box!.x + box!.width).toBeLessThanOrEqual(viewportWidth);
}
await resume.tap();
await expect(session.completedSessionBanner()).toHaveCount(0, { timeout: 60_000 });
await expect(session.activeChat().locator(".tiptap.ProseMirror:visible").first()).toBeEditable({
timeout: 60_000,
});
await session.sendMessageViaButton("/e2e:simple-message");
await session.expectChatResponseVisible("simple mock response", 1, { timeout: 60_000 });
await expect
.poll(
async () => {
const current = await apiClient.listTaskSessions(task.id);
return current.sessions.find((item) => item.id === task.session_id)?.state ?? "MISSING";
},
{ timeout: 60_000, message: "Waiting for the follow-up turn to become idle" },
)
.toBe("WAITING_FOR_INPUT");
await expect(session.activeChat().locator(".tiptap.ProseMirror:visible").first()).toBeEditable({
timeout: 60_000,
});
const after = await apiClient.listTaskSessions(task.id);
expect(after.sessions).toHaveLength(before.sessions.length);
expect(after.sessions.find((item) => item.id === task.session_id)?.is_primary).toBe(true);
expect((await apiClient.getTask(task.id)).state).toBe("COMPLETED");
await assertNoDocumentHorizontalOverflow(testPage, "mobile completed conversation resume");
});
});
@@ -75,7 +75,7 @@ test.describe("New session dialog", () => {
}
});
test("completed sessions replace the composer with a New Agent action", async ({
test("completed sessions show Resume and New Agent actions", async ({
testPage,
apiClient,
seedData,
@@ -117,7 +117,8 @@ test.describe("New session dialog", () => {
await expect(session.completedSessionNewAgentButton()).toHaveText("New Agent");
await expect(session.activeChat().locator(".tiptap.ProseMirror")).not.toBeVisible();
await expect(session.submitButton()).not.toBeVisible();
await expect(session.recoveryResumeButton()).not.toBeVisible();
await expect(session.recoveryResumeButton()).toBeVisible();
await expect(session.recoveryResumeButton()).toHaveText("Resume");
await expect(session.recoveryFreshButton()).not.toBeVisible();
await session.completedSessionNewAgentButton().click();
@@ -65,7 +65,9 @@ async function recoveryWorkflow(apiClient: ApiClient, workspaceId: string, name:
const workflow = await apiClient.createWorkflow(workspaceId, name);
const waiting = await apiClient.createWorkflowStep(workflow.id, "Waiting", 0);
const review = await apiClient.createWorkflowStep(workflow.id, "Review", 1);
const done = await apiClient.createWorkflowStep(workflow.id, "Done", 2);
const done = await apiClient.createWorkflowStep(workflow.id, "Done", 2, {
complete_task_on_enter: true,
});
await apiClient.updateWorkflowStep(review.id, {
events: { on_enter: [{ type: "auto_start_agent" }] },
});
@@ -0,0 +1,61 @@
import { test, expect } from "../../fixtures/test-base";
import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions";
import { WorkflowSettingsPage } from "../../pages/workflow-settings-page";
test.describe("Workflow task completion on mobile", () => {
test("keeps the final-step control and help disclosure touch accessible", async ({
testPage,
apiClient,
seedData,
}) => {
const workflow = await apiClient.createWorkflow(
seedData.workspaceId,
`Mobile completion editor ${Date.now()}`,
);
const first = await apiClient.createWorkflowStep(workflow.id, "Draft", 0, {
is_start_step: true,
});
const final = await apiClient.createWorkflowStep(workflow.id, "Done", 1, {
complete_task_on_enter: true,
});
const settings = new WorkflowSettingsPage(testPage);
await settings.goto(seedData.workspaceId);
const card = await settings.findWorkflowCard(workflow.name, { waitForName: true });
await settings.selectStep(card, "Draft", true);
await expect(settings.completeTaskOnEnterCheckbox(card, first.id)).toHaveCount(0);
await expect(settings.completeTaskOnEnterHelp(card, first.id)).toHaveCount(0);
await settings.selectStep(card, "Done", true);
const checkbox = settings.completeTaskOnEnterCheckbox(card, final.id);
const help = settings.completeTaskOnEnterHelp(card, final.id);
await expect(checkbox).toBeChecked();
const helpBox = await help.boundingBox();
expect(helpBox).not.toBeNull();
expect(helpBox!.height).toBeGreaterThanOrEqual(44);
await help.evaluate((element) => (element as HTMLElement).blur());
await expect(testPage.getByRole("tooltip")).toHaveCount(0);
await help.tap();
await expect(testPage.getByRole("dialog")).toContainText(
"Marks the task complete when it enters this final step",
);
await expect(checkbox).toBeChecked();
await testPage.keyboard.press("Escape");
await expect(testPage.getByRole("dialog")).toHaveCount(0);
await checkbox.tap();
await expect(checkbox).not.toBeChecked();
const viewportWidth = await testPage.evaluate(() => window.innerWidth);
const saveButton = settings.floatingSave.getByRole("button", { name: "Save changes" });
for (const control of [checkbox, help, saveButton]) {
const box = await control.boundingBox();
expect(box).not.toBeNull();
expect(box!.x).toBeGreaterThanOrEqual(0);
expect(box!.x + box!.width).toBeLessThanOrEqual(viewportWidth);
}
await settings.saveChanges(true);
const { steps } = await apiClient.listWorkflowSteps(workflow.id);
expect(steps.find((step) => step.id === final.id)?.complete_task_on_enter).toBe(false);
await assertNoDocumentHorizontalOverflow(testPage, "mobile workflow task completion");
});
});
@@ -17,9 +17,10 @@ test.describe("Workflow import/export", () => {
// Textarea should contain valid YAML with the workflow
const textarea = dialog.locator("textarea");
const yamlContent = await textarea.inputValue();
expect(yamlContent).toContain("version: 1");
expect(yamlContent).toContain("version: 2");
expect(yamlContent).toContain("type: kandev_workflow");
expect(yamlContent).toContain("E2E Workflow");
expect(yamlContent).toContain("complete_task_on_enter: false");
// Copy button should work
await dialog.getByRole("button", { name: "Copy" }).click();
@@ -0,0 +1,176 @@
import { test, expect } from "../../fixtures/test-base";
import { assertNoDocumentHorizontalOverflow } from "../../helpers/layout-assertions";
import { waitForSessionDone } from "../../helpers/session";
import { SessionPage } from "../../pages/session-page";
import { WorkflowSettingsPage } from "../../pages/workflow-settings-page";
test.describe("Workflow task completion", () => {
test("shows completion only on the final step and preserves it through edit flows", async ({
testPage,
apiClient,
seedData,
}) => {
const workflow = await apiClient.createWorkflow(
seedData.workspaceId,
`Completion editor ${Date.now()}`,
);
const start = await apiClient.createWorkflowStep(workflow.id, "Draft", 0, {
is_start_step: true,
complete_task_on_enter: false,
});
const middle = await apiClient.createWorkflowStep(workflow.id, "Review", 1, {
complete_task_on_enter: false,
});
const done = await apiClient.createWorkflowStep(workflow.id, "Done", 2, {
complete_task_on_enter: true,
});
const settings = new WorkflowSettingsPage(testPage);
await settings.goto(seedData.workspaceId);
const card = await settings.findWorkflowCard(workflow.name, { waitForName: true });
await expect(card).toBeVisible();
await settings.selectStep(card, "Review");
await expect(settings.completeTaskOnEnterCheckbox(card, middle.id)).toHaveCount(0);
await expect(settings.completeTaskOnEnterHelp(card, middle.id)).toHaveCount(0);
await settings.selectStep(card, "Done");
const checkbox = settings.completeTaskOnEnterCheckbox(card, done.id);
const help = settings.completeTaskOnEnterHelp(card, done.id);
await expect(checkbox).toBeChecked();
await expect(help).toBeVisible();
await help.hover();
await expect(testPage.getByRole("tooltip")).toContainText(
"Marks the task complete when it enters this final step",
);
await help.focus();
await expect(testPage.getByRole("tooltip")).toContainText(
"Marks the task complete when it enters this final step",
);
await help.click();
await expect(checkbox).toBeChecked();
await checkbox.click();
await expect(checkbox).not.toBeChecked();
await settings.floatingSave.getByRole("button", { name: "Reset", exact: true }).click();
await expect(checkbox).toBeChecked();
await checkbox.click();
await settings.saveChanges();
await testPage.reload();
await settings.goto(seedData.workspaceId);
const reloadedCard = await settings.findWorkflowCard(workflow.name, { waitForName: true });
await settings.selectStep(reloadedCard, "Done");
await expect(settings.completeTaskOnEnterCheckbox(reloadedCard, done.id)).not.toBeChecked();
// Move the saved step away from the end and back. Its persisted value stays
// attached to the step, while the control follows final position.
await settings.reorderStep(reloadedCard, "Done", "Review");
await settings.selectStep(reloadedCard, "Done");
await expect(settings.completeTaskOnEnterCheckbox(reloadedCard, done.id)).toHaveCount(0);
await settings.reorderStep(reloadedCard, "Done", "Review");
await settings.selectStep(reloadedCard, "Done");
await expect(settings.completeTaskOnEnterCheckbox(reloadedCard, done.id)).toBeVisible();
await expect(settings.completeTaskOnEnterCheckbox(reloadedCard, done.id)).not.toBeChecked();
if (await settings.floatingSave.isVisible().catch(() => false)) {
await settings.saveChanges();
}
const { steps } = await apiClient.listWorkflowSteps(workflow.id);
expect(steps.find((step) => step.id === start.id)?.position).toBe(0);
expect(steps.find((step) => step.id === done.id)?.position).toBe(2);
expect(steps.find((step) => step.id === done.id)?.complete_task_on_enter).toBe(false);
await assertNoDocumentHorizontalOverflow(testPage, "workflow task completion editor");
});
test("does not complete an unchecked final step and accepts a follow-up turn", async ({
testPage,
apiClient,
seedData,
}) => {
test.setTimeout(150_000);
const workflow = await apiClient.createWorkflow(
seedData.workspaceId,
`Unchecked final ${Date.now()}`,
);
const start = await apiClient.createWorkflowStep(workflow.id, "Start", 0, {
is_start_step: true,
events: { on_turn_complete: [{ type: "move_to_next" }] },
});
const final = await apiClient.createWorkflowStep(workflow.id, "Final", 1, {
complete_task_on_enter: false,
});
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
`Unchecked final task ${Date.now()}`,
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: workflow.id,
workflow_step_id: start.id,
repository_ids: [seedData.repositoryId],
},
);
if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id");
await waitForSessionDone(apiClient, task.id, task.session_id, "Waiting for start step turn");
await expect
.poll(async () => (await apiClient.getTask(task.id)).workflow_step_id, {
timeout: 60_000,
message: "Workflow did not enter the unchecked final step",
})
.toBe(final.id);
expect((await apiClient.getTask(task.id)).state).not.toBe("COMPLETED");
await testPage.goto(`/t/${task.id}`);
const session = new SessionPage(testPage);
await session.waitForLoad();
await expect(session.completedSessionBanner()).toHaveCount(0);
await session.waitForChatIdle({ timeout: 30_000, requireEditable: true });
await session.sendMessage("/e2e:simple-message");
await session.expectChatResponseVisible("simple mock response", 1, { timeout: 60_000 });
await session.waitForChatIdle({ timeout: 30_000 });
const after = await apiClient.getTask(task.id);
expect(after.state).not.toBe("COMPLETED");
expect(after.workflow_step_id).toBe(final.id);
await assertNoDocumentHorizontalOverflow(testPage, "unchecked final follow-up");
});
test("completes a task when it enters an enabled final step", async ({ apiClient, seedData }) => {
test.setTimeout(150_000);
const workflow = await apiClient.createWorkflow(
seedData.workspaceId,
`Checked final ${Date.now()}`,
);
const start = await apiClient.createWorkflowStep(workflow.id, "Start", 0, {
is_start_step: true,
events: { on_turn_complete: [{ type: "move_to_next" }] },
});
const final = await apiClient.createWorkflowStep(workflow.id, "Final", 1, {
complete_task_on_enter: true,
});
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
`Checked final task ${Date.now()}`,
seedData.agentProfileId,
{
description: "/e2e:simple-message",
workflow_id: workflow.id,
workflow_step_id: start.id,
repository_ids: [seedData.repositoryId],
},
);
if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id");
await waitForSessionDone(apiClient, task.id, task.session_id, "Waiting for checked final step");
await expect
.poll(async () => (await apiClient.getTask(task.id)).state, {
timeout: 60_000,
message: "Task did not complete on the enabled final step",
})
.toBe("COMPLETED");
expect((await apiClient.getTask(task.id)).workflow_step_id).toBe(final.id);
});
});
@@ -0,0 +1,110 @@
import { act, renderHook, waitFor } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import { useSessionRecoveryActions } from "./use-session-recovery-actions";
const mocks = vi.hoisted(() => ({
requestSessionRecover: vi.fn(),
restoreSessionWorkspace: vi.fn(),
}));
vi.mock("@/lib/services/session-recovery-service", () => ({
asRecoveryError: (error: unknown, fallback: string) =>
error instanceof Error ? error : new Error(fallback),
branchRecoveryDetails: () => null,
requestSessionRecover: mocks.requestSessionRecover,
restoreSessionWorkspace: mocks.restoreSessionWorkspace,
}));
vi.mock("react-i18next", () => ({
useTranslation: () => ({
t: (key: string, values?: Record<string, string>) =>
values ? `${key}:${JSON.stringify(values)}` : key,
}),
}));
const TASK_ID = "task-1";
const SESSION_ID = "session-1";
beforeEach(() => {
vi.clearAllMocks();
});
describe("useSessionRecoveryActions", () => {
it("clears busy and error state after a successful recovery", async () => {
mocks.requestSessionRecover.mockResolvedValueOnce(undefined);
const { result } = renderHook(() =>
useSessionRecoveryActions({ taskId: TASK_ID, sessionId: SESSION_ID }),
);
let success: boolean | undefined;
await act(async () => {
success = await result.current.handleRecover("resume");
});
expect(success).toBe(true);
expect(result.current.busyAction).toBeNull();
expect(result.current.recoveryError).toBeNull();
expect(mocks.requestSessionRecover).toHaveBeenCalledWith(
TASK_ID,
SESSION_ID,
"resume",
"task:failedToResumeSession",
);
});
it("retains a failed action for retry and releases its busy state", async () => {
mocks.requestSessionRecover
.mockRejectedValueOnce(new Error("provider unavailable"))
.mockResolvedValueOnce(undefined);
const { result } = renderHook(() =>
useSessionRecoveryActions({ taskId: TASK_ID, sessionId: SESSION_ID }),
);
await act(async () => {
await result.current.handleRecover("resume");
});
expect(result.current.recoveryError?.message).toBe("provider unavailable");
expect(result.current.busyAction).toBeNull();
await act(async () => {
await result.current.handleRetry();
});
expect(mocks.requestSessionRecover).toHaveBeenNthCalledWith(
2,
TASK_ID,
SESSION_ID,
"resume",
"task:failedToResumeSession",
);
expect(result.current.recoveryError).toBeNull();
expect(result.current.busyAction).toBeNull();
});
it("ignores an old response after the selected session changes", async () => {
const deferred = Promise.withResolvers<void>();
mocks.requestSessionRecover.mockReturnValueOnce(deferred.promise);
const { result, rerender } = renderHook(
({ sessionId }: { sessionId: string }) =>
useSessionRecoveryActions({ taskId: TASK_ID, sessionId }),
{ initialProps: { sessionId: SESSION_ID } },
);
act(() => {
void result.current.handleRecover("resume");
});
expect(result.current.busyAction).toBe("resume");
rerender({ sessionId: "session-2" });
await waitFor(() => expect(result.current.busyAction).toBeNull());
await act(async () => {
deferred.resolve();
await deferred.promise;
});
expect(result.current.recoveryError).toBeNull();
expect(result.current.busyAction).toBeNull();
});
});
@@ -1,4 +1,5 @@
import { useCallback, useMemo, useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import type { TFunction } from "i18next";
import { useTranslation } from "react-i18next";
import {
asRecoveryError,
@@ -16,9 +17,30 @@ type SessionRecoveryActionsOptions = {
sessionId: string;
};
function combineRecoveryErrors(
resumeError: Error | null,
restoreError: Error | null,
translate: TFunction,
): Error | null {
if (!resumeError || !restoreError) return restoreError ?? resumeError;
return new Error(
translate("task:resumeAndRestoreFailed", {
resumeError: resumeError.message,
restoreError: restoreError.message,
}),
);
}
/** Owns shared manual recovery state while a failed session remains visible. */
export function useSessionRecoveryActions({ taskId, sessionId }: SessionRecoveryActionsOptions) {
const { t } = useTranslation();
const requestKey = `${taskId}\u0000${sessionId}`;
const activeRequestKeyRef = useRef(requestKey);
const operationGenerationRef = useRef(0);
if (activeRequestKeyRef.current !== requestKey) {
activeRequestKeyRef.current = requestKey;
operationGenerationRef.current += 1;
}
const [busyAction, setBusyAction] = useState<SessionRecoveryBusyAction>(null);
const [resumeError, setResumeError] = useState<Error | null>(null);
const [restoreError, setRestoreError] = useState<Error | null>(null);
@@ -26,29 +48,46 @@ export function useSessionRecoveryActions({ taskId, sessionId }: SessionRecovery
const [lastFailedAction, setLastFailedAction] = useState<SessionRecoveryAction | null>(null);
const [recoveryNotice, setRecoveryNotice] = useState<string | null>(null);
const recoveryError = useMemo(() => {
if (resumeError && restoreError) {
return new Error(
t("task:resumeAndRestoreFailed", {
resumeError: resumeError.message,
restoreError: restoreError.message,
}),
);
}
return restoreError ?? resumeError;
}, [restoreError, resumeError, t]);
useEffect(() => {
setBusyAction(null);
setResumeError(null);
setRestoreError(null);
setBranchDetails(null);
setLastFailedAction(null);
setRecoveryNotice(null);
}, [requestKey]);
const beginOperation = useCallback(
(action: SessionRecoveryBusyAction) => {
const operationId = ++operationGenerationRef.current;
setBusyAction(action);
return { requestKey, operationId };
},
[requestKey],
);
const isCurrentOperation = useCallback(
(operation: { requestKey: string; operationId: number }) =>
activeRequestKeyRef.current === operation.requestKey &&
operationGenerationRef.current === operation.operationId,
[],
);
const recoveryError = combineRecoveryErrors(resumeError, restoreError, t);
const handleRecover = useCallback(
async (action: SessionRecoveryAction) => {
setBusyAction(action);
const operation = beginOperation(action);
try {
await requestSessionRecover(taskId, sessionId, action, t("task:failedToResumeSession"));
if (!isCurrentOperation(operation)) return false;
setResumeError(null);
setRestoreError(null);
setBranchDetails(null);
setLastFailedAction(null);
setRecoveryNotice(null);
} catch (cause) {
if (!isCurrentOperation(operation)) return false;
setResumeError(asRecoveryError(cause, t("task:failedToResumeSession")));
setRestoreError(null);
setBranchDetails(branchRecoveryDetails(cause));
@@ -56,30 +95,32 @@ export function useSessionRecoveryActions({ taskId, sessionId }: SessionRecovery
setRecoveryNotice(null);
return false;
} finally {
setBusyAction(null);
if (isCurrentOperation(operation)) setBusyAction(null);
}
return true;
},
[sessionId, taskId, t],
[beginOperation, isCurrentOperation, sessionId, taskId, t],
);
const handleRestore = useCallback(async () => {
setBusyAction("restore");
const operation = beginOperation("restore");
setRestoreError(null);
try {
await restoreSessionWorkspace(taskId, sessionId, t("task:failedToRestoreWorkspace"));
if (!isCurrentOperation(operation)) return;
setResumeError(null);
setRestoreError(null);
setBranchDetails(null);
setLastFailedAction(null);
setRecoveryNotice(t("task:resumeFailedWorkspaceReadOnly"));
} catch (cause) {
if (!isCurrentOperation(operation)) return;
setRestoreError(asRecoveryError(cause, t("task:failedToRestoreWorkspace")));
setRecoveryNotice(null);
} finally {
setBusyAction(null);
if (isCurrentOperation(operation)) setBusyAction(null);
}
}, [resumeError, sessionId, taskId, t]);
}, [beginOperation, isCurrentOperation, sessionId, taskId, t]);
const handleRetry = useCallback(() => {
return handleRecover(lastFailedAction ?? "resume");
@@ -404,6 +404,9 @@ export type ResumeAction = "running" | "skip" | "resume" | "restore" | "idle";
export function decideResumeAction(status: SessionStatus, preventAutoStart: boolean): ResumeAction {
if (status.is_agent_running) return "running";
// Completed sessions remain passive until the user explicitly chooses the
// completed-chat Resume action. Open-time recovery must not revive them.
if (status.state === "COMPLETED") return "idle";
if (preventAutoStart && status.needs_resume && status.is_resumable) return "skip";
if (status.needs_resume && status.is_resumable) return "resume";
if (status.needs_workspace_restore) return "restore";

Some files were not shown because too many files have changed in this diff Show More