fix(backend): record step transitions so session workflow history stops returning empty (#2520)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
nova28
2026-08-13 00:06:40 +08:00
committed by GitHub
parent ff9f0f48bc
commit 93e5c51116
34 changed files with 1647 additions and 34 deletions
@@ -1963,6 +1963,7 @@ func newBootStateTestHarness(t *testing.T) bootStateTestHarness {
eventBus := bus.NewMemoryEventBus(log)
userSvc := userservice.NewService(userRepo, eventBus, log)
workflowSvc := workflowservice.NewService(workflowRepo, log)
t.Cleanup(func() { _ = workflowSvc.Close() })
taskSvc := taskservice.NewService(
taskservice.Repos{
Workspaces: taskRepo,
+3
View File
@@ -362,6 +362,9 @@ func startServices( //nolint:cyclop
log.Error("Failed to initialize services", zap.Error(err))
return false
}
if services.Workflow != nil {
addCleanup(services.Workflow.Close)
}
services.RuntimeFlags = runtimeflags.NewService(
repos.RuntimeFlags,
runtimeflags.RuntimeOptionsFromAppliedConfig(runtimeFlagDefaults, cfg),
@@ -172,8 +172,16 @@ func provideOrchestrator(
taskSvc.PublishTaskUpdated(ctx, task)
})
// Wire workflow step getter for prompt building
// Wire workflow step getter for prompt building. The recorder is wired
// first: SetStepHistoryRecorder's own initWorkflowEngine() call is a
// no-op while workflowStepGetter is still nil, so the store/engine only
// get built once, by SetWorkflowStepGetter below, instead of twice.
if workflowSvc != nil {
// Wire the ADR 0015 audit-trail writer for auto-advance step
// transitions. workflowSvc.CreateStepTransition already matches
// orchestrator.StepHistoryRecorder structurally, so no adapter is
// needed.
orchestratorSvc.SetStepHistoryRecorder(workflowSvc)
orchestratorSvc.SetWorkflowStepGetter(&orchestratorWorkflowStepGetterAdapter{svc: workflowSvc})
}
@@ -67,5 +67,8 @@ func provideTestServices(t *testing.T, version string) (*Services, *config.Confi
if err != nil {
t.Fatalf("provideServices: %v", err)
}
if services.Workflow != nil {
t.Cleanup(func() { _ = services.Workflow.Close() })
}
return services, cfg
}
@@ -113,6 +113,14 @@ func provideServices(cfg *config.Config, log *logger.Logger, repos *Repositories
// Wire start step resolver to task service for CreateTask
taskSvc.SetStartStepResolver(&startStepResolverAdapter{svc: workflowSvc})
// Session history is owned by workflow service, but access is owned by the
// task service. Keep the authorization check at the service boundary.
workflowSvc.SetSessionAccessChecker(taskSvc.AuthorizeSessionAccess)
// Wire the ADR 0015 audit-trail writer for manual step transitions.
// workflowSvc.CreateStepTransition already matches
// taskservice.StepHistoryRecorder structurally, so no adapter is needed.
taskSvc.SetStepHistoryRecorder(workflowSvc)
// Wire workflow provider to workflow service for export/import
workflowSvc.SetWorkflowProvider(&workflowProviderAdapter{svc: taskSvc})
@@ -26,4 +26,11 @@ const (
// SessionLoadTimeout is the maximum time for ACP session/load (resume).
// Session loading may involve deserializing large conversation histories.
SessionLoadTimeout = 2 * time.Minute
// StepHistoryWriteTimeout bounds the ADR 0015 session_step_history audit
// insert issued after a step transition has already been durably
// persisted. It runs on context.WithoutCancel so a client disconnect (or
// turn-end context cancellation) cannot drop the audit row for a
// transition that already committed.
StepHistoryWriteTimeout = 5 * time.Second
)
@@ -13,6 +13,7 @@ import (
"github.com/kandev/kandev/internal/task/models"
taskrepo "github.com/kandev/kandev/internal/task/repository/sqlite"
"github.com/kandev/kandev/internal/task/service"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
v1 "github.com/kandev/kandev/pkg/api/v1"
ws "github.com/kandev/kandev/pkg/websocket"
"go.uber.org/zap"
@@ -164,6 +165,7 @@ func (h *Handlers) deferMoveTask(
WorkflowID: req.WorkflowID,
WorkflowStepID: req.WorkflowStepID,
Position: req.Position,
Actor: string(wfmodels.StepTransitionActorAgent),
})
return ws.NewResponse(msg.ID, msg.Action,
h.synthesizeMovedTaskDTO(ctx, req.TaskID, req.WorkflowID, req.WorkflowStepID, req.Position))
@@ -197,7 +199,8 @@ func (h *Handlers) applyMoveTaskImmediate(
queuedSessionID = session.ID
}
result, err := h.taskSvc.MoveTask(ctx, req.TaskID, req.WorkflowID, req.WorkflowStepID, req.Position)
result, err := h.taskSvc.MoveTaskWithOptions(ctx, req.TaskID, req.WorkflowID, req.WorkflowStepID, req.Position,
service.MoveTaskOptions{StepHistoryActor: wfmodels.StepTransitionActorAgent})
if err != nil {
// Roll back the queued prompt — without this, the next turn would
// deliver a "You were moved to this step…" message for a transition
@@ -99,6 +99,7 @@ func setupImportHandlers(t *testing.T) (*Handlers, *memWorkflowProvider, *reposi
require.NoError(t, err)
svc := workflowsvc.NewService(repo, testLogger(t))
t.Cleanup(func() { _ = svc.Close() })
provider := &memWorkflowProvider{}
svc.SetWorkflowProvider(provider)
@@ -136,6 +136,7 @@ func newTestTaskServiceWithWorkflow(t *testing.T) (*service.Service, *sqliterepo
Reviews: repo,
}, eventBus, log, service.RepositoryDiscoveryConfig{})
workflowSvc := workflowservice.NewService(workflowRepo, log)
t.Cleanup(func() { _ = workflowSvc.Close() })
return svc, repo, workflowcontroller.NewController(workflowSvc), workflowRepo
}
@@ -81,6 +81,7 @@ func setupParitySurfaces(t *testing.T) (*Handlers, *gin.Engine, *eventbus.Memory
log := testLogger(t)
svc := workflowsvc.NewService(repo, log)
t.Cleanup(func() { _ = svc.Close() })
ctrl := workflowctrl.NewController(svc)
eventBus := eventbus.NewMemoryEventBus(log)
@@ -5,9 +5,11 @@ import (
"go.uber.org/zap"
"github.com/kandev/kandev/internal/common/constants"
"github.com/kandev/kandev/internal/events"
"github.com/kandev/kandev/internal/events/bus"
"github.com/kandev/kandev/internal/task/models"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
)
// subscribeStepCompletionEvents wires the ADR 0015 out-of-band subscriber
@@ -59,6 +61,81 @@ func (s *Service) clearPendingStepSignalByID(ctx context.Context, sessionID stri
}
}
// recordAutoStepTransition writes the ADR 0015 audit row for an
// orchestrator-driven (non-manual) step transition — shared by the engine
// path (applyEngineTransition, covering on_turn_complete, on_turn_start,
// and on_children_completed) and the legacy on_turn_complete/on_turn_start
// path (executeStepTransition), so the two funnels cannot drift apart.
// The trigger is preserved so on_turn_start and on_children_completed are not
// reported as completion events. Older rows keep auto_complete for turn
// completion and remain readable.
// Nil-safe: no recorder wired, or no session to record against, is a
// no-op. Runtime writes use the workflow service's bounded worker. Failures
// are logged and swallowed because this contract is best-effort telemetry.
func (s *Service) recordAutoStepTransition(ctx context.Context, sessionID, fromStepID, toStepID string, signal *models.PendingStepCompletionSignal, triggers ...wfmodels.StepTransitionTrigger) {
if s.stepHistoryRecorder == nil || sessionID == "" {
return
}
var metadata map[string]interface{}
if signal != nil {
metadata = map[string]interface{}{
"signal_source": signal.Source,
"signal_summary": signal.Summary,
}
}
trigger := wfmodels.StepTransitionTriggerAutoComplete
if len(triggers) > 0 && triggers[0] != "" {
trigger = triggers[0]
}
if asyncRecorder, ok := s.stepHistoryRecorder.(asyncStepHistoryRecorder); ok {
asyncRecorder.EnqueueStepTransition(sessionID, fromStepID, toStepID, trigger, nil, metadata)
return
}
// Test doubles can remain synchronous. Production workflow service uses the
// queue branch above.
writeCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), constants.StepHistoryWriteTimeout)
defer cancel()
if err := s.stepHistoryRecorder.CreateStepTransition(
writeCtx, sessionID, fromStepID, toStepID, trigger, nil, metadata,
); err != nil {
s.logger.Warn("failed to record auto step transition",
zap.String("session_id", sessionID),
zap.String("from_step_id", fromStepID),
zap.String("to_step_id", toStepID),
zap.Error(err))
}
}
// recordManualStepTransition writes the ADR 0015 audit row for an
// agent-initiated move_task_kandev call that could not apply inline because
// the calling session was still RUNNING/STARTING (applyPendingMove, deferred
// via messagequeue.PendingMove). This is the agent half of
// StepTransitionTriggerManual — the user/HTTP half is recorded by
// task/service.MoveTaskWithOptions. Nil-safe and failure-swallowing, same as
// recordAutoStepTransition.
func (s *Service) recordManualStepTransition(ctx context.Context, sessionID, fromStepID, toStepID string, actors ...wfmodels.StepTransitionActor) {
if s.stepHistoryRecorder == nil || sessionID == "" {
return
}
// Test doubles can remain synchronous. Production workflow service uses the
// queue branch below.
writeCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), constants.StepHistoryWriteTimeout)
defer cancel()
if asyncRecorder, ok := s.stepHistoryRecorder.(asyncStepHistoryRecorder); ok {
asyncRecorder.EnqueueStepTransition(sessionID, fromStepID, toStepID, wfmodels.StepTransitionTriggerManual, nil, nil)
return
}
if err := s.stepHistoryRecorder.CreateStepTransition(
writeCtx, sessionID, fromStepID, toStepID, wfmodels.StepTransitionTriggerManual, nil, nil,
); err != nil {
s.logger.Warn("failed to record manual step transition",
zap.String("session_id", sessionID),
zap.String("from_step_id", fromStepID),
zap.String("to_step_id", toStepID),
zap.Error(err))
}
}
// onStepCompletionSignaled subscribes to events.WorkflowStepCompletionSignaled
// to handle the case where the agent's `step_complete_kandev` call lands
// AFTER the turn already ended — at that point processOnTurnCompleteViaEngine
@@ -353,6 +353,22 @@ func (s *Service) executeStepTransition(ctx context.Context, taskID, sessionID s
s.workflowStore.pullNextTaskOnVacate(ctx, fromStep.ID, taskID)
}
// ADR 0015 — record the audit row before the pending signal (if any)
// is cleared below. Only the triggerOnEnter=true (legacy
// on_turn_complete) branch could have consumed a signal; the
// on_turn_start branch records with no signal metadata.
var consumedSignal *models.PendingStepCompletionSignal
if triggerOnEnter && exitSession != nil {
if signal, has := models.LoadPendingStepSignal(exitSession.Metadata); has && signal.StepID == fromStep.ID {
consumedSignal = &signal
}
}
trigger := wfmodels.StepTransitionTriggerTurnStart
if triggerOnEnter {
trigger = wfmodels.StepTransitionTriggerAutoComplete
}
s.recordAutoStepTransition(ctx, sessionID, fromStep.ID, toStepID, consumedSignal, trigger)
if triggerOnEnter {
// ADR 0015 — clear any pending completion-signal bag for the
// step we just left. Only on_turn_complete transitions trigger
@@ -1340,6 +1356,17 @@ func (s *Service) applyPendingMove(ctx context.Context, taskID, sessionID string
return
}
// ADR 0015 — record the audit row now that the transition is durably
// persisted. This is the agent-initiated move_task_kandev path (the move
// couldn't apply inline because the calling session was still
// RUNNING/STARTING); the idle-session path records through
// task/service.MoveTaskWithOptions instead.
actor := wfmodels.StepTransitionActorAgent
if move.Actor != "" {
actor = wfmodels.StepTransitionActor(move.Actor)
}
s.recordManualStepTransition(ctx, sessionID, fromStepID, move.WorkflowStepID, actor)
s.logger.Info("applying pending move",
zap.String("task_id", taskID),
zap.String("session_id", sessionID),
@@ -2748,6 +2775,25 @@ func (s *Service) applyEngineTransition(
return false
}
// ADR 0015 — record the audit row before the pending signal (if any) is
// cleared. Only an on_turn_complete transition can have consumed a
// signal; on_turn_start and on_children_completed transitions record
// with no signal metadata.
var consumedSignal *models.PendingStepCompletionSignal
if trigger == engine.TriggerOnTurnComplete {
if signal, has := models.LoadPendingStepSignal(session.Metadata); has && signal.StepID == result.FromStepID {
consumedSignal = &signal
}
}
historyTrigger := wfmodels.StepTransitionTriggerAutoComplete
switch trigger {
case engine.TriggerOnTurnStart:
historyTrigger = wfmodels.StepTransitionTriggerTurnStart
case engine.TriggerOnChildrenCompleted:
historyTrigger = wfmodels.StepTransitionTriggerChildrenCompleted
}
s.recordAutoStepTransition(ctx, session.ID, result.FromStepID, result.ToStepID, consumedSignal, historyTrigger)
// ADR 0015 — a successful on_turn_complete transition consumes any
// pending step-completion signal for the source step. The bag must be
// cleared so the next step's gating starts from a clean slate.
@@ -11,6 +11,7 @@ import (
"github.com/google/uuid"
"github.com/jmoiron/sqlx"
"github.com/kandev/kandev/internal/db"
)
// sqliteRepository persists queued messages and pending moves.
@@ -80,10 +81,20 @@ func (r *sqliteRepository) initSchema() error {
workflow_id TEXT NOT NULL DEFAULT '',
workflow_step_id TEXT NOT NULL DEFAULT '',
step_position INTEGER NOT NULL DEFAULT 0,
queued_at TIMESTAMP NOT NULL
queued_at TIMESTAMP NOT NULL,
actor TEXT NOT NULL DEFAULT ''
);
`)
return err
if err != nil {
return err
}
// Existing installations may have the pre-audit shape; fresh installs
// already get the column from CREATE TABLE above, so this replays as a
// duplicate-column error there.
if _, alterErr := r.db.Exec(`ALTER TABLE pending_moves ADD COLUMN actor TEXT NOT NULL DEFAULT ''`); alterErr != nil && !db.IsDuplicateColumnError(alterErr) {
return alterErr
}
return nil
}
// Insert appends a new entry at the tail of the session's FIFO queue.
@@ -1733,16 +1744,17 @@ func (r *sqliteRepository) SetPendingMove(ctx context.Context, sessionID string,
move.QueuedAt = time.Now().UTC()
}
_, err := r.db.ExecContext(ctx, r.db.Rebind(`
INSERT INTO pending_moves (id, session_id, task_id, workflow_id, workflow_step_id, step_position, queued_at)
VALUES (?, ?, ?, ?, ?, ?, ?)
INSERT INTO pending_moves (id, session_id, task_id, workflow_id, workflow_step_id, step_position, queued_at, actor)
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
ON CONFLICT(session_id) DO UPDATE SET
task_id = excluded.task_id,
workflow_id = excluded.workflow_id,
workflow_step_id = excluded.workflow_step_id,
step_position = excluded.step_position,
queued_at = excluded.queued_at
queued_at = excluded.queued_at,
actor = excluded.actor
`),
uuid.New().String(), sessionID, move.TaskID, move.WorkflowID, move.WorkflowStepID, move.Position, move.QueuedAt,
uuid.New().String(), sessionID, move.TaskID, move.WorkflowID, move.WorkflowStepID, move.Position, move.QueuedAt, move.Actor,
)
if err != nil {
return fmt.Errorf("upsert pending move: %w", err)
@@ -1756,11 +1768,12 @@ func (r *sqliteRepository) GetPendingMove(ctx context.Context, sessionID string)
taskID, workflowID, workflowStepID string
position int
queuedAt time.Time
actor string
)
if err := r.ro.QueryRowxContext(ctx, r.db.Rebind(`
SELECT task_id, workflow_id, workflow_step_id, step_position, queued_at
SELECT task_id, workflow_id, workflow_step_id, step_position, queued_at, actor
FROM pending_moves WHERE session_id = ?
`), sessionID).Scan(&taskID, &workflowID, &workflowStepID, &position, &queuedAt); err != nil {
`), sessionID).Scan(&taskID, &workflowID, &workflowStepID, &position, &queuedAt, &actor); err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, nil
}
@@ -1772,6 +1785,7 @@ func (r *sqliteRepository) GetPendingMove(ctx context.Context, sessionID string)
WorkflowStepID: workflowStepID,
Position: position,
QueuedAt: queuedAt,
Actor: actor,
}, nil
}
@@ -1787,11 +1801,12 @@ func (r *sqliteRepository) TakePendingMove(ctx context.Context, sessionID string
taskID, workflowID, workflowStepID string
position int
queuedAt time.Time
actor string
)
if err := tx.QueryRowxContext(ctx, r.db.Rebind(`
SELECT task_id, workflow_id, workflow_step_id, step_position, queued_at
SELECT task_id, workflow_id, workflow_step_id, step_position, queued_at, actor
FROM pending_moves WHERE session_id = ?
`), sessionID).Scan(&taskID, &workflowID, &workflowStepID, &position, &queuedAt); err != nil {
`), sessionID).Scan(&taskID, &workflowID, &workflowStepID, &position, &queuedAt, &actor); err != nil {
if errors.Is(err, sql.ErrNoRows) {
return nil, nil
}
@@ -1809,6 +1824,7 @@ func (r *sqliteRepository) TakePendingMove(ctx context.Context, sessionID string
WorkflowStepID: workflowStepID,
Position: position,
QueuedAt: queuedAt,
Actor: actor,
}, nil
}
@@ -790,7 +790,7 @@ func TestSQLiteRepository_PendingMove(t *testing.T) {
t.Fatalf("expected nil move on empty, got %v err=%v", move, err)
}
move := &PendingMove{TaskID: "t1", WorkflowID: "w1", WorkflowStepID: "step-A", Position: 0}
move := &PendingMove{TaskID: "t1", WorkflowID: "w1", WorkflowStepID: "step-A", Position: 0, Actor: "agent"}
if err := repo.SetPendingMove(ctx, "s1", move); err != nil {
t.Fatalf("set pending: %v", err)
}
@@ -808,6 +808,9 @@ func TestSQLiteRepository_PendingMove(t *testing.T) {
if got == nil || got.WorkflowStepID != "step-B" {
t.Errorf("expected step-B after upsert, got %+v", got)
}
if got == nil || got.Actor != "agent" {
t.Errorf("expected agent actor after upsert, got %+v", got)
}
// Take again -> nil.
got, err = repo.TakePendingMove(ctx, "s1")
@@ -210,4 +210,7 @@ type PendingMove struct {
WorkflowStepID string `json:"workflow_step_id"`
Position int `json:"position"`
QueuedAt time.Time `json:"queued_at"`
// Actor records provenance across the deferred move boundary. Agent is the
// value used by move_task_kandev; it prevents owner identity leakage.
Actor string `json:"actor,omitempty"`
}
+29 -1
View File
@@ -161,6 +161,18 @@ type WorkflowStepGetter interface {
GetWorkflowMeta(ctx context.Context, workflowID string) (WorkflowMeta, error)
}
// StepHistoryRecorder persists an ADR 0015 session-step transition audit
// row. Optional — when unset, the auto-advance and deferred move_task_kandev
// transition paths record nothing. Errors are logged and swallowed by the
// caller: the audit trail must never fail the transition it is recording.
type StepHistoryRecorder interface {
CreateStepTransition(ctx context.Context, sessionID, fromStepID, toStepID string, trigger wfmodels.StepTransitionTrigger, actorID *string, metadata map[string]interface{}) error
}
type asyncStepHistoryRecorder interface {
EnqueueStepTransition(sessionID, fromStepID, toStepID string, trigger wfmodels.StepTransitionTrigger, actorID *string, metadata map[string]interface{})
}
// AgentFamilyResolver maps a hand-written agent family reference onto the
// canonical IDs of every agent that answers to it. Exactly one candidate
// resolves the reference; none means it names no known agent; more than one
@@ -370,6 +382,12 @@ type Service struct {
// Workflow step getter for prompt building
workflowStepGetter WorkflowStepGetter
// stepHistoryRecorder persists the ADR 0015 audit row for orchestrator
// step transitions: auto-advance (StepTransitionTriggerAutoComplete) and
// the deferred move_task_kandev path (StepTransitionTriggerManual).
// Optional.
stepHistoryRecorder StepHistoryRecorder
// Resolves the agent family names written in configure_session rules onto
// canonical agent IDs. Nil-safe: when unset, rule matching falls back to an
// exact string comparison.
@@ -1165,6 +1183,16 @@ func (s *Service) SetWorkflowStepGetter(getter WorkflowStepGetter) {
s.initWorkflowEngine()
}
// SetStepHistoryRecorder wires the ADR 0015 audit-trail writer for
// auto-advance step transitions (engine and legacy on_turn_complete paths)
// and the deferred move_task_kandev path (applyPendingMove). Optional.
func (s *Service) SetStepHistoryRecorder(recorder StepHistoryRecorder) {
s.stepHistoryRecorder = recorder
// The transition store is created when the workflow step getter is wired.
// Rebuild it so queue-promotion paths receive the same recorder.
s.initWorkflowEngine()
}
// SetAgentFamilyResolver sets the collaborator that maps the agent family names
// written in configure_session rules onto canonical agent IDs.
//
@@ -1202,7 +1230,7 @@ func (s *Service) initWorkflowEngine() {
if s.workflowStepGetter == nil {
return
}
store := newWorkflowStore(s.repo, s.workflowStepGetter, s.agentManager, s.publishTaskUpdated, s.logger, s.publishTaskMoved, s.publishTaskQueuePromoted)
store := newWorkflowStore(s.repo, s.workflowStepGetter, s.agentManager, s.publishTaskUpdated, s.logger, s.publishTaskMoved, s.publishTaskQueuePromoted, s.stepHistoryRecorder)
callbacks := buildWorkflowCallbacks(s)
s.workflowStore = store
s.workflowEngine = engine.New(store, callbacks, s.engineOptions...)
@@ -0,0 +1,455 @@
package orchestrator
import (
"context"
"sync"
"testing"
"time"
"github.com/kandev/kandev/internal/orchestrator/messagequeue"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/workflow/engine"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
)
// fakeStepHistoryRecorder captures CreateStepTransition calls for assertion.
type fakeStepHistoryRecorder struct {
mu sync.Mutex
calls []recordedStepTransition
err error
}
type recordedStepTransition struct {
sessionID string
fromStepID, toStepID string
trigger wfmodels.StepTransitionTrigger
actorID *string
metadata map[string]interface{}
// ctxErr captures ctx.Err() at call time — used to prove the recorder
// received a live (non-cancelled) context even when the caller's parent
// context was cancelled before the record* helper ran.
ctxErr error
}
func (f *fakeStepHistoryRecorder) CreateStepTransition(ctx context.Context, sessionID, fromStepID, toStepID string, trigger wfmodels.StepTransitionTrigger, actorID *string, metadata map[string]interface{}) error {
f.mu.Lock()
defer f.mu.Unlock()
f.calls = append(f.calls, recordedStepTransition{
sessionID: sessionID, fromStepID: fromStepID, toStepID: toStepID,
trigger: trigger, actorID: actorID, metadata: metadata,
ctxErr: ctx.Err(),
})
return f.err
}
func (f *fakeStepHistoryRecorder) Calls() []recordedStepTransition {
f.mu.Lock()
defer f.mu.Unlock()
out := make([]recordedStepTransition, len(f.calls))
copy(out, f.calls)
return out
}
func twoStepGetter() *mockStepGetter {
sg := newMockStepGetter()
sg.steps["step1"] = &wfmodels.WorkflowStep{ID: "step1", WorkflowID: "wf1", Name: "Step 1", Position: 0}
sg.steps["step2"] = &wfmodels.WorkflowStep{ID: "step2", WorkflowID: "wf1", Name: "Step 2", Position: 1}
return sg
}
// --- Legacy path (executeStepTransition) ---
func TestExecuteStepTransition_RecordsAutoStepHistoryOnTurnComplete(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
sg := twoStepGetter()
svc := createTestService(repo, sg, newMockTaskRepo())
recorder := &fakeStepHistoryRecorder{}
svc.stepHistoryRecorder = recorder
svc.executeStepTransition(ctx, "t1", "s1", sg.steps["step1"], "step2", true)
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
got := calls[0]
if got.sessionID != "s1" || got.fromStepID != "step1" || got.toStepID != "step2" {
t.Errorf("unexpected call: %+v", got)
}
if got.trigger != wfmodels.StepTransitionTriggerAutoComplete {
t.Errorf("trigger = %q, want auto_complete", got.trigger)
}
if got.metadata != nil {
t.Errorf("metadata = %v, want nil (no pending signal)", got.metadata)
}
}
func TestExecuteStepTransition_RecordsConsumedSignalMetadata(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
sg := twoStepGetter()
svc := createTestService(repo, sg, newMockTaskRepo())
recorder := &fakeStepHistoryRecorder{}
svc.stepHistoryRecorder = recorder
signal := models.PendingStepCompletionSignal{
StepID: "step1",
Source: models.StepCompletionSourceAgent,
Summary: "did the work",
}
if err := repo.SetSessionMetadataKey(ctx, "s1", models.SessionMetaKeyPendingStepCompletion, signal); err != nil {
t.Fatalf("seed pending signal: %v", err)
}
svc.executeStepTransition(ctx, "t1", "s1", sg.steps["step1"], "step2", true)
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
got := calls[0]
if got.metadata["signal_source"] != models.StepCompletionSourceAgent {
t.Errorf("metadata[signal_source] = %v, want %q", got.metadata["signal_source"], models.StepCompletionSourceAgent)
}
if got.metadata["signal_summary"] != "did the work" {
t.Errorf("metadata[signal_summary] = %v, want 'did the work'", got.metadata["signal_summary"])
}
}
func TestExecuteStepTransition_OnTurnStartDoesNotAttachSignal(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
sg := twoStepGetter()
svc := createTestService(repo, sg, newMockTaskRepo())
recorder := &fakeStepHistoryRecorder{}
svc.stepHistoryRecorder = recorder
// A signal is pending for step1, but this is an on_turn_start move
// (triggerOnEnter=false) — it must not consume or report the signal.
signal := models.PendingStepCompletionSignal{StepID: "step1", Source: models.StepCompletionSourceAgent, Summary: "premature"}
if err := repo.SetSessionMetadataKey(ctx, "s1", models.SessionMetaKeyPendingStepCompletion, signal); err != nil {
t.Fatalf("seed pending signal: %v", err)
}
svc.executeStepTransition(ctx, "t1", "s1", sg.steps["step1"], "step2", false)
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
got := calls[0]
if got.trigger != wfmodels.StepTransitionTriggerTurnStart {
t.Errorf("trigger = %q, want on_turn_start", got.trigger)
}
if got.metadata != nil {
t.Errorf("metadata = %v, want nil (on_turn_start does not consume the signal)", got.metadata)
}
}
func TestExecuteStepTransition_HistoryWriteFailureDoesNotBlockTransition(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
sg := twoStepGetter()
svc := createTestService(repo, sg, newMockTaskRepo())
svc.stepHistoryRecorder = &fakeStepHistoryRecorder{err: context.DeadlineExceeded}
svc.executeStepTransition(ctx, "t1", "s1", sg.steps["step1"], "step2", true)
task, err := repo.GetTask(ctx, "t1")
if err != nil {
t.Fatalf("GetTask: %v", err)
}
if task.WorkflowStepID != "step2" {
t.Fatalf("expected transition to still apply despite audit failure, got step %q", task.WorkflowStepID)
}
}
// --- Engine path (applyEngineTransition) ---
func TestApplyEngineTransition_RecordsConsumedSignalMetadata(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
sg := twoStepGetter()
svc := createTestService(repo, sg, newMockTaskRepo())
svc.SetWorkflowStepGetter(sg)
recorder := &fakeStepHistoryRecorder{}
svc.stepHistoryRecorder = recorder
onEnterDone := make(chan struct{})
svc.onProcessOnEnterComplete = func() { close(onEnterDone) }
signal := models.PendingStepCompletionSignal{
StepID: "step1",
Source: models.StepCompletionSourceAgent,
Summary: "engine path done",
}
if err := repo.SetSessionMetadataKey(ctx, "s1", models.SessionMetaKeyPendingStepCompletion, signal); err != nil {
t.Fatalf("seed pending signal: %v", err)
}
session, err := repo.GetTaskSession(ctx, "s1")
if err != nil {
t.Fatalf("GetTaskSession: %v", err)
}
result := engine.HandleResult{Transitioned: true, FromStepID: "step1", ToStepID: "step2"}
transitioned := svc.applyEngineTransition(ctx, "t1", session, result, engine.TriggerOnTurnComplete, "desc", true)
if !transitioned {
t.Fatalf("expected transition to apply")
}
select {
case <-onEnterDone:
case <-time.After(2 * time.Second):
t.Fatal("timed out waiting for on_enter processing")
}
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
got := calls[0]
if got.sessionID != "s1" || got.fromStepID != "step1" || got.toStepID != "step2" {
t.Errorf("unexpected call: %+v", got)
}
if got.trigger != wfmodels.StepTransitionTriggerAutoComplete {
t.Errorf("trigger = %q, want auto_complete", got.trigger)
}
if got.metadata["signal_source"] != models.StepCompletionSourceAgent {
t.Errorf("metadata[signal_source] = %v, want %q", got.metadata["signal_source"], models.StepCompletionSourceAgent)
}
if got.metadata["signal_summary"] != "engine path done" {
t.Errorf("metadata[signal_summary] = %v, want 'engine path done'", got.metadata["signal_summary"])
}
}
func TestApplyEngineTransition_OnTurnStartDoesNotAttachSignal(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
sg := twoStepGetter()
svc := createTestService(repo, sg, newMockTaskRepo())
svc.SetWorkflowStepGetter(sg)
recorder := &fakeStepHistoryRecorder{}
svc.stepHistoryRecorder = recorder
signal := models.PendingStepCompletionSignal{StepID: "step1", Source: models.StepCompletionSourceAgent, Summary: "premature"}
if err := repo.SetSessionMetadataKey(ctx, "s1", models.SessionMetaKeyPendingStepCompletion, signal); err != nil {
t.Fatalf("seed pending signal: %v", err)
}
session, err := repo.GetTaskSession(ctx, "s1")
if err != nil {
t.Fatalf("GetTaskSession: %v", err)
}
result := engine.HandleResult{Transitioned: true, FromStepID: "step1", ToStepID: "step2"}
transitioned := svc.applyEngineTransition(ctx, "t1", session, result, engine.TriggerOnTurnStart, "", false)
if !transitioned {
t.Fatalf("expected transition to apply")
}
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
got := calls[0]
if got.trigger != wfmodels.StepTransitionTriggerTurnStart {
t.Errorf("trigger = %q, want on_turn_start", got.trigger)
}
if got.metadata != nil {
t.Errorf("metadata = %v, want nil (on_turn_start does not consume the signal)", got.metadata)
}
}
func TestApplyEngineTransition_ChildrenCompletedRecordsNoSignalMetadata(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
sg := twoStepGetter()
svc := createTestService(repo, sg, newMockTaskRepo())
svc.SetWorkflowStepGetter(sg)
recorder := &fakeStepHistoryRecorder{}
svc.stepHistoryRecorder = recorder
onEnterDone := make(chan struct{})
svc.onProcessOnEnterComplete = func() { close(onEnterDone) }
session, err := repo.GetTaskSession(ctx, "s1")
if err != nil {
t.Fatalf("GetTaskSession: %v", err)
}
result := engine.HandleResult{Transitioned: true, FromStepID: "step1", ToStepID: "step2"}
transitioned := svc.applyEngineTransition(ctx, "t1", session, result, engine.TriggerOnChildrenCompleted, "desc", true)
if !transitioned {
t.Fatalf("expected transition to apply")
}
select {
case <-onEnterDone:
case <-time.After(2 * time.Second):
t.Fatal("timed out waiting for on_enter processing")
}
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
if calls[0].trigger != wfmodels.StepTransitionTriggerChildrenCompleted {
t.Errorf("trigger = %q, want on_children_completed", calls[0].trigger)
}
if calls[0].metadata != nil {
t.Errorf("metadata = %v, want nil (on_children_completed never consumes a signal)", calls[0].metadata)
}
}
// --- Deferred move_task_kandev path (applyPendingMove) ---
// TestApplyPendingMove_RecordsManualStepHistory covers the agent half of
// StepTransitionTriggerManual: a move_task_kandev call that couldn't apply
// inline because the calling session was RUNNING, deferred via
// messagequeue.PendingMove, and applied here at turn end. Before this test
// the identical tool call from an IDLE session recorded a row (through
// task/service.MoveTaskWithOptions) but this path recorded nothing —
// Review round 1 flagged the trail as silently non-deterministic.
func TestApplyPendingMove_RecordsManualStepHistory(t *testing.T) {
sc := buildPendingMoveScenario(t)
recorder := &fakeStepHistoryRecorder{}
sc.svc.stepHistoryRecorder = recorder
session, err := sc.repo.GetTaskSession(sc.ctx, sc.reviewSessionID)
if err != nil {
t.Fatalf("load review session: %v", err)
}
sc.svc.applyPendingMove(sc.ctx, "task-1", sc.reviewSessionID, session, &messagequeue.PendingMove{
TaskID: "task-1",
WorkflowID: "wf1",
WorkflowStepID: stepInProgressID,
})
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
got := calls[0]
if got.sessionID != sc.reviewSessionID || got.fromStepID != stepInReviewID || got.toStepID != stepInProgressID {
t.Errorf("unexpected call: %+v", got)
}
if got.trigger != wfmodels.StepTransitionTriggerManual {
t.Errorf("trigger = %q, want manual", got.trigger)
}
if got.metadata != nil {
t.Errorf("metadata = %v, want nil", got.metadata)
}
}
// TestApplyPendingMove_HistoryWriteFailureDoesNotBlockTransition mirrors the
// failure-isolation coverage of the other two funnels: an audit-write error
// must never block the transition it is recording.
func TestApplyPendingMove_HistoryWriteFailureDoesNotBlockTransition(t *testing.T) {
sc := buildPendingMoveScenario(t)
sc.svc.stepHistoryRecorder = &fakeStepHistoryRecorder{err: context.DeadlineExceeded}
session, err := sc.repo.GetTaskSession(sc.ctx, sc.reviewSessionID)
if err != nil {
t.Fatalf("load review session: %v", err)
}
sc.svc.applyPendingMove(sc.ctx, "task-1", sc.reviewSessionID, session, &messagequeue.PendingMove{
TaskID: "task-1",
WorkflowID: "wf1",
WorkflowStepID: stepInProgressID,
})
task, err := sc.repo.GetTask(sc.ctx, "task-1")
if err != nil {
t.Fatalf("GetTask: %v", err)
}
if task.WorkflowStepID != stepInProgressID {
t.Fatalf("expected transition to still apply despite audit failure, got step %q", task.WorkflowStepID)
}
}
// TestApplyPendingMove_ForeignWorkflowMismatchRecordsNoHistory extends
// TestPendingMove_DropsForeignWorkflowStepWithoutMovingTask's scenario: a
// pending move whose target step belongs to a different workflow is dropped
// before ApplyTransition ever runs, so it must not write a phantom audit row.
func TestApplyPendingMove_ForeignWorkflowMismatchRecordsNoHistory(t *testing.T) {
sc := buildPendingMoveScenario(t)
sc.stepGetter.steps["foreign-step"] = &wfmodels.WorkflowStep{
ID: "foreign-step",
WorkflowID: "wf-other",
Name: "Foreign",
Position: 1,
}
recorder := &fakeStepHistoryRecorder{}
sc.svc.stepHistoryRecorder = recorder
session, err := sc.repo.GetTaskSession(sc.ctx, sc.reviewSessionID)
if err != nil {
t.Fatalf("load review session: %v", err)
}
sc.svc.applyPendingMove(sc.ctx, "task-1", sc.reviewSessionID, session, &messagequeue.PendingMove{
TaskID: "task-1",
WorkflowID: "wf1",
WorkflowStepID: "foreign-step",
})
if calls := recorder.Calls(); len(calls) != 0 {
t.Fatalf("expected no recorded transitions for a dropped foreign-workflow move, got %d", len(calls))
}
}
// --- Detached context coverage ---
// TestRecordAutoStepTransition_SurvivesCancelledParentContext covers Review
// round 3's finding: the audit insert previously ran on the caller's ctx
// after the step change already committed, so a cancelled parent context
// (turn-end, request cancellation) could silently drop the row. The recorder
// must see a live context even when the caller passes a cancelled one.
func TestRecordAutoStepTransition_SurvivesCancelledParentContext(t *testing.T) {
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
sg := twoStepGetter()
svc := createTestService(repo, sg, newMockTaskRepo())
recorder := &fakeStepHistoryRecorder{}
svc.stepHistoryRecorder = recorder
cancelledCtx, cancel := context.WithCancel(context.Background())
cancel()
svc.recordAutoStepTransition(cancelledCtx, "s1", "step1", "step2", nil)
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
if calls[0].ctxErr != nil {
t.Errorf("recorder received a cancelled context: %v", calls[0].ctxErr)
}
}
// TestRecordManualStepTransition_SurvivesCancelledParentContext mirrors
// TestRecordAutoStepTransition_SurvivesCancelledParentContext for the
// applyPendingMove funnel's manual-trigger helper.
func TestRecordManualStepTransition_SurvivesCancelledParentContext(t *testing.T) {
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", "step1")
sg := twoStepGetter()
svc := createTestService(repo, sg, newMockTaskRepo())
recorder := &fakeStepHistoryRecorder{}
svc.stepHistoryRecorder = recorder
cancelledCtx, cancel := context.WithCancel(context.Background())
cancel()
svc.recordManualStepTransition(cancelledCtx, "s1", "step1", "step2")
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
if calls[0].ctxErr != nil {
t.Errorf("recorder received a cancelled context: %v", calls[0].ctxErr)
}
}
@@ -66,6 +66,7 @@ type workflowStore struct {
publishTaskMoved taskMovedPublisher
publishTaskPromoted taskQueuePromotedPublisher
logger *logger.Logger
stepHistoryRecorder StepHistoryRecorder
appliedOps sync.Map
}
@@ -79,6 +80,7 @@ func newWorkflowStore(
) *workflowStore {
var moved taskMovedPublisher
var promoted taskQueuePromotedPublisher
var history StepHistoryRecorder
for _, publisher := range publishers {
switch value := publisher.(type) {
case taskMovedPublisher:
@@ -87,6 +89,10 @@ func newWorkflowStore(
moved = taskMovedPublisher(value)
case taskQueuePromotedPublisher:
promoted = value
case StepHistoryRecorder:
// Keep transition-history ownership in the workflow store for
// queue promotions, which otherwise bypass the normal move API.
history = value
case func(context.Context, *models.Task):
promoted = taskQueuePromotedPublisher(value)
}
@@ -99,6 +105,7 @@ func newWorkflowStore(
publishTaskMoved: moved,
publishTaskPromoted: promoted,
logger: log,
stepHistoryRecorder: history,
}
}
@@ -380,6 +387,13 @@ func (s *workflowStore) pullOneFeederTask(
if session, err := s.repo.GetActiveTaskSessionByTaskID(ctx, candidate.ID); err == nil && session != nil {
sessionID = session.ID
}
if s.stepHistoryRecorder != nil && sessionID != "" {
if asyncRecorder, ok := s.stepHistoryRecorder.(asyncStepHistoryRecorder); ok {
asyncRecorder.EnqueueStepTransition(sessionID, fromStepID, vacatedStep.ID, wfmodels.StepTransitionTriggerQueuePromotion, nil, nil)
} else if err := s.stepHistoryRecorder.CreateStepTransition(ctx, sessionID, fromStepID, vacatedStep.ID, wfmodels.StepTransitionTriggerQueuePromotion, nil, nil); err != nil {
s.logger.Warn("failed to record queue promotion transition", zap.String("task_id", candidate.ID), zap.Error(err))
}
}
s.publishTaskMoved(ctx, candidate, fromWorkflowID, fromStepID, vacatedStep.ID, sessionID)
return true
}
@@ -24,6 +24,7 @@ import (
"github.com/kandev/kandev/internal/task/service"
"github.com/kandev/kandev/internal/task/statussummary"
usermodels "github.com/kandev/kandev/internal/user/models"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
"github.com/kandev/kandev/internal/worktree"
v1 "github.com/kandev/kandev/pkg/api/v1"
"go.uber.org/zap"
@@ -1635,7 +1636,7 @@ func (h *TaskHandlers) httpMoveTask(c *gin.Context) {
result, err := h.service.MoveTaskWithOptions(
c.Request.Context(), c.Param("id"),
body.WorkflowID, body.WorkflowStepID, body.Position,
service.MoveTaskOptions{AllowActivePrimarySession: true},
service.MoveTaskOptions{AllowActivePrimarySession: true, StepHistoryActor: wfmodels.StepTransitionActorHuman},
)
if err != nil {
handleSelectedMoveError(c, h.logger, err)
@@ -9,6 +9,7 @@ import (
"github.com/kandev/kandev/internal/task/dto"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/task/service"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
v1 "github.com/kandev/kandev/pkg/api/v1"
ws "github.com/kandev/kandev/pkg/websocket"
"go.uber.org/zap"
@@ -409,7 +410,7 @@ func (h *TaskHandlers) wsMoveTask(ctx context.Context, msg *ws.Message) (*ws.Mes
req.WorkflowID,
req.WorkflowStepID,
req.Position,
service.MoveTaskOptions{AllowActivePrimarySession: true},
service.MoveTaskOptions{AllowActivePrimarySession: true, StepHistoryActor: wfmodels.StepTransitionActorHuman},
)
if err != nil {
h.logger.Error("failed to move task", zap.Error(err))
@@ -195,6 +195,18 @@ type StartStepResolver interface {
ResolveFirstStep(ctx context.Context, workflowID string) (string, error)
}
// StepHistoryRecorder persists an ADR 0015 session-step transition audit
// row. Optional — when unset, MoveTaskWithOptions records nothing. Errors
// are logged and swallowed by the caller: the audit trail must never fail
// the move it is recording.
type StepHistoryRecorder interface {
CreateStepTransition(ctx context.Context, sessionID, fromStepID, toStepID string, trigger wfmodels.StepTransitionTrigger, actorID *string, metadata map[string]interface{}) error
}
type asyncStepHistoryRecorder interface {
EnqueueStepTransition(sessionID, fromStepID, toStepID string, trigger wfmodels.StepTransitionTrigger, actorID *string, metadata map[string]interface{})
}
var (
ErrActiveTaskSessions = errors.New("active agent sessions exist")
ErrWIPLimitExceeded = wfmodels.ErrWIPLimitExceeded
@@ -291,6 +303,7 @@ type Service struct {
workspaceBootstrapper WorkspaceBootstrapper
workflowStepGetter WorkflowStepGetter
startStepResolver StartStepResolver
stepHistoryRecorder StepHistoryRecorder
prTaskResolver PRTaskResolver
quickChatDir string // Directory for quick-chat workspaces (e.g., ~/.kandev/quick-chat)
branchFetcher *branchFetcher
@@ -506,6 +519,12 @@ func (s *Service) SetStartStepResolver(resolver StartStepResolver) {
s.startStepResolver = resolver
}
// SetStepHistoryRecorder wires the ADR 0015 audit-trail writer for manual
// step transitions (MoveTaskWithOptions). Optional.
func (s *Service) SetStepHistoryRecorder(recorder StepHistoryRecorder) {
s.stepHistoryRecorder = recorder
}
// SetPRTaskResolver wires the GitHub PR→task resolver for PR-number search.
// Optional — when unset, search by PR number is a no-op.
func (s *Service) SetPRTaskResolver(resolver PRTaskResolver) {
@@ -1372,6 +1372,7 @@ func (s *Service) UpdateTask(ctx context.Context, id string, req *UpdateTaskRequ
if err != nil {
return nil, err
}
oldWorkflowStepID := task.WorkflowStepID
var oldState *v1.TaskState
stateChanged := false
@@ -1426,6 +1427,16 @@ func (s *Service) UpdateTask(ctx context.Context, id string, req *UpdateTaskRequ
// this snapshot was stale. Publish and return the row that actually won so
// callers never receive the provisional title or pending marker again.
task = s.reloadTaskAfterMutation(ctx, id, task, "update")
if req.WorkflowStepID != nil && oldWorkflowStepID != task.WorkflowStepID {
sessionID := ""
if session := s.resolvePrimaryOrActiveSession(ctx, id); session != nil {
sessionID = session.ID
}
// Generic task updates are a mutation boundary used by plugins and
// MCP. Record them as system-originated unless a dedicated move API
// supplied stronger provenance.
s.recordManualStepTransition(ctx, sessionID, oldWorkflowStepID, task.WorkflowStepID, wfmodels.StepTransitionTriggerTaskUpdate, wfmodels.StepTransitionActorSystem)
}
// Update task repositories if provided
if req.Repositories != nil {
@@ -10,6 +10,8 @@ import (
v1 "github.com/kandev/kandev/pkg/api/v1"
"github.com/kandev/kandev/internal/auth/authn"
"github.com/kandev/kandev/internal/common/constants"
"github.com/kandev/kandev/internal/events"
"github.com/kandev/kandev/internal/task/models"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
@@ -88,7 +90,12 @@ func (s *Service) applyApprovalStepTransition(ctx context.Context, sessionID str
return nil
}
moved, err := s.MoveTask(ctx, result.Session.TaskID, step.WorkflowID, newStepID, 0)
moved, err := s.MoveTaskWithOptions(ctx, result.Session.TaskID, step.WorkflowID, newStepID, 0,
MoveTaskOptions{
StepHistoryTrigger: wfmodels.StepTransitionTriggerApproval,
StepHistorySessionID: sessionID,
StepHistoryActor: wfmodels.StepTransitionActorHuman,
})
if err != nil {
return fmt.Errorf("failed to move task to next step after approval: %w", err)
}
@@ -388,6 +395,21 @@ type MoveTaskOptions struct {
// PreserveDeferredLaunch keeps the deferred launch intent when an internal
// queue promotion changes workflow steps. Manual moves still clear it.
PreserveDeferredLaunch bool
// StepHistoryTrigger overrides the ADR 0015 audit-row trigger recorded for
// this move. Zero value defaults to StepTransitionTriggerManual — callers
// driving an approval-gated transition (ApproveSession) set
// StepTransitionTriggerApproval instead.
StepHistoryTrigger wfmodels.StepTransitionTrigger
// StepHistorySessionID pins the ADR 0015 audit-row session_id to a
// specific session, overriding the primary/active-session resolution
// MoveTaskWithOptions otherwise uses. ApproveSession sets this to the
// session it is actually approving — on a task with more than one
// active session, resolvePrimaryOrActiveSession can pick a different
// (primary) session than the one being approved.
StepHistorySessionID string
// StepHistoryActor identifies the caller. Agent moves must not inherit the
// owner identity that MCP uses for authorization.
StepHistoryActor wfmodels.StepTransitionActor
}
type workflowMoveLimitsRepository interface {
@@ -487,6 +509,11 @@ func (s *Service) MoveTaskWithOptions(
if stepChanged {
s.publishTaskMovedEvent(ctx, task, oldWorkflowID, oldStepID, workflowStepID, sessionID)
s.pullNextTaskOnVacate(ctx, oldStepID, task.ID)
historySessionID := opts.StepHistorySessionID
if historySessionID == "" {
historySessionID = sessionID
}
s.recordManualStepTransition(ctx, historySessionID, oldStepID, workflowStepID, opts.StepHistoryTrigger, opts.StepHistoryActor)
}
s.logger.Info("task moved",
@@ -687,6 +714,7 @@ func (s *Service) promoteFeederQueuedTask(ctx context.Context, candidate *models
return s.promoteNextQueuedTask(ctx, targetStep, position, skipped)
}
s.publishTaskEvent(ctx, events.TaskUpdated, candidate, nil, oldWorkflowID)
s.recordQueuedPromotion(ctx, candidate.ID, fromStepID, targetStep.ID)
s.publishTaskMovedEvent(ctx, candidate, oldWorkflowID, fromStepID, targetStep.ID, "")
return true
}
@@ -711,6 +739,23 @@ func (s *Service) promoteFeederQueuedTask(ctx context.Context, candidate *models
return true
}
func (s *Service) recordQueuedPromotion(ctx context.Context, taskID, fromStepID, toStepID string) {
if s.stepHistoryRecorder == nil {
return
}
session := s.resolvePrimaryOrActiveSession(ctx, taskID)
if session == nil {
return
}
if asyncRecorder, ok := s.stepHistoryRecorder.(asyncStepHistoryRecorder); ok {
asyncRecorder.EnqueueStepTransition(session.ID, fromStepID, toStepID, wfmodels.StepTransitionTriggerQueuePromotion, nil, nil)
return
}
if err := s.stepHistoryRecorder.CreateStepTransition(ctx, session.ID, fromStepID, toStepID, wfmodels.StepTransitionTriggerQueuePromotion, nil, nil); err != nil {
s.logger.Warn("failed to record queued task promotion", zap.String("task_id", taskID), zap.Error(err))
}
}
func (s *Service) feederCandidateBlocked(ctx context.Context, taskID string) bool {
sessions, err := s.sessions.ListTaskSessions(ctx, taskID)
if err != nil {
@@ -917,6 +962,59 @@ func (s *Service) resolvePrimaryOrActiveSession(ctx context.Context, taskID stri
return active
}
// recordManualStepTransition writes the ADR 0015 audit row for a
// user/agent-initiated move. trigger is normally StepTransitionTriggerManual;
// callers driving an approval-gated transition (ApproveSession) pass
// StepTransitionTriggerApproval — a zero value defaults to Manual. It is a
// no-op when no recorder is wired or when the task has no session to record
// against — session_step_history.session_id is a NOT NULL FK to
// task_sessions, so a session-less move cannot be recorded without a
// schema change. Runtime writes use the workflow service's bounded worker;
// failures are logged and swallowed because this is best-effort telemetry.
func (s *Service) recordManualStepTransition(ctx context.Context, sessionID, fromStepID, toStepID string, trigger wfmodels.StepTransitionTrigger, actors ...wfmodels.StepTransitionActor) {
if s.stepHistoryRecorder == nil {
return
}
if sessionID == "" {
s.logger.Debug("skipping manual step transition audit: task has no session",
zap.String("from_step_id", fromStepID),
zap.String("to_step_id", toStepID))
return
}
if trigger == "" {
trigger = wfmodels.StepTransitionTriggerManual
}
var actorID *string
actor := wfmodels.StepTransitionActorHuman
if len(actors) > 0 {
actor = actors[0]
}
if actor == wfmodels.StepTransitionActorHuman {
if identity, ok := authn.IdentityFromContext(ctx); ok && identity.UserID != "" {
actorID = &identity.UserID
}
}
if asyncRecorder, ok := s.stepHistoryRecorder.(asyncStepHistoryRecorder); ok {
asyncRecorder.EnqueueStepTransition(sessionID, fromStepID, toStepID, trigger, actorID, nil)
return
}
// The step change is already durably persisted by the time this runs.
// Use a detached, bounded context so a cancelled request context (client
// disconnect, turn-end) cannot drop the audit row for a transition that
// already committed.
writeCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), constants.StepHistoryWriteTimeout)
defer cancel()
if err := s.stepHistoryRecorder.CreateStepTransition(
writeCtx, sessionID, fromStepID, toStepID, trigger, actorID, nil,
); err != nil {
s.logger.Warn("failed to record manual step transition",
zap.String("session_id", sessionID),
zap.String("from_step_id", fromStepID),
zap.String("to_step_id", toStepID),
zap.Error(err))
}
}
func isSessionActive(state models.TaskSessionState) bool {
return state == models.TaskSessionStateCreated ||
state == models.TaskSessionStateStarting ||
@@ -0,0 +1,274 @@
package service
import (
"context"
"sync"
"testing"
"github.com/kandev/kandev/internal/task/models"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
)
// fakeStepHistoryRecorder captures CreateStepTransition calls for assertion.
type fakeStepHistoryRecorder struct {
mu sync.Mutex
calls []recordedTransition
err error
}
type recordedTransition struct {
sessionID string
fromStepID, toStepID string
trigger wfmodels.StepTransitionTrigger
actorID *string
metadata map[string]interface{}
// ctxErr captures ctx.Err() at call time — used to prove the recorder
// received a live (non-cancelled) context even when the caller's parent
// context was cancelled before the record* helper ran.
ctxErr error
}
func (f *fakeStepHistoryRecorder) CreateStepTransition(ctx context.Context, sessionID, fromStepID, toStepID string, trigger wfmodels.StepTransitionTrigger, actorID *string, metadata map[string]interface{}) error {
f.mu.Lock()
defer f.mu.Unlock()
f.calls = append(f.calls, recordedTransition{
sessionID: sessionID, fromStepID: fromStepID, toStepID: toStepID,
trigger: trigger, actorID: actorID, metadata: metadata,
ctxErr: ctx.Err(),
})
return f.err
}
func (f *fakeStepHistoryRecorder) Calls() []recordedTransition {
f.mu.Lock()
defer f.mu.Unlock()
out := make([]recordedTransition, len(f.calls))
copy(out, f.calls)
return out
}
func TestService_MoveTaskRecordsManualStepHistory(t *testing.T) {
svc, _, repo := createTestService(t)
ctx := context.Background()
seedMoveWorkflows(t, ctx, repo)
seedMoveSteps(svc)
recorder := &fakeStepHistoryRecorder{}
svc.SetStepHistoryRecorder(recorder)
createMoveTask(t, ctx, repo, "task-history", "wf-source", "step-source", nil)
createMoveSession(t, ctx, repo, "session-history", "task-history", models.TaskSessionStateWaitingForInput, "")
_, err := svc.MoveTask(ctx, "task-history", "wf-source", "step-review-target", 0)
if err != nil {
t.Fatalf("MoveTask: %v", err)
}
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
got := calls[0]
if got.sessionID != "session-history" {
t.Errorf("sessionID = %q, want session-history", got.sessionID)
}
if got.fromStepID != "step-source" {
t.Errorf("fromStepID = %q, want step-source", got.fromStepID)
}
if got.toStepID != "step-review-target" {
t.Errorf("toStepID = %q, want step-review-target", got.toStepID)
}
if got.trigger != wfmodels.StepTransitionTriggerManual {
t.Errorf("trigger = %q, want manual", got.trigger)
}
}
func TestService_MoveTaskAgentDoesNotUseOwnerActor(t *testing.T) {
svc, _, repo := createTestService(t)
ctx := context.Background()
seedMoveWorkflows(t, ctx, repo)
seedMoveSteps(svc)
recorder := &fakeStepHistoryRecorder{}
svc.SetStepHistoryRecorder(recorder)
createMoveTask(t, ctx, repo, "task-agent-history", "wf-source", "step-source", nil)
createMoveSession(t, ctx, repo, "session-agent-history", "task-agent-history", models.TaskSessionStateWaitingForInput, "")
_, err := svc.MoveTaskWithOptions(ctx, "task-agent-history", "wf-source", "step-review-target", 0, MoveTaskOptions{
StepHistoryActor: wfmodels.StepTransitionActorAgent,
})
if err != nil {
t.Fatalf("MoveTaskWithOptions: %v", err)
}
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
if calls[0].actorID != nil {
t.Fatalf("agent transition actorID = %v, want nil", *calls[0].actorID)
}
}
func TestService_MoveTaskSameStepDoesNotRecordHistory(t *testing.T) {
svc, _, repo := createTestService(t)
ctx := context.Background()
seedMoveWorkflows(t, ctx, repo)
seedMoveSteps(svc)
recorder := &fakeStepHistoryRecorder{}
svc.SetStepHistoryRecorder(recorder)
createMoveTask(t, ctx, repo, "task-samestep", "wf-source", "step-source", nil)
// Reorder within the same step — position change only, no step change.
_, err := svc.MoveTask(ctx, "task-samestep", "wf-source", "step-source", 0)
if err != nil {
t.Fatalf("MoveTask: %v", err)
}
if calls := recorder.Calls(); len(calls) != 0 {
t.Fatalf("expected no recorded transitions for a same-step move, got %d", len(calls))
}
}
func TestService_MoveTaskWithoutSessionDoesNotRecordOrFail(t *testing.T) {
svc, _, repo := createTestService(t)
ctx := context.Background()
seedMoveWorkflows(t, ctx, repo)
seedMoveSteps(svc)
recorder := &fakeStepHistoryRecorder{}
svc.SetStepHistoryRecorder(recorder)
// No session created for this task at all.
createMoveTask(t, ctx, repo, "task-no-session", "wf-source", "step-source", nil)
_, err := svc.MoveTask(ctx, "task-no-session", "wf-source", "step-review-target", 0)
if err != nil {
t.Fatalf("MoveTask should not fail when there is no session to record against: %v", err)
}
if calls := recorder.Calls(); len(calls) != 0 {
t.Fatalf("expected no recorded transitions when sessionID is empty, got %d", len(calls))
}
}
func TestService_ApproveSessionRecordsApprovalStepHistoryNotManual(t *testing.T) {
svc, _, repo := createTestService(t)
ctx := context.Background()
seedMoveWorkflows(t, ctx, repo)
seedMoveSteps(svc)
getter := svc.workflowStepGetter.(*fakeWorkflowStepGetter)
getter.steps["step-done"] = &wfmodels.WorkflowStep{
ID: "step-done", WorkflowID: "wf-source", Name: "Approved", Position: 2,
}
recorder := &fakeStepHistoryRecorder{}
svc.SetStepHistoryRecorder(recorder)
createMoveTask(t, ctx, repo, "task-approval-history", "wf-source", "step-review-target", nil)
createMoveSession(t, ctx, repo, "session-approval-history", "task-approval-history",
models.TaskSessionStateWaitingForInput, models.ReviewStatusPending)
if _, err := svc.ApproveSession(ctx, "session-approval-history"); err != nil {
t.Fatalf("ApproveSession: %v", err)
}
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
got := calls[0]
if got.sessionID != "session-approval-history" || got.fromStepID != "step-review-target" || got.toStepID != "step-done" {
t.Errorf("unexpected call: %+v", got)
}
if got.trigger != wfmodels.StepTransitionTriggerApproval {
t.Errorf("trigger = %q, want approval (not manual)", got.trigger)
}
}
// TestService_ApproveSessionRecordsApprovalStepHistoryForApprovedSessionNotPrimary
// covers a task with two active sessions where the primary session is NOT the
// one being approved. Review round 3 flagged that MoveTaskWithOptions re-derives
// the audit-row session via resolvePrimaryOrActiveSession instead of using the
// session ApproveSession actually approved, so the row was attributed to the
// primary session instead of the approved one.
func TestService_ApproveSessionRecordsApprovalStepHistoryForApprovedSessionNotPrimary(t *testing.T) {
svc, _, repo := createTestService(t)
ctx := context.Background()
seedMoveWorkflows(t, ctx, repo)
seedMoveSteps(svc)
getter := svc.workflowStepGetter.(*fakeWorkflowStepGetter)
getter.steps["step-done"] = &wfmodels.WorkflowStep{
ID: "step-done", WorkflowID: "wf-source", Name: "Approved", Position: 2,
}
recorder := &fakeStepHistoryRecorder{}
svc.SetStepHistoryRecorder(recorder)
createMoveTask(t, ctx, repo, "task-multi-session", "wf-source", "step-review-target", nil)
must(t, repo.CreateTaskSession(ctx, &models.TaskSession{
ID: "session-primary",
TaskID: "task-multi-session",
State: models.TaskSessionStateWaitingForInput,
IsPrimary: true,
}))
must(t, repo.CreateTaskSession(ctx, &models.TaskSession{
ID: "session-review",
TaskID: "task-multi-session",
State: models.TaskSessionStateWaitingForInput,
IsPrimary: false,
ReviewStatus: models.ReviewStatusPending,
}))
if _, err := svc.ApproveSession(ctx, "session-review"); err != nil {
t.Fatalf("ApproveSession: %v", err)
}
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
got := calls[0]
if got.sessionID != "session-review" {
t.Errorf("sessionID = %q, want session-review (the approved session, not the primary)", got.sessionID)
}
}
func TestService_MoveTaskHistoryWriteFailureDoesNotFailMove(t *testing.T) {
svc, _, repo := createTestService(t)
ctx := context.Background()
seedMoveWorkflows(t, ctx, repo)
seedMoveSteps(svc)
recorder := &fakeStepHistoryRecorder{err: context.DeadlineExceeded}
svc.SetStepHistoryRecorder(recorder)
createMoveTask(t, ctx, repo, "task-history-fail", "wf-source", "step-source", nil)
createMoveSession(t, ctx, repo, "session-history-fail", "task-history-fail", models.TaskSessionStateWaitingForInput, "")
moved, err := svc.MoveTask(ctx, "task-history-fail", "wf-source", "step-review-target", 0)
if err != nil {
t.Fatalf("MoveTask must not fail when the audit write fails: %v", err)
}
if moved.Task.WorkflowStepID != "step-review-target" {
t.Fatalf("expected task to still move to step-review-target, got %s", moved.Task.WorkflowStepID)
}
}
// TestService_RecordManualStepTransitionSurvivesCancelledParentContext covers
// Review round 3's finding: the audit insert previously ran on the caller's
// ctx after the step change already committed, so a cancelled parent context
// (client disconnect on httpMoveTask) could silently drop the row. The
// recorder must see a live context even when the caller passes a cancelled
// one.
func TestService_RecordManualStepTransitionSurvivesCancelledParentContext(t *testing.T) {
svc, _, _ := createTestService(t)
recorder := &fakeStepHistoryRecorder{}
svc.SetStepHistoryRecorder(recorder)
cancelledCtx, cancel := context.WithCancel(context.Background())
cancel()
svc.recordManualStepTransition(cancelledCtx, "session-x", "step-a", "step-b", wfmodels.StepTransitionTriggerManual)
calls := recorder.Calls()
if len(calls) != 1 {
t.Fatalf("expected 1 recorded transition, got %d", len(calls))
}
if calls[0].ctxErr != nil {
t.Errorf("recorder received a cancelled context: %v", calls[0].ctxErr)
}
}
@@ -14,6 +14,8 @@ import (
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/events"
"github.com/kandev/kandev/internal/events/bus"
taskmodels "github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/task/repository/repoerrors"
"github.com/kandev/kandev/internal/workflow/controller"
"github.com/kandev/kandev/internal/workflow/models"
"github.com/kandev/kandev/internal/workflow/service"
@@ -283,7 +285,15 @@ func (h *Handlers) httpListHistoryBySession(c *gin.Context) {
})
if err != nil {
h.logger.Error("failed to list history", zap.Error(err))
c.JSON(http.StatusInternalServerError, gin.H{"error": "Failed to list history"})
// Session IDs are opaque, so denied/missing access is reported as
// not-found rather than leaking whether another workspace owns the
// session. Any other error (e.g. a repository read failure) is a
// genuine server error and must not be masked as not-found.
if errors.Is(err, taskmodels.ErrTaskSessionNotFound) || errors.Is(err, repoerrors.ErrTaskNotFound) {
c.JSON(http.StatusNotFound, gin.H{"error": "session not found"})
return
}
c.JSON(http.StatusInternalServerError, gin.H{"error": "failed to list history"})
return
}
c.JSON(http.StatusOK, resp)
@@ -109,7 +109,9 @@ func setupStepRouter(t *testing.T) (*gin.Engine, *bus.MemoryEventBus, *repositor
}
log := logger.Default()
eventBus := bus.NewMemoryEventBus(log)
h := NewHandlers(controller.NewController(service.NewService(repo, log)), eventBus, log)
workflowSvc := service.NewService(repo, log)
t.Cleanup(func() { _ = workflowSvc.Close() })
h := NewHandlers(controller.NewController(workflowSvc), eventBus, log)
gin.SetMode(gin.TestMode)
router := gin.New()
@@ -277,6 +277,25 @@ const (
StepTransitionTriggerManual StepTransitionTrigger = "manual"
StepTransitionTriggerAutoComplete StepTransitionTrigger = "auto_complete"
StepTransitionTriggerApproval StepTransitionTrigger = "approval"
// StepTransitionTriggerTurnStart identifies an automatic transition caused
// by the on_turn_start workflow event.
StepTransitionTriggerTurnStart StepTransitionTrigger = "on_turn_start"
// StepTransitionTriggerChildrenCompleted identifies an automatic transition
// caused by the on_children_completed workflow event.
StepTransitionTriggerChildrenCompleted StepTransitionTrigger = "on_children_completed"
StepTransitionTriggerTaskUpdate StepTransitionTrigger = "task_update"
StepTransitionTriggerQueuePromotion StepTransitionTrigger = "queue_promotion"
)
// StepTransitionActor identifies the source of a move. Human identity is
// taken only from authenticated HTTP or WebSocket boundaries. Agent and
// system moves do not inherit the owner's user identity.
type StepTransitionActor string
const (
StepTransitionActorHuman StepTransitionActor = "human"
StepTransitionActorAgent StepTransitionActor = "agent"
StepTransitionActorSystem StepTransitionActor = "system"
)
// SessionStepHistory represents an audit trail entry for session step transitions
@@ -0,0 +1,174 @@
package repository
import (
"context"
"testing"
"github.com/kandev/kandev/internal/workflow/models"
)
func TestCreateHistory_RoundTrip(t *testing.T) {
repo, sqlxDB := setupTestRepoWithDB(t)
ctx := context.Background()
if _, err := sqlxDB.Exec(`INSERT INTO task_sessions (id) VALUES ('sess-1')`); err != nil {
t.Fatalf("failed to insert test session: %v", err)
}
fromStepID := "step-a"
actorID := "user-1"
history := &models.SessionStepHistory{
SessionID: "sess-1",
FromStepID: &fromStepID,
ToStepID: "step-b",
Trigger: models.StepTransitionTriggerManual,
ActorID: &actorID,
Metadata: map[string]interface{}{
"signal_source": "agent",
"signal_summary": "did the thing",
},
}
if err := repo.CreateHistory(ctx, history); err != nil {
t.Fatalf("CreateHistory failed: %v", err)
}
if history.ID == 0 {
t.Fatalf("expected CreateHistory to populate ID, got 0")
}
if history.CreatedAt.IsZero() {
t.Fatalf("expected CreateHistory to populate CreatedAt")
}
result, err := repo.ListHistoryBySession(ctx, "sess-1")
if err != nil {
t.Fatalf("ListHistoryBySession failed: %v", err)
}
if len(result) != 1 {
t.Fatalf("expected 1 history row, got %d", len(result))
}
got := result[0]
if got.SessionID != "sess-1" {
t.Errorf("SessionID = %q, want sess-1", got.SessionID)
}
if got.FromStepID == nil || *got.FromStepID != "step-a" {
t.Errorf("FromStepID = %v, want step-a", got.FromStepID)
}
if got.ToStepID != "step-b" {
t.Errorf("ToStepID = %q, want step-b", got.ToStepID)
}
if got.Trigger != models.StepTransitionTriggerManual {
t.Errorf("Trigger = %q, want manual", got.Trigger)
}
if got.ActorID == nil || *got.ActorID != "user-1" {
t.Errorf("ActorID = %v, want user-1", got.ActorID)
}
if got.Metadata["signal_source"] != "agent" {
t.Errorf("Metadata[signal_source] = %v, want agent", got.Metadata["signal_source"])
}
if got.Metadata["signal_summary"] != "did the thing" {
t.Errorf("Metadata[signal_summary] = %v, want 'did the thing'", got.Metadata["signal_summary"])
}
}
func TestCreateHistory_NullableFromStepAndActor(t *testing.T) {
repo, sqlxDB := setupTestRepoWithDB(t)
ctx := context.Background()
if _, err := sqlxDB.Exec(`INSERT INTO task_sessions (id) VALUES ('sess-2')`); err != nil {
t.Fatalf("failed to insert test session: %v", err)
}
history := &models.SessionStepHistory{
SessionID: "sess-2",
ToStepID: "step-first",
Trigger: models.StepTransitionTriggerAutoComplete,
}
if err := repo.CreateHistory(ctx, history); err != nil {
t.Fatalf("CreateHistory failed: %v", err)
}
result, err := repo.ListHistoryBySession(ctx, "sess-2")
if err != nil {
t.Fatalf("ListHistoryBySession failed: %v", err)
}
if len(result) != 1 {
t.Fatalf("expected 1 history row, got %d", len(result))
}
got := result[0]
if got.FromStepID != nil {
t.Errorf("FromStepID = %v, want nil", got.FromStepID)
}
if got.ActorID != nil {
t.Errorf("ActorID = %v, want nil", got.ActorID)
}
if got.Metadata != nil {
t.Errorf("Metadata = %v, want nil", got.Metadata)
}
}
func TestListHistoryBySession_OrderedByCreatedAt(t *testing.T) {
repo, sqlxDB := setupTestRepoWithDB(t)
ctx := context.Background()
if _, err := sqlxDB.Exec(`INSERT INTO task_sessions (id) VALUES ('sess-3')`); err != nil {
t.Fatalf("failed to insert test session: %v", err)
}
steps := []string{"step-1", "step-2", "step-3"}
for _, to := range steps {
h := &models.SessionStepHistory{
SessionID: "sess-3",
ToStepID: to,
Trigger: models.StepTransitionTriggerManual,
}
if err := repo.CreateHistory(ctx, h); err != nil {
t.Fatalf("CreateHistory(%s) failed: %v", to, err)
}
}
result, err := repo.ListHistoryBySession(ctx, "sess-3")
if err != nil {
t.Fatalf("ListHistoryBySession failed: %v", err)
}
if len(result) != len(steps) {
t.Fatalf("expected %d history rows, got %d", len(steps), len(result))
}
for i, want := range steps {
if result[i].ToStepID != want {
t.Errorf("result[%d].ToStepID = %q, want %q", i, result[i].ToStepID, want)
}
}
}
func TestListHistoryBySession_ScopedToSession(t *testing.T) {
repo, sqlxDB := setupTestRepoWithDB(t)
ctx := context.Background()
if _, err := sqlxDB.Exec(`INSERT INTO task_sessions (id) VALUES ('sess-a'), ('sess-b')`); err != nil {
t.Fatalf("failed to insert test sessions: %v", err)
}
for _, sid := range []string{"sess-a", "sess-b"} {
h := &models.SessionStepHistory{
SessionID: sid,
ToStepID: "step-x",
Trigger: models.StepTransitionTriggerManual,
}
if err := repo.CreateHistory(ctx, h); err != nil {
t.Fatalf("CreateHistory failed: %v", err)
}
}
result, err := repo.ListHistoryBySession(ctx, "sess-a")
if err != nil {
t.Fatalf("ListHistoryBySession failed: %v", err)
}
if len(result) != 1 {
t.Fatalf("expected 1 history row scoped to sess-a, got %d", len(result))
}
if result[0].SessionID != "sess-a" {
t.Errorf("SessionID = %q, want sess-a", result[0].SessionID)
}
}
@@ -0,0 +1,126 @@
package service
import (
"context"
"sync"
"testing"
"time"
"github.com/stretchr/testify/require"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/workflow/models"
)
// TestEnqueueStepTransition_PersistsThroughQueue exercises the actual async
// path (EnqueueStepTransition -> runHistoryWriter -> CreateStepTransition)
// rather than calling CreateStepTransition directly, so a regression in the
// queue/writer plumbing itself would be caught here.
func TestEnqueueStepTransition_PersistsThroughQueue(t *testing.T) {
svc, _ := setupTestService(t)
actorID := "user-1"
svc.EnqueueStepTransition("sess-async-1", "step-a", "step-b", models.StepTransitionTriggerManual, &actorID, nil)
require.Eventually(t, func() bool {
history, err := svc.ListHistoryBySession(context.Background(), "sess-async-1")
return err == nil && len(history) == 1
}, 2*time.Second, 5*time.Millisecond, "enqueued transition was never persisted")
history, err := svc.ListHistoryBySession(context.Background(), "sess-async-1")
require.NoError(t, err)
require.Len(t, history, 1)
require.Equal(t, "step-b", history[0].ToStepID)
require.Equal(t, models.StepTransitionTriggerManual, history[0].Trigger)
}
// TestEnqueueStepTransition_PreservesFIFOOrder proves the single writer
// goroutine drains the queue in enqueue order — the history rows must read
// back as a single monotonic step chain.
func TestEnqueueStepTransition_PreservesFIFOOrder(t *testing.T) {
svc, _ := setupTestService(t)
const steps = 20
for i := 0; i < steps; i++ {
from := stepName(i)
to := stepName(i + 1)
svc.EnqueueStepTransition("sess-async-fifo", from, to, models.StepTransitionTriggerManual, nil, nil)
}
require.Eventually(t, func() bool {
history, err := svc.ListHistoryBySession(context.Background(), "sess-async-fifo")
return err == nil && len(history) == steps
}, 2*time.Second, 5*time.Millisecond, "not all enqueued transitions were persisted")
history, err := svc.ListHistoryBySession(context.Background(), "sess-async-fifo")
require.NoError(t, err)
require.Len(t, history, steps)
for i, row := range history {
require.Equal(t, stepName(i+1), row.ToStepID, "row %d out of FIFO order", i)
}
}
func stepName(i int) string {
return "step-" + string(rune('a'+i))
}
// TestEnqueueStepTransition_QueueFullDropsWithoutBlocking builds the Service
// directly with a one-slot queue and no writer goroutine draining it, so the
// second enqueue call must hit the full-queue branch and return immediately
// rather than block the caller.
func TestEnqueueStepTransition_QueueFullDropsWithoutBlocking(t *testing.T) {
log, err := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "console"})
require.NoError(t, err)
svc := &Service{
logger: log,
historyQueue: make(chan historyWrite, 1),
}
svc.EnqueueStepTransition("sess-full-1", "", "step-a", models.StepTransitionTriggerManual, nil, nil)
require.Len(t, svc.historyQueue, 1)
done := make(chan struct{})
go func() {
svc.EnqueueStepTransition("sess-full-2", "", "step-b", models.StepTransitionTriggerManual, nil, nil)
close(done)
}()
select {
case <-done:
case <-time.After(time.Second):
t.Fatal("EnqueueStepTransition blocked on a full queue instead of dropping")
}
require.Len(t, svc.historyQueue, 1, "queue-full enqueue must be dropped, not buffered")
}
// TestEnqueueStepTransition_RejectsAfterClose covers the enqueue/Close race
// the maintainer flagged: once Close has returned, the writer goroutine is
// gone, so anything still landing in historyQueue would sit there forever.
// EnqueueStepTransition must reject rather than silently queue it.
func TestEnqueueStepTransition_RejectsAfterClose(t *testing.T) {
svc, _ := setupTestService(t)
require.NoError(t, svc.Close())
svc.EnqueueStepTransition("sess-after-close", "", "step-a", models.StepTransitionTriggerManual, nil, nil)
require.Empty(t, svc.historyQueue, "enqueue after Close must be dropped, not silently queued")
}
// TestEnqueueStepTransition_ConcurrentWithClose_NoRace races many concurrent
// enqueues against Close under go test -race to prove the historyMu guard
// actually serializes the check-and-send with the closed-flag flip.
func TestEnqueueStepTransition_ConcurrentWithClose_NoRace(t *testing.T) {
svc, _ := setupTestService(t)
var wg sync.WaitGroup
wg.Add(1)
go func() {
defer wg.Done()
for i := 0; i < 50; i++ {
svc.EnqueueStepTransition("sess-race", "", "step", models.StepTransitionTriggerManual, nil, nil)
}
}()
require.NoError(t, svc.Close())
wg.Wait()
}
+132 -12
View File
@@ -4,11 +4,13 @@ package service
import (
"context"
"fmt"
"sync"
"github.com/google/uuid"
"go.uber.org/zap"
workflowcfg "github.com/kandev/kandev/config/workflows"
"github.com/kandev/kandev/internal/common/constants"
"github.com/kandev/kandev/internal/common/logger"
taskmodels "github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/workflow/models"
@@ -25,13 +27,34 @@ type WorkflowProvider interface {
// Service provides workflow business logic
type Service struct {
repo *repository.Repository
logger *logger.Logger
workflowProvider WorkflowProvider
workspaceProvider WorkspaceProvider
resolveProfile models.AgentProfileResolver
matchProfile models.AgentProfileMatcher
syncOps SyncWorkflowOps
repo *repository.Repository
logger *logger.Logger
workflowProvider WorkflowProvider
workspaceProvider WorkspaceProvider
resolveProfile models.AgentProfileResolver
matchProfile models.AgentProfileMatcher
syncOps SyncWorkflowOps
sessionAccessChecker func(context.Context, string) error
historyQueue chan historyWrite
historyStop chan struct{}
historyDone chan struct{}
historyCloseOnce sync.Once
historyMu sync.RWMutex
historyClosed bool
}
type historyWrite struct {
sessionID, fromStepID, toStepID string
trigger models.StepTransitionTrigger
actorID *string
metadata map[string]interface{}
}
// SetSessionAccessChecker wires the task-domain authorization check. The
// workflow package owns the history read, but the task package owns session
// and workspace permissions.
func (s *Service) SetSessionAccessChecker(checker func(context.Context, string) error) {
s.sessionAccessChecker = checker
}
// WorkspaceProvider resolves a workspace by ID so the read-only guard can tell
@@ -59,10 +82,98 @@ func (s *Service) SetAgentProfileFuncs(resolve models.AgentProfileResolver, matc
// NewService creates a new workflow service
func NewService(repo *repository.Repository, log *logger.Logger) *Service {
return &Service{
repo: repo,
logger: log.WithFields(zap.String("component", "workflow-service")),
s := &Service{
repo: repo,
logger: log.WithFields(zap.String("component", "workflow-service")),
historyQueue: make(chan historyWrite, 256),
historyStop: make(chan struct{}),
historyDone: make(chan struct{}),
}
go s.runHistoryWriter()
return s
}
func (s *Service) runHistoryWriter() {
defer close(s.historyDone)
for {
select {
case write := <-s.historyQueue:
ctx, cancel := context.WithTimeout(context.Background(), constants.StepHistoryWriteTimeout)
if err := s.CreateStepTransition(ctx, write.sessionID, write.fromStepID, write.toStepID, write.trigger, write.actorID, write.metadata); err != nil {
s.logger.Warn("failed to write queued step transition", zap.Error(err))
}
cancel()
case <-s.historyStop:
s.drainHistoryQueue()
return
}
}
}
// drainHistoryQueue flushes whatever is left in the queue at shutdown under a
// single bounded deadline shared by every remaining row, rather than a fresh
// StepHistoryWriteTimeout per row — a stalled database and a full 256-row
// queue would otherwise be able to block graceful shutdown for minutes. Rows
// that don't fit inside the deadline are dropped; this audit trail is
// best-effort and must never hold up shutdown.
func (s *Service) drainHistoryQueue() {
ctx, cancel := context.WithTimeout(context.Background(), constants.StepHistoryWriteTimeout)
defer cancel()
for {
select {
case write := <-s.historyQueue:
if err := s.CreateStepTransition(ctx, write.sessionID, write.fromStepID, write.toStepID, write.trigger, write.actorID, write.metadata); err != nil {
s.logger.Warn("failed to write queued step transition during shutdown", zap.Error(err))
}
case <-ctx.Done():
if dropped := len(s.historyQueue); dropped > 0 {
s.logger.Warn("dropped queued step transitions at shutdown deadline", zap.Int("dropped", dropped))
}
return
default:
return
}
}
}
// EnqueueStepTransition records transition history outside the caller's
// event-reader path. The queue is bounded. A full queue, or an enqueue after
// Close has begun, is logged as a dropped best-effort telemetry row while the
// workflow mutation itself succeeds.
func (s *Service) EnqueueStepTransition(sessionID, fromStepID, toStepID string, trigger models.StepTransitionTrigger, actorID *string, metadata map[string]interface{}) {
if sessionID == "" {
return
}
s.historyMu.RLock()
defer s.historyMu.RUnlock()
if s.historyClosed {
s.logger.Warn("dropped step transition enqueued after shutdown", zap.String("session_id", sessionID))
return
}
select {
case s.historyQueue <- historyWrite{sessionID: sessionID, fromStepID: fromStepID, toStepID: toStepID, trigger: trigger, actorID: actorID, metadata: metadata}:
default:
s.logger.Warn("step transition history queue is full", zap.String("session_id", sessionID))
}
}
// Close stops accepting new history writes, drains the queue under a bounded
// deadline, and waits for the writer goroutine to exit. The closed flag is
// set under the same lock EnqueueStepTransition holds across its
// check-and-send, so no enqueue can land in the queue after a concurrent
// Close has started shutting it down.
func (s *Service) Close() error {
if s.historyStop == nil {
return nil
}
s.historyCloseOnce.Do(func() {
s.historyMu.Lock()
s.historyClosed = true
s.historyMu.Unlock()
close(s.historyStop)
})
<-s.historyDone
return nil
}
// ============================================================================
@@ -430,13 +541,17 @@ func (s *Service) ReorderSteps(ctx context.Context, workflowID string, stepIDs [
// History Operations
// ============================================================================
// CreateStepTransition creates a new step transition history entry.
func (s *Service) CreateStepTransition(ctx context.Context, sessionID string, fromStepID, toStepID string, trigger models.StepTransitionTrigger, actorID *string) error {
// CreateStepTransition creates a new step transition history entry. metadata
// is optional (nil for a plain move) and carries the ADR 0015 consumed-signal
// shape ({"signal_source", "signal_summary"}) when a completion signal drove
// the transition.
func (s *Service) CreateStepTransition(ctx context.Context, sessionID string, fromStepID, toStepID string, trigger models.StepTransitionTrigger, actorID *string, metadata map[string]interface{}) error {
history := &models.SessionStepHistory{
SessionID: sessionID,
ToStepID: toStepID,
Trigger: trigger,
ActorID: actorID,
Metadata: metadata,
}
if fromStepID != "" {
@@ -462,6 +577,11 @@ func (s *Service) CreateStepTransition(ctx context.Context, sessionID string, fr
// ListHistoryBySession returns all step history entries for a session.
func (s *Service) ListHistoryBySession(ctx context.Context, sessionID string) ([]*models.SessionStepHistory, error) {
if s.sessionAccessChecker != nil {
if err := s.sessionAccessChecker(ctx, sessionID); err != nil {
return nil, err
}
}
history, err := s.repo.ListHistoryBySession(ctx, sessionID)
if err != nil {
s.logger.Error("failed to list history by session", zap.String("session_id", sessionID), zap.Error(err))
@@ -22,6 +22,11 @@ import (
func setupTestService(t *testing.T) (*Service, *sqlx.DB) {
rawDB, err := sql.Open("sqlite3", ":memory:")
require.NoError(t, err)
// Pin the pool to one connection: every connection to an in-memory SQLite
// DB gets its own database, so a second one (e.g. the async history
// writer goroutine racing a test's read) would not see the schema,
// causing flaky "no such table" failures.
rawDB.SetMaxOpenConns(1)
sqlxDB := sqlx.NewDb(rawDB, "sqlite3")
t.Cleanup(func() { _ = sqlxDB.Close() })
@@ -37,7 +42,9 @@ func setupTestService(t *testing.T) (*Service, *sqlx.DB) {
require.NoError(t, err)
log, _ := logger.NewLogger(logger.LoggingConfig{Level: "error", Format: "console"})
return NewService(repo, log), sqlxDB
svc := NewService(repo, log)
t.Cleanup(func() { _ = svc.Close() })
return svc, sqlxDB
}
func insertWorkflow(t *testing.T, db *sqlx.DB, id, name string) {
@@ -0,0 +1,71 @@
package service
import (
"context"
"errors"
"testing"
"github.com/stretchr/testify/require"
"github.com/kandev/kandev/internal/workflow/models"
)
func TestListHistoryBySession_AuthorizesBeforeRead(t *testing.T) {
svc, _ := setupTestService(t)
denied := errors.New("session access denied")
called := false
svc.SetSessionAccessChecker(func(context.Context, string) error {
called = true
return denied
})
history, err := svc.ListHistoryBySession(context.Background(), "sess-foreign")
require.ErrorIs(t, err, denied)
require.Nil(t, history)
require.True(t, called)
}
func TestCreateStepTransition_PersistsMetadataAndActor(t *testing.T) {
svc, _ := setupTestService(t)
ctx := context.Background()
actorID := "user-1"
metadata := map[string]interface{}{
"signal_source": "agent",
"signal_summary": "finished the step",
}
err := svc.CreateStepTransition(ctx, "sess-1", "step-a", "step-b", models.StepTransitionTriggerAutoComplete, &actorID, metadata)
require.NoError(t, err)
history, err := svc.ListHistoryBySession(ctx, "sess-1")
require.NoError(t, err)
require.Len(t, history, 1)
got := history[0]
require.NotNil(t, got.FromStepID)
require.Equal(t, "step-a", *got.FromStepID)
require.Equal(t, "step-b", got.ToStepID)
require.Equal(t, models.StepTransitionTriggerAutoComplete, got.Trigger)
require.NotNil(t, got.ActorID)
require.Equal(t, "user-1", *got.ActorID)
require.Equal(t, "agent", got.Metadata["signal_source"])
require.Equal(t, "finished the step", got.Metadata["signal_summary"])
}
func TestCreateStepTransition_NilMetadataAndFromStep(t *testing.T) {
svc, _ := setupTestService(t)
ctx := context.Background()
err := svc.CreateStepTransition(ctx, "sess-2", "", "step-first", models.StepTransitionTriggerManual, nil, nil)
require.NoError(t, err)
history, err := svc.ListHistoryBySession(ctx, "sess-2")
require.NoError(t, err)
require.Len(t, history, 1)
got := history[0]
require.Nil(t, got.FromStepID)
require.Nil(t, got.ActorID)
require.Nil(t, got.Metadata)
}
+1 -1
View File
@@ -108,7 +108,7 @@ export async function createWorkflowStep(
// Session Step History operations
export async function listSessionStepHistory(sessionId: string, options?: ApiRequestOptions) {
return fetchJson<ListSessionStepHistoryResponse>(
`/api/v1/workflow/history?session_id=${sessionId}`,
`/api/v1/sessions/${encodeURIComponent(sessionId)}/workflow/history`,
options,
);
}
@@ -54,7 +54,9 @@ Today, a workflow step's `on_turn_complete` actions (most commonly `move_to_next
- **Read** by `processOnTurnComplete` and by the `onStepCompletionSignaled` subscriber. Stale entries whose `StepID` no longer matches the session's current step are treated as absent and cleared on read.
- **Cleared** on (a) successful transition, (b) user message arriving before the transition runs (re-open semantics), (c) any other step change.
- **Idempotent.** Repeated calls within the same step are no-ops: if `existing != nil && existing.StepID == currentStepID`, the MCP tool returns `{accepted: false, reason: "already_signaled"}` without overwriting.
- **Audit (deferred).** Persisting consumed-signal context on the transition row (`SessionStepHistory.Metadata["signal_source"]` ∈ {`"agent"`, `"manual_fallback"`} plus `Metadata["signal_summary"]`) is the planned audit shape but is not yet implemented. Today the bag is cleared on transition and the only history is the chat transcript. Cancelled signals (overwritten by user reply) are deliberately not retained.
- **Audit (implemented for the trigger-enum call sites).** Consumed-signal context is persisted on the transition row (`SessionStepHistory.Metadata["signal_source"]` ∈ {`"agent"`, `"manual_fallback"`} plus `Metadata["signal_summary"]`), written by `orchestrator.Service.recordAutoStepTransition` immediately before the bag is cleared, for both the workflow-engine (`applyEngineTransition`) and legacy (`executeStepTransition`) transition paths. The three `StepTransitionTrigger` funnels are wired: manual moves (`StepTransitionTriggerManual`, from `task/service.MoveTaskWithOptions`, including the deferred `move_task_kandev` path via `orchestrator.applyPendingMove`), approval-gated moves (`StepTransitionTriggerApproval`, from `applyApprovalStepTransition`), and orchestrator-driven moves (`StepTransitionTriggerAutoComplete`, covering on_turn_complete, on_turn_start, and on_children_completed via `applyEngineTransition`/`executeStepTransition`) — but only an on_turn_complete transition that actually consumed a matching pending signal carries the metadata above; other automated transitions record with no metadata. Cancelled signals (overwritten by user reply) are still not retained — the bag is cleared, not archived.
Transition ownership is now explicit at the mutation boundaries. Generic `task/service.UpdateTask` changes use `task_update`; queue-promotion paths use `queue_promotion`; engine and legacy orchestrator paths preserve their event trigger; and human or agent move paths carry explicit actor provenance. The workflow service owns the history writer. Runtime callers enqueue rows on its bounded worker, which drains during service shutdown. This is best-effort telemetry: a full queue or database failure is logged and does not roll back an already-committed task mutation. A durable audit ledger would require an outbox transaction in a future change.
- **Chat visibility (deferred).** Surfacing a `step_complete_signaled` system message in the existing message stream is planned but not yet emitted. Today the only user-visible cue is the next-step transition itself; the bag remains the only truth source.
3. **System-prompt injection.** When `auto_advance_requires_signal` is true on the current workflow step, `internal/sysprompt/` prepends a short instruction block to the agent's system prompt at launch / resume time, in the same path that already injects MCP tool descriptions. The text is fixed, terse, and points at the tool by name: