dreampiler/kandev
1
0
Fork 0
Code Issues 43 Pull Requests Actions 6 Packages Projects Releases Wiki Activity

fix(workflow): dispatch on_enter actions for review/approval steps (#2907)

Browse Source 
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
Henry
2026-08-27 22:19:00 +00:00
committed by GitHub
parent 144106c72c
commit 5ae96c324f
29 changed files with 4368 additions and 116 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/backend/internal/db/dialect/dialect.go
+10
View File
@@ -11,6 +11,16 @@ func IsPostgres(driver string) bool {
return driver == PGX
}
// AutoIncrementIDColumn returns the dialect-specific column definition for an
// auto-incrementing integer primary key named "id", for use in CREATE TABLE
// statements shared between SQLite and PostgreSQL.
func AutoIncrementIDColumn(driver string) string {
if IsPostgres(driver) {
return "id BIGSERIAL PRIMARY KEY"
}
return "id INTEGER PRIMARY KEY AUTOINCREMENT"
}
// BoolToInt converts a boolean to an integer for SQL storage.
func BoolToInt(value bool) int {
if value {
apps/backend/internal/orchestrator/event_handlers_workflow.go
+351 -23
View File
@@ -24,8 +24,10 @@ import (
"github.com/kandev/kandev/internal/steptelemetry"
"github.com/kandev/kandev/internal/sysprompt"
"github.com/kandev/kandev/internal/task/models"
workflowadapters "github.com/kandev/kandev/internal/workflow/adapters"
"github.com/kandev/kandev/internal/workflow/engine"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
"github.com/kandev/kandev/internal/workflow/stepentry"
v1 "github.com/kandev/kandev/pkg/api/v1"
)
@@ -1953,7 +1955,14 @@ func (s *Service) finalizeStepEnter(ctx context.Context, taskID, sessionID strin
return fmt.Errorf("load session for on_enter: %w", err)
}
s.processOnEnter(ctx, taskID, session, targetStep, taskDescription)
// entryID 0: this path (manual move / legacy on_turn_start/complete) does
// not attach a step-entry ResultHolder before ApplyTransition, so no
// entry was allocated for this step-entry. processOnEnter's engine-owned
// action cases treat entryID==0 as "not this Build round's dispatch
// path" and skip with a log rather than executing — see
// docs/specs/workflow-on-enter-action-dispatch/spec.md and the task
// plan's scope note for why E2-E5 dispatch is deferred.
s.processOnEnter(ctx, taskID, session, targetStep, taskDescription, 0)
return nil
}
@@ -2360,8 +2369,98 @@ func (s *Service) maybySwitchSessionForProfile(
return effective, true
}
// dispatchOnEnterActions runs each on_enter action declared on step, in
// order, and reports whether an auto_start_agent action was among them.
// Extracted from processOnEnter to keep that function's cognitive
// complexity within the repo's lint threshold.
//
// AC-C2: if clear_decisions fails, no further on_enter action for this step
// entry may execute — stale decision rows plus a fresh participant fan-out
// could otherwise satisfy wait_for_quorum without every current-round
// participant actually voting. The loop stops entirely (not just further
// engine-owned actions) the moment that happens.
//
// AC-F1: a caller that loses a *live* claim on an engine-owned position
// (dispatchEngineOwnedOnEnterAction's abandon=true) must abandon the entry
// the same way — it does not skip to a later position, because that later
// action could execute while the winner is still committing an earlier
// one (e.g. enqueueing queue_run_for_each_participant's fan-out before the
// winner's clear_decisions delete commits). This is a normal outcome, not
// a fault, so the loop stops silently (no WARNING/ERROR) rather than via
// AC-C2's error-logged abort.
type onEnterDispatchResult struct {
hasAutoStart bool
aborted bool
}
func (s *Service) dispatchOnEnterActions(ctx context.Context, taskID string, session *models.TaskSession, step *wfmodels.WorkflowStep, entryID int64, isPassthrough, hasPlanMode bool) onEnterDispatchResult {
result := onEnterDispatchResult{}
dispatchLoop:
for i, action := range step.Events.OnEnter {
switch action.Type {
case wfmodels.OnEnterEnablePlanMode:
// Skip plan mode for passthrough — CLI manages its own state.
// Also skip if agent doesn't support MCP (hasPlanMode is already false above).
if !isPassthrough && hasPlanMode {
s.setSessionPlanMode(ctx, session, true)
}
case wfmodels.OnEnterSetSessionMode:
mode, _ := action.Config["mode"].(string)
s.applyStepSessionMode(ctx, session, mode, isPassthrough)
case wfmodels.OnEnterAutoStartAgent:
result.hasAutoStart = true
case wfmodels.OnEnterResetAgentContext, wfmodels.OnEnterConfigureSession:
// Already handled earlier in processOnEnter (context reset must run
// before auto_start_agent; session config runs right after it).
case wfmodels.OnEnterClearDecisions, wfmodels.OnEnterQueueRunForEachParticipant:
abandon, failed, cause := s.dispatchEngineOwnedOnEnterAction(ctx, taskID, step, action, i, entryID)
if abandon {
s.logger.Debug("processOnEnter: lost a live claim to a concurrent dispatch of this step entry, abandoning remaining on_enter actions",
zap.String("workflow_id", step.WorkflowID),
zap.String("step_id", step.ID),
zap.String("task_id", taskID),
zap.String("action_type", string(action.Type)),
)
result.aborted = true
break dispatchLoop
}
if failed && action.Type == wfmodels.OnEnterClearDecisions {
s.logger.Error("processOnEnter: clear_decisions failed, aborting remaining on_enter actions for step entry",
zap.String("workflow_id", step.WorkflowID),
zap.String("step_id", step.ID),
zap.String("task_id", taskID),
zap.String("cause", cause),
)
result.aborted = true
break dispatchLoop
}
case wfmodels.OnEnterQueueRun, wfmodels.OnEnterRunCodeReview:
// Engine-owned per the spec, but their on_enter dispatch (AC-A7/
// AC-A8) is explicitly deferred to a later Build round — see
// docs/specs/workflow-on-enter-action-dispatch/spec.md. This is a
// known, recognized type, not the AC-A6 default warning case.
default:
// AC-A6: a genuinely unrecognized on_enter action type. Warn
// instead of silently discarding it — this is the exact failure
// mode the step-entry dispatch fix exists to close.
s.logger.Warn("processOnEnter: unrecognized on_enter action type",
zap.String("task_id", taskID),
zap.String("step_id", step.ID),
zap.String("action_type", string(action.Type)),
)
}
}
return result
}
// processOnEnter processes the on_enter events for a step after transitioning to it.
func (s *Service) processOnEnter(ctx context.Context, taskID string, session *models.TaskSession, step *wfmodels.WorkflowStep, taskDescription string) {
// entryID is the workflow_step_entries row allocated for this step-entry by
// the write site that persisted the transition (0 when no entry was
// allocated for this call path — see finalizeStepEnter's call site). Only a
// non-zero entryID lets the engine-owned on_enter cases below dispatch;
// this is this Build round's E1-only scope boundary, not a general
// precondition of the marker CAS mechanism itself.
func (s *Service) processOnEnter(ctx context.Context, taskID string, session *models.TaskSession, step *wfmodels.WorkflowStep, taskDescription string, entryID int64) {
// One GetWorkflowMeta read shared by profile resolution and prompt build.
ctx = withWorkflowMetaCache(ctx)
@@ -2421,23 +2520,28 @@ func (s *Service) processOnEnter(ctx context.Context, taskID string, session *mo
// auto-start prompt is dispatched. It never switches or creates a tab.
s.applyWorkflowSessionConfigOnEnter(ctx, taskID, session, step)
hasAutoStart := false
for _, action := range step.Events.OnEnter {
switch action.Type {
case wfmodels.OnEnterEnablePlanMode:
// Skip plan mode for passthrough — CLI manages its own state.
// Also skip if agent doesn't support MCP (hasPlanMode is already false above).
if !isPassthrough && hasPlanMode {
s.setSessionPlanMode(ctx, session, true)
}
case wfmodels.OnEnterSetSessionMode:
mode, _ := action.Config["mode"].(string)
s.applyStepSessionMode(ctx, session, mode, isPassthrough)
case wfmodels.OnEnterAutoStartAgent:
hasAutoStart = true
}
dispatchResult := s.dispatchOnEnterActions(ctx, taskID, session, step, entryID, isPassthrough, hasPlanMode)
if dispatchResult.aborted {
return
}
s.launchAfterOnEnterDispatch(ctx, taskID, session, step, taskDescription, hasPlanMode, dispatchResult.hasAutoStart, sessionSwitched)
}
// launchAfterOnEnterDispatch runs the auto-start decision that follows
// on_enter action dispatch: passthrough vs. ACP auto-start, the
// profile-switch fallback launch, and the plain wait-for-input path.
// Extracted from processOnEnter to keep that function's complexity within
// the repo's lint thresholds.
//
//nolint:cyclop,funlen,gocognit // profile-switch recovery has independent terminal and retry branches.
func (s *Service) launchAfterOnEnterDispatch(
ctx context.Context, taskID string, session *models.TaskSession, step *wfmodels.WorkflowStep,
taskDescription string, hasPlanMode, hasAutoStart, sessionSwitched bool,
) {
sessionID := session.ID
isPassthrough := s.agentManager.IsPassthroughSession(ctx, sessionID)
switch {
case hasAutoStart && isPassthrough && session.State != models.TaskSessionStateCreated:
// Started passthrough path: write prompt directly to PTY stdin.
@@ -2605,9 +2709,6 @@ func (s *Service) processOnEnter(ctx context.Context, taskID string, session *mo
}()
return
}
// Same active-turn guard as the no-on_enter branch above: if the agent
// is still mid-turn, leave state alone so handleAgentReady can run on
// turn end. See that branch for the full rationale.
if session.State == models.TaskSessionStateRunning || session.State == models.TaskSessionStateStarting {
return
}
@@ -2620,6 +2721,126 @@ func (s *Service) processOnEnter(ctx context.Context, taskID string, session *mo
// from inline processOnEnter.
s.drainQueuedMessageForPromptableSession(ctx, sessionID)
}
}
// dispatchEngineOwnedOnEnterAction executes an engine-owned on_enter action
// once for this step-entry. The marker CAS prevents duplicate execution.
func (s *Service) dispatchEngineOwnedOnEnterAction(
ctx context.Context, taskID string, step *wfmodels.WorkflowStep, action wfmodels.OnEnterAction, position int, entryID int64,
) (abandon, failed bool, cause string) {
if entryID == 0 {
return false, false, ""
}
compiled, ok := engine.CompileOnEnterAction(action)
if !ok {
return false, false, ""
}
callback := s.engineOnEnterCallback(action.Type)
if callback == nil {
return false, false, ""
}
operationID := fmt.Sprintf("step_entry:%d:%d", entryID, position)
claimed, err := s.repo.ClaimStepEntryMarker(ctx, entryID, position, string(action.Type), operationID, time.Now())
if err != nil {
s.logger.Error("processOnEnter: claim step entry marker failed",
zap.Error(err), zap.String("task_id", taskID), zap.String("step_id", step.ID),
zap.Int64("entry_id", entryID), zap.Int("position", position))
return false, true, err.Error()
}
if !claimed {
priorState, priorCause, found, stateErr := s.repo.GetStepEntryMarkerState(ctx, entryID, position)
if stateErr != nil || !found {
return false, false, ""
}
switch priorState {
case stepentry.MarkerFailed:
return false, true, priorCause
case stepentry.MarkerInProgress:
return true, false, ""
default:
return false, false, ""
}
}
if action.Type == wfmodels.OnEnterClearDecisions {
if handled, atomicFailed, atomicCause := s.dispatchClearDecisionsAtomic(ctx, taskID, step, entryID, position); handled {
return false, atomicFailed, atomicCause
}
}
in := engine.ActionInput{
Trigger: engine.TriggerOnEnter,
State: engine.MachineState{TaskID: taskID, CurrentStepID: step.ID, WorkflowID: step.WorkflowID},
Step: engine.StepSpec{ID: step.ID, WorkflowID: step.WorkflowID, Name: step.Name, Position: step.Position},
Action: compiled,
OperationID: operationID,
}
state, execCause := stepentry.MarkerDone, ""
if _, execErr := callback.Execute(ctx, in); execErr != nil {
state, execCause = stepentry.MarkerFailed, execErr.Error()
s.logger.Error("processOnEnter: engine-owned on_enter action failed",
zap.Error(execErr), zap.String("task_id", taskID), zap.String("step_id", step.ID),
zap.String("action_type", string(action.Type)))
}
if err := s.repo.CompleteStepEntryMarker(ctx, entryID, position, state, execCause, time.Now()); err != nil {
s.logger.Error("processOnEnter: complete step entry marker failed",
zap.Error(err), zap.String("task_id", taskID), zap.String("step_id", step.ID),
zap.Int64("entry_id", entryID), zap.Int("position", position))
}
return false, state == stepentry.MarkerFailed, execCause
}
// dispatchClearDecisionsAtomic executes clear_decisions through
// Repository.ClearStepDecisionsAndCompleteMarker (AC-B6) when it is safe to
// do so: only when engineDecisions is confirmed to be
// *workflowadapters.DecisionAdapter, the type SetEngineDecisionStore is
// wired with at the one production construction site
// (backendapp/main.go:1501, NewDecisionAdapter(repos.Workflow) —
// repos.Workflow is always the real workflow repository, sharing s.repo's
// writer *sqlx.DB per backendapp/storage.go). Test doubles
// (fakeDecisionStore, failingDecisionStore in step_entry_dispatch_test.go)
// are not that type — no test constructs a DecisionAdapter — so handled is
// false for them and the caller falls back to the pre-existing, non-atomic
// callback.Execute + CompleteStepEntryMarker sequence, preserving every
// existing AC-C2/AC-D3/AC-D4 test's error-injection behavior unchanged.
//
// When handled is true, failed/cause report the outcome exactly like
// dispatchEngineOwnedOnEnterAction's own return values. On error, the
// transaction rolled back — neither the delete nor the marker update
// committed, so the marker is left in_progress, which is precisely AC-B6's
// invariant (in_progress proves the delete did not commit). No compensating
// write is attempted; returning failed=true is enough to trigger AC-C2's
// abort in dispatchOnEnterActions.
func (s *Service) dispatchClearDecisionsAtomic(
ctx context.Context, taskID string, step *wfmodels.WorkflowStep, entryID int64, position int,
) (handled, failed bool, cause string) {
if _, ok := s.engineDecisions.(*workflowadapters.DecisionAdapter); !ok {
return false, false, ""
}
if _, err := s.repo.ClearStepDecisionsAndCompleteMarker(ctx, taskID, step.ID, entryID, position, time.Now()); err != nil {
s.logger.Error("processOnEnter: atomic clear_decisions failed",
zap.Error(err), zap.String("task_id", taskID), zap.String("step_id", step.ID),
zap.Int64("entry_id", entryID), zap.Int("position", position))
return true, true, err.Error()
}
return true, false, ""
}
// engineOnEnterCallback returns the Phase 2 callback for an engine-owned
// on_enter action kind. The returned callback's Execute reports its own
// ErrActionNotYetWired when the required adapter isn't wired (kanban-only
// deployments) — that surfaces as a failed marker with a clear cause rather
// than a silent no-op, matching AC-A6's "never discard" intent.
func (s *Service) engineOnEnterCallback(t wfmodels.OnEnterActionType) engine.ActionCallback {
switch t {
case wfmodels.OnEnterClearDecisions:
return engine.ClearDecisionsCallback{Decisions: s.engineDecisions}
case wfmodels.OnEnterQueueRunForEachParticipant:
return engine.QueueRunForEachParticipantCallback{Adapter: s.engineRunQueue, Participants: s.engineParticipants}
default:
return nil
}
}
// applyPendingMove applies a deferred move_task_kandev call now that the agent's
@@ -4252,6 +4473,12 @@ func (s *Service) processOnTurnCompleteViaEngineWithCause(
return false
}
unlock, task, proceed := s.acquireTurnCompletionCriticalSection(ctx, taskID, session, task)
defer unlock()
if !proceed {
return false
}
if s.workflowEngine == nil {
return s.processOnTurnCompleteWithCause(ctx, task, session, cause)
}
@@ -4294,6 +4521,91 @@ func (s *Service) processOnTurnCompleteViaEngineWithCause(
return s.applyEngineTransition(ctx, taskID, session, result, engine.TriggerOnTurnComplete, task.Description, true)
}
// acquireTurnCompletionCriticalSection serializes on_turn_complete
// processing per session (see turnCompletionLocks' field comment for the
// double-allocation/double-dispatch race this closes) and detects a
// duplicate call that lost the race for the lock. proceed is false when
// this call must stop and treat itself as a redundant duplicate; the
// caller must still invoke unlock regardless of proceed. session == nil
// callers (none exist today, but the type allows it) skip locking
// entirely, matching pre-fix behavior.
//
// Two independent duplicate signals are checked, because neither alone
// covers every case:
//
// 1. Lock contention (acquireTurnCompletionLock's contended return).
// Several independent event sources (normal agent turn completion,
// agent-exit, step_complete_kandev's out-of-band signal, user
// cancellation) can each call this for the same session; if this call
// had to wait for another one already in flight for the same session,
// that other call is processing (or has just finished processing) the
// same physical turn, so this one backs off unconditionally — no DB
// read is needed or even safe to reason from, since a call that lost
// the lock race has no reliable way to tell whether the winner's
// commit is what it would observe. This is the only signal that
// reliably catches a duplicate whose entire pre-lock read happens
// *after* the winner already committed: e.g. two concurrent normal
// agent-turn-completion signals for the same physical turn (one via
// the ready path, one via agent-exit), where the second caller's very
// first DB read can already reflect the first caller's full commit,
// making any comparison of "my early read" vs. "my read under the
// lock" pass trivially even though the two calls are genuine
// duplicates. Contention can only be true when two calls for the same
// session truly overlap in time, so it never misfires against a
// legitimate call that arrives well after a prior one has finished
// (cancellation reconciliation, onStepCompletionSignaled's retry,
// clarification-dismissal) — those find the lock free.
// 2. The WorkflowStepID comparison (observedTask vs. a fresh read taken
// after acquiring the lock without contention). This catches a
// duplicate whose pre-lock snapshot predates the winner's commit: the
// step moved out from under it while it waited for the (uncontended,
// from this call's perspective) lock.
// 3. The session-generation comparison (turnCompletionConsumedGeneration):
// closes the one gap the first two leave open. When two calls for the
// same session are fully sequential — no true overlap, so contention
// never fires — the second caller's own pre-lock GetTask already
// reflects the first caller's commit, so signal 2 also passes
// trivially: current and observedTask agree, just both already at the
// winner's destination. Nothing in either DB read distinguishes that
// from a legitimate, later call that happens to arrive once the
// session already sits at that step. What does distinguish them is
// the caller-supplied session snapshot's UpdatedAt: a genuinely new
// call only exists because something touched the session after the
// winner's transition committed (a new turn starting, cancellation
// reconciliation, ...), which bumps UpdatedAt past the generation the
// winner recorded. A duplicate redelivery carries the same (or an
// older) snapshot it was constructed from before that commit, so its
// UpdatedAt is not strictly newer, and it is rejected here instead of
// being evaluated against the step the winner already moved to.
func (s *Service) acquireTurnCompletionCriticalSection(
ctx context.Context, taskID string, session *models.TaskSession, observedTask *models.Task,
) (unlock func(), fresh *models.Task, proceed bool) {
if session == nil {
return func() {}, observedTask, true
}
unlock, contended := s.acquireTurnCompletionLock(session.ID)
if contended {
return unlock, observedTask, false
}
if lastConsumed, ok := s.turnCompletionConsumedGeneration.Load(session.ID); ok {
if !session.UpdatedAt.After(lastConsumed.(time.Time)) {
return unlock, observedTask, false
}
}
current, err := s.repo.GetTask(ctx, taskID)
if err != nil {
// Best-effort re-check: fall back to the already-validated snapshot
// rather than fail a call that succeeded its primary read.
s.turnCompletionConsumedGeneration.Store(session.ID, session.UpdatedAt)
return unlock, observedTask, true
}
if current.WorkflowStepID != observedTask.WorkflowStepID {
return unlock, current, false
}
s.turnCompletionConsumedGeneration.Store(session.ID, session.UpdatedAt)
return unlock, current, true
}
func (s *Service) prepareEngineTurnCompletion(
ctx context.Context,
taskID string,
@@ -4527,7 +4839,18 @@ func (s *Service) applyEngineTransitionWithCommit(
s.processOnExit(ctx, taskID, session, fromStep)
}
applied, err := commit(ctx)
// A ResultHolder is only attached when this transition will actually
// trigger on_enter (triggerOnEnter) — an on_turn_start transition never
// reaches launchProcessOnEnter below, so allocating a step-entry it will
// never dispatch would be a needless write. See applyTransition's
// matching gate in workflow_store.go.
var stepEntry *stepentry.AllocationResult
applyCtx := ctx
if triggerOnEnter {
stepEntry = &stepentry.AllocationResult{}
applyCtx = stepentry.WithResultHolder(applyCtx, stepEntry)
}
applied, err := commit(applyCtx)
if err != nil {
s.logger.Error("failed to apply engine transition",
zap.String("task_id", taskID),
@@ -4629,7 +4952,11 @@ func (s *Service) applyEngineTransitionWithCommit(
// ResetAgentContext → sendStreamRequest, which blocks G_reader waiting for a response
// that can only be delivered by G_reader reading from the same WebSocket — a deadlock.
// The DB transition is already persisted above, so it's safe to process on_enter async.
s.launchProcessOnEnter(context.WithoutCancel(ctx), taskID, session, targetStep, taskDescription)
var stepEntryID int64
if stepEntry != nil {
stepEntryID = stepEntry.EntryID
}
s.launchProcessOnEnter(context.WithoutCancel(ctx), taskID, session, targetStep, taskDescription, stepEntryID)
return true
}
@@ -4639,6 +4966,7 @@ func (s *Service) launchProcessOnEnter(
session *models.TaskSession,
targetStep *wfmodels.WorkflowStep,
taskDescription string,
entryID int64,
) {
go func() {
defer func() {
@@ -4646,7 +4974,7 @@ func (s *Service) launchProcessOnEnter(
s.onProcessOnEnterComplete()
}
}()
s.processOnEnter(ctx, taskID, session, targetStep, taskDescription)
s.processOnEnter(ctx, taskID, session, targetStep, taskDescription, entryID)
}()
}
apps/backend/internal/orchestrator/event_handlers_workflow_lazy_resume_test.go
+1 -1
View File
@@ -49,7 +49,7 @@ func TestProcessOnEnterResetAgentContext_ClearsLazyResumeTokenWithoutLiveExecuti
t.Fatalf("load session: %v", err)
}
svc.processOnEnter(ctx, "task-lazy-resume", session, step, "review task")
svc.processOnEnter(ctx, "task-lazy-resume", session, step, "review task", 0)
if len(agentManager.restartProcessCalls) != 0 {
t.Fatalf("expected no reset against a missing live execution, got %d calls", len(agentManager.restartProcessCalls))
apps/backend/internal/orchestrator/event_handlers_workflow_on_enter_warn_test.go
+83
View File
@@ -0,0 +1,83 @@
package orchestrator
import (
"context"
"testing"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/task/models"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
)
// TestProcessOnEnter_UnrecognizedActionType_WarnsAndContinuesDispatch covers
// AC-A6 — the bug report's own headline acceptance bullet: "An on_enter
// action the handler does not recognise emits a WARNING naming the step and
// action type. The silent discard is what made this take a full afternoon
// to find." Build round 1 (commit d4c3dd42a) added the default-branch
// warning in dispatchOnEnterActions but shipped with zero test coverage for
// it; Testing round 1 verified the behavior with an uncommitted throwaway
// probe and routed the card back for exactly this permanent test.
func TestProcessOnEnter_UnrecognizedActionType_WarnsAndContinuesDispatch(t *testing.T) {
ctx := context.Background()
repo := setupTestRepo(t)
seedSession(t, repo, "task-a6", "session-a6", "step-a6")
session, err := repo.GetTaskSession(ctx, "session-a6")
if err != nil {
t.Fatalf("load session: %v", err)
}
agent := &mockAgentManager{isAgentRunning: true}
svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), agent)
core, logs := observer.New(zapcore.WarnLevel)
log, err := logger.NewFromZap(zap.New(core))
if err != nil {
t.Fatalf("build observed logger: %v", err)
}
svc.logger = log
// A genuinely unrecognized action type followed by a real one: the
// warning must not abort the loop, so set_session_mode must still run
// and persist.
step := &wfmodels.WorkflowStep{
ID: "step-a6",
WorkflowID: "wf-a6",
Name: "AC-A6 Step",
Events: wfmodels.StepEvents{OnEnter: []wfmodels.OnEnterAction{
{Type: wfmodels.OnEnterActionType("totally_bogus_action")},
{Type: wfmodels.OnEnterSetSessionMode, Config: map[string]interface{}{"mode": "acceptEdits"}},
}},
}
svc.processOnEnter(ctx, "task-a6", session, step, "", 0)
var warnings []observer.LoggedEntry
for _, e := range logs.All() {
if e.Message == "processOnEnter: unrecognized on_enter action type" {
warnings = append(warnings, e)
}
}
if len(warnings) != 1 {
t.Fatalf("got %d WARNING(s) for the unrecognized action, want exactly 1 (all entries: %+v)", len(warnings), logs.All())
}
fields := warnings[0].ContextMap()
if fields["step_id"] != "step-a6" {
t.Errorf("warning step_id = %v, want %q", fields["step_id"], "step-a6")
}
if fields["action_type"] != "totally_bogus_action" {
t.Errorf("warning action_type = %v, want %q", fields["action_type"], "totally_bogus_action")
}
reloaded, err := repo.GetTaskSession(ctx, "session-a6")
if err != nil {
t.Fatalf("reload session: %v", err)
}
if got := reloaded.Metadata[models.SessionMetaKeySessionMode]; got != "acceptEdits" {
t.Errorf("set_session_mode after the unrecognized action = %v, want %q — the warning must not abort dispatch of subsequent actions", got, "acceptEdits")
}
}
apps/backend/internal/orchestrator/event_handlers_workflow_profile_test.go
+6 -6
View File
@@ -1416,7 +1416,7 @@ func TestProcessOnEnter_ProfileSwitch(t *testing.T) {
scheduler: sched,
}
svc.processOnEnter(ctx, "t1", session, step, "desc")
svc.processOnEnter(ctx, "t1", session, step, "desc", 0)
// The old session should be completed
oldSession, err := repo.GetTaskSession(ctx, "s1")
@@ -1484,7 +1484,7 @@ func TestProcessOnEnter_ProfileSwitch(t *testing.T) {
sg.steps["step1"] = step
svc := createTestService(repo, sg, newMockTaskRepo())
svc.processOnEnter(ctx, "t1", session, step, "desc")
svc.processOnEnter(ctx, "t1", session, step, "desc", 0)
// Session should remain running (not completed)
updatedSession, err := repo.GetTaskSession(ctx, "s1")
@@ -1548,7 +1548,7 @@ func TestProcessOnEnter_ProfileSwitch(t *testing.T) {
agentMgr := &mockAgentManager{isPassthrough: true}
svc := createTestServiceWithAgent(repo, sg, newMockTaskRepo(), agentMgr)
svc.processOnEnter(ctx, "t1", session, step, "desc")
svc.processOnEnter(ctx, "t1", session, step, "desc", 0)
// Session should NOT be completed (passthrough skips profile switch)
updatedSession, err := repo.GetTaskSession(ctx, "s1")
@@ -1596,7 +1596,7 @@ func TestProcessOnEnter_ProfileSwitch(t *testing.T) {
sg.steps["step1"] = step
svc := createTestService(repo, sg, newMockTaskRepo())
svc.processOnEnter(ctx, "t1", session, step, "desc")
svc.processOnEnter(ctx, "t1", session, step, "desc", 0)
// Session should remain running
sessions, err := repo.ListTaskSessions(ctx, "t1")
@@ -1657,7 +1657,7 @@ func TestProcessOnEnter_ProfileSwitch(t *testing.T) {
sg.steps["step1"] = step
svc := createTestService(repo, sg, newMockTaskRepo())
svc.processOnEnter(ctx, "t1", session, step, "desc")
svc.processOnEnter(ctx, "t1", session, step, "desc", 0)
// Critical: no new profile-a session should be spawned, and the
// user-chosen profile-b session must NOT be marked COMPLETED.
@@ -1744,7 +1744,7 @@ func TestProcessOnEnter_ProfileSwitch(t *testing.T) {
scheduler: sched,
}
svc.processOnEnter(ctx, "t1", session, step, "desc")
svc.processOnEnter(ctx, "t1", session, step, "desc", 0)
updated, err := repo.GetTaskSession(ctx, "s1")
if err != nil {
apps/backend/internal/orchestrator/event_handlers_workflow_prompt_claim_test.go
+2 -2
View File
@@ -459,7 +459,7 @@ func testProcessOnEnterTerminalizedAfterPromptClaim(t *testing.T, terminalState
done := make(chan struct{})
go func() {
svc.processOnEnter(ctx, source.TaskID, source, step, "Test")
svc.processOnEnter(ctx, source.TaskID, source, step, "Test", 0)
close(done)
}()
@@ -756,7 +756,7 @@ func testProcessOnEnterImplicitProfileSwitchTerminalizedGuard(
done := make(chan struct{})
go func() {
svc.processOnEnter(ctx, source.TaskID, source, step, "Test")
svc.processOnEnter(ctx, source.TaskID, source, step, "Test", 0)
close(done)
}()
apps/backend/internal/orchestrator/event_handlers_workflow_session_config_test.go
+14 -14
View File
@@ -31,7 +31,7 @@ func TestProcessOnEnterConfigureSessionAppliesMatchingOriginalSession(t *testing
svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), agent)
step := configureSessionStep("step-config", "codex", "set", "gpt-5.6-luna", map[string]string{"reasoning_effort": "max"})
svc.processOnEnter(ctx, "task-config", session, step, "")
svc.processOnEnter(ctx, "task-config", session, step, "", 0)
if len(agent.setSessionModelCalls) != 1 || agent.setSessionModelCalls[0].ModelID != "gpt-5.6-luna" {
t.Fatalf("model calls = %#v, want one luna switch", agent.setSessionModelCalls)
@@ -70,7 +70,7 @@ func TestProcessOnEnterConfigureSessionSkipsNonMatchingAgent(t *testing.T) {
svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), agent)
step := configureSessionStep("step-config-skip", "codex", "set", "gpt-5.6-luna", map[string]string{"reasoning_effort": "max"})
svc.processOnEnter(ctx, "task-config-skip", session, step, "")
svc.processOnEnter(ctx, "task-config-skip", session, step, "", 0)
if len(agent.setSessionModelCalls) != 0 || len(agent.setSessionConfigCalls) != 0 {
t.Fatalf("non-matching agent was configured: model=%#v config=%#v", agent.setSessionModelCalls, agent.setSessionConfigCalls)
@@ -104,7 +104,7 @@ func TestProcessOnEnterConfigureSessionRestoreOriginal(t *testing.T) {
svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), agent)
step := configureSessionStep("step-config-restore", "claude", "restore_original", "", nil)
svc.processOnEnter(ctx, "task-config-restore", session, step, "")
svc.processOnEnter(ctx, "task-config-restore", session, step, "", 0)
if len(agent.setSessionModelCalls) != 1 || agent.setSessionModelCalls[0].ModelID != "claude-sonnet-original" {
t.Fatalf("restore model calls = %#v", agent.setSessionModelCalls)
@@ -175,7 +175,7 @@ func TestProcessOnEnterConfigureSessionMatchesAgentDisplayName(t *testing.T) {
svc.messageCreator = messages
step := configureSessionStep("step-config-display", "Claude", "set", "opus[1m]", nil)
svc.processOnEnter(ctx, "task-config-display", session, step, "")
svc.processOnEnter(ctx, "task-config-display", session, step, "", 0)
if len(agent.setSessionModelCalls) != 1 || agent.setSessionModelCalls[0].ModelID != "opus[1m]" {
t.Fatalf("model calls = %#v, want one opus[1m] switch", agent.setSessionModelCalls)
@@ -217,7 +217,7 @@ func TestProcessOnEnterConfigureSessionUnknownAgentFamilyWarns(t *testing.T) {
svc.messageCreator = messages
step := configureSessionStep("step-config-unknown", "grok-9000", "set", "opus[1m]", nil)
svc.processOnEnter(ctx, "task-config-unknown", session, step, "")
svc.processOnEnter(ctx, "task-config-unknown", session, step, "", 0)
if len(agent.setSessionModelCalls) != 0 {
t.Fatalf("unknown family was configured: %#v", agent.setSessionModelCalls)
@@ -251,7 +251,7 @@ func TestProcessOnEnterConfigureSessionKnownOtherAgentStaysSilent(t *testing.T)
svc.messageCreator = messages
step := configureSessionStep("step-config-other", "Codex", "set", "gpt-5.6-luna", nil)
svc.processOnEnter(ctx, "task-config-other", session, step, "")
svc.processOnEnter(ctx, "task-config-other", session, step, "", 0)
if len(agent.setSessionModelCalls) != 0 {
t.Fatalf("non-matching agent was configured: %#v", agent.setSessionModelCalls)
@@ -324,7 +324,7 @@ func TestProcessOnEnterConfigureSessionWithoutResolverRequiresExactAgentName(t *
svc.agentFamilyResolver = nil
svc.processOnEnter(ctx, "task-config-noresolver", session,
configureSessionStep("step-config-noresolver", "Claude", "set", "opus[1m]", nil), "")
configureSessionStep("step-config-noresolver", "Claude", "set", "opus[1m]", nil), "", 0)
if len(agent.setSessionModelCalls) != 0 {
t.Fatalf("display name matched without a resolver: %#v", agent.setSessionModelCalls)
}
@@ -335,7 +335,7 @@ func TestProcessOnEnterConfigureSessionWithoutResolverRequiresExactAgentName(t *
}
svc.processOnEnter(ctx, "task-config-noresolver", session,
configureSessionStep("step-config-noresolver", "claude-acp", "set", "opus[1m]", nil), "")
configureSessionStep("step-config-noresolver", "claude-acp", "set", "opus[1m]", nil), "", 0)
if len(agent.setSessionModelCalls) != 1 || agent.setSessionModelCalls[0].ModelID != "opus[1m]" {
t.Fatalf("exact agent name did not match without a resolver: %#v", agent.setSessionModelCalls)
}
@@ -461,7 +461,7 @@ func TestProcessOnEnterConfigureSessionAmbiguousAgentFamilyWarns(t *testing.T) {
svc.agentFamilyResolver = shadowed
svc.processOnEnter(ctx, "task-ambiguous", session,
configureSessionStep("step-ambiguous", "Claude", "set", "opus[1m]", nil), "")
configureSessionStep("step-ambiguous", "Claude", "set", "opus[1m]", nil), "", 0)
if len(agent.setSessionModelCalls) != 0 {
t.Fatalf("ambiguous family was applied to an agent: %#v", agent.setSessionModelCalls)
@@ -503,7 +503,7 @@ func TestProcessOnEnterConfigureSessionUnrelatedAmbiguousRuleStillApplies(t *tes
svc.processOnEnter(ctx, "task-unrelated", session, multiRuleConfigureSessionStep("step-unrelated",
configureSessionRule{agentName: "Codex", model: "gpt-5.6-luna"},
configureSessionRule{agentName: "Claude", model: "opus[1m]"},
), "")
), "", 0)
if len(agent.setSessionModelCalls) != 1 || agent.setSessionModelCalls[0].ModelID != "gpt-5.6-luna" {
t.Fatalf("model calls = %#v, want one gpt-5.6-luna switch", agent.setSessionModelCalls)
@@ -567,7 +567,7 @@ func TestProcessOnEnterConfigureSessionRelevantAmbiguityRefusesDespiteExactRule(
svc.agentFamilyResolver = shadowed
svc.processOnEnter(ctx, "task-relevant", session,
multiRuleConfigureSessionStep("step-relevant", tc.rules...), "")
multiRuleConfigureSessionStep("step-relevant", tc.rules...), "", 0)
if len(agent.setSessionModelCalls) != 0 {
t.Fatalf("relevant ambiguity applied a model: %#v", agent.setSessionModelCalls)
@@ -610,7 +610,7 @@ func TestProcessOnEnterConfigureSessionAmbiguousSessionFamilyRefuses(t *testing.
// The rule names one agent unambiguously; the session is the ambiguous side.
svc.processOnEnter(ctx, "task-session-ambiguous", session,
configureSessionStep("step-session-ambiguous", "claude-acp", "set", "opus[1m]", nil), "")
configureSessionStep("step-session-ambiguous", "claude-acp", "set", "opus[1m]", nil), "", 0)
if len(agent.setSessionModelCalls) != 0 {
t.Fatalf("ambiguous session family applied a model: %#v", agent.setSessionModelCalls)
@@ -640,7 +640,7 @@ func TestProcessOnEnterConfigureSessionConflictingRulesWarn(t *testing.T) {
svc.processOnEnter(ctx, "task-conflict", session, multiRuleConfigureSessionStep("step-conflict",
configureSessionRule{agentName: "Claude", model: "opus[1m]"},
configureSessionRule{agentName: "claude-acp", model: "sonnet"},
), "")
), "", 0)
if len(agent.setSessionModelCalls) != 0 {
t.Fatalf("conflicting rules applied a model: %#v", agent.setSessionModelCalls)
@@ -669,7 +669,7 @@ func TestProcessOnEnterConfigureSessionDistinctFamiliesStillApply(t *testing.T)
configureSessionRule{agentName: "Codex", model: "gpt-5.6-luna"},
configureSessionRule{agentName: "Claude", model: "opus[1m]"},
configureSessionRule{agentName: "Gemini", model: "gemini-pro"},
), "")
), "", 0)
if len(agent.setSessionModelCalls) != 1 || agent.setSessionModelCalls[0].ModelID != "opus[1m]" {
t.Fatalf("model calls = %#v, want one opus[1m] switch", agent.setSessionModelCalls)
apps/backend/internal/orchestrator/event_handlers_workflow_triggers_test.go
+15 -15
View File
@@ -340,7 +340,7 @@ func TestProcessOnEnterSetSessionMode(t *testing.T) {
}
session, _ := repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "test task")
svc.processOnEnter(ctx, "t1", session, step, "test task", 0)
updated, _ := repo.GetTaskSession(ctx, "s1")
if got, _ := updated.Metadata[models.SessionMetaKeySessionMode].(string); got != "acceptEdits" {
@@ -374,7 +374,7 @@ func TestProcessOnEnter(t *testing.T) {
}
session, _ := repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "test task")
svc.processOnEnter(ctx, "t1", session, step, "test task", 0)
session, _ = repo.GetTaskSession(ctx, "s1")
if session.Metadata == nil {
@@ -403,7 +403,7 @@ func TestProcessOnEnter(t *testing.T) {
}
session, _ = repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "test task")
svc.processOnEnter(ctx, "t1", session, step, "test task", 0)
// Plan mode should persist — only explicit on_exit/on_turn_complete
// disable_plan_mode actions should clear it.
@@ -445,7 +445,7 @@ func TestProcessOnEnter(t *testing.T) {
}
session, _ = repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, reviewStep, "task description")
svc.processOnEnter(ctx, "t1", session, reviewStep, "task description", 0)
select {
case <-agentMgr.promptDone:
@@ -492,7 +492,7 @@ func TestProcessOnEnter(t *testing.T) {
}
session, _ := repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "queued prompt content")
svc.processOnEnter(ctx, "t1", session, step, "queued prompt content", 0)
deadline := time.Now().Add(2 * time.Second)
for {
@@ -744,7 +744,7 @@ func TestProcessOnEnterPassthrough(t *testing.T) {
}
session, _ := repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "test task")
svc.processOnEnter(ctx, "t1", session, step, "test task", 0)
session, _ = repo.GetTaskSession(ctx, "s1")
if session.Metadata != nil {
@@ -771,7 +771,7 @@ func TestProcessOnEnterPassthrough(t *testing.T) {
}
session, _ = repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "test task")
svc.processOnEnter(ctx, "t1", session, step, "test task", 0)
// plan_mode should still be set since passthrough sessions skip plan mode management
updated, _ := repo.GetTaskSession(ctx, "s1")
@@ -849,7 +849,7 @@ func TestProcessOnEnterResetAgentContext(t *testing.T) {
}
session, _ = repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "review task")
svc.processOnEnter(ctx, "t1", session, step, "review task", 0)
published := eventBus.published()
if len(published) < 2 {
@@ -960,7 +960,7 @@ func TestProcessOnEnterResetAgentContext(t *testing.T) {
}
session, _ = repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "review task")
svc.processOnEnter(ctx, "t1", session, step, "review task", 0)
// Verify RestartAgentProcess was called with the correct execution ID
if len(agentMgr.restartProcessCalls) != 1 {
@@ -996,7 +996,7 @@ func TestProcessOnEnterResetAgentContext(t *testing.T) {
}
session, _ := repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "review task")
svc.processOnEnter(ctx, "t1", session, step, "review task", 0)
// Verify RestartAgentProcess was NOT called (no execution ID)
if len(agentMgr.restartProcessCalls) != 0 {
@@ -1135,7 +1135,7 @@ func TestProcessOnEnterResetAgentContext(t *testing.T) {
}
session, _ = repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "Run the work")
svc.processOnEnter(ctx, "t1", session, step, "Run the work", 0)
select {
case <-startAgentProcessCalled:
case <-time.After(time.Second):
@@ -1177,7 +1177,7 @@ func TestProcessOnEnterResetAgentContext(t *testing.T) {
}
session, _ = repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "review task")
svc.processOnEnter(ctx, "t1", session, step, "review task", 0)
// Verify RestartAgentProcess was called even for passthrough sessions
if len(agentMgr.restartProcessCalls) != 1 {
@@ -1211,7 +1211,7 @@ func TestProcessOnEnterResetAgentContext(t *testing.T) {
}
session, _ = repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "review task")
svc.processOnEnter(ctx, "t1", session, step, "review task", 0)
updated, _ := repo.GetTaskSession(ctx, "s1")
if updated.State != models.TaskSessionStateWaitingForInput {
@@ -1255,7 +1255,7 @@ func TestProcessOnEnterResetAgentContext(t *testing.T) {
t.Fatalf("precondition: seed should start session as RUNNING, got %q", session.State)
}
svc.processOnEnter(ctx, "t1", session, step, "review task")
svc.processOnEnter(ctx, "t1", session, step, "review task", 0)
// In-memory session must also be updated — queueAutoStartPromptIfRunning
// (further down in processOnEnter when auto_start_agent is present)
@@ -1304,7 +1304,7 @@ func TestProcessOnEnterResetAgentContext(t *testing.T) {
}
session, _ = repo.GetTaskSession(ctx, "s1")
svc.processOnEnter(ctx, "t1", session, step, "review task")
svc.processOnEnter(ctx, "t1", session, step, "review task", 0)
// Positive assertion: pin the expected state to RUNNING so any other
// unintended mutation (COMPLETED, FAILED, etc.) also fails the test,
apps/backend/internal/orchestrator/service.go
+70
View File
@@ -37,6 +37,7 @@ import (
taskservice "github.com/kandev/kandev/internal/task/service"
"github.com/kandev/kandev/internal/workflow/engine"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
"github.com/kandev/kandev/internal/workflow/stepentry"
v1 "github.com/kandev/kandev/pkg/api/v1"
)
@@ -348,6 +349,22 @@ type sessionExecutorStore interface {
GetTaskEnvironmentByTaskID(ctx context.Context, taskID string) (*models.TaskEnvironment, error)
CreateTaskEnvironment(ctx context.Context, env *models.TaskEnvironment) error
UpdateTaskEnvironment(ctx context.Context, env *models.TaskEnvironment) error
// Step-entry CAS markers (see internal/workflow/stepentry) — claim/complete
// an engine-owned on_enter action at most once per step-entry.
ClaimStepEntryMarker(ctx context.Context, entryID int64, position int, kind, operationID string, claimedAt time.Time) (bool, error)
CompleteStepEntryMarker(ctx context.Context, entryID int64, position int, state stepentry.MarkerState, cause string, completedAt time.Time) error
// GetStepEntryMarkerState reads back a marker's terminal (or in-progress)
// state after a lost CAS claim, so the caller can tell "already failed"
// apart from "already done" or "still in progress elsewhere."
GetStepEntryMarkerState(ctx context.Context, entryID int64, position int) (state stepentry.MarkerState, cause string, found bool, err error)
// ClearStepDecisionsAndCompleteMarker satisfies AC-B6: clears every
// decision for (taskID, stepID) and completes the clear_decisions
// marker in one database transaction, so a crash between the two can
// never leave the marker showing in_progress after the delete already
// committed. Only usable when the DecisionStore backing clear_decisions
// shares this repository's writer DB — see the capability check at
// dispatchClearDecisionsAtomic's call site.
ClearStepDecisionsAndCompleteMarker(ctx context.Context, taskID, stepID string, entryID int64, position int, now time.Time) (int64, error)
}
// ClaimTaskTitleSession claims the first-turn generated-title handoff for a
@@ -907,6 +924,33 @@ type Service struct {
// Entries are not deleted: deleting a lock can let a new caller create a
// second mutex while an existing waiter still owns the old one.
sessionLifecycleLocks sync.Map // map[sessionID]*sync.Mutex
// turnCompletionLocks serializes processOnTurnCompleteViaEngineWithCause
// per session. Several independent event sources (normal agent turn
// completion, agent-exit, step_complete_kandev's out-of-band signal,
// user cancellation) can each call it for the same session; without
// serialization, two concurrent calls can both read the same
// pre-transition engine state and each call applyEngineTransition,
// allocating two independent workflow_step_entries rows for what should
// be a single logical transition and double-dispatching that entry's
// on_enter actions (clear_decisions, queue_run_for_each_participant).
// Entries are not deleted — see sessionLifecycleLocks above for why.
turnCompletionLocks sync.Map // map[sessionID]*sync.Mutex
// turnCompletionConsumedGeneration records, per session, the
// TaskSession.UpdatedAt of the most recent session snapshot that was
// already let past acquireTurnCompletionCriticalSection. It closes the
// gap turnCompletionLocks' contention signal cannot: two calls whose
// entire lifetimes are fully sequential (the second's own pre-lock
// GetTask already reflects the first's commit, so the WorkflowStepID
// comparison passes trivially) never contend on the mutex at all. A
// caller's session argument only carries a newer UpdatedAt than the
// stored generation when something legitimately touched the session
// (a new turn starting, cancellation reconciliation, ...) after the
// last transition was consumed; a stale or identical snapshot — the
// hallmark of a duplicate delivery for the same physical turn — is
// rejected instead of being re-evaluated against the step the winner
// already moved to. See acquireTurnCompletionCriticalSection.
// Entries are not deleted — see sessionLifecycleLocks above for why.
turnCompletionConsumedGeneration sync.Map // map[sessionID]time.Time
// contextWindowGuards serializes live context usage writes and gives each
// session a generation boundary that reset_agent_context can invalidate.
contextWindowGuards sync.Map
@@ -2269,6 +2313,32 @@ func (s *Service) acquireSessionLifecycleLock(sessionID string) func() {
return lock.Unlock
}
// acquireTurnCompletionLock serializes on_turn_complete processing for a
// single session — see turnCompletionLocks' field comment for the race it
// closes. A caller with no session ID (defensive callers pass "" rather than
// skip the lock call) gets a no-op unlock and contended=false.
//
// contended reports whether another caller already held this session's lock
// at the moment this call tried to acquire it — i.e. another
// processOnTurnCompleteViaEngineWithCause call for the same session is (or
// very recently was) actively executing right now. That is a direct,
// timing-independent duplicate signal: unlike comparing two DB reads taken
// at different times (which a sufficiently late duplicate can pass
// trivially — see acquireTurnCompletionCriticalSection), lock contention can
// only be true when two calls for the same session genuinely overlapped.
func (s *Service) acquireTurnCompletionLock(sessionID string) (unlock func(), contended bool) {
if sessionID == "" {
return func() {}, false
}
value, _ := s.turnCompletionLocks.LoadOrStore(sessionID, &sync.Mutex{})
lock := value.(*sync.Mutex)
if lock.TryLock() {
return lock.Unlock, false
}
lock.Lock()
return lock.Unlock, true
}
func (s *Service) isSessionResetInProgress(sessionID string) bool {
if sessionID == "" {
return false
apps/backend/internal/orchestrator/step_entry_dispatch_atomic_test.go
+155
View File
@@ -0,0 +1,155 @@
package orchestrator
// step_entry_dispatch_atomic_test.go covers AC-B6: when engineDecisions is
// wired the way production actually wires it — workflowadapters.NewDecisionAdapter
// over the real workflow repository, sharing the same writer DB as the task
// repository (backendapp/main.go:1501, backendapp/storage.go) — clear_decisions
// must go through dispatchClearDecisionsAtomic's one-transaction path, not the
// generic callback.Execute + CompleteStepEntryMarker sequence the other
// step_entry_dispatch_test.go fixtures exercise with a fakeDecisionStore.
//
// This test cannot observe the transaction boundary directly (that is
// step_entries_test.go's job, at the repository layer), but it does prove the
// capability-detection wiring actually engages against a real
// *workflowadapters.DecisionAdapter — a wrong type assertion, a mismatched
// entryID/position, or a step_id mixup would surface here as decisions never
// clearing or the marker never completing, exactly like it would in
// production.
import (
"context"
"path/filepath"
"testing"
"github.com/jmoiron/sqlx"
_ "github.com/mattn/go-sqlite3"
"github.com/kandev/kandev/internal/db"
"github.com/kandev/kandev/internal/task/repository"
workflowadapters "github.com/kandev/kandev/internal/workflow/adapters"
"github.com/kandev/kandev/internal/workflow/engine"
workflowrepository "github.com/kandev/kandev/internal/workflow/repository"
"github.com/kandev/kandev/internal/workflow/stepentry"
v1 "github.com/kandev/kandev/pkg/api/v1"
)
// newSharedDBReviewLoopFixture is newReviewLoopFixture's sibling: it wires
// svc.engineDecisions to a real *workflowadapters.DecisionAdapter backed by a
// real *workflowrepository.Repository sharing the task repository's writer
// *sqlx.DB, matching production's backendapp/storage.go wiring exactly.
func newSharedDBReviewLoopFixture(t *testing.T) *reviewLoopFixture {
t.Helper()
sg, nameToID := buildWorkflowFromJSON(t, reviewLoopWorkflowJSON)
tmpDir := t.TempDir()
dbConn, err := db.OpenSQLite(filepath.Join(tmpDir, "test.db"))
if err != nil {
t.Fatalf("open test database: %v", err)
}
sqlxDB := sqlx.NewDb(dbConn, "sqlite3")
t.Cleanup(func() { _ = sqlxDB.Close() })
taskRepo, cleanup, err := repository.Provide(sqlxDB, sqlxDB, nil)
if err != nil {
t.Fatalf("create task repository: %v", err)
}
t.Cleanup(func() { _ = cleanup() })
workflowRepo, err := workflowrepository.NewWithDB(sqlxDB, sqlxDB, nil)
if err != nil {
t.Fatalf("create workflow repository: %v", err)
}
seedSession(t, taskRepo, "t1", "s1", nameToID["Work"])
setSessionExecID(t, taskRepo, "s1", "exec-1")
agentMgr := &mockAgentManager{repoForExecutionLookup: taskRepo, isAgentRunning: true}
svc := createEngineService(t, taskRepo, sg, agentMgr)
runQueue := &stepEntryFakeRunQueue{}
participants := &fakeParticipantStore{participants: []engine.ParticipantInfo{
{ID: "participant-1", StepID: nameToID["Review"], Role: "reviewer", AgentProfileID: "agent-reviewer-1"},
}}
svc.SetEngineRunQueue(runQueue)
svc.SetEngineParticipantStore(participants)
svc.SetEngineDecisionStore(workflowadapters.NewDecisionAdapter(workflowRepo))
mockRepo := svc.taskRepo.(*mockTaskRepo)
seedMockTaskState(mockRepo, "t1", v1.TaskStateInProgress)
return &reviewLoopFixture{
svc: svc, repo: taskRepo, agentMgr: agentMgr,
runQueue: runQueue, nameToID: nameToID, taskRepoMk: mockRepo,
}
}
// TestProcessOnEnter_ClearDecisions_RealDecisionAdapter_ClearsAndCompletesAtomically
// covers AC-B6 end to end: with engineDecisions wired as the real production
// adapter, an existing decision from a prior round is cleared and the
// clear_decisions marker lands done — the same observable outcome the
// fallback path produces, but now driven through
// Repository.ClearStepDecisionsAndCompleteMarker's single transaction rather
// than two independent calls.
func TestProcessOnEnter_ClearDecisions_RealDecisionAdapter_ClearsAndCompletesAtomically(t *testing.T) {
ctx := context.Background()
f := newSharedDBReviewLoopFixture(t)
reviewStepID := f.nameToID["Review"]
decisions := workflowadapters.NewDecisionAdapter(mustWorkflowRepoFromDecisionStore(t, f.svc))
if err := decisions.RecordStepDecision(ctx, engine.DecisionInfo{
ID: "dec-stale", TaskID: "t1", StepID: reviewStepID, ParticipantID: "participant-0", Decision: "approve",
}); err != nil {
t.Fatalf("seed stale decision: %v", err)
}
if !f.fireOnTurnComplete(t, ctx) {
t.Fatalf("expected a transition from Work -> Review, got none")
}
assertStepByName(t, ctx, f.repo, "s1", "Review", f.nameToID)
if got := f.runQueue.callCount(); got != 1 {
t.Fatalf("queued run count = %d, want exactly 1", got)
}
remaining, err := decisions.ListStepDecisions(ctx, "t1", reviewStepID)
if err != nil {
t.Fatalf("ListStepDecisions: %v", err)
}
if len(remaining) != 0 {
t.Fatalf("remaining decisions = %d, want 0 (AC-B6 atomic clear_decisions)", len(remaining))
}
// The run-queued assertion above only proves dispatchOnEnterActions
// continued past clear_decisions — it does not directly prove
// ClearStepDecisionsAndCompleteMarker's own marker-write half of its one
// transaction actually committed "done" rather than, say, leaving the
// marker in_progress while a downstream code path incorrectly continued
// anyway. entryID 1: the fresh per-test database's very first allocated
// workflow_step_entries row (same reasoning as
// step_entry_dispatch_cas_loser_test.go's sibling tests); position 0 is
// clear_decisions, the first action reviewLoopWorkflowJSON's Review step
// declares.
const entryID, clearDecisionsPosition = int64(1), 0
state, cause, found, err := f.repo.GetStepEntryMarkerState(ctx, entryID, clearDecisionsPosition)
if err != nil {
t.Fatalf("GetStepEntryMarkerState: %v", err)
}
if !found {
t.Fatalf("expected a clear_decisions marker to exist for entry %d position %d", entryID, clearDecisionsPosition)
}
if state != stepentry.MarkerDone {
t.Fatalf("clear_decisions marker state = %q (cause=%q), want %q (AC-B6: the atomic path must complete the marker inside the same transaction as the delete)", state, cause, stepentry.MarkerDone)
}
}
// mustWorkflowRepoFromDecisionStore extracts the *workflowadapters.DecisionAdapter
// wired by newSharedDBReviewLoopFixture, so the test can seed/read decisions
// through the identical repository instance the atomic path writes through.
func mustWorkflowRepoFromDecisionStore(t *testing.T, svc *Service) workflowadapters.WorkflowRepo {
t.Helper()
adapter, ok := svc.engineDecisions.(*workflowadapters.DecisionAdapter)
if !ok {
t.Fatalf("svc.engineDecisions is %T, want *workflowadapters.DecisionAdapter", svc.engineDecisions)
}
return adapter.Repo
}
apps/backend/internal/orchestrator/step_entry_dispatch_cas_loser_test.go
+255
View File
@@ -0,0 +1,255 @@
package orchestrator
// step_entry_dispatch_cas_loser_test.go covers two MEDIUM/P2 findings from
// this Build round's Review, both rooted in
// dispatchEngineOwnedOnEnterAction's !claimed branch:
//
// 1. It unconditionally returned "not failed", so a retry of a step-entry
// whose clear_decisions had already recorded a terminal failure would
// silently fall through to dispatching queue_run_for_each_participant —
// the exact fall-through AC-C2's break exists to close for a fresh
// failure, just reachable a second time through the CAS-loser path
// instead.
// 2. It also returned "not failed" — indistinguishable from "already
// done" — when the existing marker was still in_progress, i.e. a *live*
// concurrent dispatch of the same step entry hadn't finished yet.
// dispatchOnEnterActions only aborted on failed==true for
// clear_decisions specifically, so the loser of that live claim would
// skip ahead to queue_run_for_each_participant instead of abandoning
// the entry — violating docs/specs/workflow-on-enter-action-dispatch/
// spec.md's AC-F1 ("A dispatcher that loses a claim stops. It does not
// skip ahead.") and risking the exact hazard AC-F1's rationale names:
// enqueueing the fan-out before the winner's clear_decisions delete
// (AC-A3) commits.
//
// See GetStepEntryMarkerState and its call site in
// dispatchEngineOwnedOnEnterAction for both.
import (
"context"
"testing"
"time"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/workflow/stepentry"
)
// TestProcessOnEnter_ClearDecisionsPriorFailure_BlocksRetryFromDispatchingSubsequentActions
// drives a step entry's on_enter dispatch twice: once with a failing
// decisions store (mirroring AC-C2's failure test, which records the
// clear_decisions marker as terminally failed for this entry), then again
// with a passing store — modelling a retry after whatever caused the first
// failure was fixed, but for the *same* step entry. The retry must still
// see clear_decisions as failed (from the stored marker state, since its
// CAS claim is lost to the earlier row) and must not proceed to dispatch
// queue_run_for_each_participant.
func TestProcessOnEnter_ClearDecisionsPriorFailure_BlocksRetryFromDispatchingSubsequentActions(t *testing.T) {
ctx := context.Background()
f := newReviewLoopFixture(t)
f.svc.SetEngineDecisionStore(&failingDecisionStore{})
if !f.fireOnTurnComplete(t, ctx) {
t.Fatalf("expected a transition from Work -> Review, got none")
}
if got := f.runQueue.callCount(); got != 0 {
t.Fatalf("queued run count after clear_decisions failure = %d, want 0 (AC-C2)", got)
}
// Swap in a passing decisions store, as if the underlying problem that
// caused the first clear_decisions attempt to fail were now resolved.
f.svc.SetEngineDecisionStore(&fakeDecisionStore{})
session, err := f.repo.GetTaskSession(ctx, "s1")
if err != nil {
t.Fatalf("failed to load session: %v", err)
}
sg, nameToID := buildWorkflowFromJSON(t, reviewLoopWorkflowJSON)
reviewStep, ok := sg.steps[nameToID["Review"]]
if !ok {
t.Fatalf("Review step not found in rebuilt workflow")
}
// entryID 1: the fresh per-test database's very first allocated
// workflow_step_entries row, created by fireOnTurnComplete's Work ->
// Review transition above (no other entry allocation happens before
// it in this fixture).
const retryEntryID = int64(1)
dispatchResult := f.svc.dispatchOnEnterActions(ctx, "t1", session, reviewStep, retryEntryID, false, false)
if dispatchResult.hasAutoStart {
t.Errorf("hasAutoStart = true, want false (Review declares no auto_start_agent)")
}
if got := f.runQueue.callCount(); got != 0 {
t.Fatalf("queued run count after retry = %d, want still 0 (clear_decisions already failed terminally for this entry)", got)
}
}
// blockingDecisionStore wraps fakeDecisionStore but parks ClearStepDecisions
// on a channel the test controls, so another goroutine can observe this
// entry's clear_decisions marker verifiably in_progress — a live claim, not
// a hypothetical race — before the test lets it proceed to completion.
type blockingDecisionStore struct {
fakeDecisionStore
proceed chan struct{}
}
func (f *blockingDecisionStore) ClearStepDecisions(ctx context.Context, taskID, stepID string) (int64, error) {
<-f.proceed
return f.fakeDecisionStore.ClearStepDecisions(ctx, taskID, stepID)
}
// TestProcessOnEnter_ConcurrentDispatchLosesLiveClaim_AbandonsEntryWithoutSkippingAhead
// drives two genuinely concurrent dispatches of the *same* step entry. The
// winner is parked mid-flight inside ClearStepDecisions, so its position-0
// marker is confirmed in_progress (polled, not assumed) before the second,
// would-be-loser dispatchOnEnterActions call races in on the same entryID.
// The loser must abandon the entry — no run queued, no WARNING/ERROR log
// (AC-F1: losing a claim is a normal outcome) — rather than skip ahead to
// queue_run_for_each_participant. Once the winner is released, it alone
// completes both actions.
func TestProcessOnEnter_ConcurrentDispatchLosesLiveClaim_AbandonsEntryWithoutSkippingAhead(t *testing.T) {
ctx := context.Background()
f := newReviewLoopFixture(t)
store := &blockingDecisionStore{proceed: make(chan struct{})}
f.svc.SetEngineDecisionStore(store)
core, logs := observer.New(zapcore.DebugLevel)
log, err := logger.NewFromZap(zap.New(core))
if err != nil {
t.Fatalf("build observed logger: %v", err)
}
f.svc.logger = log
setSessionState(t, ctx, f.repo, "s1", models.TaskSessionStateRunning)
onEnterDone := make(chan struct{})
f.svc.onProcessOnEnterComplete = func() { close(onEnterDone) }
session, err := f.repo.GetTaskSession(ctx, "s1")
if err != nil {
t.Fatalf("failed to load session: %v", err)
}
go f.svc.processOnTurnCompleteViaEngine(ctx, "t1", session)
// entryID 1: the fresh per-test database's very first allocated
// workflow_step_entries row (same reasoning as the sibling test above).
const entryID = int64(1)
const clearDecisionsPosition = 0
deadline := time.Now().Add(2 * time.Second)
for {
state, _, found, stateErr := f.repo.GetStepEntryMarkerState(ctx, entryID, clearDecisionsPosition)
if stateErr != nil {
t.Fatalf("GetStepEntryMarkerState: %v", stateErr)
}
if found && state == stepentry.MarkerInProgress {
break
}
if time.Now().After(deadline) {
t.Fatalf("timed out waiting for position %d to become in_progress (found=%v state=%v)", clearDecisionsPosition, found, state)
}
time.Sleep(time.Millisecond)
}
sg, nameToID := buildWorkflowFromJSON(t, reviewLoopWorkflowJSON)
reviewStep, ok := sg.steps[nameToID["Review"]]
if !ok {
t.Fatalf("Review step not found in rebuilt workflow")
}
// The concurrent, would-be-loser dispatch: same entry, same session,
// racing while the winner above is still parked inside clear_decisions.
dispatchResult := f.svc.dispatchOnEnterActions(ctx, "t1", session, reviewStep, entryID, false, false)
if dispatchResult.hasAutoStart {
t.Errorf("hasAutoStart = true, want false (Review declares no auto_start_agent)")
}
if got := f.runQueue.callCount(); got != 0 {
t.Fatalf("queued run count after losing a live claim = %d, want 0 (AC-F1: loser must not skip ahead to queue_run_for_each_participant)", got)
}
for _, e := range logs.All() {
if e.Level >= zapcore.WarnLevel {
t.Errorf("unexpected %s log for a normal claim loss (AC-F1: not a fault): %s %+v", e.Level, e.Message, e.ContextMap())
}
}
close(store.proceed)
select {
case <-onEnterDone:
case <-time.After(2 * time.Second):
t.Fatal("timed out waiting for winner's on_enter processing to finish")
}
if got := f.runQueue.callCount(); got != 1 {
t.Fatalf("queued run count after winner finished = %d, want exactly 1 (winner alone dispatches queue_run_for_each_participant)", got)
}
store.mu.Lock()
clearCalls := store.clearCalls
store.mu.Unlock()
if clearCalls != 1 {
t.Errorf("clear_decisions calls = %d, want exactly 1 (loser must not have re-executed it)", clearCalls)
}
}
// TestProcessOnEnter_RedispatchAfterEntryAlreadyDone_IsANoOp covers the third
// !claimed outcome GetStepEntryMarkerState can report — MarkerDone — which
// neither sibling test above exercises: the first test's retry sees
// MarkerFailed (a prior clear_decisions error), and the concurrent test's
// loser sees MarkerInProgress (a live peer). Here every position for the
// entry has already finished successfully via a normal, uncontested
// fireOnTurnComplete; a second, later dispatchOnEnterActions call for that
// same entryID (e.g. a duplicate trigger delivery, or any caller re-driving
// on_enter for a step entry the engine already fully processed) must fall
// through both positions as already-done and produce no new side effects,
// per the "MarkerDone: ... Safe, and required, to continue past it" branch
// in dispatchEngineOwnedOnEnterAction.
func TestProcessOnEnter_RedispatchAfterEntryAlreadyDone_IsANoOp(t *testing.T) {
ctx := context.Background()
f := newReviewLoopFixture(t)
if !f.fireOnTurnComplete(t, ctx) {
t.Fatalf("expected a transition from Work -> Review, got none")
}
if got := f.runQueue.callCount(); got != 1 {
t.Fatalf("queued run count after the original dispatch = %d, want exactly 1", got)
}
f.decisions.mu.Lock()
clearCallsAfterOriginal := f.decisions.clearCalls
f.decisions.mu.Unlock()
if clearCallsAfterOriginal != 1 {
t.Fatalf("clear_decisions calls after the original dispatch = %d, want exactly 1", clearCallsAfterOriginal)
}
session, err := f.repo.GetTaskSession(ctx, "s1")
if err != nil {
t.Fatalf("failed to load session: %v", err)
}
sg, nameToID := buildWorkflowFromJSON(t, reviewLoopWorkflowJSON)
reviewStep, ok := sg.steps[nameToID["Review"]]
if !ok {
t.Fatalf("Review step not found in rebuilt workflow")
}
// entryID 1: same fully-completed entry the fireOnTurnComplete call above
// just processed; both of its on_enter positions are now MarkerDone.
const entryID = int64(1)
dispatchResult := f.svc.dispatchOnEnterActions(ctx, "t1", session, reviewStep, entryID, false, false)
if dispatchResult.hasAutoStart {
t.Errorf("hasAutoStart = true, want false (Review declares no auto_start_agent)")
}
if got := f.runQueue.callCount(); got != 1 {
t.Fatalf("queued run count after redispatching an already-done entry = %d, want still 1 (no re-execution)", got)
}
f.decisions.mu.Lock()
clearCallsAfterRedispatch := f.decisions.clearCalls
f.decisions.mu.Unlock()
if clearCallsAfterRedispatch != 1 {
t.Errorf("clear_decisions calls after redispatching an already-done entry = %d, want still 1 (no re-execution)", clearCallsAfterRedispatch)
}
}
apps/backend/internal/orchestrator/step_entry_dispatch_concurrent_test.go
+90
View File
@@ -0,0 +1,90 @@
package orchestrator
// step_entry_dispatch_concurrent_test.go covers the HIGH duplicate-entry
// finding from this Build round's Review: two independent event sources
// (agent turn completion, agent-exit, the step_complete_kandev signal, user
// cancellation) can each call processOnTurnCompleteViaEngine for the same
// session. Without serialization, two concurrent calls could both read the
// same pre-transition step, both call applyEngineTransition, and each
// allocate its own workflow_step_entries row for what should be a single
// logical transition — double-dispatching that entry's on_enter actions
// (clear_decisions, queue_run_for_each_participant). See
// acquireTurnCompletionCriticalSection and turnCompletionLocks' field
// comment in service.go for the fix.
import (
"context"
"sync"
"testing"
"time"
"github.com/kandev/kandev/internal/task/models"
)
// TestProcessOnTurnComplete_ConcurrentDuplicateCalls_OnlyOneTransitionApplies
// fires two concurrent processOnTurnCompleteViaEngine calls for the same
// session, both observing the same pre-transition step (Work). Exactly one
// must win: apply the Work -> Review transition and dispatch Review's
// on_enter actions exactly once. The other must recognize (after losing the
// race for the per-session lock and re-reading fresh task state) that
// another caller already handled this turn, and return false without
// re-evaluating on_turn_complete against Review's own declaration — which
// would otherwise spuriously fire Review -> Work for a turn nobody in
// Review actually completed.
func TestProcessOnTurnComplete_ConcurrentDuplicateCalls_OnlyOneTransitionApplies(t *testing.T) {
ctx := context.Background()
f := newReviewLoopFixture(t)
setSessionState(t, ctx, f.repo, "s1", models.TaskSessionStateRunning)
onEnterDone := make(chan struct{}, 2)
f.svc.onProcessOnEnterComplete = func() { onEnterDone <- struct{}{} }
session, err := f.repo.GetTaskSession(ctx, "s1")
if err != nil {
t.Fatalf("failed to load session: %v", err)
}
const callers = 2
results := make([]bool, callers)
start := make(chan struct{})
var wg sync.WaitGroup
for i := 0; i < callers; i++ {
wg.Add(1)
go func(i int) {
defer wg.Done()
<-start
results[i] = f.svc.processOnTurnCompleteViaEngine(ctx, "t1", session)
}(i)
}
close(start)
wg.Wait()
select {
case <-onEnterDone:
case <-time.After(2 * time.Second):
t.Fatal("timed out waiting for on_enter processing")
}
transitionedCount := 0
for _, r := range results {
if r {
transitionedCount++
}
}
if transitionedCount != 1 {
t.Fatalf("transitioned count = %d, want exactly 1 of %d concurrent calls", transitionedCount, callers)
}
assertStepByName(t, ctx, f.repo, "s1", "Review", f.nameToID)
if got := f.runQueue.callCount(); got != 1 {
t.Fatalf("queued run count = %d, want exactly 1 (no double-dispatch)", got)
}
f.decisions.mu.Lock()
clearCalls := f.decisions.clearCalls
f.decisions.mu.Unlock()
if clearCalls != 1 {
t.Fatalf("clear_decisions calls = %d, want exactly 1 (no double-dispatch)", clearCalls)
}
}
apps/backend/internal/orchestrator/step_entry_dispatch_test.go
+351
View File
@@ -0,0 +1,351 @@
package orchestrator
import (
"context"
"errors"
"sync"
"testing"
"time"
"go.uber.org/zap"
"go.uber.org/zap/zapcore"
"go.uber.org/zap/zaptest/observer"
"github.com/kandev/kandev/internal/common/logger"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/workflow/engine"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
v1 "github.com/kandev/kandev/pkg/api/v1"
)
// step_entry_dispatch_test.go covers the defect this Build round fixes: the
// Office Default workflow's Review/Approval steps declare
// on_enter: [clear_decisions, queue_run_for_each_participant], but the
// legacy processOnEnter switch silently discarded both action types (no
// case, no default), so no reviewer/approver was ever woken. These tests
// drive the real E1 entry path (engine-driven on_turn_complete →
// applyEngineTransition → launchProcessOnEnter → processOnEnter) end to end
// against the real sqlite repo, so step-entry allocation and CAS-marker
// claiming are exercised for real, not stubbed.
// reviewLoopWorkflowJSON models a two-step Work/Review loop: Work's
// on_turn_complete advances into Review; Review declares the two
// engine-owned on_enter actions under test and its own on_turn_complete
// sends the task back to Work (modelling a rejection round for AC-D4).
const reviewLoopWorkflowJSON = `{
"version": 1,
"type": "kandev_workflow",
"workflows": [
{
"name": "ReviewLoop",
"steps": [
{
"name": "Work",
"position": 0,
"color": "bg-blue-500",
"events": {
"on_turn_complete": [{"type": "move_to_step", "config": {"step_position": 1}}]
},
"is_start_step": true,
"allow_manual_move": true
},
{
"name": "Review",
"position": 1,
"color": "bg-yellow-500",
"events": {
"on_enter": [
{"type": "clear_decisions"},
{"type": "queue_run_for_each_participant", "config": {"role": "reviewer"}}
],
"on_turn_complete": [{"type": "move_to_step", "config": {"step_position": 0}}]
},
"allow_manual_move": true
}
]
}
]
}`
// stepEntryFakeRunQueue records every QueueRun call. Safe for concurrent use —
// processOnEnter's dispatch runs on a goroutine.
type stepEntryFakeRunQueue struct {
mu sync.Mutex
calls []engine.QueueRunRequest
}
func (f *stepEntryFakeRunQueue) QueueRun(_ context.Context, req engine.QueueRunRequest) (engine.QueueOutcome, error) {
f.mu.Lock()
defer f.mu.Unlock()
f.calls = append(f.calls, req)
return "", nil
}
func (f *stepEntryFakeRunQueue) callCount() int {
f.mu.Lock()
defer f.mu.Unlock()
return len(f.calls)
}
// fakeParticipantStore returns a fixed participant list regardless of
// stepID/taskID — sufficient for a single-step-under-test scenario.
type fakeParticipantStore struct {
participants []engine.ParticipantInfo
}
func (f *fakeParticipantStore) ListStepParticipants(_ context.Context, _, _ string) ([]engine.ParticipantInfo, error) {
return f.participants, nil
}
func (f *fakeParticipantStore) ListTaskParticipants(_ context.Context, _ string) ([]engine.ParticipantInfo, error) {
return f.participants, nil
}
// fakeDecisionStore is a minimal DecisionStore recording ClearStepDecisions
// calls; ListStepDecisions/RecordStepDecision are unused by this test.
type fakeDecisionStore struct {
mu sync.Mutex
clearCalls int
}
func (f *fakeDecisionStore) ListStepDecisions(_ context.Context, _, _ string) ([]engine.DecisionInfo, error) {
return nil, nil
}
func (f *fakeDecisionStore) RecordStepDecision(_ context.Context, _ engine.DecisionInfo) error {
return nil
}
func (f *fakeDecisionStore) ClearStepDecisions(_ context.Context, _, _ string) (int64, error) {
f.mu.Lock()
defer f.mu.Unlock()
f.clearCalls++
return 0, nil
}
// reviewLoopFixture bundles the wiring shared by the AC-D3/AC-D4 tests.
type reviewLoopFixture struct {
svc *Service
repo sessionExecutorStore
agentMgr *mockAgentManager
runQueue *stepEntryFakeRunQueue
decisions *fakeDecisionStore
nameToID map[string]string
taskRepoMk *mockTaskRepo
}
func newReviewLoopFixture(t *testing.T) *reviewLoopFixture {
t.Helper()
sg, nameToID := buildWorkflowFromJSON(t, reviewLoopWorkflowJSON)
repo := setupTestRepo(t)
seedSession(t, repo, "t1", "s1", nameToID["Work"])
setSessionExecID(t, repo, "s1", "exec-1")
agentMgr := &mockAgentManager{repoForExecutionLookup: repo, isAgentRunning: true}
svc := createEngineService(t, repo, sg, agentMgr)
runQueue := &stepEntryFakeRunQueue{}
participants := &fakeParticipantStore{participants: []engine.ParticipantInfo{
{ID: "participant-1", StepID: nameToID["Review"], Role: "reviewer", AgentProfileID: "agent-reviewer-1"},
}}
decisions := &fakeDecisionStore{}
svc.SetEngineRunQueue(runQueue)
svc.SetEngineParticipantStore(participants)
svc.SetEngineDecisionStore(decisions)
taskRepo := svc.taskRepo.(*mockTaskRepo)
seedMockTaskState(taskRepo, "t1", v1.TaskStateInProgress)
return &reviewLoopFixture{
svc: svc, repo: repo, agentMgr: agentMgr,
runQueue: runQueue, decisions: decisions,
nameToID: nameToID, taskRepoMk: taskRepo,
}
}
// fireOnTurnComplete sets the session RUNNING, fires on_turn_complete, and
// blocks until processOnEnter's goroutine has finished — so assertions right
// after this call see the dispatch's side effects.
func (f *reviewLoopFixture) fireOnTurnComplete(t *testing.T, ctx context.Context) bool {
t.Helper()
setSessionState(t, ctx, f.repo, "s1", models.TaskSessionStateRunning)
onEnterDone := make(chan struct{})
f.svc.onProcessOnEnterComplete = func() { close(onEnterDone) }
session, err := f.repo.GetTaskSession(ctx, "s1")
if err != nil {
t.Fatalf("failed to load session: %v", err)
}
transitioned := f.svc.processOnTurnCompleteViaEngine(ctx, "t1", session)
select {
case <-onEnterDone:
case <-time.After(2 * time.Second):
t.Fatal("timed out waiting for on_enter processing")
}
return transitioned
}
// TestProcessOnEnter_QueueRunForEachParticipant_SingleReviewerEntry_QueuesExactlyOneRun
// covers AC-D3: a single reviewer entering Review via an ordinary
// on_turn_complete transition produces exactly one queued run, and
// clear_decisions runs alongside it.
func TestProcessOnEnter_QueueRunForEachParticipant_SingleReviewerEntry_QueuesExactlyOneRun(t *testing.T) {
ctx := context.Background()
f := newReviewLoopFixture(t)
transitioned := f.fireOnTurnComplete(t, ctx)
if !transitioned {
t.Fatalf("expected a transition from Work -> Review, got none")
}
assertStepByName(t, ctx, f.repo, "s1", "Review", f.nameToID)
if got := f.runQueue.callCount(); got != 1 {
t.Fatalf("queued run count = %d, want exactly 1 (AC-D3)", got)
}
if got := f.runQueue.calls[0].AgentProfileID; got != "agent-reviewer-1" {
t.Errorf("queued run agent = %q, want %q", got, "agent-reviewer-1")
}
f.decisions.mu.Lock()
clearCalls := f.decisions.clearCalls
f.decisions.mu.Unlock()
if clearCalls != 1 {
t.Errorf("clear_decisions calls = %d, want 1", clearCalls)
}
}
// TestProcessOnEnter_QueueRunForEachParticipant_SecondEntryAfterRejection_QueuesAnotherRun
// covers AC-D4: after a rejection round sends the task back to Work and it
// re-enters Review, the step-entry CAS marker for the *new* entry has never
// been claimed, so a further run is queued — the at-most-once guarantee is
// per step-entry, not a blanket suppression of repeat dispatch.
func TestProcessOnEnter_QueueRunForEachParticipant_SecondEntryAfterRejection_QueuesAnotherRun(t *testing.T) {
ctx := context.Background()
f := newReviewLoopFixture(t)
// Work -> Review (first entry).
if !f.fireOnTurnComplete(t, ctx) {
t.Fatalf("expected a transition from Work -> Review, got none")
}
if got := f.runQueue.callCount(); got != 1 {
t.Fatalf("queued run count after first entry = %d, want 1", got)
}
// Review -> Work (rejection round).
if !f.fireOnTurnComplete(t, ctx) {
t.Fatalf("expected a transition from Review -> Work, got none")
}
assertStepByName(t, ctx, f.repo, "s1", "Work", f.nameToID)
if got := f.runQueue.callCount(); got != 1 {
t.Fatalf("queued run count after rejection = %d, want still 1 (Work declares no queue_run_for_each_participant)", got)
}
// Work -> Review (second entry).
if !f.fireOnTurnComplete(t, ctx) {
t.Fatalf("expected a transition from Work -> Review, got none")
}
assertStepByName(t, ctx, f.repo, "s1", "Review", f.nameToID)
if got := f.runQueue.callCount(); got != 2 {
t.Fatalf("queued run count after second Review entry = %d, want 2 (AC-D4: not suppressed)", got)
}
}
// failingDecisionStore wraps fakeDecisionStore but always fails
// ClearStepDecisions, for AC-C2 below.
type failingDecisionStore struct {
fakeDecisionStore
}
func (f *failingDecisionStore) ClearStepDecisions(_ context.Context, _, _ string) (int64, error) {
f.mu.Lock()
f.clearCalls++
f.mu.Unlock()
return 0, errors.New("boom: clear_decisions failure")
}
// TestProcessOnEnter_ClearDecisionsFailure_BlocksSubsequentOnEnterActions
// covers AC-C2: "IF clear_decisions fails, THEN the system shall not
// execute any remaining on_enter action for that step entry." Review's
// on_enter declares queue_run_for_each_participant immediately after
// clear_decisions (the exact order the Office Default workflow uses) — a
// stale decision from a previous round combined with a fresh participant
// fan-out could otherwise satisfy wait_for_quorum without every
// current-round reviewer actually voting.
func TestProcessOnEnter_ClearDecisionsFailure_BlocksSubsequentOnEnterActions(t *testing.T) {
ctx := context.Background()
f := newReviewLoopFixture(t)
f.svc.SetEngineDecisionStore(&failingDecisionStore{})
core, logs := observer.New(zapcore.ErrorLevel)
log, err := logger.NewFromZap(zap.New(core))
if err != nil {
t.Fatalf("build observed logger: %v", err)
}
f.svc.logger = log
if !f.fireOnTurnComplete(t, ctx) {
t.Fatalf("expected a transition from Work -> Review, got none")
}
if got := f.runQueue.callCount(); got != 0 {
t.Fatalf("queued run count after clear_decisions failure = %d, want 0 (AC-C2)", got)
}
var acC2Errors []observer.LoggedEntry
for _, e := range logs.All() {
if e.Message == "processOnEnter: clear_decisions failed, aborting remaining on_enter actions for step entry" {
acC2Errors = append(acC2Errors, e)
}
}
if len(acC2Errors) != 1 {
t.Fatalf("got %d AC-C2 ERROR log(s), want exactly 1 (all entries: %+v)", len(acC2Errors), logs.All())
}
fields := acC2Errors[0].ContextMap()
if fields["workflow_id"] != "wf1" {
t.Errorf("AC-C2 error workflow_id = %v, want %q", fields["workflow_id"], "wf1")
}
if fields["step_id"] != f.nameToID["Review"] {
t.Errorf("AC-C2 error step_id = %v, want %q", fields["step_id"], f.nameToID["Review"])
}
if fields["task_id"] != "t1" {
t.Errorf("AC-C2 error task_id = %v, want %q", fields["task_id"], "t1")
}
if cause, _ := fields["cause"].(string); cause == "" {
t.Errorf("AC-C2 error missing cause field: %+v", fields)
}
}
// TestProcessOnEnter_ClearDecisionsFailure_DoesNotLaunchEarlierAutoStart
// covers AC-C2 when auto_start_agent appears before clear_decisions. The
// auto-start action keeps its fixed final execution position, so a failed
// clear must abort the launch as well as later engine-owned actions.
func TestProcessOnEnter_ClearDecisionsFailure_DoesNotLaunchEarlierAutoStart(t *testing.T) {
ctx := context.Background()
f := newReviewLoopFixture(t)
f.svc.SetEngineDecisionStore(&failingDecisionStore{})
stepGetter, ok := f.svc.workflowStepGetter.(*mockStepGetter)
if !ok {
t.Fatalf("workflow step getter type = %T, want *mockStepGetter", f.svc.workflowStepGetter)
}
reviewStep := stepGetter.steps[f.nameToID["Review"]]
reviewStep.Prompt = "review prompt"
reviewStep.Events.OnEnter = []wfmodels.OnEnterAction{
{Type: wfmodels.OnEnterAutoStartAgent},
{Type: wfmodels.OnEnterClearDecisions},
}
if !f.fireOnTurnComplete(t, ctx) {
t.Fatalf("expected a transition from Work -> Review, got none")
}
f.agentMgr.mu.Lock()
promptCount := len(f.agentMgr.capturedPrompts)
f.agentMgr.mu.Unlock()
if promptCount != 0 {
t.Fatalf("auto-start prompt count after clear_decisions failure = %d, want 0", promptCount)
}
}
apps/backend/internal/orchestrator/workflow_store.go
+16
View File
@@ -15,6 +15,7 @@ import (
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/workflow/engine"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
"github.com/kandev/kandev/internal/workflow/stepentry"
v1 "github.com/kandev/kandev/pkg/api/v1"
)
@@ -308,6 +309,21 @@ func (s *workflowStore) applyTransition(ctx context.Context, taskID, sessionID,
if !steptelemetry.HasTrigger(transitionCtx) {
transitionCtx = engineTransitionAttribution(transitionCtx, sessionID, trigger)
}
// Only attach a PendingAllocation when the caller already opted in by
// wrapping ctx with a ResultHolder (applyEngineTransition does this for
// the engine-driven on_turn_complete path). Attaching it unconditionally
// would allocate workflow_step_entries rows for every transition into a
// step with engine-owned on_enter actions, including the callers (manual
// move, deferred move) that don't yet dispatch through them this Build
// round — see docs/specs/workflow-on-enter-action-dispatch/spec.md and
// the task plan's scope note for why those entry paths are deferred.
if targetStep != nil {
if _, wantsAllocation := stepentry.ResultHolderFromContext(ctx); wantsAllocation {
if pending, ok := stepentry.BuildPendingAllocation(targetStep.ID, targetStep.Events.OnEnter); ok {
transitionCtx = stepentry.WithPendingAllocation(transitionCtx, pending)
}
}
}
if err := s.updateTransitionTask(transitionCtx, task, targetStep); err != nil {
return fmt.Errorf("update task workflow step: %w", err)
}
apps/backend/internal/task/repository/sqlite/base_schema.go
+2 -4
View File
@@ -24,6 +24,7 @@ func (r *Repository) initSchema() error {
r.initSessionSchema,
r.initDynamicRoutingSchema,
r.initStepTransitionsSchema,
r.initStepEntriesSchema,
r.initTaskUsageEventsSchema,
r.initAttachmentsSchema,
r.initTaskResourceCleanupSchema,
@@ -825,10 +826,7 @@ func (r *Repository) initSubagentContextSchema() error {
// workflows get deleted, and the historical fact that a card was in a
// now-deleted step must survive that deletion.
func (r *Repository) initStepTransitionsSchema() error {
idCol := "id INTEGER PRIMARY KEY AUTOINCREMENT"
if dialect.IsPostgres(r.db.DriverName()) {
idCol = "id BIGSERIAL PRIMARY KEY"
}
idCol := dialect.AutoIncrementIDColumn(r.db.DriverName())
_, err := r.db.Exec(`
CREATE TABLE IF NOT EXISTS task_step_transitions (
` + idCol + `,
apps/backend/internal/task/repository/sqlite/step_entries.go
+272
View File
@@ -0,0 +1,272 @@
package sqlite
// workflow_step_entries / workflow_step_entry_markers back the step-entry
// allocation and per-action CAS markers described in
// docs/specs/workflow-on-enter-action-dispatch/spec.md. This Build round
// wires allocation into exactly one chokepoint (updateTaskTx, shared by
// UpdateTask and UpdateTaskWithWorkflowStepAdmission(AndState)) via the
// context-carried stepentry.PendingAllocation/AllocationResult contract —
// see internal/workflow/stepentry for why a context value was chosen over
// changing ApplyTransition's or the admission methods' signatures.
//
// Scope note: this is a deliberately simplified, single-writer-per-entry CAS
// model, not the spec's full multi-process epoch/staleness reconciliation.
// A marker only ever has one row; claiming is "insert from absent", and a
// UNIQUE-constraint conflict on that insert means "already claimed or
// handled — skip", not "steal the marker from a stale epoch". Full
// epoch/staleness reconciliation under real contention and the AC-B9
// startup recovery scan are explicitly deferred to a later Build round.
import (
"context"
"database/sql"
"errors"
"fmt"
"strings"
"time"
"github.com/jackc/pgx/v5/pgconn"
"github.com/kandev/kandev/internal/db/dialect"
"github.com/kandev/kandev/internal/workflow/stepentry"
)
// StepEntryMarkerState is a marker's terminal (or in-progress) state. It is
// an alias for stepentry.MarkerState so orchestrator-side dispatch callers
// can pass the same constants without importing this repository package.
type StepEntryMarkerState = stepentry.MarkerState
const (
// StepEntryMarkerInProgress is set the moment a claim succeeds.
StepEntryMarkerInProgress = stepentry.MarkerInProgress
// StepEntryMarkerDone marks a successfully executed action.
StepEntryMarkerDone = stepentry.MarkerDone
// StepEntryMarkerFailed marks an action whose callback returned an
// error; the error column records the cause.
StepEntryMarkerFailed = stepentry.MarkerFailed
)
// stepEntryMarkerUniqueMessage is the substring go-sqlite3 puts in a
// UNIQUE-constraint error for the (entry_id, position) index — see
// isExternalIDUniqueViolation in task_external_id.go for the same
// cross-dialect approach applied to a different index.
const stepEntryMarkerUniqueMessage = "UNIQUE constraint failed: workflow_step_entry_markers.entry_id, workflow_step_entry_markers.position"
const stepEntryMarkerIndexName = "uniq_workflow_step_entry_markers_entry_position"
func isStepEntryMarkerUniqueViolation(err error) bool {
if err == nil {
return false
}
var pgErr *pgconn.PgError
if errors.As(err, &pgErr) {
return pgErr.Code == "23505" && pgErr.ConstraintName == stepEntryMarkerIndexName
}
return strings.Contains(err.Error(), stepEntryMarkerUniqueMessage)
}
// initStepEntriesSchema creates workflow_step_entries and
// workflow_step_entry_markers. New tables, not columns added to existing
// ones, so CREATE TABLE IF NOT EXISTS in the init block is correct — see
// initStepTransitionsSchema's comment for why that rule doesn't apply here.
func (r *Repository) initStepEntriesSchema() error {
idCol := dialect.AutoIncrementIDColumn(r.db.DriverName())
_, err := r.db.Exec(`
CREATE TABLE IF NOT EXISTS workflow_step_entries (
` + idCol + `,
task_id TEXT NOT NULL REFERENCES tasks(id) ON DELETE CASCADE,
step_id TEXT NOT NULL,
entry_seq INTEGER NOT NULL,
digest TEXT NOT NULL,
created_at TIMESTAMP NOT NULL,
UNIQUE(task_id, step_id, entry_seq)
);
CREATE INDEX IF NOT EXISTS idx_workflow_step_entries_task_step
ON workflow_step_entries(task_id, step_id);
CREATE TABLE IF NOT EXISTS workflow_step_entry_markers (
` + idCol + `,
entry_id INTEGER NOT NULL REFERENCES workflow_step_entries(id) ON DELETE CASCADE,
position INTEGER NOT NULL,
kind TEXT NOT NULL,
state TEXT NOT NULL,
operation_id TEXT,
error TEXT,
claimed_at TIMESTAMP,
completed_at TIMESTAMP,
CONSTRAINT uniq_workflow_step_entry_markers_entry_position UNIQUE(entry_id, position)
);
CREATE INDEX IF NOT EXISTS idx_workflow_step_entry_markers_entry
ON workflow_step_entry_markers(entry_id);
`)
if err != nil {
return fmt.Errorf("init step entries schema: %w", err)
}
return nil
}
// allocateStepEntryIfPending reads a PendingAllocation off ctx (attached by
// the caller that resolved this transition's target step — see
// workflow_store.applyTransition) and, when present, allocates a new
// workflow_step_entries row inside tx. entry_seq is the 1-based count of
// prior entries into (task_id, step_id) for this task, so a task
// re-entering the same step after a rejection round gets a fresh entry
// rather than reusing the first one. When a ResultHolder is also attached,
// the allocated (entry_id, entry_seq) is written back through it so a
// caller several calls further up the stack can read it after commit.
//
// No-op (returns nil) when no PendingAllocation is attached — the common
// case for write paths that haven't opted into step-entry allocation this
// round (e.g. manual moves, still using the legacy processOnEnter path).
func (r *Repository) allocateStepEntryIfPending(ctx context.Context, tx stepTransitionTx, taskID string, occurredAt time.Time) error {
pending, ok := stepentry.FromContext(ctx)
if !ok {
return nil
}
var seqCount int64
err := tx.QueryRowContext(ctx, r.db.Rebind(`
SELECT COUNT(*) FROM workflow_step_entries WHERE task_id = ? AND step_id = ?
`), taskID, pending.StepID).Scan(&seqCount)
if err != nil {
return fmt.Errorf("count prior step entries: %w", err)
}
entrySeq := seqCount + 1
var entryID int64
err = tx.QueryRowContext(ctx, r.db.Rebind(`
INSERT INTO workflow_step_entries (task_id, step_id, entry_seq, digest, created_at)
VALUES (?, ?, ?, ?, ?)
RETURNING id
`), taskID, pending.StepID, entrySeq, pending.Digest, occurredAt).Scan(&entryID)
if err != nil {
return fmt.Errorf("allocate step entry: %w", err)
}
if holder, ok := stepentry.ResultHolderFromContext(ctx); ok {
holder.EntryID = entryID
holder.EntrySeq = entrySeq
}
return nil
}
// ClaimStepEntryMarker attempts to claim (entryID, position) for execution,
// transitioning it from absent (no row) to in_progress. claimed is false —
// not an error — when the marker already has a row (already claimed by a
// concurrent or prior attempt, or already terminal): the caller must treat
// that as "do not execute the action again", per AC-D3/AC-D4's at-most-once
// requirement for a given step-entry.
func (r *Repository) ClaimStepEntryMarker(ctx context.Context, entryID int64, position int, kind, operationID string, claimedAt time.Time) (bool, error) {
_, err := r.db.ExecContext(ctx, r.db.Rebind(`
INSERT INTO workflow_step_entry_markers (entry_id, position, kind, state, operation_id, claimed_at)
VALUES (?, ?, ?, ?, ?, ?)
`), entryID, position, kind, string(StepEntryMarkerInProgress), operationID, claimedAt)
if err != nil {
if isStepEntryMarkerUniqueViolation(err) {
return false, nil
}
return false, fmt.Errorf("claim step entry marker: %w", err)
}
return true, nil
}
// CompleteStepEntryMarker transitions a claimed marker to a terminal state
// (done/failed). It is a no-op — not an error — when the marker was never
// claimed (no row at all): there is nothing to complete, and fabricating a
// terminal row here would let a later, legitimate claim believe the action
// already ran.
func (r *Repository) CompleteStepEntryMarker(ctx context.Context, entryID int64, position int, state StepEntryMarkerState, cause string, completedAt time.Time) error {
var errCol sql.NullString
if cause != "" {
errCol = sql.NullString{String: cause, Valid: true}
}
_, err := r.db.ExecContext(ctx, r.db.Rebind(`
UPDATE workflow_step_entry_markers
SET state = ?, error = ?, completed_at = ?
WHERE entry_id = ? AND position = ? AND state = ?
`), string(state), errCol, completedAt, entryID, position, string(StepEntryMarkerInProgress))
if err != nil {
return fmt.Errorf("complete step entry marker: %w", err)
}
return nil
}
// GetStepEntryMarkerState reads back a marker's current state and stored
// failure cause (if any). found is false when no marker row exists for
// (entryID, position) — the marker was never claimed. Used by the CAS-loser
// path in dispatchEngineOwnedOnEnterAction: losing the claim to an existing
// row means "already claimed or handled," but that covers three different
// outcomes (still in progress elsewhere, already done, or already failed
// terminally), and only this read can tell them apart.
func (r *Repository) GetStepEntryMarkerState(ctx context.Context, entryID int64, position int) (state StepEntryMarkerState, cause string, found bool, err error) {
var row struct {
State string `db:"state"`
Error sql.NullString `db:"error"`
}
queryErr := r.db.GetContext(ctx, &row, r.db.Rebind(`
SELECT state, error FROM workflow_step_entry_markers WHERE entry_id = ? AND position = ?
`), entryID, position)
if errors.Is(queryErr, sql.ErrNoRows) {
return "", "", false, nil
}
if queryErr != nil {
return "", "", false, fmt.Errorf("get step entry marker state: %w", queryErr)
}
return StepEntryMarkerState(row.State), row.Error.String, true, nil
}
// ClearStepDecisionsAndCompleteMarker satisfies AC-B6: it deletes every
// decision for (taskID, stepID) and marks the clear_decisions position
// done, in one database transaction. A crash between the delete and the
// marker update is therefore impossible to observe as "delete committed,
// marker still in_progress" — the transaction either commits both or
// neither, which is the invariant a future AC-B9 recovery scan needs to
// safely decide whether re-running clear_decisions for a non-terminal
// marker is necessary.
//
// This duplicates the DELETE FROM workflow_step_decisions statement in
// workflow/repository.Repository.ClearStepDecisions rather than depending
// on that package: AC-B6 and AC-B7 together require the decisions table
// and the step-entry tables to be written through the same transaction,
// and workflow/repository.Repository exposes no way to join a
// caller-supplied *sql.Tx into its own writes. Both tables live in the same
// database reached through the same writer *sqlx.DB (see
// internal/backendapp/storage.go), so running both statements here, against
// this repository's own r.db, is safe — see
// orchestrator.dispatchClearDecisionsAtomic for the call site and the
// capability check that keeps this path off whenever the DecisionStore
// isn't actually backed by this database (e.g. every existing test double).
func (r *Repository) ClearStepDecisionsAndCompleteMarker(
ctx context.Context, taskID, stepID string, entryID int64, position int, now time.Time,
) (int64, error) {
if taskID == "" || stepID == "" {
return 0, errors.New("task_id and step_id are required")
}
tx, err := r.db.BeginTx(ctx, nil)
if err != nil {
return 0, fmt.Errorf("begin clear decisions transaction: %w", err)
}
defer func() { _ = tx.Rollback() }()
res, err := tx.ExecContext(ctx, r.db.Rebind(
`DELETE FROM workflow_step_decisions WHERE task_id = ? AND step_id = ?`,
), taskID, stepID)
if err != nil {
return 0, fmt.Errorf("clear step decisions: %w", err)
}
rows, _ := res.RowsAffected()
_, err = tx.ExecContext(ctx, r.db.Rebind(`
UPDATE workflow_step_entry_markers
SET state = ?, error = NULL, completed_at = ?
WHERE entry_id = ? AND position = ? AND state = ?
`), string(StepEntryMarkerDone), now, entryID, position, string(StepEntryMarkerInProgress))
if err != nil {
return 0, fmt.Errorf("complete step entry marker: %w", err)
}
if err := tx.Commit(); err != nil {
return 0, fmt.Errorf("commit clear decisions transaction: %w", err)
}
return rows, nil
}
apps/backend/internal/task/repository/sqlite/step_entries_postgres_test.go
+141
View File
@@ -0,0 +1,141 @@
package sqlite
import (
"context"
"fmt"
"sync"
"testing"
"time"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/testutil"
)
// TestPostgresClaimStepEntryMarker_ConcurrentRace is the PostgreSQL
// counterpart to the CAS-loser coverage in
// internal/orchestrator/step_entry_dispatch_cas_loser_test.go: it proves
// ClaimStepEntryMarker's at-most-once guarantee (AC-D3/AC-D4,
// docs/specs/workflow-on-enter-action-dispatch/spec.md) holds against
// Postgres's real UNIQUE-violation error shape
// (isStepEntryMarkerUniqueViolation's *pgconn.PgError branch), not only
// against go-sqlite3's string-matched one every orchestrator dispatch test
// in this Build round already exercises. Several real connections race a
// claim for the same (entry_id, position); exactly one must win. Skips
// unless KANDEV_TEST_POSTGRES_DSN is set.
func TestPostgresClaimStepEntryMarker_ConcurrentRace(t *testing.T) {
const concurrency = 8
db := openIsolatedPostgresMultiConn(t, testutil.PostgresDSNFromEnv(t), concurrency)
repo, err := NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("init postgres schema: %v", err)
}
ctx := context.Background()
if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "postgres-cas-marker-workspace", Name: "Postgres CAS marker workspace"}); err != nil {
t.Fatalf("seed workspace: %v", err)
}
if err := repo.CreateTask(ctx, &models.Task{
ID: "postgres-cas-marker-task", WorkspaceID: "postgres-cas-marker-workspace",
WorkflowID: "postgres-cas-marker-workflow", WorkflowStepID: "postgres-cas-marker-step",
Title: "CAS marker candidate",
}); err != nil {
t.Fatalf("seed task: %v", err)
}
var entryID int64
if err := db.QueryRowContext(ctx, db.Rebind(`
INSERT INTO workflow_step_entries (task_id, step_id, entry_seq, digest, created_at)
VALUES (?, ?, ?, ?, ?)
RETURNING id
`), "postgres-cas-marker-task", "postgres-cas-marker-step", 1, "digest-1", time.Now()).Scan(&entryID); err != nil {
t.Fatalf("seed step entry: %v", err)
}
start := make(chan struct{})
results := make(chan bool, concurrency)
var wg sync.WaitGroup
for i := 0; i < concurrency; i++ {
wg.Add(1)
go func(i int) {
defer wg.Done()
<-start
claimed, claimErr := repo.ClaimStepEntryMarker(ctx, entryID, 0, "clear_decisions", fmt.Sprintf("op-%d", i), time.Now())
if claimErr != nil {
t.Errorf("concurrent claim attempt %d: %v", i, claimErr)
return
}
results <- claimed
}(i)
}
close(start)
wg.Wait()
close(results)
claimedCount := 0
for c := range results {
if c {
claimedCount++
}
}
if claimedCount != 1 {
t.Fatalf("expected exactly 1 concurrent claim to win, got %d", claimedCount)
}
state, _, found, err := repo.GetStepEntryMarkerState(ctx, entryID, 0)
if err != nil {
t.Fatalf("GetStepEntryMarkerState: %v", err)
}
if !found {
t.Fatalf("expected a marker row to exist after the race")
}
if state != StepEntryMarkerInProgress {
t.Fatalf("marker state after the race = %q, want %q (only the winner claimed it, nobody completed it)", state, StepEntryMarkerInProgress)
}
}
// TestPostgresIsStepEntryMarkerUniqueViolation_MatchesRealConstraintError
// drives the actual UNIQUE-constraint index through Postgres directly (a
// second raw INSERT for the same (entry_id, position), no ClaimStepEntryMarker
// involved) and asserts isStepEntryMarkerUniqueViolation classifies the
// resulting *pgconn.PgError the same way its already-tested SQLite string
// match does. This is what ClaimStepEntryMarker's !claimed branch relies on
// for every CAS-loser test in internal/orchestrator — without this,
// SQLite's error-shape coverage tells us nothing about the Postgres branch
// of the same function.
func TestPostgresIsStepEntryMarkerUniqueViolation_MatchesRealConstraintError(t *testing.T) {
db := openIsolatedPostgresMultiConn(t, testutil.PostgresDSNFromEnv(t), 2)
repo, err := NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("init postgres schema: %v", err)
}
ctx := context.Background()
if err := repo.CreateWorkspace(ctx, &models.Workspace{ID: "postgres-cas-marker-dup-workspace", Name: "Postgres CAS marker dup workspace"}); err != nil {
t.Fatalf("seed workspace: %v", err)
}
if err := repo.CreateTask(ctx, &models.Task{
ID: "postgres-cas-marker-dup-task", WorkspaceID: "postgres-cas-marker-dup-workspace",
WorkflowID: "postgres-cas-marker-dup-workflow", WorkflowStepID: "postgres-cas-marker-dup-step",
Title: "CAS marker dup candidate",
}); err != nil {
t.Fatalf("seed task: %v", err)
}
var entryID int64
if err := db.QueryRowContext(ctx, db.Rebind(`
INSERT INTO workflow_step_entries (task_id, step_id, entry_seq, digest, created_at)
VALUES (?, ?, ?, ?, ?)
RETURNING id
`), "postgres-cas-marker-dup-task", "postgres-cas-marker-dup-step", 1, "digest-1", time.Now()).Scan(&entryID); err != nil {
t.Fatalf("seed step entry: %v", err)
}
claimed, err := repo.ClaimStepEntryMarker(ctx, entryID, 0, "clear_decisions", "op-first", time.Now())
if err != nil || !claimed {
t.Fatalf("first claim: claimed=%v err=%v", claimed, err)
}
claimed, err = repo.ClaimStepEntryMarker(ctx, entryID, 0, "clear_decisions", "op-second", time.Now())
if err != nil {
t.Fatalf("second claim on the same (entry_id, position) returned a real error instead of claimed=false: %v", err)
}
if claimed {
t.Fatalf("second claim on the same (entry_id, position) succeeded, want claimed=false (UNIQUE violation)")
}
}
apps/backend/internal/task/repository/sqlite/step_entries_test.go
+514
View File
@@ -0,0 +1,514 @@
package sqlite
// Coverage for workflow_step_entries / workflow_step_entry_markers: the
// step-entry allocation hook inside updateTaskTx (via stepentry's
// context-carried PendingAllocation/AllocationResult), entry_seq
// incrementing across repeated entries into the same step, and the
// CAS claim/complete semantics markers use to guarantee at-most-once
// dispatch of an engine-owned on_enter action.
//
// See docs/specs/workflow-on-enter-action-dispatch/spec.md and this Build
// round's task plan scope note: this file exercises only the write path this
// round wires (plain UpdateTask through updateTaskTx), not the full
// multi-process epoch/staleness reconciliation the spec describes.
import (
"context"
"path/filepath"
"testing"
"time"
"github.com/jmoiron/sqlx"
dbutil "github.com/kandev/kandev/internal/db"
"github.com/kandev/kandev/internal/task/models"
"github.com/kandev/kandev/internal/workflow/stepentry"
)
func newStepEntriesTestRepo(t *testing.T) *Repository {
t.Helper()
dbPath := filepath.Join(t.TempDir(), "step-entries.db")
dbConn, err := dbutil.OpenSQLite(dbPath)
if err != nil {
t.Fatalf("open sqlite: %v", err)
}
db := sqlx.NewDb(dbConn, "sqlite3")
t.Cleanup(func() { _ = db.Close() })
repo, err := NewWithDB(db, db, nil)
if err != nil {
t.Fatalf("initialize schema: %v", err)
}
return repo
}
func createStepEntriesTestTask(t *testing.T, repo *Repository, taskID, workflowID, stepID string) *models.Task {
t.Helper()
task := &models.Task{
ID: taskID,
WorkspaceID: "ws-1",
WorkflowID: workflowID,
WorkflowStepID: stepID,
Title: "Test Task",
Priority: "medium",
}
if err := repo.CreateTask(context.Background(), task); err != nil {
t.Fatalf("CreateTask: %v", err)
}
return task
}
type stepEntryRow struct {
id int64
taskID string
stepID string
entrySeq int64
digest string
createdAt time.Time
}
func stepEntryRowsForTask(t *testing.T, repo *Repository, taskID string) []stepEntryRow {
t.Helper()
rows, err := repo.db.QueryContext(context.Background(), repo.db.Rebind(`
SELECT id, task_id, step_id, entry_seq, digest, created_at
FROM workflow_step_entries WHERE task_id = ? ORDER BY entry_seq ASC
`), taskID)
if err != nil {
t.Fatalf("query workflow_step_entries: %v", err)
}
defer func() { _ = rows.Close() }()
var out []stepEntryRow
for rows.Next() {
var r stepEntryRow
if err := rows.Scan(&r.id, &r.taskID, &r.stepID, &r.entrySeq, &r.digest, &r.createdAt); err != nil {
t.Fatalf("scan row: %v", err)
}
out = append(out, r)
}
return out
}
func TestUpdateTaskAllocatesStepEntryWhenPendingAllocationPresent(t *testing.T) {
repo := newStepEntriesTestRepo(t)
task := createStepEntriesTestTask(t, repo, "task-1", "wf-1", "step-a")
task.WorkflowStepID = "step-review"
holder := &stepentry.AllocationResult{}
ctx := stepentry.WithResultHolder(context.Background(), holder)
ctx = stepentry.WithPendingAllocation(ctx, stepentry.PendingAllocation{
StepID: "step-review",
Digest: "digest-1",
Positions: []stepentry.EnginePosition{
{Position: 0, Kind: "clear_decisions"},
{Position: 1, Kind: "queue_run_for_each_participant"},
},
})
if err := repo.UpdateTask(ctx, task); err != nil {
t.Fatalf("UpdateTask: %v", err)
}
if holder.EntryID == 0 {
t.Fatalf("expected holder.EntryID to be populated, got 0")
}
if holder.EntrySeq != 1 {
t.Fatalf("EntrySeq = %d, want 1", holder.EntrySeq)
}
rows := stepEntryRowsForTask(t, repo, "task-1")
if len(rows) != 1 {
t.Fatalf("rows = %d, want 1", len(rows))
}
if rows[0].stepID != "step-review" {
t.Fatalf("stepID = %q, want step-review", rows[0].stepID)
}
if rows[0].digest != "digest-1" {
t.Fatalf("digest = %q, want digest-1", rows[0].digest)
}
if rows[0].id != holder.EntryID {
t.Fatalf("row id = %d, want holder.EntryID = %d", rows[0].id, holder.EntryID)
}
}
func TestUpdateTaskWithoutPendingAllocationAllocatesNothing(t *testing.T) {
repo := newStepEntriesTestRepo(t)
task := createStepEntriesTestTask(t, repo, "task-2", "wf-1", "step-a")
task.WorkflowStepID = "step-b"
if err := repo.UpdateTask(context.Background(), task); err != nil {
t.Fatalf("UpdateTask: %v", err)
}
rows := stepEntryRowsForTask(t, repo, "task-2")
if len(rows) != 0 {
t.Fatalf("rows = %d, want 0 (no pending allocation attached to ctx)", len(rows))
}
}
func TestUpdateTaskStepEntrySeqIncrementsAcrossRepeatedEntries(t *testing.T) {
repo := newStepEntriesTestRepo(t)
task := createStepEntriesTestTask(t, repo, "task-3", "wf-1", "step-a")
enter := func(stepID string) *stepentry.AllocationResult {
task.WorkflowStepID = stepID
holder := &stepentry.AllocationResult{}
ctx := stepentry.WithResultHolder(context.Background(), holder)
ctx = stepentry.WithPendingAllocation(ctx, stepentry.PendingAllocation{
StepID: stepID,
Digest: "digest",
Positions: []stepentry.EnginePosition{
{Position: 0, Kind: "clear_decisions"},
},
})
if err := repo.UpdateTask(ctx, task); err != nil {
t.Fatalf("UpdateTask: %v", err)
}
return holder
}
first := enter("step-review")
if first.EntrySeq != 1 {
t.Fatalf("first EntrySeq = %d, want 1", first.EntrySeq)
}
// Round-trip through a different step and back into step-review — a
// second entry into the same step (e.g. after a rejection round).
enter("step-work")
second := enter("step-review")
if second.EntrySeq != 2 {
t.Fatalf("second EntrySeq = %d, want 2", second.EntrySeq)
}
if second.EntryID == first.EntryID {
t.Fatalf("expected a distinct entry row for the second entry, got the same id %d", second.EntryID)
}
rows := stepEntryRowsForTask(t, repo, "task-3")
if len(rows) != 3 {
t.Fatalf("rows = %d, want 3 (step-review, step-work, step-review)", len(rows))
}
}
func allocateOneStepEntry(t *testing.T, repo *Repository, taskID, stepID string) int64 {
t.Helper()
task := createStepEntriesTestTask(t, repo, taskID, "wf-1", "step-a")
task.WorkflowStepID = stepID
holder := &stepentry.AllocationResult{}
ctx := stepentry.WithResultHolder(context.Background(), holder)
ctx = stepentry.WithPendingAllocation(ctx, stepentry.PendingAllocation{
StepID: stepID,
Digest: "digest",
Positions: []stepentry.EnginePosition{
{Position: 0, Kind: "clear_decisions"},
{Position: 1, Kind: "queue_run_for_each_participant"},
},
})
if err := repo.UpdateTask(ctx, task); err != nil {
t.Fatalf("UpdateTask: %v", err)
}
return holder.EntryID
}
func TestClaimStepEntryMarkerFirstClaimSucceeds(t *testing.T) {
repo := newStepEntriesTestRepo(t)
entryID := allocateOneStepEntry(t, repo, "task-4", "step-review")
claimed, err := repo.ClaimStepEntryMarker(context.Background(), entryID, 0, "clear_decisions", "op-1", time.Now().UTC())
if err != nil {
t.Fatalf("ClaimStepEntryMarker: %v", err)
}
if !claimed {
t.Fatalf("expected the first claim on an absent marker to succeed")
}
}
func TestClaimStepEntryMarkerDoubleClaimIsRejected(t *testing.T) {
repo := newStepEntriesTestRepo(t)
entryID := allocateOneStepEntry(t, repo, "task-5", "step-review")
ctx := context.Background()
claimed, err := repo.ClaimStepEntryMarker(ctx, entryID, 0, "clear_decisions", "op-1", time.Now().UTC())
if err != nil {
t.Fatalf("ClaimStepEntryMarker (first): %v", err)
}
if !claimed {
t.Fatalf("expected the first claim to succeed")
}
claimedAgain, err := repo.ClaimStepEntryMarker(ctx, entryID, 0, "clear_decisions", "op-2", time.Now().UTC())
if err != nil {
t.Fatalf("ClaimStepEntryMarker (second): %v", err)
}
if claimedAgain {
t.Fatalf("expected a second claim on an already-claimed marker to be rejected, not silently succeed")
}
}
func TestClaimStepEntryMarkerDistinctPositionsClaimIndependently(t *testing.T) {
repo := newStepEntriesTestRepo(t)
entryID := allocateOneStepEntry(t, repo, "task-6", "step-review")
ctx := context.Background()
claimedA, err := repo.ClaimStepEntryMarker(ctx, entryID, 0, "clear_decisions", "op-1", time.Now().UTC())
if err != nil {
t.Fatalf("ClaimStepEntryMarker (position 0): %v", err)
}
claimedB, err := repo.ClaimStepEntryMarker(ctx, entryID, 1, "queue_run_for_each_participant", "op-2", time.Now().UTC())
if err != nil {
t.Fatalf("ClaimStepEntryMarker (position 1): %v", err)
}
if !claimedA || !claimedB {
t.Fatalf("expected both distinct positions to claim independently, got (%v, %v)", claimedA, claimedB)
}
}
func TestCompleteStepEntryMarkerDoneAfterClaim(t *testing.T) {
repo := newStepEntriesTestRepo(t)
entryID := allocateOneStepEntry(t, repo, "task-7", "step-review")
ctx := context.Background()
if _, err := repo.ClaimStepEntryMarker(ctx, entryID, 0, "clear_decisions", "op-1", time.Now().UTC()); err != nil {
t.Fatalf("ClaimStepEntryMarker: %v", err)
}
if err := repo.CompleteStepEntryMarker(ctx, entryID, 0, StepEntryMarkerDone, "", time.Now().UTC()); err != nil {
t.Fatalf("CompleteStepEntryMarker: %v", err)
}
state, markerErr := stepEntryMarkerState(t, repo, entryID, 0)
if state != string(StepEntryMarkerDone) {
t.Fatalf("state = %q, want %q", state, StepEntryMarkerDone)
}
if markerErr != "" {
t.Fatalf("marker error = %q, want empty", markerErr)
}
}
func TestCompleteStepEntryMarkerFailedRecordsCause(t *testing.T) {
repo := newStepEntriesTestRepo(t)
entryID := allocateOneStepEntry(t, repo, "task-8", "step-review")
ctx := context.Background()
if _, err := repo.ClaimStepEntryMarker(ctx, entryID, 1, "queue_run_for_each_participant", "op-1", time.Now().UTC()); err != nil {
t.Fatalf("ClaimStepEntryMarker: %v", err)
}
if err := repo.CompleteStepEntryMarker(ctx, entryID, 1, StepEntryMarkerFailed, "queue_run for participant p1: boom", time.Now().UTC()); err != nil {
t.Fatalf("CompleteStepEntryMarker: %v", err)
}
state, markerErr := stepEntryMarkerState(t, repo, entryID, 1)
if state != string(StepEntryMarkerFailed) {
t.Fatalf("state = %q, want %q", state, StepEntryMarkerFailed)
}
if markerErr != "queue_run for participant p1: boom" {
t.Fatalf("marker error = %q, want the recorded failure cause", markerErr)
}
}
func TestCompleteStepEntryMarkerWithoutClaimIsNoop(t *testing.T) {
repo := newStepEntriesTestRepo(t)
entryID := allocateOneStepEntry(t, repo, "task-9", "step-review")
ctx := context.Background()
// Completing a marker that was never claimed (state is still absent — no
// row at all) must not fabricate a terminal row: there is nothing to
// transition, and a spurious "done" row would let a later claim think
// the action already ran when it never did.
if err := repo.CompleteStepEntryMarker(ctx, entryID, 0, StepEntryMarkerDone, "", time.Now().UTC()); err != nil {
t.Fatalf("CompleteStepEntryMarker: %v", err)
}
state, _ := stepEntryMarkerState(t, repo, entryID, 0)
if state != "" {
t.Fatalf("state = %q, want empty (no row, since no claim ever happened)", state)
}
}
func TestGetStepEntryMarkerStateNoRowReturnsNotFound(t *testing.T) {
repo := newStepEntriesTestRepo(t)
entryID := allocateOneStepEntry(t, repo, "task-10", "step-review")
ctx := context.Background()
_, _, found, err := repo.GetStepEntryMarkerState(ctx, entryID, 0)
if err != nil {
t.Fatalf("GetStepEntryMarkerState: %v", err)
}
if found {
t.Fatalf("found = true, want false (no claim ever happened)")
}
}
func TestGetStepEntryMarkerStateReportsDone(t *testing.T) {
repo := newStepEntriesTestRepo(t)
entryID := allocateOneStepEntry(t, repo, "task-11", "step-review")
ctx := context.Background()
if _, err := repo.ClaimStepEntryMarker(ctx, entryID, 0, "clear_decisions", "op-1", time.Now().UTC()); err != nil {
t.Fatalf("ClaimStepEntryMarker: %v", err)
}
if err := repo.CompleteStepEntryMarker(ctx, entryID, 0, StepEntryMarkerDone, "", time.Now().UTC()); err != nil {
t.Fatalf("CompleteStepEntryMarker: %v", err)
}
state, cause, found, err := repo.GetStepEntryMarkerState(ctx, entryID, 0)
if err != nil {
t.Fatalf("GetStepEntryMarkerState: %v", err)
}
if !found {
t.Fatalf("found = false, want true")
}
if state != StepEntryMarkerDone {
t.Fatalf("state = %q, want %q", state, StepEntryMarkerDone)
}
if cause != "" {
t.Fatalf("cause = %q, want empty", cause)
}
}
func TestGetStepEntryMarkerStateReportsFailedWithCause(t *testing.T) {
repo := newStepEntriesTestRepo(t)
entryID := allocateOneStepEntry(t, repo, "task-12", "step-review")
ctx := context.Background()
if _, err := repo.ClaimStepEntryMarker(ctx, entryID, 0, "clear_decisions", "op-1", time.Now().UTC()); err != nil {
t.Fatalf("ClaimStepEntryMarker: %v", err)
}
if err := repo.CompleteStepEntryMarker(ctx, entryID, 0, StepEntryMarkerFailed, "boom", time.Now().UTC()); err != nil {
t.Fatalf("CompleteStepEntryMarker: %v", err)
}
state, cause, found, err := repo.GetStepEntryMarkerState(ctx, entryID, 0)
if err != nil {
t.Fatalf("GetStepEntryMarkerState: %v", err)
}
if !found {
t.Fatalf("found = false, want true")
}
if state != StepEntryMarkerFailed {
t.Fatalf("state = %q, want %q", state, StepEntryMarkerFailed)
}
if cause != "boom" {
t.Fatalf("cause = %q, want %q", cause, "boom")
}
}
// seedWorkflowStepDecisionsTable creates workflow_step_decisions directly
// (rather than importing internal/workflow/repository, which owns it in
// production) so ClearStepDecisionsAndCompleteMarker has something to
// delete against — see phase2_sqlite.go's decisionsSchema for the DDL this
// mirrors.
func seedWorkflowStepDecisionsTable(t *testing.T, repo *Repository) {
t.Helper()
_, err := repo.db.Exec(`
CREATE TABLE IF NOT EXISTS workflow_step_decisions (
id TEXT PRIMARY KEY,
task_id TEXT NOT NULL,
step_id TEXT NOT NULL,
participant_id TEXT NOT NULL,
decision TEXT NOT NULL,
decided_at TIMESTAMP NOT NULL
)
`)
if err != nil {
t.Fatalf("create workflow_step_decisions: %v", err)
}
}
func insertStepDecision(t *testing.T, repo *Repository, id, taskID, stepID string) {
t.Helper()
_, err := repo.db.ExecContext(context.Background(), repo.db.Rebind(`
INSERT INTO workflow_step_decisions (id, task_id, step_id, participant_id, decision, decided_at)
VALUES (?, ?, ?, ?, ?, ?)
`), id, taskID, stepID, "participant-1", "approve", time.Now().UTC())
if err != nil {
t.Fatalf("insert workflow_step_decisions row: %v", err)
}
}
func countStepDecisions(t *testing.T, repo *Repository, taskID, stepID string) int {
t.Helper()
var n int
err := repo.db.QueryRowContext(context.Background(), repo.db.Rebind(
`SELECT COUNT(*) FROM workflow_step_decisions WHERE task_id = ? AND step_id = ?`,
), taskID, stepID).Scan(&n)
if err != nil {
t.Fatalf("count workflow_step_decisions: %v", err)
}
return n
}
func TestClearStepDecisionsAndCompleteMarkerDeletesDecisionsAndCompletesMarker(t *testing.T) {
repo := newStepEntriesTestRepo(t)
seedWorkflowStepDecisionsTable(t, repo)
entryID := allocateOneStepEntry(t, repo, "task-10", "step-review")
ctx := context.Background()
insertStepDecision(t, repo, "dec-1", "task-10", "step-review")
insertStepDecision(t, repo, "dec-2", "task-10", "step-review")
if _, err := repo.ClaimStepEntryMarker(ctx, entryID, 0, "clear_decisions", "op-1", time.Now().UTC()); err != nil {
t.Fatalf("ClaimStepEntryMarker: %v", err)
}
rows, err := repo.ClearStepDecisionsAndCompleteMarker(ctx, "task-10", "step-review", entryID, 0, time.Now().UTC())
if err != nil {
t.Fatalf("ClearStepDecisionsAndCompleteMarker: %v", err)
}
if rows != 2 {
t.Fatalf("rows = %d, want 2", rows)
}
if n := countStepDecisions(t, repo, "task-10", "step-review"); n != 0 {
t.Fatalf("remaining decisions = %d, want 0", n)
}
state, markerErr := stepEntryMarkerState(t, repo, entryID, 0)
if state != string(StepEntryMarkerDone) {
t.Fatalf("marker state = %q, want %q", state, StepEntryMarkerDone)
}
if markerErr != "" {
t.Fatalf("marker error = %q, want empty", markerErr)
}
}
func TestClearStepDecisionsAndCompleteMarkerRejectsMissingIDs(t *testing.T) {
repo := newStepEntriesTestRepo(t)
seedWorkflowStepDecisionsTable(t, repo)
entryID := allocateOneStepEntry(t, repo, "task-11", "step-review")
ctx := context.Background()
insertStepDecision(t, repo, "dec-1", "task-11", "step-review")
if _, err := repo.ClaimStepEntryMarker(ctx, entryID, 0, "clear_decisions", "op-1", time.Now().UTC()); err != nil {
t.Fatalf("ClaimStepEntryMarker: %v", err)
}
if _, err := repo.ClearStepDecisionsAndCompleteMarker(ctx, "", "step-review", entryID, 0, time.Now().UTC()); err == nil {
t.Fatalf("expected an error for an empty task_id")
}
// Nothing should have changed: the validation guard runs before BeginTx.
if n := countStepDecisions(t, repo, "task-11", "step-review"); n != 1 {
t.Fatalf("remaining decisions = %d, want 1 (untouched)", n)
}
state, _ := stepEntryMarkerState(t, repo, entryID, 0)
if state != string(StepEntryMarkerInProgress) {
t.Fatalf("marker state = %q, want %q (untouched)", state, StepEntryMarkerInProgress)
}
}
func stepEntryMarkerState(t *testing.T, repo *Repository, entryID int64, position int) (state, markerErr string) {
t.Helper()
var s, e *string
err := repo.db.QueryRowContext(context.Background(), repo.db.Rebind(`
SELECT state, error FROM workflow_step_entry_markers WHERE entry_id = ? AND position = ?
`), entryID, position).Scan(&s, &e)
if err != nil {
return "", ""
}
if s != nil {
state = *s
}
if e != nil {
markerErr = *e
}
return state, markerErr
}
apps/backend/internal/task/repository/sqlite/task.go
+8
View File
@@ -597,6 +597,14 @@ func (r *Repository) updateTaskTx(ctx context.Context, tx *sql.Tx, task *models.
task.WorkflowStepTransitionID = transitionID
entryID = formatEntryID(transitionID)
// Both sides kept. pr-2907's allocateStepEntryIfPending is a write with its
// own concern; #3043's entryID is a pure format of transitionID (line above),
// so neither subsumes the other and dropping either loses real behaviour.
// The two-value return is #3043's signature, which this function now carries.
if err := r.allocateStepEntryIfPending(ctx, tx, task.ID, task.UpdatedAt); err != nil {
return "", err
}
if err := syncRunnerInTx(ctx, tx, r.db.Rebind, task.WorkflowStepID, task.ID, task.AssigneeAgentProfileID); err != nil {
return "", err
}
apps/backend/internal/workflow/engine/entrydispatch_test.go
+12 -10
View File
@@ -14,18 +14,20 @@ import (
// TestSessionShapedAndSessionIndependentKindsPartitionCompiledOnEnterKinds
// asserts the two classification maps in this file are exhaustive and
// disjoint over exactly the ActionKind values compileOnEnter (types.go) can
// emit. It parses that function's source rather than hand-listing the
// kinds, so a kind added to the compiler later without a deliberate
// classification decision fails this test instead of silently defaulting
// into double execution (dispatched from both HandleTriggerSessionShapedOnly
// and DispatchStepEntry) or silence (dispatched from neither) — see
// disjoint over exactly the ActionKind values CompileOnEnterAction (types.go)
// can emit for a single on_enter declaration — the per-kind switch
// compileOnEnter delegates to. It parses that function's source rather than
// hand-listing the kinds, so a kind added to the compiler later without a
// deliberate classification decision fails this test instead of silently
// defaulting into double execution (dispatched from both
// HandleTriggerSessionShapedOnly and DispatchStepEntry) or silence
// (dispatched from neither) — see
// docs/specs/office/system-design/step-entry-sequence-execution.md
// ("The two lists together are exhaustive").
func TestSessionShapedAndSessionIndependentKindsPartitionCompiledOnEnterKinds(t *testing.T) {
compiled, err := actionKindsAssignedInFunc("types.go", "compileOnEnter")
compiled, err := actionKindsAssignedInFunc("types.go", "CompileOnEnterAction")
require.NoError(t, err)
require.NotEmpty(t, compiled, "compileOnEnter must assign at least one ActionKind for this test to be meaningful")
require.NotEmpty(t, compiled, "CompileOnEnterAction must assign at least one ActionKind for this test to be meaningful")
for _, kind := range compiled {
shaped := isSessionShapedActionKind(kind)
@@ -38,11 +40,11 @@ func TestSessionShapedAndSessionIndependentKindsPartitionCompiledOnEnterKinds(t
for kind := range sessionShapedActionKinds {
assert.Contains(t, compiled, kind,
"sessionShapedActionKinds lists %q, which compileOnEnter no longer emits", kind)
"sessionShapedActionKinds lists %q, which CompileOnEnterAction no longer emits", kind)
}
for kind := range sessionIndependentActionKinds {
assert.Contains(t, compiled, kind,
"sessionIndependentActionKinds lists %q, which compileOnEnter no longer emits", kind)
"sessionIndependentActionKinds lists %q, which CompileOnEnterAction no longer emits", kind)
}
}
apps/backend/internal/workflow/engine/phase2_callbacks.go
+22 -1
View File
@@ -454,6 +454,18 @@ func (c QueueRunForEachParticipantCallback) Execute(ctx context.Context, in Acti
return ActionResult{}, fmt.Errorf("queue_run_for_each_participant list participants: %w", err)
}
reason := queueRunForEachParticipantReason(in)
// Collect-and-continue (AC-C1): one participant's QueueRun failure must
// not abort the fan-out to their siblings — a reviewer whose queue is
// briefly unavailable should not silently block every other reviewer's
// run from ever being queued. Every matching participant gets an attempt
// regardless of earlier failures; the errors are joined and returned
// together so the caller still sees (and can log/retry) every failure.
//
// Merge note (local/integration): #2969 (WO-30) replaced the raw
// participant list with roleSeatsForFanOut, which already filters by
// role and canonicalizes the slate, so #2907's inline `p.Role != cfg.Role`
// guard is redundant here and is dropped rather than duplicated.
var errs []error
for _, p := range seats {
req := QueueRunRequest{
AgentProfileID: p.AgentProfileID,
@@ -463,10 +475,19 @@ func (c QueueRunForEachParticipantCallback) Execute(ctx context.Context, in Acti
IdempotencyKey: idempotencyKey(in, p.AgentProfileID, taskID),
Payload: queueRunPayload(in, cfg.Payload, taskID),
}
// Merge note (local/integration): #3011 gave QueueRun a second return
// value; #2907's fan-out collects errors rather than aborting (AC-C1,
// see the comment above). Take #3011's signature and keep the
// collect-and-continue policy — returning early here would abort the
// fan-out on one participant's failure, which is the exact behaviour
// AC-C1 exists to prevent.
if _, err := c.Adapter.QueueRun(ctx, req); err != nil {
return ActionResult{}, fmt.Errorf("queue_run for participant %s: %w", p.ID, err)
errs = append(errs, fmt.Errorf("queue_run for participant %s: %w", p.ID, err))
}
}
if len(errs) > 0 {
return ActionResult{}, errors.Join(errs...)
}
return ActionResult{}, nil
}
apps/backend/internal/workflow/engine/queue_run_test.go
+60
View File
@@ -799,3 +799,63 @@ func TestQueueRunForEachParticipantCallback_MissingDeps_Errors(t *testing.T) {
t.Fatalf("expected ErrActionNotYetWired, got %v", err)
}
}
// selectiveFailRunQueue fails QueueRun only for the agent profile ids listed
// in failFor, recording every call (including the ones it fails) so a test
// can assert the fan-out kept going past the failure.
type selectiveFailRunQueue struct {
calls []QueueRunRequest
failFor map[string]error
}
func (f *selectiveFailRunQueue) QueueRun(_ context.Context, req QueueRunRequest) (QueueOutcome, error) {
f.calls = append(f.calls, req)
if err, ok := f.failFor[req.AgentProfileID]; ok {
return "", err
}
return "", nil
}
// TestQueueRunForEachParticipantCallback_OneParticipantFailureDoesNotAbortFanOut
// pins AC-C1: a single participant's QueueRun error must not stop the
// remaining participants from being queued. Before this fix the loop
// returned on the first error, so rev-B and rev-C never got a call.
func TestQueueRunForEachParticipantCallback_OneParticipantFailureDoesNotAbortFanOut(t *testing.T) {
boom := errors.New("boom")
q := &selectiveFailRunQueue{failFor: map[string]error{"rev-A": boom}}
parts := fakeParticipants{list: []ParticipantInfo{
{ID: "p1", Role: "reviewer", AgentProfileID: "rev-A"},
{ID: "p2", Role: "reviewer", AgentProfileID: "rev-B"},
{ID: "p3", Role: "reviewer", AgentProfileID: "rev-C"},
}}
cb := QueueRunForEachParticipantCallback{Adapter: q, Participants: parts}
in := ActionInput{
Trigger: TriggerOnEnter,
State: MachineState{TaskID: "task-1"},
Step: StepSpec{ID: "step-1"},
Action: Action{
Kind: ActionQueueRunForEachParticipant,
QueueRunForEachParticipant: &QueueRunForEachParticipantAction{
Role: "reviewer",
},
},
}
_, err := cb.Execute(context.Background(), in)
if err == nil {
t.Fatalf("expected the joined error from rev-A's failure to be returned")
}
if !errors.Is(err, boom) {
t.Fatalf("expected the returned error to wrap the participant failure, got %v", err)
}
if len(q.calls) != 3 {
t.Fatalf("expected all 3 participants to receive a QueueRun attempt despite rev-A's failure, got %d calls: %#v", len(q.calls), q.calls)
}
got := []string{q.calls[0].AgentProfileID, q.calls[1].AgentProfileID, q.calls[2].AgentProfileID}
want := []string{"rev-A", "rev-B", "rev-C"}
for i, w := range want {
if got[i] != w {
t.Fatalf("call %d agent = %q, want %q (order: %#v)", i, got[i], w, got)
}
}
}
apps/backend/internal/workflow/engine/types.go
+63 -38
View File
@@ -311,49 +311,74 @@ func CompileStep(step *wfmodels.WorkflowStep) StepSpec {
func compileOnEnter(step *wfmodels.WorkflowStep) []Action {
actions := make([]Action, 0, len(step.Events.OnEnter))
for _, action := range step.Events.OnEnter {
switch action.Type {
case wfmodels.OnEnterEnablePlanMode:
actions = append(actions, Action{Kind: ActionEnablePlanMode})
case wfmodels.OnEnterAutoStartAgent:
actions = append(actions, Action{Kind: ActionAutoStartAgent, AutoStartAgent: &AutoStartAgentAction{QueueIfBusy: true}})
case wfmodels.OnEnterResetAgentContext:
actions = append(actions, Action{Kind: ActionResetAgentContext})
case wfmodels.OnEnterSetSessionMode:
mode := readSessionMode(action.Config)
if mode == "" {
continue // skip set_session_mode actions with no target mode
}
actions = append(actions, Action{Kind: ActionSetSessionMode, SetSessionMode: &SetSessionModeAction{Mode: mode}})
case wfmodels.OnEnterRunCodeReview:
actions = append(actions, Action{
Kind: ActionRunCodeReview,
RunCodeReview: &RunCodeReviewAction{AgentProfileID: readReviewAgentProfileID(action.Config)},
})
case wfmodels.OnEnterClearDecisions:
actions = append(actions, Action{
Kind: ActionClearDecisions,
ClearDecisions: &ClearDecisionsAction{},
})
case wfmodels.OnEnterQueueRunForEachParticipant:
actions = append(actions, Action{
Kind: ActionQueueRunForEachParticipant,
QueueRunForEachParticipant: readQueueRunForEachParticipantConfig(action.Config),
})
case wfmodels.OnEnterQueueRun:
actions = append(actions, Action{
Kind: ActionQueueRun,
QueueRun: readQueueRunConfig(action.Config),
})
case wfmodels.OnEnterEnsureParticipantSeat:
actions = append(actions, Action{
Kind: ActionEnsureParticipantSeat,
EnsureParticipantSeat: readEnsureParticipantSeatConfig(action.Config),
})
if compiled, ok := CompileOnEnterAction(action); ok {
actions = append(actions, compiled)
}
}
return actions
}
// CompileOnEnterAction compiles a single on_enter declaration into its
// engine Action, or (Action{}, false) when the declaration produces no
// action (an unrecognised type, or a recognised type with config that
// doesn't resolve to anything runnable — e.g. set_session_mode with no
// target mode).
//
// Exported so callers outside the engine's own on_enter evaluation loop —
// specifically internal/orchestrator's processOnEnter, which dispatches
// clear_decisions/queue_run_for_each_participant directly via a step-entry
// marker rather than through Engine.HandleTrigger — can reuse the exact
// same per-kind config parsing instead of duplicating it. See
// docs/specs/workflow-on-enter-action-dispatch/spec.md.
func CompileOnEnterAction(action wfmodels.OnEnterAction) (Action, bool) {
switch action.Type {
case wfmodels.OnEnterEnablePlanMode:
return Action{Kind: ActionEnablePlanMode}, true
case wfmodels.OnEnterAutoStartAgent:
return Action{Kind: ActionAutoStartAgent, AutoStartAgent: &AutoStartAgentAction{QueueIfBusy: true}}, true
case wfmodels.OnEnterResetAgentContext:
return Action{Kind: ActionResetAgentContext}, true
case wfmodels.OnEnterSetSessionMode:
mode := readSessionMode(action.Config)
if mode == "" {
return Action{}, false // no target mode: nothing to run
}
return Action{Kind: ActionSetSessionMode, SetSessionMode: &SetSessionModeAction{Mode: mode}}, true
case wfmodels.OnEnterRunCodeReview:
return Action{
Kind: ActionRunCodeReview,
RunCodeReview: &RunCodeReviewAction{AgentProfileID: readReviewAgentProfileID(action.Config)},
}, true
case wfmodels.OnEnterClearDecisions:
return Action{
Kind: ActionClearDecisions,
ClearDecisions: &ClearDecisionsAction{},
}, true
case wfmodels.OnEnterQueueRunForEachParticipant:
return Action{
Kind: ActionQueueRunForEachParticipant,
QueueRunForEachParticipant: readQueueRunForEachParticipantConfig(action.Config),
}, true
case wfmodels.OnEnterQueueRun:
return Action{
Kind: ActionQueueRun,
QueueRun: readQueueRunConfig(action.Config),
}, true
case wfmodels.OnEnterEnsureParticipantSeat:
// Carried across the CompileOnEnterAction refactor from #3043's
// open-coded switch. Without this case the compiler silently drops
// every ensure_participant_seat action, which is the whole point of
// that PR: nothing else creates reviewer/approver seats, so the slate
// stays empty and queue_run_for_each_participant has nobody to queue.
return Action{
Kind: ActionEnsureParticipantSeat,
EnsureParticipantSeat: readEnsureParticipantSeatConfig(action.Config),
}, true
default:
return Action{}, false
}
}
func compileOnTurnStart(step *wfmodels.WorkflowStep) []Action {
actions := make([]Action, 0, len(step.Events.OnTurnStart))
for _, action := range step.Events.OnTurnStart {
apps/backend/internal/workflow/stepentry/stepentry.go
+153
View File
@@ -0,0 +1,153 @@
// Package stepentry carries the durable "step-entry" allocation contract from
// the caller that resolves a workflow step transition (and therefore knows
// the landing step's on_enter declaration) down to the task repository
// transaction that persists the step change. It intentionally has no
// dependency on the task repository package; the repository reads only the
// already-resolved PendingAllocation/AllocationResult shapes carried on
// context, mirroring how internal/steptelemetry threads attribution.
//
// Scope note: this package only recognises clear_decisions and
// queue_run_for_each_participant as engine-owned on_enter kinds for now.
// queue_run and run_code_review on_enter dispatch (AC-A7/AC-A8 in
// docs/specs/workflow-on-enter-action-dispatch/spec.md) are explicitly
// deferred to a later Build round — see that spec's Verification section and
// the task plan's scope note.
package stepentry
import (
"context"
"crypto/sha256"
"encoding/hex"
"strconv"
"strings"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
)
// EnginePosition is one engine-owned on_enter action's declared position
// (0-based, index into the step's on_enter list) and kind, as resolved by
// the caller that has the workflow step in hand.
type EnginePosition struct {
Position int
Kind string
}
// PendingAllocation is what the caller resolving a step transition passes
// down to the write-site transaction: which step is being entered, the
// digest of its full on_enter declaration, and the ordered list of
// engine-owned positions the write site should allocate markers for.
type PendingAllocation struct {
StepID string
Digest string
Positions []EnginePosition
}
// AllocationResult is a mutable out-parameter: the caller creates one, wraps
// it onto the context via WithResultHolder, and reads it back after the
// write-site transaction commits. It is populated by the repository layer.
type AllocationResult struct {
EntryID int64
EntrySeq int64
}
// MarkerState is a step-entry marker's terminal (or in-progress) state.
// Declared here rather than in the task repository package so that
// dispatch-side callers (internal/orchestrator) can reference it without
// importing the repository package's implementation types.
type MarkerState string
const (
// MarkerInProgress is set the moment a claim succeeds.
MarkerInProgress MarkerState = "in_progress"
// MarkerDone marks a successfully executed action.
MarkerDone MarkerState = "done"
// MarkerFailed marks an action whose callback returned an error; the
// error is recorded alongside the marker.
MarkerFailed MarkerState = "failed"
)
type pendingAllocationKey struct{}
type resultHolderKey struct{}
// WithPendingAllocation attaches the resolved allocation input to ctx.
func WithPendingAllocation(ctx context.Context, p PendingAllocation) context.Context {
return context.WithValue(ctx, pendingAllocationKey{}, p)
}
// FromContext reads back a PendingAllocation previously attached with
// WithPendingAllocation. ok is false when none was attached (the common
// case for callers that have not opted into step-entry allocation yet).
func FromContext(ctx context.Context) (PendingAllocation, bool) {
p, ok := ctx.Value(pendingAllocationKey{}).(PendingAllocation)
return p, ok
}
// WithResultHolder attaches a mutable AllocationResult pointer to ctx so the
// repository layer can report the allocated entry identity back to a caller
// several function calls away without changing every intermediate
// signature — the same shape steptelemetry uses for attribution, just with
// an output instead of an input.
func WithResultHolder(ctx context.Context, r *AllocationResult) context.Context {
return context.WithValue(ctx, resultHolderKey{}, r)
}
// ResultHolderFromContext reads back the AllocationResult pointer previously
// attached with WithResultHolder.
func ResultHolderFromContext(ctx context.Context) (*AllocationResult, bool) {
r, ok := ctx.Value(resultHolderKey{}).(*AllocationResult)
return r, ok
}
// IsEngineOwnedOnEnter reports whether an on_enter action kind is dispatched
// by the step-entry marker system as of this Build round. See the package
// doc's scope note for the kinds deliberately excluded for now.
func IsEngineOwnedOnEnter(t wfmodels.OnEnterActionType) bool {
switch t {
case wfmodels.OnEnterClearDecisions, wfmodels.OnEnterQueueRunForEachParticipant:
return true
default:
return false
}
}
// BuildPendingAllocation inspects a step's on_enter declaration and returns
// the PendingAllocation the write site should allocate, plus false when the
// step declares no engine-owned on_enter actions (nothing to allocate).
func BuildPendingAllocation(stepID string, actions []wfmodels.OnEnterAction) (PendingAllocation, bool) {
positions := make([]EnginePosition, 0, len(actions))
for i, action := range actions {
if IsEngineOwnedOnEnter(action.Type) {
positions = append(positions, EnginePosition{Position: i, Kind: string(action.Type)})
}
}
if len(positions) == 0 {
return PendingAllocation{}, false
}
return PendingAllocation{
StepID: stepID,
Digest: ComputeDigest(actions),
Positions: positions,
}, true
}
// ComputeDigest returns a stable digest of a step's full on_enter
// declaration (every action, not just the engine-owned ones), so a
// mid-flight template edit can eventually be detected by comparing an
// entry's stored digest against a freshly computed one. The algorithm and
// encoding are this package's choice (AC-H7): sha256 hex over the ordered
// "position:kind" pairs. Config payloads are deliberately excluded — this
// digest only needs to notice a shape change (actions added/removed/
// reordered/retyped), not a config tweak.
func ComputeDigest(actions []wfmodels.OnEnterAction) string {
var b strings.Builder
for i, action := range actions {
if i > 0 {
b.WriteByte('\n')
}
b.WriteString(strconv.Itoa(i))
b.WriteByte(':')
b.WriteString(string(action.Type))
}
sum := sha256.Sum256([]byte(b.String()))
return hex.EncodeToString(sum[:])
}
apps/backend/internal/workflow/stepentry/stepentry_test.go
+129
View File
@@ -0,0 +1,129 @@
package stepentry
import (
"context"
"reflect"
"testing"
wfmodels "github.com/kandev/kandev/internal/workflow/models"
)
func TestComputeDigestDeterministicAndPositionSensitive(t *testing.T) {
a := []wfmodels.OnEnterAction{
{Type: wfmodels.OnEnterClearDecisions},
{Type: wfmodels.OnEnterQueueRunForEachParticipant},
}
b := []wfmodels.OnEnterAction{
{Type: wfmodels.OnEnterClearDecisions},
{Type: wfmodels.OnEnterQueueRunForEachParticipant},
}
if ComputeDigest(a) != ComputeDigest(b) {
t.Fatalf("expected identical declarations to produce identical digests")
}
reordered := []wfmodels.OnEnterAction{
{Type: wfmodels.OnEnterQueueRunForEachParticipant},
{Type: wfmodels.OnEnterClearDecisions},
}
if ComputeDigest(a) == ComputeDigest(reordered) {
t.Fatalf("expected reordered declaration to change the digest")
}
withConfigOnly := []wfmodels.OnEnterAction{
{Type: wfmodels.OnEnterClearDecisions, Config: map[string]interface{}{"unused": "value"}},
{Type: wfmodels.OnEnterQueueRunForEachParticipant},
}
if ComputeDigest(a) != ComputeDigest(withConfigOnly) {
t.Fatalf("expected a config-only change to leave the digest unchanged (shape digest, not config digest)")
}
}
func TestIsEngineOwnedOnEnter(t *testing.T) {
cases := map[wfmodels.OnEnterActionType]bool{
wfmodels.OnEnterClearDecisions: true,
wfmodels.OnEnterQueueRunForEachParticipant: true,
wfmodels.OnEnterQueueRun: false,
wfmodels.OnEnterRunCodeReview: false,
wfmodels.OnEnterEnablePlanMode: false,
wfmodels.OnEnterAutoStartAgent: false,
wfmodels.OnEnterResetAgentContext: false,
wfmodels.OnEnterSetSessionMode: false,
}
for kind, want := range cases {
if got := IsEngineOwnedOnEnter(kind); got != want {
t.Errorf("IsEngineOwnedOnEnter(%s) = %v, want %v", kind, got, want)
}
}
}
func TestBuildPendingAllocationNoEngineOwnedActions(t *testing.T) {
actions := []wfmodels.OnEnterAction{
{Type: wfmodels.OnEnterEnablePlanMode},
{Type: wfmodels.OnEnterAutoStartAgent},
}
_, ok := BuildPendingAllocation("step-1", actions)
if ok {
t.Fatalf("expected no allocation for a step with no engine-owned on_enter actions")
}
}
func TestBuildPendingAllocationTracksDeclaredPositions(t *testing.T) {
actions := []wfmodels.OnEnterAction{
{Type: wfmodels.OnEnterEnablePlanMode},
{Type: wfmodels.OnEnterClearDecisions},
{Type: wfmodels.OnEnterQueueRunForEachParticipant},
}
pending, ok := BuildPendingAllocation("step-1", actions)
if !ok {
t.Fatalf("expected an allocation for a step declaring engine-owned actions")
}
if pending.StepID != "step-1" {
t.Fatalf("expected StepID to be carried through, got %q", pending.StepID)
}
if len(pending.Positions) != 2 {
t.Fatalf("expected 2 engine-owned positions, got %d: %+v", len(pending.Positions), pending.Positions)
}
if pending.Positions[0].Position != 1 || pending.Positions[0].Kind != string(wfmodels.OnEnterClearDecisions) {
t.Errorf("unexpected first position: %+v", pending.Positions[0])
}
if pending.Positions[1].Position != 2 || pending.Positions[1].Kind != string(wfmodels.OnEnterQueueRunForEachParticipant) {
t.Errorf("unexpected second position: %+v", pending.Positions[1])
}
if pending.Digest != ComputeDigest(actions) {
t.Errorf("expected pending.Digest to match ComputeDigest(actions)")
}
}
func TestPendingAllocationContextRoundTrip(t *testing.T) {
ctx := context.Background()
if _, ok := FromContext(ctx); ok {
t.Fatalf("expected no pending allocation on a bare context")
}
pending := PendingAllocation{StepID: "step-1", Digest: "abc"}
ctx = WithPendingAllocation(ctx, pending)
got, ok := FromContext(ctx)
if !ok {
t.Fatalf("expected pending allocation to round-trip through context")
}
if !reflect.DeepEqual(got, pending) {
t.Errorf("got %+v, want %+v", got, pending)
}
}
func TestResultHolderContextRoundTrip(t *testing.T) {
ctx := context.Background()
if _, ok := ResultHolderFromContext(ctx); ok {
t.Fatalf("expected no result holder on a bare context")
}
holder := &AllocationResult{}
ctx = WithResultHolder(ctx, holder)
got, ok := ResultHolderFromContext(ctx)
if !ok {
t.Fatalf("expected result holder to round-trip through context")
}
got.EntryID = 42
got.EntrySeq = 3
if holder.EntryID != 42 || holder.EntrySeq != 3 {
t.Fatalf("expected mutations through the round-tripped pointer to reach the original holder")
}
}
docs/specs/INDEX.md
+1
View File
@@ -36,5 +36,6 @@ The following specification was added after this migration and remains in the le
- [Task Cost & Token Ledger](task-cost-ledger/spec.md) (draft)
- [Startup listener before recovery](startup-listener-before-recovery/spec.md) (draft)
- [Workflow on_enter action dispatch](workflow-on-enter-action-dispatch/spec.md) (draft)
## Authoring rule
docs/specs/spec-lint.json
+2 -1
View File
@@ -11,6 +11,7 @@
},
"legacy_size_exceptions": {
"docs/specs/office-task-content-editing/spec.md": 84103,
"docs/specs/task-cost-ledger/spec.md": 91931
"docs/specs/task-cost-ledger/spec.md": 91931,
"docs/specs/workflow-on-enter-action-dispatch/spec.md": 116869
}
}
docs/specs/ui/requirements/sidebar-repository-grouping.md
-1
View File
@@ -43,4 +43,3 @@ The UI system owns this presentation contract. The task system supplies the orde
- Changes to repository attachment order.
- Changes to saved-view persistence or group-collapse persistence.
- Changes to group-header layout, truncation, or touch behavior.
docs/specs/workflow-on-enter-action-dispatch/spec.md
+1570
View File
File diff suppressed because it is too large Load Diff