fix: quiesce active turns before workflow context reset (#3156)
This commit is contained in:
@@ -222,6 +222,11 @@ func (m *Manager) claimPromptCompletion(
|
||||
) (promptCompletionClaim, bool) {
|
||||
claim := promptCompletionClaim{}
|
||||
if event.PromptGeneration == 0 {
|
||||
if execution.dispatchedPromptPending.Load() {
|
||||
m.logger.Debug("ignoring unnumbered completion while a dispatched prompt is pending",
|
||||
zap.String("execution_id", execution.ID))
|
||||
return claim, false
|
||||
}
|
||||
return claim, true
|
||||
}
|
||||
|
||||
|
||||
@@ -1666,6 +1666,58 @@ func TestHandleAgentEvent_DelayedCompleteCannotFinishReplacementPrompt(t *testin
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleAgentEvent_UnnumberedCompleteCannotReleasePendingPrompt(t *testing.T) {
|
||||
mgr, eventBus := createTestManagerWithTracking()
|
||||
execution := createTestExecution("exec-1", "task-1", "session-1")
|
||||
if err := mgr.executionStore.Add(execution); err != nil {
|
||||
t.Fatalf("add execution: %v", err)
|
||||
}
|
||||
generation, err := mgr.executionStore.BeginPrompt(execution.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("begin prompt: %v", err)
|
||||
}
|
||||
mgr.executionStore.MarkPromptDispatched(execution.ID, generation)
|
||||
execution.dispatchedPromptPending.Store(true)
|
||||
|
||||
if mgr.handleCompleteEvent(execution, &agentctl.AgentEvent{
|
||||
Type: streams.EventTypeComplete,
|
||||
SessionID: execution.SessionID,
|
||||
Data: map[string]any{"stop_reason": "end_turn"},
|
||||
}) {
|
||||
t.Fatal("unnumbered completion was accepted while a numbered prompt was pending")
|
||||
}
|
||||
select {
|
||||
case signal := <-execution.promptDoneCh:
|
||||
t.Fatalf("unnumbered completion released pending prompt: %+v", signal)
|
||||
default:
|
||||
}
|
||||
if !execution.dispatchedPromptPending.Load() {
|
||||
t.Fatal("unnumbered completion cleared the pending prompt gate")
|
||||
}
|
||||
for _, published := range eventBus.PublishedEvents {
|
||||
if published.Subject == events.AgentReady {
|
||||
t.Fatal("unnumbered completion published AgentReady for pending prompt")
|
||||
}
|
||||
}
|
||||
|
||||
if !mgr.handleCompleteEvent(execution, &agentctl.AgentEvent{
|
||||
Type: streams.EventTypeComplete,
|
||||
SessionID: execution.SessionID,
|
||||
PromptGeneration: generation,
|
||||
Data: map[string]any{"stop_reason": "end_turn"},
|
||||
}) {
|
||||
t.Fatal("numbered completion was rejected after unnumbered completion")
|
||||
}
|
||||
select {
|
||||
case signal := <-execution.promptDoneCh:
|
||||
if signal.PromptGeneration != generation {
|
||||
t.Fatalf("completion generation = %d, want %d", signal.PromptGeneration, generation)
|
||||
}
|
||||
default:
|
||||
t.Fatal("numbered completion did not signal the pending prompt")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleAgentEvent_ForegroundIdlePreservesPromptGeneration(t *testing.T) {
|
||||
mgr, eventBus := createTestManagerWithTracking()
|
||||
execution := createTestExecution("exec-foreground-idle", "task-1", "session-1")
|
||||
|
||||
@@ -26,11 +26,29 @@ import (
|
||||
)
|
||||
|
||||
const (
|
||||
modelConfigOptionID = "model"
|
||||
attachmentDeliveryPath = "path"
|
||||
attachmentDeliveryPrompt = "prompt"
|
||||
modelConfigOptionID = "model"
|
||||
attachmentDeliveryPath = "path"
|
||||
attachmentDeliveryPrompt = "prompt"
|
||||
pendingDispatchedPromptWaitTimeout = 10 * time.Second
|
||||
)
|
||||
|
||||
// PendingDispatchedPromptTimeoutError reports that a successor prompt could
|
||||
// not observe completion of an earlier dispatch-only prompt within the
|
||||
// bounded admission interval. The pending gate remains set so cancellation
|
||||
// escalation remains the only path that can release the predecessor.
|
||||
type PendingDispatchedPromptTimeoutError struct {
|
||||
ExecutionID string
|
||||
Timeout time.Duration
|
||||
}
|
||||
|
||||
func (e *PendingDispatchedPromptTimeoutError) Error() string {
|
||||
return fmt.Sprintf(
|
||||
"pending dispatched prompt completion timed out for execution %q after %s",
|
||||
e.ExecutionID,
|
||||
e.Timeout,
|
||||
)
|
||||
}
|
||||
|
||||
// SessionManager handles ACP session initialization and management
|
||||
type SessionManager struct {
|
||||
logger *logger.Logger
|
||||
@@ -1530,12 +1548,27 @@ func waitForPendingDispatchedPrompt(ctx context.Context, execution *AgentExecuti
|
||||
if !execution.dispatchedPromptPending.Load() {
|
||||
return nil
|
||||
}
|
||||
timer := time.NewTimer(pendingDispatchedPromptWaitTimeout)
|
||||
defer timer.Stop()
|
||||
select {
|
||||
case <-execution.promptDoneCh:
|
||||
execution.dispatchedPromptPending.Store(false)
|
||||
return nil
|
||||
case <-ctx.Done():
|
||||
return ctx.Err()
|
||||
case <-timer.C:
|
||||
// Prefer a completion that arrived at the timeout boundary. If no
|
||||
// signal is available, retain the gate for cancellation escalation.
|
||||
select {
|
||||
case <-execution.promptDoneCh:
|
||||
execution.dispatchedPromptPending.Store(false)
|
||||
return nil
|
||||
default:
|
||||
return &PendingDispatchedPromptTimeoutError{
|
||||
ExecutionID: execution.ID,
|
||||
Timeout: pendingDispatchedPromptWaitTimeout,
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
package lifecycle
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
"testing/synctest"
|
||||
"time"
|
||||
)
|
||||
|
||||
func pendingPromptExecution() *AgentExecution {
|
||||
execution := &AgentExecution{
|
||||
ID: "execution-pending",
|
||||
promptDoneCh: make(chan PromptCompletionSignal, 1),
|
||||
}
|
||||
execution.dispatchedPromptPending.Store(true)
|
||||
return execution
|
||||
}
|
||||
|
||||
func TestWaitForPendingDispatchedPrompt_TimesOutWithoutClearingGate(t *testing.T) {
|
||||
synctest.Test(t, func(t *testing.T) {
|
||||
execution := pendingPromptExecution()
|
||||
result := make(chan error, 1)
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
t.Cleanup(cancel)
|
||||
go func() {
|
||||
result <- waitForPendingDispatchedPrompt(ctx, execution)
|
||||
}()
|
||||
|
||||
time.Sleep(10 * time.Second)
|
||||
synctest.Wait()
|
||||
select {
|
||||
case err := <-result:
|
||||
var timeoutErr *PendingDispatchedPromptTimeoutError
|
||||
if !errors.As(err, &timeoutErr) {
|
||||
t.Fatalf("error = %v, want PendingDispatchedPromptTimeoutError", err)
|
||||
}
|
||||
if timeoutErr.ExecutionID != execution.ID {
|
||||
t.Fatalf("timeout execution ID = %q, want %q", timeoutErr.ExecutionID, execution.ID)
|
||||
}
|
||||
if !execution.dispatchedPromptPending.Load() {
|
||||
t.Fatal("timeout cleared the pending prompt gate")
|
||||
}
|
||||
default:
|
||||
t.Fatal("pending prompt wait did not return at its timeout")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestWaitForPendingDispatchedPrompt_ReturnsCallerCancellationWithoutClearingGate(t *testing.T) {
|
||||
execution := pendingPromptExecution()
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
cancel()
|
||||
|
||||
err := waitForPendingDispatchedPrompt(ctx, execution)
|
||||
if !errors.Is(err, context.Canceled) {
|
||||
t.Fatalf("wait error = %v, want context canceled", err)
|
||||
}
|
||||
if !execution.dispatchedPromptPending.Load() {
|
||||
t.Fatal("caller cancellation cleared the pending prompt gate")
|
||||
}
|
||||
}
|
||||
|
||||
func TestWaitForPendingDispatchedPrompt_ConsumesCompletionSignal(t *testing.T) {
|
||||
execution := pendingPromptExecution()
|
||||
execution.promptDoneCh <- PromptCompletionSignal{StopReason: "end_turn"}
|
||||
|
||||
if err := waitForPendingDispatchedPrompt(context.Background(), execution); err != nil {
|
||||
t.Fatalf("waitForPendingDispatchedPrompt: %v", err)
|
||||
}
|
||||
if execution.dispatchedPromptPending.Load() {
|
||||
t.Fatal("completion signal did not clear the pending prompt gate")
|
||||
}
|
||||
}
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
||||
)
|
||||
@@ -49,3 +50,13 @@ func TestErrorsAreClassifiable(t *testing.T) {
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func TestIsTransientPromptError_PendingCompletionTimeout(t *testing.T) {
|
||||
err := fmt.Errorf("prompt admission: %w", &lifecycle.PendingDispatchedPromptTimeoutError{
|
||||
ExecutionID: "execution-1",
|
||||
Timeout: time.Second,
|
||||
})
|
||||
if !isTransientPromptError(err) {
|
||||
t.Fatal("pending dispatched prompt timeout must be transient")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -775,9 +775,10 @@ func (s *Service) cancelAgentSilentActionWithKind(
|
||||
}
|
||||
|
||||
// cancelAgentSilentActionWithKindExclusive is the non-joining cancellation
|
||||
// path used by Send Now. A second Send Now click or an explicit cancellation
|
||||
// that already owns the session is reported as a conflict instead of joining
|
||||
// and inheriting the first operation's reconciliation semantics.
|
||||
// path used by Send Now and workflow context reset. A second Send Now click or
|
||||
// another cancellation that already owns the session is reported as a conflict
|
||||
// instead of joining and inheriting the first operation's reconciliation
|
||||
// semantics.
|
||||
func (s *Service) cancelAgentSilentActionWithKindExclusive(
|
||||
ctx context.Context,
|
||||
taskID, sessionID string,
|
||||
@@ -785,8 +786,47 @@ func (s *Service) cancelAgentSilentActionWithKindExclusive(
|
||||
kind cancellationKind,
|
||||
expectedTurnID string,
|
||||
) (bool, error) {
|
||||
return s.cancelAgentSilentActionWithKindExclusiveConflict(
|
||||
ctx, taskID, sessionID, action, kind, expectedTurnID, ErrSendNowConflict,
|
||||
)
|
||||
}
|
||||
|
||||
func (s *Service) cancelAgentSilentActionWithKindExclusiveConflict(
|
||||
ctx context.Context,
|
||||
taskID, sessionID string,
|
||||
action func(context.Context) (bool, error),
|
||||
kind cancellationKind,
|
||||
expectedTurnID string,
|
||||
conflictErr error,
|
||||
) (bool, error) {
|
||||
operation, registered, err := s.startExclusiveSilentCancellation(
|
||||
ctx, taskID, sessionID, action, kind, expectedTurnID, conflictErr,
|
||||
)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
if err := operation.wait(ctx); err != nil {
|
||||
return false, err
|
||||
}
|
||||
if registered == nil {
|
||||
return false, nil
|
||||
}
|
||||
return registered.wait(ctx)
|
||||
}
|
||||
|
||||
// startExclusiveSilentCancellation registers an exclusive cancellation before
|
||||
// the caller releases a session guard. Reset uses this hand-off to make its
|
||||
// reset marker and cancellation claim one admission boundary.
|
||||
func (s *Service) startExclusiveSilentCancellation(
|
||||
ctx context.Context,
|
||||
taskID, sessionID string,
|
||||
action func(context.Context) (bool, error),
|
||||
kind cancellationKind,
|
||||
expectedTurnID string,
|
||||
conflictErr error,
|
||||
) (*cancelOperation, *cancellationAction, error) {
|
||||
if s.repo == nil {
|
||||
return false, errors.New("cancel agent silently: repository is not configured")
|
||||
return nil, nil, errors.New("cancel agent silently: repository is not configured")
|
||||
}
|
||||
var registeredAction func(context.Context, *cancelOperation) (bool, error)
|
||||
if action != nil {
|
||||
@@ -798,19 +838,13 @@ func (s *Service) cancelAgentSilentActionWithKindExclusive(
|
||||
sessionID, kind, registeredAction,
|
||||
)
|
||||
if !accepted {
|
||||
return false, ErrSendNowConflict
|
||||
return nil, nil, conflictErr
|
||||
}
|
||||
if owner {
|
||||
s.setCancellationExpectedTurn(sessionID, operation, expectedTurnID)
|
||||
go s.runSilentCancellation(ctx, taskID, sessionID, operation)
|
||||
}
|
||||
if err := operation.wait(ctx); err != nil {
|
||||
return false, err
|
||||
}
|
||||
if registered == nil {
|
||||
return false, nil
|
||||
}
|
||||
return registered.wait(ctx)
|
||||
return operation, registered, nil
|
||||
}
|
||||
|
||||
func (s *Service) runSilentCancellation(requestCtx context.Context, taskID, sessionID string, operation *cancelOperation) {
|
||||
@@ -976,6 +1010,40 @@ func (s *Service) cancelAgentSilentWithGuardActionKindExclusive(
|
||||
return s.cancelAgentSilentActionWithKindExclusive(ctx, taskID, sessionID, action, kind, expectedTurnID)
|
||||
}
|
||||
|
||||
// cancelAgentSilentWithGuardActionKindExclusiveConflict claims cancellation
|
||||
// while the caller still owns the session guard, then releases that guard
|
||||
// while the lifecycle cancellation waits. This is the reset variant: a
|
||||
// prompt cannot enter between reset-marker publication and the exclusive
|
||||
// cancellation claim, and the cancellation owner can still reacquire the
|
||||
// guard for its lifecycle reconciliation.
|
||||
func (s *Service) cancelAgentSilentWithGuardActionKindExclusiveConflict(
|
||||
ctx context.Context,
|
||||
taskID, sessionID string,
|
||||
unlockGuard, relockGuard func(),
|
||||
action func(context.Context) (bool, error),
|
||||
kind cancellationKind,
|
||||
expectedTurnID string,
|
||||
conflictErr error,
|
||||
) (bool, error) {
|
||||
operation, registered, err := s.startExclusiveSilentCancellation(
|
||||
ctx, taskID, sessionID, action, kind, expectedTurnID, conflictErr,
|
||||
)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
if unlockGuard != nil {
|
||||
unlockGuard()
|
||||
defer relockGuard()
|
||||
}
|
||||
if err := operation.wait(ctx); err != nil {
|
||||
return false, err
|
||||
}
|
||||
if registered == nil {
|
||||
return false, nil
|
||||
}
|
||||
return registered.wait(ctx)
|
||||
}
|
||||
|
||||
func (s *Service) logSilentCancelReconciled(taskID, sessionID string, err error) {
|
||||
if errors.Is(err, lifecycle.ErrCancelEscalated) {
|
||||
s.logger.Warn("agent did not acknowledge silent clarification cancel; reconciling session state",
|
||||
|
||||
@@ -37,6 +37,7 @@ var (
|
||||
errDeferredMoveAlreadyApplied = errors.New("deferred move already applied")
|
||||
errReusableSessionNoLongerActive = errors.New("reusable session is no longer active")
|
||||
errWorkflowAutoStartSessionTerminalized = errors.New("workflow auto-start session terminalized")
|
||||
errContextResetCancellationConflict = errors.New("context reset cancellation is already in progress")
|
||||
)
|
||||
|
||||
type workflowAutoStartSessionTerminalizedError struct {
|
||||
@@ -4077,8 +4078,23 @@ func (s *Service) resetAgentContext(ctx context.Context, taskID string, session
|
||||
|
||||
releaseLifecycleLock := s.acquireSessionLifecycleLock(sessionID)
|
||||
defer releaseLifecycleLock()
|
||||
resetGuard := s.lockCancelInFlightGuard(sessionID)
|
||||
s.setSessionResetInProgress(sessionID, true)
|
||||
defer s.setSessionResetInProgress(sessionID, false)
|
||||
defer func() {
|
||||
// Publish the end of reset while the same guard is still held. A prompt
|
||||
// that acquires the guard after this point observes a settled marker.
|
||||
s.setSessionResetInProgress(sessionID, false)
|
||||
resetGuard.release()
|
||||
}()
|
||||
|
||||
if err := s.quiesceActiveResetTurn(ctx, taskID, sessionID, stepName, resetGuard); err != nil {
|
||||
s.logger.Error("failed to quiesce active turn before context reset",
|
||||
zap.String("task_id", taskID),
|
||||
zap.String("session_id", sessionID),
|
||||
zap.String("step_name", stepName),
|
||||
zap.Error(err))
|
||||
return false
|
||||
}
|
||||
|
||||
executionID, err := s.agentManager.GetExecutionIDForSession(ctx, sessionID)
|
||||
if err != nil || executionID == "" {
|
||||
@@ -4145,6 +4161,66 @@ func (s *Service) resetAgentContext(ctx context.Context, taskID string, session
|
||||
return true
|
||||
}
|
||||
|
||||
// quiesceActiveResetTurn stops an in-flight turn through the internal silent
|
||||
// cancellation coordinator before the provider conversation is replaced. It
|
||||
// deliberately does not call Service.CancelAgent: that path evaluates the
|
||||
// user's configured cancellation completion and creates a visible message.
|
||||
func (s *Service) quiesceActiveResetTurn(
|
||||
ctx context.Context,
|
||||
taskID, sessionID, stepName string,
|
||||
resetGuard *lockedCancelInFlightGuard,
|
||||
) error {
|
||||
turnID, err := s.activeResetTurnID(ctx, sessionID)
|
||||
if err != nil {
|
||||
return fmt.Errorf("inspect active turn for context reset: %w", err)
|
||||
}
|
||||
if turnID == "" {
|
||||
if s.currentCancellation(sessionID) != nil {
|
||||
return errContextResetCancellationConflict
|
||||
}
|
||||
return nil
|
||||
}
|
||||
if s.turnService == nil {
|
||||
// The in-memory cache is the only available identity when no durable
|
||||
// turn service is wired. The lifecycle capture then applies its own
|
||||
// best-effort cancellation behavior without an expected durable ID.
|
||||
turnID = ""
|
||||
}
|
||||
if _, err := s.cancelAgentSilentWithGuardActionKindExclusiveConflict(
|
||||
ctx, taskID, sessionID, resetGuard.unlock, resetGuard.relock,
|
||||
nil, cancellationKindInternal, turnID, errContextResetCancellationConflict,
|
||||
); err != nil {
|
||||
return fmt.Errorf("cancel active turn for context reset at %s: %w", stepName, err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// hasActiveResetTurn combines the in-memory admission records with the
|
||||
// durable turn service. Either record is enough to fail closed before a
|
||||
// provider context replacement; missing an active turn could let the old
|
||||
// provider stream race the new conversation.
|
||||
func (s *Service) hasActiveResetTurn(ctx context.Context, sessionID string) (bool, error) {
|
||||
turnID, err := s.activeResetTurnID(ctx, sessionID)
|
||||
return turnID != "", err
|
||||
}
|
||||
|
||||
func (s *Service) activeResetTurnID(ctx context.Context, sessionID string) (string, error) {
|
||||
if s.turnService != nil {
|
||||
turnID, err := s.peekActiveTurnID(ctx, sessionID)
|
||||
if err != nil {
|
||||
return "", err
|
||||
}
|
||||
if turnID != "" {
|
||||
return turnID, nil
|
||||
}
|
||||
}
|
||||
if value, ok := s.activeTurns.Load(sessionID); ok {
|
||||
turnID, _ := value.(string)
|
||||
return turnID, nil
|
||||
}
|
||||
return s.reservedPromptTurnID(sessionID), nil
|
||||
}
|
||||
|
||||
// reconcileFailedContextReset handles a reset that moved the live ACP session
|
||||
// before a later reset step failed. The old cursor and context metadata belong
|
||||
// to the previous ACP session and must not survive that partial transition.
|
||||
|
||||
@@ -0,0 +1,285 @@
|
||||
package orchestrator
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/orchestrator/executor"
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
sqliterepo "github.com/kandev/kandev/internal/task/repository/sqlite"
|
||||
wfmodels "github.com/kandev/kandev/internal/workflow/models"
|
||||
v1 "github.com/kandev/kandev/pkg/api/v1"
|
||||
)
|
||||
|
||||
type orderedResetAgentManager struct {
|
||||
*mockAgentManager
|
||||
events chan string
|
||||
}
|
||||
|
||||
func (m *orderedResetAgentManager) CancelAgent(ctx context.Context, sessionID string) error {
|
||||
m.events <- "cancel"
|
||||
return m.mockAgentManager.CancelAgent(ctx, sessionID)
|
||||
}
|
||||
|
||||
func (m *orderedResetAgentManager) ResetAgentContext(ctx context.Context, executionID string) error {
|
||||
m.events <- "reset"
|
||||
return m.mockAgentManager.ResetAgentContext(ctx, executionID)
|
||||
}
|
||||
|
||||
func newActiveResetTestService(t *testing.T) (*Service, *sqliterepo.Repository, *orderedResetAgentManager, *models.TaskSession) {
|
||||
t.Helper()
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "task1", "session1", "step1")
|
||||
seedExecutorRunning(t, repo, "session1", "task1", "execution1")
|
||||
|
||||
baseManager := &mockAgentManager{repoForExecutionLookup: repo}
|
||||
manager := &orderedResetAgentManager{
|
||||
mockAgentManager: baseManager,
|
||||
events: make(chan string, 4),
|
||||
}
|
||||
svc := createTestServiceWithAgent(repo, newMockStepGetter(), newMockTaskRepo(), manager)
|
||||
svc.turnService = &repoTurnService{repo: repo}
|
||||
turn, err := svc.turnService.StartTurn(ctx, "session1")
|
||||
if err != nil {
|
||||
t.Fatalf("start active turn: %v", err)
|
||||
}
|
||||
svc.activeTurns.Store("session1", turn.ID)
|
||||
|
||||
session, err := repo.GetTaskSession(ctx, "session1")
|
||||
if err != nil {
|
||||
t.Fatalf("load session: %v", err)
|
||||
}
|
||||
return svc, repo, manager, session
|
||||
}
|
||||
|
||||
func requireResetEvents(t *testing.T, events <-chan string, want ...string) {
|
||||
t.Helper()
|
||||
for _, expected := range want {
|
||||
select {
|
||||
case got := <-events:
|
||||
if got != expected {
|
||||
t.Fatalf("event order = %q, want next event %q", got, expected)
|
||||
}
|
||||
case <-time.After(time.Second):
|
||||
t.Fatalf("timed out waiting for %q", expected)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestResetAgentContext_QuiescesActiveTurnBeforeProviderReset(t *testing.T) {
|
||||
svc, _, manager, session := newActiveResetTestService(t)
|
||||
messages := &mockMessageCreator{}
|
||||
svc.messageCreator = messages
|
||||
|
||||
if !svc.resetAgentContext(context.Background(), "task1", session, "Successor") {
|
||||
t.Fatal("resetAgentContext returned false")
|
||||
}
|
||||
|
||||
requireResetEvents(t, manager.events, "cancel", "reset")
|
||||
if len(messages.sessionMessages) != 0 {
|
||||
t.Fatalf("internal reset cancellation created %d session messages", len(messages.sessionMessages))
|
||||
}
|
||||
}
|
||||
|
||||
func TestResetAgentContext_CancellationConflictStopsProviderReset(t *testing.T) {
|
||||
svc, _, manager, session := newActiveResetTestService(t)
|
||||
operation, owner := svc.claimCancellation("session1", cancellationKindExplicit)
|
||||
if !owner {
|
||||
t.Fatal("test setup failed to claim explicit cancellation")
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
svc.finishCancellation("session1", operation, nil)
|
||||
})
|
||||
|
||||
if svc.resetAgentContext(context.Background(), "task1", session, "Successor") {
|
||||
t.Fatal("resetAgentContext returned true while explicit cancellation owned the session")
|
||||
}
|
||||
select {
|
||||
case event := <-manager.events:
|
||||
t.Fatalf("provider operation %q started despite cancellation conflict", event)
|
||||
default:
|
||||
}
|
||||
}
|
||||
|
||||
func TestResetAgentContext_CancellationConflictWithoutActiveTurnStopsProviderReset(t *testing.T) {
|
||||
svc, _, manager, session := newActiveResetTestService(t)
|
||||
turnIDValue, ok := svc.activeTurns.Load(session.ID)
|
||||
if !ok {
|
||||
t.Fatal("test setup did not create an active turn")
|
||||
}
|
||||
turnID, ok := turnIDValue.(string)
|
||||
if !ok || turnID == "" {
|
||||
t.Fatalf("active turn cache value = %#v, want turn ID", turnIDValue)
|
||||
}
|
||||
if err := svc.turnService.CompleteTurn(context.Background(), turnID); err != nil {
|
||||
t.Fatalf("complete active turn: %v", err)
|
||||
}
|
||||
svc.activeTurns.Delete(session.ID)
|
||||
|
||||
operation, owner := svc.claimCancellation(session.ID, cancellationKindExplicit)
|
||||
if !owner {
|
||||
t.Fatal("test setup failed to claim explicit cancellation")
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
svc.finishCancellation(session.ID, operation, nil)
|
||||
})
|
||||
|
||||
if svc.resetAgentContext(context.Background(), session.TaskID, session, "Successor") {
|
||||
t.Fatal("resetAgentContext returned true while explicit cancellation owned an idle session")
|
||||
}
|
||||
select {
|
||||
case event := <-manager.events:
|
||||
t.Fatalf("provider operation %q started despite cancellation conflict", event)
|
||||
default:
|
||||
}
|
||||
}
|
||||
|
||||
func TestHasActiveResetTurn_ReservedPromptOnly(t *testing.T) {
|
||||
svc := createTestService(setupTestRepo(t), newMockStepGetter(), newMockTaskRepo())
|
||||
svc.reservedPromptTurns.Store("session1", newReservedPromptTurn("reserved-turn"))
|
||||
|
||||
active, err := svc.hasActiveResetTurn(context.Background(), "session1")
|
||||
if err != nil {
|
||||
t.Fatalf("hasActiveResetTurn: %v", err)
|
||||
}
|
||||
if !active {
|
||||
t.Fatal("reserved prompt turn was not treated as active")
|
||||
}
|
||||
}
|
||||
|
||||
func TestResetAgentContext_ActiveTurnAllowsSuccessorPrompt(t *testing.T) {
|
||||
svc, repo, manager, session := newActiveResetTestService(t)
|
||||
manager.isAgentRunning = true
|
||||
manager.promptAgentFunc = func(
|
||||
_ context.Context,
|
||||
_ string,
|
||||
_ string,
|
||||
_ []v1.MessageAttachment,
|
||||
_ bool,
|
||||
) (*executor.PromptResult, error) {
|
||||
manager.events <- "prompt"
|
||||
return &executor.PromptResult{}, nil
|
||||
}
|
||||
svc.executor = executor.NewExecutor(manager, repo, testLogger(), executor.ExecutorConfig{})
|
||||
svc.messageCreator = &mockMessageCreator{}
|
||||
|
||||
if !svc.resetAgentContext(context.Background(), "task1", session, "Successor") {
|
||||
t.Fatal("resetAgentContext returned false")
|
||||
}
|
||||
resetSession, err := svc.repo.GetTaskSession(context.Background(), "session1")
|
||||
if err != nil {
|
||||
t.Fatalf("reload reset session: %v", err)
|
||||
}
|
||||
step := &wfmodels.WorkflowStep{
|
||||
ID: "step2",
|
||||
WorkflowID: "wf1",
|
||||
Name: "Successor",
|
||||
Events: wfmodels.StepEvents{OnEnter: []wfmodels.OnEnterAction{
|
||||
{Type: wfmodels.OnEnterResetAgentContext},
|
||||
}},
|
||||
}
|
||||
if err := svc.autoStartStepPrompt(
|
||||
context.Background(), "task1", resetSession, step, "successor prompt", false, false,
|
||||
); err != nil {
|
||||
t.Fatalf("auto-start successor prompt: %v", err)
|
||||
}
|
||||
|
||||
requireResetEvents(t, manager.events, "cancel", "reset", "prompt")
|
||||
}
|
||||
|
||||
func TestResetAgentContext_CancelFailureStopsProviderReset(t *testing.T) {
|
||||
svc, _, manager, session := newActiveResetTestService(t)
|
||||
manager.cancelAgentErr = errors.New("cancel failed")
|
||||
|
||||
if svc.resetAgentContext(context.Background(), "task1", session, "Successor") {
|
||||
t.Fatal("resetAgentContext returned true after cancellation failure")
|
||||
}
|
||||
|
||||
requireResetEvents(t, manager.events, "cancel")
|
||||
if got := len(manager.restartProcessCalls); got != 0 {
|
||||
t.Fatalf("provider reset calls = %d, want 0", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestResetAgentContext_SerializesPromptAdmission stages a prompt immediately
|
||||
// before its final guarded claim, then starts reset while that claim is paused.
|
||||
// If reset wins the shared guard, the marker rejects the prompt while reset is
|
||||
// waiting on cancellation. If the prompt wins, reset must observe that turn
|
||||
// and cancel it before replacing the provider session.
|
||||
func TestResetAgentContext_SerializesPromptAdmission(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
svc, repo, manager, session := newActiveResetTestService(t)
|
||||
session.State = models.TaskSessionStateWaitingForInput
|
||||
if err := repo.UpdateTaskSession(ctx, session); err != nil {
|
||||
t.Fatalf("set session waiting for input: %v", err)
|
||||
}
|
||||
cancelEntered := make(chan struct{}, 1)
|
||||
cancelRelease := make(chan struct{})
|
||||
manager.cancelAgentEntered = cancelEntered
|
||||
manager.cancelAgentBlock = cancelRelease
|
||||
var releaseCancellation sync.Once
|
||||
releaseCancel := func() { releaseCancellation.Do(func() { close(cancelRelease) }) }
|
||||
t.Cleanup(releaseCancel)
|
||||
|
||||
guard, releaseGuard := svc.acquireCancelInFlightGuard(session.ID)
|
||||
guard.Lock()
|
||||
promptStarted := make(chan struct{})
|
||||
promptDone := make(chan error, 1)
|
||||
go func() {
|
||||
close(promptStarted)
|
||||
_, _, _, _, _, err := svc.claimSessionRunningForPrompt(
|
||||
ctx, session.TaskID, session.ID, "", false, nil, nil, "", false,
|
||||
)
|
||||
promptDone <- err
|
||||
}()
|
||||
<-promptStarted
|
||||
|
||||
resetStarted := make(chan struct{})
|
||||
resetDone := make(chan bool, 1)
|
||||
go func() {
|
||||
close(resetStarted)
|
||||
resetDone <- svc.resetAgentContext(ctx, session.TaskID, session, "Successor")
|
||||
}()
|
||||
<-resetStarted
|
||||
|
||||
guard.Unlock()
|
||||
releaseGuard()
|
||||
|
||||
var promptErr error
|
||||
resetWonGuard := false
|
||||
select {
|
||||
case promptErr = <-promptDone:
|
||||
case <-cancelEntered:
|
||||
// Reset won the admission guard and is now waiting for its internal
|
||||
// cancellation. The prompt must observe the still-published marker.
|
||||
resetWonGuard = true
|
||||
select {
|
||||
case promptErr = <-promptDone:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("timed out waiting for reset-blocked prompt admission")
|
||||
}
|
||||
}
|
||||
if resetWonGuard && !errors.Is(promptErr, ErrSessionResetInProgress) {
|
||||
t.Fatalf("prompt admission after reset won guard = %v, want %v", promptErr, ErrSessionResetInProgress)
|
||||
}
|
||||
releaseCancel()
|
||||
select {
|
||||
case resetOK := <-resetDone:
|
||||
if !resetOK {
|
||||
t.Fatal("resetAgentContext returned false")
|
||||
}
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("timed out waiting for context reset")
|
||||
}
|
||||
|
||||
if !resetWonGuard && promptErr != nil && !errors.Is(promptErr, ErrSessionResetInProgress) {
|
||||
t.Fatalf("prompt admission error = %v, want reset sentinel or successful claim", promptErr)
|
||||
}
|
||||
// A prompt that won the guard must have been visible as the active turn to
|
||||
// reset. The provider replacement is therefore ordered after cancellation.
|
||||
requireResetEvents(t, manager.events, "cancel", "reset")
|
||||
}
|
||||
@@ -162,6 +162,10 @@ func isTransientPromptError(err error) bool {
|
||||
if err == nil {
|
||||
return false
|
||||
}
|
||||
var pendingCompletionTimeout *lifecycle.PendingDispatchedPromptTimeoutError
|
||||
if errors.As(err, &pendingCompletionTimeout) {
|
||||
return true
|
||||
}
|
||||
msg := strings.ToLower(err.Error())
|
||||
return strings.Contains(msg, "agent stream disconnected") ||
|
||||
strings.Contains(msg, "use of closed network connection")
|
||||
@@ -4673,6 +4677,9 @@ func (s *Service) claimSessionRunningForPrompt(
|
||||
if err := s.waitForCancellationWithGuard(ctx, sessionID, lock.Unlock, lock.Lock); err != nil {
|
||||
return nil, "", "", false, nil, err
|
||||
}
|
||||
if s.isSessionResetInProgress(sessionID) {
|
||||
return nil, "", "", false, nil, ErrSessionResetInProgress
|
||||
}
|
||||
if expectedCurrentTurnID != "" {
|
||||
if s.turnService == nil {
|
||||
return nil, "", "", false, nil, errors.New("cannot verify expected prompt turn without turn service")
|
||||
@@ -4773,6 +4780,9 @@ func (s *Service) claimLifecycleSessionRunning(
|
||||
if err := s.waitForCancellationWithGuard(ctx, sessionID, lock.Unlock, lock.Lock); err != nil {
|
||||
return nil, "", err
|
||||
}
|
||||
if s.isSessionResetInProgress(sessionID) {
|
||||
return nil, "", ErrSessionResetInProgress
|
||||
}
|
||||
|
||||
if claimEntryID != "" && !s.isCurrentQueuedDispatch(sessionID, claimEntryID) {
|
||||
return nil, "", errQueuedDispatchSuperseded
|
||||
|
||||
@@ -0,0 +1,47 @@
|
||||
# ADR-2026-08-30-context-reset-quiesces-active-turn: Quiesce Active Turns Before Context Reset
|
||||
|
||||
**Status:** accepted
|
||||
**Date:** 2026-08-30
|
||||
**Area:** backend, workflow
|
||||
|
||||
## Context
|
||||
|
||||
A workflow transition can enter a step with `reset_agent_context` while the current session still owns an active turn.
|
||||
|
||||
The reset currently replaces the ACP session and drains a shared completion channel. The old turn can then finish against retired session state.
|
||||
|
||||
A later prompt waits for that lost completion while it holds the session dispatch guard. The same guard prevents explicit cancellation from reaching the lifecycle manager.
|
||||
|
||||
Context reset needs one owner for turn quiescence. The owner must preserve workflow cancellation semantics and prompt-generation ownership.
|
||||
|
||||
## Decision
|
||||
|
||||
A workflow context reset is a system-owned turn-replacement operation when the target session has an active turn.
|
||||
|
||||
The orchestrator takes the shared per-session cancellation guard before it sets the reset marker. It then claims the session's cancellation coordinator exclusively for an internal cancellation operation and uses the existing bounded path to stop and reconcile the active turn. The guard is released while that operation waits and reacquired before provider replacement. If another cancellation already owns the session, reset fails closed rather than inheriting that source's reconciliation semantics. A reset with no active turn also fails closed when a cancellation is already in flight.
|
||||
|
||||
This cancellation does not create a user-cancellation message. It does not evaluate `cancel_triggers_turn_complete` or `on_turn_complete`.
|
||||
|
||||
The lifecycle manager replaces the provider session only after internal cancellation finishes. Normal and lifecycle prompt claims perform their final marker check under the same guard. The reset marker prevents new prompt admission until provider state and persisted reset state are settled.
|
||||
|
||||
A successor that waits for an unresolved dispatch-only completion has a 10-second bound. Timeout returns a typed transient error without clearing the predecessor gate.
|
||||
|
||||
Existing deferred cleanup releases prompt serialization and the session dispatch guard. The normal cancellation escalation path remains the only owner that clears the stale gate.
|
||||
|
||||
Prompt generations remain the authority for terminal events. A completion from the replaced turn cannot finish a later generation. An unnumbered completion also cannot release a pending numbered dispatch-only prompt. Synthetic or delayed completion events without ownership identity are ignored at that boundary.
|
||||
|
||||
## Consequences
|
||||
|
||||
- A workflow reset can stop an old turn before it starts work in the destination step.
|
||||
- Internal reset cancellation cannot trigger user-configured workflow completion.
|
||||
- A cancellation escalation can add up to the existing bounded cancellation interval before provider reset.
|
||||
- A stale predecessor wait fails before dispatch and leaves cancellation available.
|
||||
- Existing sessions that already contain the stuck state do not receive automatic reconciliation.
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
- **Replace the provider session immediately.** Rejected because it can orphan the active completion signal and block all later session operations.
|
||||
- **Wait for `agent.ready` before reset.** Rejected because workflow entry remains incomplete while the old turn can continue under the destination step.
|
||||
- **Use explicit user cancellation.** Rejected because configured user cancellation can evaluate `on_turn_complete` and move the task again.
|
||||
- **Clear the pending gate when its wait expires.** Rejected because the predecessor can still run and corrupt successor buffers or completion ownership.
|
||||
- **Restart the agent process for every active reset.** Rejected because process replacement is slower and does not replace cancellation reconciliation.
|
||||
@@ -216,3 +216,4 @@ Read individual ADRs for full context. Create new ones via `/record decision` or
|
||||
| 2026-08-27-preserve-legacy-sqlite-before-default-initialization | [Preserve Legacy SQLite Data Before Default Initialization](2026-08-27-preserve-legacy-sqlite-before-default-initialization.md) | accepted | backend, cli, operations | 2026-08-27 |
|
||||
| 2026-08-27-mixed-git-change-facets | [Preserve mixed Git changes as path facets](2026-08-27-mixed-git-change-facets.md) | accepted | backend, agentctl, frontend, protocol | 2026-08-27 |
|
||||
| 2026-08-28-bind-github-auto-merge-attempts-to-reviewed-head | [Bind GitHub Auto-Merge Attempts to the Reviewed Head](2026-08-28-bind-github-auto-merge-attempts-to-reviewed-head.md) | accepted | backend, frontend, protocol, security, GitHub | 2026-08-28 |
|
||||
| 2026-08-30-context-reset-quiesces-active-turn | [Quiesce Active Turns Before Context Reset](2026-08-30-context-reset-quiesces-active-turn.md) | accepted | backend, workflow | 2026-08-30 |
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
---
|
||||
created: 2026-08-30
|
||||
status: implemented
|
||||
requirements:
|
||||
- REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002
|
||||
system_design:
|
||||
- ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md
|
||||
legacy_specs:
|
||||
- ../../specs/workflow-on-enter-action-dispatch/spec.md
|
||||
---
|
||||
|
||||
# Implementation Plan: Workflow Context Reset Quiescence
|
||||
|
||||
## Overview
|
||||
|
||||
This plan stops an active turn before workflow context replacement. Then it bounds any remaining wait for a dispatch-only predecessor.
|
||||
|
||||
The quiescence change comes first because it removes the known cause. The timeout follows as a recovery boundary for other lost completions.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Use internal cancellation before a workflow reset replaces an active provider session.
|
||||
- Preserve explicit user-cancellation and workflow-completion behavior.
|
||||
- Prevent automatic step prompt dispatch when quiescence or reset fails.
|
||||
- Bound the wait for an unresolved dispatch-only completion.
|
||||
- Keep delayed completion events isolated by prompt generation.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- Automatic repair of sessions that became stuck before this change.
|
||||
- A REST cancellation endpoint or new UI recovery controls.
|
||||
- Provider-specific ACP reset behavior.
|
||||
- Runtime configuration restoration during reset.
|
||||
- Changes to explicit user cancellation or `cancel_triggers_turn_complete`.
|
||||
|
||||
## Technical approach
|
||||
|
||||
### Workflow reset quiescence
|
||||
|
||||
Update `orchestrator.Service.resetAgentContext` in `apps/backend/internal/orchestrator/event_handlers_workflow.go`.
|
||||
|
||||
Keep the session lifecycle lock and the shared per-session `cancelInFlight` guard as the outer boundary. Publish the reset marker while that guard is held, and recheck the marker under the same guard in both normal and lifecycle prompt claims. If the session owns an active turn, register the internal cancellation coordinator exclusively before releasing the guard for its potentially blocking wait. Reacquire the guard before `AgentManager.ResetAgentContext` and persisted reset cleanup.
|
||||
|
||||
If no active turn exists but another cancellation owns the session, fail closed instead of allowing reset to continue with stale workflow inputs.
|
||||
|
||||
Use the existing bounded lifecycle cancellation and escalation path. Do not call `Service.CancelAgent`, because that path can evaluate user-configured workflow completion.
|
||||
|
||||
Return reset failure when internal cancellation fails. The existing `processOnEnter` flow then blocks automatic prompt dispatch.
|
||||
|
||||
### Bounded predecessor completion
|
||||
|
||||
Add a typed lifecycle error and a 10-second timeout in `apps/backend/internal/agent/runtime/lifecycle/session.go`.
|
||||
|
||||
Return the error from `waitForPendingDispatchedPrompt` without clearing `dispatchedPromptPending`. Deferred cleanup releases `promptMu` and the orchestrator dispatch guard.
|
||||
|
||||
Classify the typed error as transient in `apps/backend/internal/orchestrator/task_operations.go`. Existing queue logic then preserves automatic prompts that did not dispatch.
|
||||
|
||||
Keep `lifecycle.Manager.CancelAgent` as the only path that clears the stale pending gate through bounded escalation.
|
||||
|
||||
## Tests
|
||||
|
||||
| Acceptance criterion | Evidence |
|
||||
| --- | --- |
|
||||
| `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.1` | `TestResetAgentContext_QuiescesActiveTurnBeforeProviderReset`, `TestResetAgentContext_CancellationConflictStopsProviderReset`, and `TestHasActiveResetTurn_ReservedPromptOnly` in `event_handlers_workflow_reset_quiescence_test.go` |
|
||||
| `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.2` | The same test asserts internal cancellation and no explicit completion path. |
|
||||
| `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.3` | `TestResetAgentContext_ActiveTurnAllowsSuccessorPrompt` and `TestResetAgentContext_SerializesPromptAdmission` in `event_handlers_workflow_reset_quiescence_test.go` |
|
||||
| `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.4` | `TestResetAgentContext_CancelFailureStopsProviderReset` in `event_handlers_workflow_reset_quiescence_test.go` |
|
||||
| `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.5` | `TestWaitForPendingDispatchedPrompt_TimesOutWithoutClearingGate` in `session_pending_prompt_test.go` and transient classification coverage in `errors_test.go` |
|
||||
| `AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.6` | `TestHandleAgentEvent_UnnumberedCompleteCannotReleasePendingPrompt` in `manager_events_test.go`, plus existing generation tests in `manager_events_test.go` and `execution_store_test.go`, remain green. |
|
||||
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Quiesce active reset turns](task-01-quiesce-active-reset-turns.md) (`done`)
|
||||
- [x] [Task 02: Bound predecessor completion waits](task-02-bound-predecessor-completion-waits.md) (`done`)
|
||||
|
||||
## Verification results
|
||||
|
||||
Implemented and verified.
|
||||
|
||||
- Task 01 focused regression: 7 tests passed.
|
||||
- Task 02 focused regressions: 4 tests passed across the lifecycle and orchestrator packages.
|
||||
- Combined race-enabled affected-package suite: 4321 tests passed across the two affected packages.
|
||||
- Backend lint reported 0 issues and specification lint passed.
|
||||
- Red tests confirmed the pre-fix provider-reset ordering, cancellation fail-open, stale barrier hang, and missing transient classification.
|
||||
- `make -C apps/backend test` passed the lifecycle and orchestrator packages but the overall target failed in unrelated config-discovery and launcher tests because this workspace's `/root/.kandev/config.yaml` took precedence over their temporary home directories.
|
||||
|
||||
## Risks
|
||||
|
||||
- Internal cancellation can race with natural completion. The existing cancellation identity and reconciliation rules must make both outcomes idempotent.
|
||||
- Reset-related ready events can re-enter workflow handling. The reset marker must remain active until provider and persistence work finishes.
|
||||
- The timeout must not clear a predecessor gate. Only cancellation can safely release that ownership.
|
||||
|
||||
## PR fixup results
|
||||
|
||||
- Reset quiescence now claims cancellation exclusively and fails closed on an existing cancellation operation.
|
||||
- Reset marker publication and final normal/lifecycle prompt admission now share the per-session guard. Active reset cancellation releases that guard only while waiting, then reacquires it before provider replacement.
|
||||
- Added reservation-only active-turn coverage and rejected unnumbered completion events while a numbered dispatch-only prompt is pending.
|
||||
- Stabilized the fake-time timeout assertion and corrected the lifecycle test file manifest.
|
||||
@@ -0,0 +1,92 @@
|
||||
---
|
||||
id: "01-quiesce-active-reset-turns"
|
||||
title: "Quiesce active reset turns"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002
|
||||
acceptance_criteria:
|
||||
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.1
|
||||
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.2
|
||||
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.3
|
||||
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.4
|
||||
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.6
|
||||
system_design:
|
||||
- ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md
|
||||
---
|
||||
|
||||
# Task 01: Quiesce Active Reset Turns
|
||||
|
||||
## Summary
|
||||
|
||||
Stop and reconcile an active turn before workflow context replacement. Keep this operation separate from explicit user cancellation.
|
||||
|
||||
## In scope
|
||||
|
||||
- Detect an active turn after the reset marker becomes active.
|
||||
- Use internal cancellation before provider context replacement.
|
||||
- Keep the reset marker active through cancellation and reset settlement.
|
||||
- Serialize reset-marker publication and final normal/lifecycle prompt admission through the shared per-session cancellation guard.
|
||||
- Stop automatic prompt dispatch when cancellation fails.
|
||||
- Cover natural completion races and provider reset ordering.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Generic stale-prompt timeout behavior.
|
||||
- Explicit user cancellation semantics.
|
||||
- Automatic repair of an existing stuck session.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- Provider reset starts only after the active turn is reconciled.
|
||||
- Internal cancellation does not evaluate user-configured workflow completion.
|
||||
- A prompt paused before final admission is rejected during reset, or reset observes and cancels it before provider replacement.
|
||||
- A cancellation error prevents provider reset and automatic prompt dispatch.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
go test ./internal/orchestrator -run 'Test(ResetAgentContext_(QuiescesActiveTurnBeforeProviderReset|CancellationConflict(StopsProviderReset|WithoutActiveTurnStopsProviderReset)|ActiveTurnAllowsSuccessorPrompt|CancelFailureStopsProviderReset|SerializesPromptAdmission)|HasActiveResetTurn_ReservedPromptOnly)' -count=1
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/orchestrator/event_handlers_workflow.go`
|
||||
- `apps/backend/internal/orchestrator/event_handlers_clarification.go`
|
||||
- `apps/backend/internal/orchestrator/task_operations.go`
|
||||
- `apps/backend/internal/orchestrator/event_handlers_workflow_reset_quiescence_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Risks
|
||||
|
||||
- A natural completion can race with internal cancellation.
|
||||
- A ready event can arrive while the reset marker is active.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002`
|
||||
- `docs/specs/tasks/system-design/workflow-step-agent-start-ownership.md`
|
||||
- `docs/decisions/2026-08-30-context-reset-quiesces-active-turn.md`
|
||||
- Existing silent cancellation in `event_handlers_clarification.go`
|
||||
|
||||
## Results
|
||||
|
||||
Implemented.
|
||||
|
||||
- Added active-turn detection using durable and in-memory turn ownership records.
|
||||
- Routed reset quiescence through the internal silent cancellation coordinator before provider reset.
|
||||
- Made reset quiescence use an exclusive internal cancellation claim and fail closed when another cancellation already owns the session.
|
||||
- Serialized reset-marker publication and final normal/lifecycle prompt admission through the shared per-session cancellation guard. The guard is released while internal cancellation waits and reacquired before provider replacement.
|
||||
- Preserved the reset marker through cancellation and provider replacement, and failed closed when cancellation failed.
|
||||
- Added ordering, cancellation-conflict, reservation-only, prompt-admission race, successor-prompt, and cancellation-failure regressions in `event_handlers_workflow_reset_quiescence_test.go`.
|
||||
- Red: the pre-fix tests observed provider reset without cancellation and allowed reset after cancellation failure.
|
||||
- Green: the exact verification command passed 7 tests.
|
||||
+89
@@ -0,0 +1,89 @@
|
||||
---
|
||||
id: "02-bound-predecessor-completion-waits"
|
||||
title: "Bound predecessor completion waits"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on:
|
||||
- "01-quiesce-active-reset-turns"
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002
|
||||
acceptance_criteria:
|
||||
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.3
|
||||
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.5
|
||||
- AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.6
|
||||
system_design:
|
||||
- ../../specs/tasks/system-design/workflow-step-agent-start-ownership.md
|
||||
---
|
||||
|
||||
# Task 02: Bound Predecessor Completion Waits
|
||||
|
||||
## Summary
|
||||
|
||||
End a stale dispatch-only predecessor wait within a fixed interval. Preserve the pending gate so cancellation remains its only safe release owner.
|
||||
|
||||
## In scope
|
||||
|
||||
- Add a typed lifecycle error for a stale predecessor wait.
|
||||
- Add a 10-second timeout to the pending dispatch-only barrier.
|
||||
- Leave `dispatchedPromptPending` set after timeout.
|
||||
- Treat the new error as transient in orchestrator prompt handling.
|
||||
- Cover timeout, caller cancellation, signal arrival, and error classification.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Automatic cancellation when the timeout expires.
|
||||
- Changes to normal prompt completion duration.
|
||||
- A new user-facing error message or UI state.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- A stale predecessor wait returns before it can hold session admission forever.
|
||||
- The timed-out successor never reaches agentctl and never clears predecessor ownership.
|
||||
- Existing cancellation escalation can clear the gate after the timeout releases session admission.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
go test ./internal/agent/runtime/lifecycle ./internal/orchestrator -run 'Test(WaitForPendingDispatchedPrompt|IsTransientPromptError_PendingCompletionTimeout)' -count=1
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/agent/runtime/lifecycle/session.go`
|
||||
- `apps/backend/internal/agent/runtime/lifecycle/session_test.go`
|
||||
- `apps/backend/internal/orchestrator/task_operations.go`
|
||||
- `apps/backend/internal/orchestrator/errors_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
Task 01 establishes the primary active-reset correction.
|
||||
|
||||
## Risks
|
||||
|
||||
- A timeout that clears the gate can admit concurrent active prompts. The implementation must not clear it.
|
||||
- A non-transient classification can drop an automatic step prompt instead of preserving it.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002`
|
||||
- `docs/specs/tasks/system-design/workflow-step-agent-start-ownership.md`
|
||||
- `docs/decisions/0035-version-agent-ready-events-by-prompt-generation.md`
|
||||
- Existing cancellation escalation in `manager_interaction.go`
|
||||
|
||||
## Results
|
||||
|
||||
Implemented.
|
||||
|
||||
- Added `PendingDispatchedPromptTimeoutError` and a 10-second pending-completion bound.
|
||||
- Preserved `dispatchedPromptPending` on timeout and caller cancellation; completion signals still clear it.
|
||||
- Classified the typed timeout as transient so queued workflow prompts remain retryable.
|
||||
- Added lifecycle barrier tests in `session_pending_prompt_test.go` and orchestrator classification coverage in `errors_test.go`.
|
||||
- Rejected unnumbered completion events while a numbered dispatch-only prompt is pending, so delayed synthetic completion cannot release its gate.
|
||||
- Made the timeout test use a nonblocking post-`synctest.Wait` assertion and `t.Cleanup` for cancellation.
|
||||
- Red: the pre-fix stale barrier did not return and the typed timeout was not transient.
|
||||
- Green: the exact verification command passed 4 tests.
|
||||
@@ -126,6 +126,7 @@ signals, and task-scoped scheduling contracts.
|
||||
- [Task Launch Failure Recovery](system-design/task-launch-failure-recovery.md)
|
||||
- [WIP Limits and Visible Overflow Queues](system-design/wip-limit-pull-system.md)
|
||||
- [Workflow quorum decision recording](system-design/workflow-quorum-decision-recording.md)
|
||||
- [Workflow Step Agent Start Ownership](system-design/workflow-step-agent-start-ownership.md)
|
||||
- [Workflow Step Fixed-Profile Routing](system-design/workflow-step-fixed-profile-routing.md)
|
||||
- [Workflow task-step transition ledger](system-design/workflow-task-step-transition-ledger.md)
|
||||
|
||||
|
||||
@@ -21,6 +21,19 @@ This document is the migrated task-system source for the capability. The source
|
||||
|
||||
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-001.1:** When a consumer uses this capability, the system shall provide the observable behavior and exclusions documented below.
|
||||
|
||||
### REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002: Context reset turn quiescence
|
||||
|
||||
**Intent:** A workflow step can replace agent context without losing turn completion or blocking later session operations.
|
||||
|
||||
#### Acceptance criteria
|
||||
|
||||
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.1:** When a workflow context reset finds an active turn, the system shall stop and reconcile that turn before it replaces the provider context.
|
||||
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.2:** The system-owned stop shall not create a user-cancellation message or evaluate configured user-cancellation completion actions.
|
||||
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.3:** When the reset succeeds, the next automatic step prompt shall reach the new provider context without waiting for the replaced turn.
|
||||
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.4:** When turn quiescence or context replacement fails, the system shall not dispatch the automatic step prompt.
|
||||
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.5:** When a prompt waits for an unresolved dispatch-only completion, the wait shall end within a bounded period and release session admission.
|
||||
- **AC-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002.6:** A delayed completion from the replaced turn shall not complete or release a later prompt generation.
|
||||
|
||||
## Migrated source detail
|
||||
|
||||
## Why
|
||||
@@ -138,4 +151,4 @@ read, which is a design change beyond this repair.
|
||||
- The `-32002 Resource not found` resume failure, which is a consequence of the
|
||||
force-kill rather than an independent defect.
|
||||
- Changing task `FAILED` semantics, the reconciliation path, or the
|
||||
lazy-recovery boot that follows a backend restart.
|
||||
lazy-recovery boot that follows a backend restart.
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
---
|
||||
status: draft
|
||||
system: tasks
|
||||
requirements:
|
||||
- REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-001
|
||||
- REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002
|
||||
---
|
||||
|
||||
# Workflow Step Agent Start Ownership System Design
|
||||
|
||||
## Purpose and boundaries
|
||||
|
||||
The task system owns workflow entry and agent turn admission. The agent runtime owns provider sessions and prompt completion signals.
|
||||
|
||||
This design defines the reset boundary between these systems. It covers never-started, idle, and active task sessions.
|
||||
|
||||
The design preserves runtime configuration through the existing reset contract. It does not change provider reset support or explicit user cancellation.
|
||||
|
||||
## Requirement mapping
|
||||
|
||||
| Requirement | Design sections |
|
||||
| --- | --- |
|
||||
| `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-001` | [Session states](#session-states) |
|
||||
| `REQ-TASKS-WORKFLOW-STEP-AGENT-START-OWNERSHIP-002` | [Active-turn reset flow](#active-turn-reset-flow), [Bounded predecessor wait](#bounded-predecessor-wait) |
|
||||
|
||||
## Components and responsibilities
|
||||
|
||||
`orchestrator.Service.resetAgentContext` owns workflow reset semantics. It also owns the session reset marker, the session lifecycle lock, and the shared per-session cancellation guard that closes prompt-admission races.
|
||||
|
||||
The cancellation coordinator owns active-turn quiescence. It uses the internal cancellation path, not the explicit user cancellation path.
|
||||
|
||||
`lifecycle.Manager.ResetAgentContext` replaces the provider session after quiescence. It restores runtime configuration through the existing reset contract.
|
||||
|
||||
`lifecycle.SessionManager` owns prompt serialization and the dispatch-only completion barrier. Prompt generations continue to identify completion ownership.
|
||||
|
||||
## Session states
|
||||
|
||||
A `CREATED` session has no conversation to reset. The workflow reset skips provider work and leaves the first start to `auto_start_agent`.
|
||||
|
||||
An idle session has no active turn. The workflow reset replaces its provider context directly.
|
||||
|
||||
An active session has a current turn. The workflow reset uses the active-turn flow before it replaces the provider context.
|
||||
|
||||
## Active-turn reset flow
|
||||
|
||||
The workflow reset takes the session lifecycle lock and the shared per-session cancellation guard. It sets the session reset marker while holding that guard before it starts quiescence.
|
||||
|
||||
The reset marker rejects new prompt admission. Normal and lifecycle prompt claims recheck the marker immediately before their final guarded claim. This check and marker publication use the same guard.
|
||||
|
||||
If the session owns an active turn, the orchestrator starts an exclusive internal cancellation operation, then releases the guard while it waits for the bounded lifecycle cancellation and escalation path. It reacquires the guard before provider replacement. If another cancellation already owns the session, reset fails closed instead of inheriting that operation's source-specific reconciliation. A reset with no active turn also fails closed when a cancellation operation is already in flight.
|
||||
|
||||
The internal operation reconciles the active turn and session state. It does not create the visible user-cancellation message or evaluate `cancel_triggers_turn_complete`.
|
||||
|
||||
The orchestrator calls `lifecycle.Manager.ResetAgentContext` only after the internal operation finishes. An error stops the workflow entry before automatic prompt dispatch.
|
||||
|
||||
After a successful reset, the existing entry flow marks the session idle. Then `auto_start_agent` dispatches the step prompt into the new provider session.
|
||||
|
||||
The reset marker stays active through quiescence, provider replacement, configuration restoration, and reset-state persistence. The guard and marker together prevent successor admission races.
|
||||
|
||||
## Bounded predecessor wait
|
||||
|
||||
A dispatch-only prompt leaves `dispatchedPromptPending` set until its completion signal arrives. A later prompt waits at this barrier before it resets shared buffers.
|
||||
|
||||
`waitForPendingDispatchedPrompt` uses a 10-second internal timeout. The caller context can end the wait sooner.
|
||||
|
||||
If the timeout expires, the function returns a typed transient error. It does not clear the pending flag or dispatch the successor prompt.
|
||||
|
||||
The error releases `promptMu` and the orchestrator dispatch guard through existing deferred cleanup. Queued workflow prompts return to the queue through existing transient-error handling.
|
||||
|
||||
After guard release, cancellation can use its existing escalation path. That path clears the pending flag and emits a generation-bound synthetic completion signal.
|
||||
|
||||
## Completion ownership
|
||||
|
||||
Provider completion events keep their `(agent_execution_id, prompt_generation)` identity. The lifecycle manager rejects a completion that does not own the current generation.
|
||||
|
||||
An unnumbered completion cannot release a pending numbered dispatch-only prompt. This prevents a delayed synthetic completion from being consumed as the predecessor's completion.
|
||||
|
||||
Internal cancellation finishes or escalates the old generation before provider replacement. The reset does not drain a completion signal from an active generation.
|
||||
|
||||
## Failure and recovery
|
||||
|
||||
If internal cancellation fails, the provider session remains unchanged. The workflow entry records the reset error and does not send the automatic prompt.
|
||||
|
||||
If provider reset or configuration restoration fails, the existing reset reconciliation applies. The automatic prompt remains blocked.
|
||||
|
||||
If a stale predecessor barrier times out, the successor prompt fails before dispatch. The session guard becomes available for cancellation and recovery.
|
||||
|
||||
This repair does not reconcile sessions that became stuck before the new boundary existed. Users can replace such a session with a new session.
|
||||
|
||||
## Observability
|
||||
|
||||
The workflow reset logs the task, session, workflow step, and execution identifiers. Internal cancellation uses the existing cancellation and escalation logs.
|
||||
|
||||
The bounded wait error names the session execution and the timeout. Existing prompt failure logs show that the successor did not reach agentctl.
|
||||
|
||||
## Related decisions
|
||||
|
||||
- [Quiesce active turns before context reset](../../../decisions/2026-08-30-context-reset-quiesces-active-turn.md)
|
||||
- [Preserve ACP runtime configuration across context reset](../../../decisions/2026-08-18-context-reset-preserves-runtime-configuration.md)
|
||||
- [Version AgentReady events by prompt generation](../../../decisions/0035-version-agent-ready-events-by-prompt-generation.md)
|
||||
Reference in New Issue
Block a user