fix(office): retry a transient agent failure before it auto-pauses the agent (#3725)
Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com> Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
@@ -144,6 +144,12 @@ func (e *Error) Error() string {
|
||||
return fmt.Sprintf("%s: %s", e.Code, e.ClassifierRule)
|
||||
}
|
||||
|
||||
// ShouldShortRetry reports whether a classified failure is worth retrying
|
||||
// against the same provider before falling back or escalating.
|
||||
func (e *Error) ShouldShortRetry() bool {
|
||||
return e != nil && e.Class == ClassTransient && e.AutoRetryable && e.FallbackAllowed
|
||||
}
|
||||
|
||||
// Input is the raw signal bundle adapters pass to Classify.
|
||||
type Input struct {
|
||||
Phase Phase
|
||||
|
||||
@@ -0,0 +1,60 @@
|
||||
package routingerr
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestShouldShortRetry(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
err *Error
|
||||
want bool
|
||||
}{
|
||||
{
|
||||
name: "transient auto-retryable fallback-allowed",
|
||||
err: &Error{
|
||||
Class: ClassTransient,
|
||||
AutoRetryable: true,
|
||||
FallbackAllowed: true,
|
||||
},
|
||||
want: true,
|
||||
},
|
||||
{
|
||||
name: "hard class",
|
||||
err: &Error{
|
||||
Class: ClassHard,
|
||||
AutoRetryable: true,
|
||||
FallbackAllowed: true,
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "not auto-retryable",
|
||||
err: &Error{
|
||||
Class: ClassTransient,
|
||||
AutoRetryable: false,
|
||||
FallbackAllowed: true,
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "fallback not allowed (cursor retriable-stream-reset shape)",
|
||||
err: &Error{
|
||||
Class: ClassTransient,
|
||||
AutoRetryable: true,
|
||||
FallbackAllowed: false,
|
||||
},
|
||||
want: false,
|
||||
},
|
||||
{
|
||||
name: "nil error",
|
||||
err: nil,
|
||||
want: false,
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := tt.err.ShouldShortRetry(); got != tt.want {
|
||||
t.Fatalf("ShouldShortRetry() = %v, want %v", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -77,8 +77,7 @@ func (ss *SchedulerService) HandlePostStartFailure(
|
||||
// to candidate switching by default; a selected dynamic policy can add a
|
||||
// retry through the shared runtime evaluator.
|
||||
func officeShortRetryAllowed(classified *routingerr.Error) bool {
|
||||
return classified != nil && classified.FallbackAllowed &&
|
||||
classified.Class == routingerr.ClassTransient && classified.AutoRetryable
|
||||
return classified.ShouldShortRetry()
|
||||
}
|
||||
|
||||
const officeShortRetryMaxAttempts = 3
|
||||
|
||||
@@ -153,14 +153,21 @@ type ApprovalResolvedData struct {
|
||||
// same identity, so taskless fallback resolution and summary writes use this
|
||||
// field, not AgentID.
|
||||
type AgentLifecycleData struct {
|
||||
TaskID string `json:"task_id"`
|
||||
RunID string `json:"run_id"`
|
||||
AgentID string `json:"agent_id"`
|
||||
AgentProfileID string `json:"agent_profile_id"`
|
||||
SessionID string `json:"session_id"`
|
||||
TurnID string `json:"turn_id,omitempty"`
|
||||
ErrorMessage string `json:"error_message"`
|
||||
ProviderError *streams.ProviderError `json:"provider_error,omitempty"`
|
||||
TaskID string `json:"task_id"`
|
||||
RunID string `json:"run_id"`
|
||||
AgentID string `json:"agent_id"`
|
||||
AgentProfileID string `json:"agent_profile_id"`
|
||||
SessionID string `json:"session_id"`
|
||||
TurnID string `json:"turn_id,omitempty"`
|
||||
ErrorMessage string `json:"error_message"`
|
||||
ProviderError *streams.ProviderError `json:"provider_error,omitempty"`
|
||||
AgentExecutionID string `json:"agent_execution_id"`
|
||||
PromptGeneration uint64 `json:"prompt_generation,omitempty"`
|
||||
EvidenceKnown bool `json:"evidence_known,omitempty"`
|
||||
OutputObserved bool `json:"output_observed,omitempty"`
|
||||
EffectObserved bool `json:"effect_observed,omitempty"`
|
||||
ProviderDiagnosticCandidate bool `json:"provider_diagnostic_candidate,omitempty"`
|
||||
ProviderDiagnosticText string `json:"provider_diagnostic_text,omitempty"`
|
||||
}
|
||||
|
||||
// resolveLifecycleRun resolves the exact claimed run named by a lifecycle
|
||||
@@ -179,6 +186,9 @@ func (s *Service) resolveLifecycleRun(ctx context.Context, data AgentLifecycleDa
|
||||
if data.TaskID != "" && taskIDFromRunPayload(run.Payload) != data.TaskID {
|
||||
return nil, sql.ErrNoRows
|
||||
}
|
||||
if data.SessionID != "" && run.SessionID != "" && data.SessionID != run.SessionID {
|
||||
return nil, sql.ErrNoRows
|
||||
}
|
||||
return run, nil
|
||||
}
|
||||
if data.TaskID != "" {
|
||||
@@ -743,12 +753,24 @@ func (s *Service) handleAgentFailed(ctx context.Context, event *bus.Event) error
|
||||
if s.tryPostStartFallback(ctx, run, data.ErrorMessage, data.ProviderError) {
|
||||
return nil
|
||||
}
|
||||
// Office failure path (v1): every agent error is terminal. The
|
||||
// retry-by-classifier path lives behind HandleRunFailure for
|
||||
// rate-limit-retry callers; we deliberately do NOT call into it
|
||||
// here. See docs/specs/office/requirements/runtime.md.
|
||||
// Office failure path: terminal for every error except a classified-
|
||||
// transient failure, which HandleAgentFailure itself retries a
|
||||
// bounded number of times before it counts toward auto-pause
|
||||
// (AC-OFFICE-RUNTIME-001.11). The rate-limit-retry path behind
|
||||
// HandleRunFailure is a separate, pre-launch tier; we deliberately
|
||||
// do NOT call into it here. See docs/specs/office/requirements/runtime.md.
|
||||
errMsg := enrichModelFailureMessage(run, data.ErrorMessage)
|
||||
wrote, err := s.HandleAgentFailure(ctx, run, errMsg)
|
||||
wrote, err := s.HandleAgentFailure(ctx, run, errMsg, data.AgentID, data.ProviderError, AgentFailureEvidence{
|
||||
RunID: data.RunID,
|
||||
SessionID: data.SessionID,
|
||||
AgentExecutionID: data.AgentExecutionID,
|
||||
PromptGeneration: data.PromptGeneration,
|
||||
EvidenceKnown: data.EvidenceKnown,
|
||||
OutputObserved: data.OutputObserved,
|
||||
EffectObserved: data.EffectObserved,
|
||||
ProviderDiagnosticCandidate: data.ProviderDiagnosticCandidate,
|
||||
ProviderDiagnosticText: data.ProviderDiagnosticText,
|
||||
})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
@@ -200,7 +200,7 @@ func TestAutoPostAgentComment_RecordsSuccessForActingAgent(t *testing.T) {
|
||||
taskID := uuidish("task-fail-runner", i)
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "runner-pm4")
|
||||
w := queueAndReadRun(t, svc, "runner-pm4", taskID)
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom", "", nil); err != nil {
|
||||
t.Fatalf("handle failure (runner) %d: %v", i, err)
|
||||
}
|
||||
}
|
||||
@@ -208,7 +208,7 @@ func TestAutoPostAgentComment_RecordsSuccessForActingAgent(t *testing.T) {
|
||||
taskID := uuidish("task-fail-critic", i)
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "critic4")
|
||||
w := queueAndReadRun(t, svc, "critic4", taskID)
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom", "", nil); err != nil {
|
||||
t.Fatalf("handle failure (critic) %d: %v", i, err)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -10,6 +10,8 @@ import (
|
||||
|
||||
"go.uber.org/zap"
|
||||
|
||||
"github.com/kandev/kandev/internal/agent/runtime/routingerr"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
"github.com/kandev/kandev/internal/events/bus"
|
||||
"github.com/kandev/kandev/internal/office/models"
|
||||
officesqlite "github.com/kandev/kandev/internal/office/repository/sqlite"
|
||||
@@ -23,27 +25,90 @@ const (
|
||||
RunReasonManualResumeAfterFailure = "manual_resume_after_failure"
|
||||
)
|
||||
|
||||
// officeLegacyTransientMaxRetries bounds how many times a classified-transient
|
||||
// post-start failure on the legacy (HandleAgentFailure) path is retried
|
||||
// before it counts toward auto-pause like any other failure. Shared with
|
||||
// the pre-launch retry tier's run.RetryCount rather than a dedicated
|
||||
// column: a run that already burned pre-launch retries gets no transient
|
||||
// retry here, which under-retries in a rare case but can never lengthen
|
||||
// the pre-launch budget.
|
||||
const officeLegacyTransientMaxRetries = 2
|
||||
|
||||
// officeLegacyTransientBackoff mirrors the first two steps of the routing
|
||||
// tier's short-retry backoff (routing_lifecycle.go's officeShortRetryBackoff).
|
||||
var officeLegacyTransientBackoff = []time.Duration{
|
||||
5 * time.Second,
|
||||
10 * time.Second,
|
||||
}
|
||||
|
||||
// AgentFailureEvidence is the lifecycle snapshot for the invocation that
|
||||
// produced an agent failure. The legacy retry path requires this evidence to
|
||||
// prove that the failure belongs to this run and invocation, and happened
|
||||
// before output or effects.
|
||||
type AgentFailureEvidence struct {
|
||||
RunID string
|
||||
SessionID string
|
||||
AgentExecutionID string
|
||||
PromptGeneration uint64
|
||||
EvidenceKnown bool
|
||||
OutputObserved bool
|
||||
EffectObserved bool
|
||||
ProviderDiagnosticCandidate bool
|
||||
ProviderDiagnosticText string
|
||||
}
|
||||
|
||||
// HandleAgentFailure is the v1 office failure path: every agent error
|
||||
// is treated as terminal. The run is marked failed with the verbatim
|
||||
// error message, the consecutive-failure counter is incremented, and
|
||||
// when it crosses the effective threshold the agent is auto-paused.
|
||||
// It also stamps office_agent_runtime.last_run_finished_at the same as
|
||||
// the completed/stopped paths, so cooldown_sec paces the agent's next
|
||||
// heartbeat-driven fire regardless of why the previous run ended.
|
||||
// is treated as terminal, except a classified-transient post-start
|
||||
// failure that still has retry budget, which is requeued instead (see
|
||||
// tryLegacyTransientRetry). A terminal failure marks the run failed with
|
||||
// the verbatim error message, increments the consecutive-failure
|
||||
// counter, and auto-pauses the agent once it crosses the effective
|
||||
// threshold. It also stamps office_agent_runtime.last_run_finished_at
|
||||
// the same as the completed/stopped paths, so cooldown_sec paces the
|
||||
// agent's next heartbeat-driven fire regardless of why the previous run
|
||||
// ended.
|
||||
//
|
||||
// No retry is scheduled — the user resolves via Resume session in the
|
||||
// chat or Mark fixed in the inbox.
|
||||
// Beyond the transient retry above, no other retry is scheduled — the
|
||||
// user resolves via Resume session in the chat or Mark fixed in the
|
||||
// inbox.
|
||||
//
|
||||
// tryLegacyTransientRetry runs before MarkRunFailed, not after: every
|
||||
// policy cancel (task-tree cancel, workspace pause, participant
|
||||
// eviction) guards its write to status IN ('queued','claimed'), and
|
||||
// MarkRunFailed always used its own 'claimed' guard, so marking the run
|
||||
// 'failed' first and requeuing second would leave a claimed -> failed
|
||||
// -> queued window neither guard covers — a cancel landing in that
|
||||
// window matches nothing, and the unconditional requeue write would
|
||||
// resurrect the run anyway. Classifying and attempting the guarded
|
||||
// requeue first means a retry-eligible run never visits 'failed' at
|
||||
// all: it goes claimed -> queued directly, or (if a concurrent writer
|
||||
// already moved it off 'claimed') the requeue itself no-ops and
|
||||
// MarkRunFailed's identical guard below catches it the same way it
|
||||
// always has.
|
||||
//
|
||||
// Returns wrote=false when MarkRunFailed's guarded write was a no-op —
|
||||
// the run reached a terminal state through another writer (e.g. a
|
||||
// concurrent cancel) between the caller's read and this call — so
|
||||
// callers know not to treat a cancelled/already-terminal run as a
|
||||
// genuine agent failure (Review round 3, R3-1).
|
||||
// genuine agent failure. wrote=false also covers a scheduled transient
|
||||
// retry: the run was requeued, not terminalized, so callers must not
|
||||
// escalate or publish for it either.
|
||||
func (s *Service) HandleAgentFailure(
|
||||
ctx context.Context,
|
||||
run *models.Run,
|
||||
errorMessage string,
|
||||
agentID string,
|
||||
providerError *streams.ProviderError,
|
||||
evidence ...AgentFailureEvidence,
|
||||
) (bool, error) {
|
||||
var failureEvidence AgentFailureEvidence
|
||||
if len(evidence) > 0 {
|
||||
failureEvidence = evidence[0]
|
||||
}
|
||||
if s.tryLegacyTransientRetry(ctx, run, errorMessage, agentID, providerError, failureEvidence) {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
wrote, err := s.repo.MarkRunFailed(ctx, run.ID, errorMessage)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("mark run failed: %w", err)
|
||||
@@ -99,6 +164,160 @@ func (s *Service) HandleAgentFailure(
|
||||
return true, nil
|
||||
}
|
||||
|
||||
// tryLegacyTransientRetry requeues run for another attempt instead of
|
||||
// letting HandleAgentFailure treat it as terminal, when the failure
|
||||
// classifies as transient (ClassTransient, AutoRetryable, FallbackAllowed)
|
||||
// and the run still has legacy-transient retry budget. Called before
|
||||
// MarkRunFailed runs at all, so a successful retry never marks the row
|
||||
// 'failed' — the requeue write is itself
|
||||
// guarded to status = 'claimed', mirroring MarkRunFailed's own guard,
|
||||
// so the caller must not also run the terminal-shape/counter/auto-pause
|
||||
// accounting below — that is exactly what returning true signals. A
|
||||
// false return means either the failure is not retry-eligible, or the
|
||||
// guarded requeue lost a race against a concurrent writer (a cancel,
|
||||
// pause, or eviction that moved the run off 'claimed' first) — either
|
||||
// way the caller falls through to MarkRunFailed, whose identical guard
|
||||
// resolves the second case the same way it always has.
|
||||
//
|
||||
// providerError.Message is preferred over the bare errorMessage when
|
||||
// present: a raw agent stderr string ("Overloaded", a bare 429) usually
|
||||
// classifies unclassified, while the structured provider error carries
|
||||
// the signal the classifier needs.
|
||||
//
|
||||
// Provider rules are keyed by agent ID (agentID, e.g. "claude-acp"), not
|
||||
// providerError.ProviderID — some adapters put a model-provider id there
|
||||
// instead, which has no rules of its own. The provider id is substituted
|
||||
// only when the agent id itself has no rules and the provider id does,
|
||||
// mirroring classifyKanbanFailure's resolution
|
||||
// (internal/orchestrator/event_handlers_transient.go). Without this, a
|
||||
// provider rate limit — the canonical transient failure this retry exists
|
||||
// to cover — falls through the provider-specific rules unmatched and
|
||||
// classifies as an unretryable agent_runtime_error instead.
|
||||
func (s *Service) tryLegacyTransientRetry(
|
||||
ctx context.Context, run *models.Run, errorMessage string, agentID string,
|
||||
providerError *streams.ProviderError, evidence AgentFailureEvidence,
|
||||
) bool {
|
||||
if !legacyTransientRetryEvidenceSafe(run, evidence) {
|
||||
return false
|
||||
}
|
||||
delay, ok := legacyTransientRetryDelay(run)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
classified, ok := classifyLegacyTransientFailure(errorMessage, agentID, providerError, evidence)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
|
||||
retryAt := time.Now().UTC().Add(delay)
|
||||
newRetryCount := run.RetryCount + 1
|
||||
|
||||
wrote, err := s.repo.ScheduleRetryIfClaimed(ctx, run.ID, retryAt, newRetryCount)
|
||||
if err != nil {
|
||||
s.logger.Error("failed to schedule legacy transient retry",
|
||||
zap.String("run_id", run.ID), zap.Error(err))
|
||||
return false
|
||||
}
|
||||
if !wrote {
|
||||
// A concurrent writer (cancel, pause, or eviction) already moved
|
||||
// the run off 'claimed': it is no longer ours to resurrect.
|
||||
// MarkRunFailed's identical guard, called next by the caller,
|
||||
// will see the same thing and no-op the same way.
|
||||
return false
|
||||
}
|
||||
// Release ownership only after the requeue is confirmed durable — the
|
||||
// same ordering as the terminal path below. Releasing first would leave
|
||||
// the run 'claimed' with no checkout or working owner if this write (or
|
||||
// the caller's fallthrough MarkRunFailed) then failed.
|
||||
s.releaseTaskCheckoutForRun(ctx, run)
|
||||
s.clearAgentWorking(ctx, run.AgentProfileID, run.ID)
|
||||
s.logger.Info("retrying transient post-start failure before it counts toward auto-pause",
|
||||
zap.String("run_id", run.ID),
|
||||
zap.String("code", string(classified.Code)),
|
||||
zap.Int("retry_count", newRetryCount),
|
||||
zap.Duration("delay", delay))
|
||||
return true
|
||||
}
|
||||
|
||||
func legacyTransientRetryDelay(run *models.Run) (time.Duration, bool) {
|
||||
if run == nil || run.RetryCount >= officeLegacyTransientMaxRetries {
|
||||
return 0, false
|
||||
}
|
||||
if stale, _ := isRetryStale(run); stale {
|
||||
return 0, false
|
||||
}
|
||||
delay := officeLegacyTransientBackoff[run.RetryCount]
|
||||
// A requeued run is re-evaluated by evaluateRunStaleness the next time
|
||||
// it is claimed, which cancels any run with retry_count > 0 once
|
||||
// run.RequestedAt is older than staleRunThreshold. Test the age it will
|
||||
// have on arrival so the backoff cannot move it into that cancellation
|
||||
// window after this handler schedules the retry.
|
||||
if !run.RequestedAt.IsZero() && time.Since(run.RequestedAt)+delay > staleRunThreshold {
|
||||
return 0, false
|
||||
}
|
||||
return delay, true
|
||||
}
|
||||
|
||||
func classifyLegacyTransientFailure(
|
||||
errorMessage, agentID string,
|
||||
providerError *streams.ProviderError,
|
||||
evidence AgentFailureEvidence,
|
||||
) (*routingerr.Error, bool) {
|
||||
message := errorMessage
|
||||
providerID := agentID
|
||||
if providerError != nil {
|
||||
if providerError.Message != "" {
|
||||
message = providerError.Message
|
||||
}
|
||||
if id := providerError.ProviderID; id != "" &&
|
||||
!routingerr.HasProviderRules(providerID) && routingerr.HasProviderRules(id) {
|
||||
providerID = id
|
||||
}
|
||||
}
|
||||
classified := routingerr.Classify(routingerr.Input{
|
||||
Phase: routingerr.PhaseStreaming,
|
||||
ProviderID: providerID,
|
||||
Stderr: message,
|
||||
})
|
||||
if !classified.ShouldShortRetry() {
|
||||
return nil, false
|
||||
}
|
||||
if !evidence.ProviderDiagnosticCandidate {
|
||||
return classified, true
|
||||
}
|
||||
diagnostic := routingerr.Classify(routingerr.Input{
|
||||
Phase: routingerr.PhasePromptSend,
|
||||
ProviderID: providerID,
|
||||
Stderr: evidence.ProviderDiagnosticText,
|
||||
})
|
||||
diagnosticText := normalizeFailureText(evidence.ProviderDiagnosticText)
|
||||
if diagnostic.Confidence != routingerr.ConfHigh || !diagnostic.ShouldShortRetry() ||
|
||||
diagnostic.Code != classified.Code || diagnosticText == "" ||
|
||||
!strings.Contains(normalizeFailureText(message), diagnosticText) {
|
||||
return nil, false
|
||||
}
|
||||
return classified, true
|
||||
}
|
||||
|
||||
func legacyTransientRetryEvidenceSafe(run *models.Run, evidence AgentFailureEvidence) bool {
|
||||
if run == nil || evidence.RunID == "" || evidence.RunID != run.ID || evidence.SessionID == "" ||
|
||||
evidence.AgentExecutionID == "" || evidence.PromptGeneration == 0 ||
|
||||
!evidence.EvidenceKnown || evidence.OutputObserved || evidence.EffectObserved {
|
||||
return false
|
||||
}
|
||||
if run.SessionID != "" && evidence.SessionID != "" && run.SessionID != evidence.SessionID {
|
||||
return false
|
||||
}
|
||||
if evidence.ProviderDiagnosticCandidate && normalizeFailureText(evidence.ProviderDiagnosticText) == "" {
|
||||
return false
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func normalizeFailureText(value string) string {
|
||||
return streams.SanitizeProviderMessage(value)
|
||||
}
|
||||
|
||||
// RecordAgentSuccess resets the consecutive-failure counter for the
|
||||
// agent. Called from the AgentTurnMessageSaved bridge so any
|
||||
// successful turn (which is what produces the bridged comment) clears
|
||||
|
||||
@@ -102,7 +102,7 @@ func TestHandleAgentFailure_AutoPausesAtThreshold(t *testing.T) {
|
||||
taskID := uuidish("task-pause", i)
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-pause")
|
||||
w := queueAndReadRun(t, svc, "agent-pause", taskID)
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom", "", nil); err != nil {
|
||||
t.Fatalf("handle failure %d: %v", i, err)
|
||||
}
|
||||
}
|
||||
@@ -136,7 +136,7 @@ func TestRecordAgentSuccess_ResetsCounter(t *testing.T) {
|
||||
taskID := uuidish("task-succ", i)
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-success")
|
||||
w := queueAndReadRun(t, svc, "agent-success", taskID)
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom", "", nil); err != nil {
|
||||
t.Fatalf("handle failure %d: %v", i, err)
|
||||
}
|
||||
}
|
||||
@@ -180,7 +180,7 @@ func TestHandleAgentFailure_RespectsPerAgentThreshold(t *testing.T) {
|
||||
taskID := "task-tight-1"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-tight")
|
||||
w := queueAndReadRun(t, svc, "agent-tight", taskID)
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom", "", nil); err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
|
||||
@@ -206,7 +206,7 @@ func autoPauseAgent(
|
||||
taskID := agentID + "-task-" + uuidish("p", i)
|
||||
insertSyntheticTask(t, svc, taskID, wsID, agentID)
|
||||
w := queueAndReadRun(t, svc, agentID, taskID)
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom", "", nil); err != nil {
|
||||
t.Fatalf("handle failure %d: %v", i, err)
|
||||
}
|
||||
}
|
||||
@@ -336,7 +336,7 @@ func TestMarkAgentRunFailedFixed_LeavesInboxWhenRequeueFails(t *testing.T) {
|
||||
taskID := "task-retry-after-queue-error"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-task-retry")
|
||||
run := queueAndReadRun(t, svc, "agent-task-retry", taskID)
|
||||
if _, err := svc.HandleAgentFailure(ctx, run, "boom"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, run, "boom", "", nil); err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if err := svc.UpdateAgentStatusFields(ctx, "agent-task-retry",
|
||||
@@ -366,7 +366,7 @@ func TestMarkAgentPausedFixed_UsesPauseSnapshot(t *testing.T) {
|
||||
oldTaskID := "old-failure-task"
|
||||
insertSyntheticTask(t, svc, oldTaskID, "ws-1", "agent-snapshot")
|
||||
oldRun := queueAndReadRun(t, svc, "agent-snapshot", oldTaskID)
|
||||
if _, err := svc.HandleAgentFailure(ctx, oldRun, "old failure"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, oldRun, "old failure", "", nil); err != nil {
|
||||
t.Fatalf("old failure: %v", err)
|
||||
}
|
||||
svc.RecordAgentSuccess(ctx, "agent-snapshot")
|
||||
@@ -400,7 +400,7 @@ func TestMarkAgentPausedFixed_DiscardsRecoveryForReassignedTask(t *testing.T) {
|
||||
reassignedTaskID := "reassigned-task"
|
||||
insertSyntheticTask(t, svc, reassignedTaskID, "ws-1", "agent-reassigned-from")
|
||||
failedRun := queueAndReadRun(t, svc, "agent-reassigned-from", reassignedTaskID)
|
||||
if _, err := svc.HandleAgentFailure(ctx, failedRun, "boom"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, failedRun, "boom", "", nil); err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
// Two more failures (on other tasks) to cross the default threshold
|
||||
@@ -470,7 +470,7 @@ func TestMarkAgentPausedFixed_DiscardsRecoveryForStaleFailedRun(t *testing.T) {
|
||||
staleTaskID := "stale-task"
|
||||
insertSyntheticTask(t, svc, staleTaskID, "ws-1", "agent-stale-run")
|
||||
failedRun := queueAndReadRun(t, svc, "agent-stale-run", staleTaskID)
|
||||
if _, err := svc.HandleAgentFailure(ctx, failedRun, "boom"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, failedRun, "boom", "", nil); err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
// Two more failures (on other tasks) to cross the default threshold
|
||||
@@ -628,7 +628,7 @@ func TestHandleAgentFailure_RecordsTerminalShape(t *testing.T) {
|
||||
key := service.LoopMetricLabel("workspace", "ws-1", "shape", string(service.ShapeLaunchedFailed))
|
||||
before := terminalShapeExpvarInt(t, key)
|
||||
|
||||
if _, err := svc.HandleAgentFailure(ctx, run, "boom"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, run, "boom", "", nil); err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
|
||||
@@ -650,7 +650,7 @@ func TestOnAssigneeChanged_DismissesPriorEntryWithoutResettingCounter(t *testing
|
||||
taskID := "task-reassign-1"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "old-agent")
|
||||
w := queueAndReadRun(t, svc, "old-agent", taskID)
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom"); err != nil {
|
||||
if _, err := svc.HandleAgentFailure(ctx, w, "boom", "", nil); err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,963 @@
|
||||
package service_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
"github.com/kandev/kandev/internal/events/bus"
|
||||
"github.com/kandev/kandev/internal/office/models"
|
||||
"github.com/kandev/kandev/internal/office/service"
|
||||
)
|
||||
|
||||
// transientMessage classifies as ClassTransient/AutoRetryable/FallbackAllowed
|
||||
// (routingerr's provider-neutral network_unavailable rule) — the fixture
|
||||
// used throughout this file for "a retryable blip".
|
||||
const transientMessage = "connection reset by peer"
|
||||
|
||||
func safeTransientEvidence(run *models.Run) service.AgentFailureEvidence {
|
||||
return service.AgentFailureEvidence{
|
||||
RunID: run.ID,
|
||||
SessionID: "session-test",
|
||||
AgentExecutionID: "execution-test",
|
||||
PromptGeneration: 1,
|
||||
EvidenceKnown: true,
|
||||
}
|
||||
}
|
||||
|
||||
// assertScheduledRetryNear pins the exact AC-OFFICE-RUNTIME-001.11 backoff
|
||||
// value ("5s then 10s"), not just that a retry was scheduled at all
|
||||
// (Review round 3, R3-3: collapsing officeLegacyTransientBackoff to
|
||||
// {1ms, 1ms} passed every prior assertion in this file).
|
||||
func assertScheduledRetryNear(t *testing.T, got *time.Time, want time.Duration) {
|
||||
t.Helper()
|
||||
if got == nil {
|
||||
t.Fatal("expected scheduled_retry_at to be set")
|
||||
}
|
||||
const tolerance = 2 * time.Second
|
||||
if delta := time.Until(*got); delta < want-tolerance || delta > want+tolerance {
|
||||
t.Fatalf("scheduled_retry_at = now+%v, want approximately now+%v", delta, want)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_TransientDoesNotCountTowardAutoPause is AC-4.2's
|
||||
// stated observable: a classified-transient post-start failure on the
|
||||
// legacy path must not count toward auto-pause on its first occurrence.
|
||||
func TestHandleAgentFailure_TransientDoesNotCountTowardAutoPause(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-transient")
|
||||
taskID := "task-transient"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-transient")
|
||||
run := queueAndReadRun(t, svc, "agent-transient", taskID)
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, transientMessage, "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if wrote {
|
||||
t.Fatal("wrote = true, want false (retry scheduled, run not terminal)")
|
||||
}
|
||||
|
||||
agent, err := svc.GetAgentInstance(ctx, "agent-transient")
|
||||
if err != nil {
|
||||
t.Fatalf("get agent: %v", err)
|
||||
}
|
||||
if agent.ConsecutiveFailures != 0 {
|
||||
t.Fatalf("consecutive_failures = %d, want 0", agent.ConsecutiveFailures)
|
||||
}
|
||||
if agent.Status == models.AgentStatusPaused {
|
||||
t.Fatal("agent auto-paused on a single transient failure")
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusQueued {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusQueued)
|
||||
}
|
||||
assertScheduledRetryNear(t, refreshed.ScheduledRetryAt, 5*time.Second)
|
||||
if refreshed.RetryCount != 1 {
|
||||
t.Fatalf("retry_count = %d, want 1", refreshed.RetryCount)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_TransientRetryBounded pins the retry budget: the
|
||||
// third consecutive classified-transient failure on the same run (which by
|
||||
// then carries retry_count == officeLegacyTransientMaxRetries) falls
|
||||
// through to today's terminal accounting instead of scheduling a third
|
||||
// retry.
|
||||
func TestHandleAgentFailure_TransientRetryBounded(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-transient-bounded")
|
||||
taskID := "task-transient-bounded"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-transient-bounded")
|
||||
run := queueAndReadRun(t, svc, "agent-transient-bounded", taskID)
|
||||
|
||||
// AC-OFFICE-RUNTIME-001.11's stated backoff: 5s on the first retry, 10s
|
||||
// on the second.
|
||||
wantDelay := []time.Duration{5 * time.Second, 10 * time.Second}
|
||||
|
||||
for i := 0; i < 2; i++ {
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, transientMessage, "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure %d: %v", i, err)
|
||||
}
|
||||
if wrote {
|
||||
t.Fatalf("attempt %d: wrote = true, want false", i)
|
||||
}
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run %d: %v", i, err)
|
||||
}
|
||||
if refreshed.RetryCount != i+1 {
|
||||
t.Fatalf("attempt %d: retry_count = %d, want %d", i, refreshed.RetryCount, i+1)
|
||||
}
|
||||
assertScheduledRetryNear(t, refreshed.ScheduledRetryAt, wantDelay[i])
|
||||
// Simulate the dispatcher re-claiming the requeued run for its
|
||||
// next launch attempt, the precondition every production caller
|
||||
// of HandleAgentFailure actually has.
|
||||
svc.ExecSQL(t, `UPDATE runs SET status = 'claimed', claimed_at = ? WHERE id = ?`,
|
||||
time.Now().UTC(), refreshed.ID)
|
||||
refreshed.Status = service.RunStatusClaimed
|
||||
run = refreshed
|
||||
}
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, transientMessage, "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure (bound): %v", err)
|
||||
}
|
||||
if !wrote {
|
||||
t.Fatal("wrote = false, want true once the retry budget is exhausted")
|
||||
}
|
||||
|
||||
agent, err := svc.GetAgentInstance(ctx, "agent-transient-bounded")
|
||||
if err != nil {
|
||||
t.Fatalf("get agent: %v", err)
|
||||
}
|
||||
if agent.ConsecutiveFailures != 1 {
|
||||
t.Fatalf("consecutive_failures = %d, want 1", agent.ConsecutiveFailures)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_NonTransientUnchanged pins that an unclassified
|
||||
// message ("boom", the classifier-evidence fixture from triage) behaves
|
||||
// exactly as it did before this change: no retry branch fires.
|
||||
func TestHandleAgentFailure_NonTransientUnchanged(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-non-transient")
|
||||
taskID := "task-non-transient"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-non-transient")
|
||||
run := queueAndReadRun(t, svc, "agent-non-transient", taskID)
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, "boom", "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if !wrote {
|
||||
t.Fatal("wrote = false, want true (unclassified failures stay terminal)")
|
||||
}
|
||||
|
||||
agent, err := svc.GetAgentInstance(ctx, "agent-non-transient")
|
||||
if err != nil {
|
||||
t.Fatalf("get agent: %v", err)
|
||||
}
|
||||
if agent.ConsecutiveFailures != 1 {
|
||||
t.Fatalf("consecutive_failures = %d, want 1", agent.ConsecutiveFailures)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusFailed {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusFailed)
|
||||
}
|
||||
if refreshed.ScheduledRetryAt != nil {
|
||||
t.Fatal("expected scheduled_retry_at to remain unset")
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_UnlaunchableMessageNotRetried pins triage finding
|
||||
// 2: failUnlaunchableRun's wiring-fault message ("office service has no
|
||||
// task starter configured" et al.) classifies unclassified from text
|
||||
// alone, so the deliberate "do not retry a permanent wiring fault"
|
||||
// decision (scheduler_integration.go) survives this change untouched.
|
||||
func TestHandleAgentFailure_UnlaunchableMessageNotRetried(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-unlaunchable")
|
||||
taskID := "task-unlaunchable"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-unlaunchable")
|
||||
run := queueAndReadRun(t, svc, "agent-unlaunchable", taskID)
|
||||
|
||||
const msg = "scheduler cannot launch run: no task starter is configured"
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, msg, "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if !wrote {
|
||||
t.Fatal("wrote = false, want true (a wiring fault must not retry)")
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusFailed {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusFailed)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_StaleRunNotRetried pins the 24h retryMaxAge
|
||||
// abandon rule shared with the pre-launch tier: a transient failure on a
|
||||
// run requested more than 24h ago falls through to today's accounting
|
||||
// instead of scheduling a retry that would never plausibly help.
|
||||
func TestHandleAgentFailure_StaleRunNotRetried(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-stale")
|
||||
taskID := "task-stale"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-stale")
|
||||
run := queueAndReadRun(t, svc, "agent-stale", taskID)
|
||||
|
||||
staleRequestedAt := time.Now().UTC().Add(-25 * time.Hour)
|
||||
svc.ExecSQL(t, `UPDATE runs SET requested_at = ? WHERE id = ?`, staleRequestedAt, run.ID)
|
||||
run.RequestedAt = staleRequestedAt
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, transientMessage, "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if !wrote {
|
||||
t.Fatal("wrote = false, want true (a stale run must not retry, it is already marked failed)")
|
||||
}
|
||||
|
||||
agent, err := svc.GetAgentInstance(ctx, "agent-stale")
|
||||
if err != nil {
|
||||
t.Fatalf("get agent: %v", err)
|
||||
}
|
||||
if agent.ConsecutiveFailures != 1 {
|
||||
t.Fatalf("consecutive_failures = %d, want 1", agent.ConsecutiveFailures)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusFailed {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusFailed)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_ClaimStaleRunNotRetried pins review round 2
|
||||
// finding R2-1: scheduling a retry always leaves retry_count > 0, and
|
||||
// evaluateRunStaleness cancels any claimed run with retry_count > 0 once
|
||||
// requested_at is older than the 2h staleRunThreshold — well before the
|
||||
// 24h retryMaxAge this tier otherwise shares with the pre-launch retry
|
||||
// tier. Without this gate, a transient failure on a run in that window
|
||||
// would schedule a retry that the claim path immediately cancels,
|
||||
// destroying the failure with no consecutive_failures increment and no
|
||||
// inbox row. A run in the 2h-24h band must instead fall through to
|
||||
// today's terminal accounting, exactly like a run past 24h already does.
|
||||
func TestHandleAgentFailure_ClaimStaleRunNotRetried(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-claim-stale")
|
||||
taskID := "task-claim-stale"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-claim-stale")
|
||||
run := queueAndReadRun(t, svc, "agent-claim-stale", taskID)
|
||||
|
||||
// Inside the 24h retryMaxAge but past the 2h staleRunThreshold: a
|
||||
// long-running Office agent session is routine, not an edge case.
|
||||
requestedAt := time.Now().UTC().Add(-3 * time.Hour)
|
||||
svc.ExecSQL(t, `UPDATE runs SET requested_at = ? WHERE id = ?`, requestedAt, run.ID)
|
||||
run.RequestedAt = requestedAt
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, transientMessage, "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if !wrote {
|
||||
t.Fatal("wrote = false, want true (a claim-stale run must not retry, it is already marked failed)")
|
||||
}
|
||||
|
||||
agent, err := svc.GetAgentInstance(ctx, "agent-claim-stale")
|
||||
if err != nil {
|
||||
t.Fatalf("get agent: %v", err)
|
||||
}
|
||||
if agent.ConsecutiveFailures != 1 {
|
||||
t.Fatalf("consecutive_failures = %d, want 1 (the failure must count, not vanish)", agent.ConsecutiveFailures)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusFailed {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusFailed)
|
||||
}
|
||||
if refreshed.ScheduledRetryAt != nil {
|
||||
t.Fatal("expected scheduled_retry_at to remain unset")
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_ScheduledArrivalPastStaleThresholdNotRetried pins
|
||||
// review round 5 finding R5-1: the 2h staleRunThreshold gate must account
|
||||
// for the backoff it is about to schedule, not just the run's current age.
|
||||
// A run requested (2h - 3s) ago is still inside the threshold right now,
|
||||
// but scheduling the first-attempt 5s retry would land its claimable time
|
||||
// 2s past the threshold, where evaluateRunStaleness would cancel it with no
|
||||
// consecutive_failures increment and no inbox row — the exact silent-loss
|
||||
// outcome R2-1's gate exists to prevent, just displaced by the width of the
|
||||
// backoff this same function schedules. The gate must refuse before that
|
||||
// happens, falling through to today's terminal accounting instead.
|
||||
func TestHandleAgentFailure_ScheduledArrivalPastStaleThresholdNotRetried(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-arrival-stale-first")
|
||||
taskID := "task-arrival-stale-first"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-arrival-stale-first")
|
||||
run := queueAndReadRun(t, svc, "agent-arrival-stale-first", taskID)
|
||||
|
||||
// retry_count == 0: the pending backoff is 5s. 2h-3s+5s = 2h+2s, past
|
||||
// the threshold.
|
||||
requestedAt := time.Now().UTC().Add(-(2*time.Hour - 3*time.Second))
|
||||
svc.ExecSQL(t, `UPDATE runs SET requested_at = ? WHERE id = ?`, requestedAt, run.ID)
|
||||
run.RequestedAt = requestedAt
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, transientMessage, "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if !wrote {
|
||||
t.Fatal("wrote = false, want true (the scheduled arrival is past the stale threshold; the failure must count, not vanish)")
|
||||
}
|
||||
|
||||
agent, err := svc.GetAgentInstance(ctx, "agent-arrival-stale-first")
|
||||
if err != nil {
|
||||
t.Fatalf("get agent: %v", err)
|
||||
}
|
||||
if agent.ConsecutiveFailures != 1 {
|
||||
t.Fatalf("consecutive_failures = %d, want 1 (the failure must count, not vanish)", agent.ConsecutiveFailures)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusFailed {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusFailed)
|
||||
}
|
||||
if refreshed.ScheduledRetryAt != nil {
|
||||
t.Fatal("expected scheduled_retry_at to remain unset")
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_ScheduledArrivalPastStaleThresholdNotRetriedSecondAttempt
|
||||
// pins the same R5-1 gap at the second backoff step (10s, retry_count ==
|
||||
// 1), which a fix using a constant or first-step width would still miss: a
|
||||
// run requested (2h - 8s) ago is inside the threshold under EITHER the old
|
||||
// gate (time.Since alone, ~2h-8s < 2h) or a fixed-5s-width gate (2h-8s+5s <
|
||||
// 2h), and only fails when the gate reads the actual per-attempt backoff
|
||||
// (officeLegacyTransientBackoff[run.RetryCount] == 10s: 2h-8s+10s = 2h+2s).
|
||||
func TestHandleAgentFailure_ScheduledArrivalPastStaleThresholdNotRetriedSecondAttempt(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-arrival-stale-second")
|
||||
taskID := "task-arrival-stale-second"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-arrival-stale-second")
|
||||
run := queueAndReadRun(t, svc, "agent-arrival-stale-second", taskID)
|
||||
|
||||
requestedAt := time.Now().UTC().Add(-(2*time.Hour - 8*time.Second))
|
||||
svc.ExecSQL(t, `UPDATE runs SET requested_at = ?, retry_count = 1 WHERE id = ?`,
|
||||
requestedAt, run.ID)
|
||||
run.RequestedAt = requestedAt
|
||||
run.RetryCount = 1
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, transientMessage, "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if !wrote {
|
||||
t.Fatal("wrote = false, want true (the scheduled arrival is past the stale threshold; the failure must count, not vanish)")
|
||||
}
|
||||
|
||||
agent, err := svc.GetAgentInstance(ctx, "agent-arrival-stale-second")
|
||||
if err != nil {
|
||||
t.Fatalf("get agent: %v", err)
|
||||
}
|
||||
if agent.ConsecutiveFailures != 1 {
|
||||
t.Fatalf("consecutive_failures = %d, want 1 (the failure must count, not vanish)", agent.ConsecutiveFailures)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusFailed {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusFailed)
|
||||
}
|
||||
if refreshed.ScheduledRetryAt != nil {
|
||||
t.Fatal("expected scheduled_retry_at to remain unset")
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_ProviderErrorMessagePreferred pins the reason the
|
||||
// providerError parameter is in scope at all: a bare agent stderr string
|
||||
// like "Overloaded" classifies unclassified from text alone, but the
|
||||
// structured ProviderError.Message carries the signal the classifier
|
||||
// needs.
|
||||
func TestHandleAgentFailure_ProviderErrorMessagePreferred(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-provider-error")
|
||||
taskID := "task-provider-error"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-provider-error")
|
||||
run := queueAndReadRun(t, svc, "agent-provider-error", taskID)
|
||||
|
||||
providerErr := &streams.ProviderError{
|
||||
Source: "adapter",
|
||||
Message: transientMessage,
|
||||
OccurredAt: time.Now().UTC(),
|
||||
}
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, "Overloaded", "", providerErr, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if wrote {
|
||||
t.Fatal("wrote = true, want false: providerError.Message should have classified transient")
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusQueued {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusQueued)
|
||||
}
|
||||
if refreshed.RetryCount != 1 {
|
||||
t.Fatalf("retry_count = %d, want 1", refreshed.RetryCount)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_TransientRetryClearsDeadSession pins review round
|
||||
// 1 finding R1-1: a run that already launched (and so already carries the
|
||||
// session_id of the attempt that just failed) must have that session_id
|
||||
// cleared on requeue. Otherwise the relaunch resolves the dead session in
|
||||
// context_builder, mints its runtime JWT and prompt against it, and every
|
||||
// runtime action the new attempt takes gets attributed to a terminated
|
||||
// session until persistLaunchedSession finally overwrites the row.
|
||||
func TestHandleAgentFailure_TransientRetryClearsDeadSession(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-dead-session")
|
||||
taskID := "task-dead-session"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-dead-session")
|
||||
run := queueAndReadRun(t, svc, "agent-dead-session", taskID)
|
||||
|
||||
const deadSessionID = "session-from-failed-attempt"
|
||||
svc.ExecSQL(t, `UPDATE runs SET session_id = ? WHERE id = ?`, deadSessionID, run.ID)
|
||||
run.SessionID = deadSessionID
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, transientMessage, "", nil, service.AgentFailureEvidence{
|
||||
RunID: run.ID,
|
||||
SessionID: deadSessionID,
|
||||
AgentExecutionID: "execution-dead-session",
|
||||
PromptGeneration: 1,
|
||||
EvidenceKnown: true,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if wrote {
|
||||
t.Fatal("wrote = true, want false (retry scheduled, run not terminal)")
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.SessionID != "" {
|
||||
t.Fatalf("session_id = %q, want empty: a relaunch must not resolve the dead session from the failed attempt", refreshed.SessionID)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_ProviderIDRequiredForTransientClassification pins
|
||||
// review round 3 finding R3-1: routingerr's provider rules are matched
|
||||
// before the provider-neutral rules, so classifying a raw provider rate
|
||||
// limit string without the agent id misses "claude.stderr.rate.v1" and
|
||||
// falls back to an unretryable agent_runtime_error — the canonical
|
||||
// transient failure AC-4.2 exists to cover, silently un-covered. Threading
|
||||
// the agent id ("claude-acp", which has provider rules) must retry; the
|
||||
// identical message with no agent id must not.
|
||||
func TestHandleAgentFailure_ProviderIDRequiredForTransientClassification(t *testing.T) {
|
||||
const rateLimitMessage = "rate limit exceeded"
|
||||
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-rate-limit-id")
|
||||
taskID := "task-rate-limit-id"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-rate-limit-id")
|
||||
run := queueAndReadRun(t, svc, "agent-rate-limit-id", taskID)
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, rateLimitMessage, "claude-acp", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure (with agent id): %v", err)
|
||||
}
|
||||
if wrote {
|
||||
t.Fatal("wrote = true, want false: claude-acp's provider rule should have classified transient")
|
||||
}
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusQueued {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusQueued)
|
||||
}
|
||||
|
||||
svc.ExecSQL(t, `UPDATE runs SET status = 'claimed', claimed_at = ? WHERE id = ?`,
|
||||
time.Now().UTC(), refreshed.ID)
|
||||
refreshed.Status = service.RunStatusClaimed
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-rate-limit-no-id")
|
||||
taskID2 := "task-rate-limit-no-id"
|
||||
insertSyntheticTask(t, svc, taskID2, "ws-1", "agent-rate-limit-no-id")
|
||||
run2 := queueAndReadRun(t, svc, "agent-rate-limit-no-id", taskID2)
|
||||
|
||||
wrote2, err := svc.HandleAgentFailure(ctx, run2, rateLimitMessage, "", nil, safeTransientEvidence(run2))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure (no agent id): %v", err)
|
||||
}
|
||||
if !wrote2 {
|
||||
t.Fatal("wrote = false, want true: without a provider id the same message classifies unclassified, not transient")
|
||||
}
|
||||
refreshed2, err := svc.GetRun(ctx, run2.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run 2: %v", err)
|
||||
}
|
||||
if refreshed2.Status != service.RunStatusFailed {
|
||||
t.Fatalf("run status = %q, want %q", refreshed2.Status, service.RunStatusFailed)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_ProviderErrorIDSubstitutedWhenAgentIDHasNoRules pins
|
||||
// review round 4 finding R4-1: the substitution branch in
|
||||
// tryLegacyTransientRetry (providerError.ProviderID stands in for agentID
|
||||
// only when agentID itself has no provider rules) had no test exercising it
|
||||
// — every existing case either supplied an agentID with rules (claude-acp)
|
||||
// or left providerError.ProviderID empty. "too many requests" matches only
|
||||
// codex-acp's rate rule, not any provider-neutral rule, so a retry here is
|
||||
// evidence only of the substitution, not of some other classification path.
|
||||
func TestHandleAgentFailure_ProviderErrorIDSubstitutedWhenAgentIDHasNoRules(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-opencode-go")
|
||||
taskID := "task-provider-id-substituted"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-opencode-go")
|
||||
run := queueAndReadRun(t, svc, "agent-opencode-go", taskID)
|
||||
|
||||
providerErr := &streams.ProviderError{
|
||||
Source: "adapter",
|
||||
ProviderID: "codex-acp",
|
||||
Message: "too many requests",
|
||||
OccurredAt: time.Now().UTC(),
|
||||
}
|
||||
|
||||
// "opencode-go" is a real model-provider id (routingerr/rules.go's
|
||||
// HasProviderRules doc comment) with no rules of its own, so this only
|
||||
// retries if providerErr.ProviderID is substituted in.
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, "too many requests", "opencode-go", providerErr, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if wrote {
|
||||
t.Fatal("wrote = true, want false: codex-acp's provider rule should have classified transient via providerError.ProviderID")
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusQueued {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusQueued)
|
||||
}
|
||||
if refreshed.RetryCount != 1 {
|
||||
t.Fatalf("retry_count = %d, want 1", refreshed.RetryCount)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_ScheduleRetryErrorFallsThroughToTerminal pins
|
||||
// review round 3 finding R3-3: when the retry write itself errors,
|
||||
// tryLegacyTransientRetry must return false so the caller falls through to
|
||||
// terminal accounting, rather than the failure vanishing between
|
||||
// MarkRunFailed and the retry write. A trigger targets exactly the column
|
||||
// ScheduleRetry sets and MarkRunFailed does not, so MarkRunFailed's own
|
||||
// write still succeeds.
|
||||
func TestHandleAgentFailure_ScheduleRetryErrorFallsThroughToTerminal(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-schedule-retry-error")
|
||||
taskID := "task-schedule-retry-error"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-schedule-retry-error")
|
||||
run := queueAndReadRun(t, svc, "agent-schedule-retry-error", taskID)
|
||||
|
||||
svc.ExecSQL(t, `
|
||||
CREATE TRIGGER block_schedule_retry_test
|
||||
BEFORE UPDATE OF retry_count ON runs
|
||||
BEGIN
|
||||
SELECT RAISE(FAIL, 'retry_count update blocked for test');
|
||||
END`)
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, transientMessage, "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if !wrote {
|
||||
t.Fatal("wrote = false, want true: a ScheduleRetry error must fall through to terminal accounting")
|
||||
}
|
||||
|
||||
agent, err := svc.GetAgentInstance(ctx, "agent-schedule-retry-error")
|
||||
if err != nil {
|
||||
t.Fatalf("get agent: %v", err)
|
||||
}
|
||||
if agent.ConsecutiveFailures != 1 {
|
||||
t.Fatalf("consecutive_failures = %d, want 1 (the failure must count, not vanish)", agent.ConsecutiveFailures)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusFailed {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusFailed)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentFailedEvent_ThreadsAgentIDThroughToTransientRetry drives the
|
||||
// real production call site end-to-end: publishing an AgentFailed bus
|
||||
// event (not calling HandleAgentFailure directly, as every other test in
|
||||
// this file does) with AgentLifecycleData.AgentID set to a provider with
|
||||
// rules. R3-1 added a second identity threaded from the decoded event
|
||||
// through to the classifier; this is the only test that would catch it
|
||||
// being mis-threaded or dropped between the subscriber and the retry.
|
||||
func TestAgentFailedEvent_ThreadsAgentIDThroughToTransientRetry(t *testing.T) {
|
||||
svc, eb := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-e2e-ratelimit")
|
||||
taskID := "task-agent-failed-e2e"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-e2e-ratelimit")
|
||||
run := queueAndReadRun(t, svc, "agent-e2e-ratelimit", taskID)
|
||||
|
||||
event := bus.NewEvent(events.AgentFailed, "orchestrator", service.AgentLifecycleData{
|
||||
TaskID: taskID,
|
||||
RunID: run.ID,
|
||||
AgentID: "claude-acp",
|
||||
AgentProfileID: "agent-e2e-ratelimit",
|
||||
SessionID: "session-e2e-ratelimit",
|
||||
AgentExecutionID: "execution-e2e-ratelimit",
|
||||
PromptGeneration: 1,
|
||||
EvidenceKnown: true,
|
||||
ErrorMessage: "rate limit exceeded",
|
||||
ProviderError: &streams.ProviderError{
|
||||
Source: "adapter",
|
||||
Message: "rate limit exceeded",
|
||||
OccurredAt: time.Now().UTC(),
|
||||
},
|
||||
})
|
||||
if err := eb.Publish(ctx, events.AgentFailed, event); err != nil {
|
||||
t.Fatalf("publish: %v", err)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusQueued {
|
||||
t.Fatalf("run status = %q, want %q: a rate limit from an agent id with provider rules must retry",
|
||||
refreshed.Status, service.RunStatusQueued)
|
||||
}
|
||||
if refreshed.RetryCount != 1 {
|
||||
t.Fatalf("retry_count = %d, want 1", refreshed.RetryCount)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAgentFailedEvent_UnknownOrEffectfulEvidenceFallsThroughToTerminal(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
evidenceKnown bool
|
||||
outputObserved bool
|
||||
effectObserved bool
|
||||
}{
|
||||
{name: "unknown", evidenceKnown: false},
|
||||
{name: "output", evidenceKnown: true, outputObserved: true},
|
||||
{name: "effect", evidenceKnown: true, effectObserved: true},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
svc, eb := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
agentID := "agent-evidence-" + tc.name
|
||||
taskID := "task-evidence-" + tc.name
|
||||
createTestAgent(t, svc, "ws-1", agentID)
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", agentID)
|
||||
run := queueAndReadRun(t, svc, agentID, taskID)
|
||||
|
||||
event := bus.NewEvent(events.AgentFailed, "orchestrator", service.AgentLifecycleData{
|
||||
TaskID: taskID,
|
||||
RunID: run.ID,
|
||||
AgentID: "claude-acp",
|
||||
AgentProfileID: agentID,
|
||||
AgentExecutionID: "execution-" + tc.name,
|
||||
PromptGeneration: 1,
|
||||
EvidenceKnown: tc.evidenceKnown,
|
||||
OutputObserved: tc.outputObserved,
|
||||
EffectObserved: tc.effectObserved,
|
||||
ErrorMessage: "rate limit exceeded",
|
||||
})
|
||||
if err := eb.Publish(ctx, events.AgentFailed, event); err != nil {
|
||||
t.Fatalf("publish: %v", err)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusFailed {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusFailed)
|
||||
}
|
||||
if refreshed.RetryCount != 0 {
|
||||
t.Fatalf("retry_count = %d, want 0", refreshed.RetryCount)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAgentFailedEvent_DiagnosticMustMatchTerminalFailure(t *testing.T) {
|
||||
svc, eb := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
createTestAgent(t, svc, "ws-1", "agent-diagnostic-mismatch")
|
||||
taskID := "task-diagnostic-mismatch"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-diagnostic-mismatch")
|
||||
run := queueAndReadRun(t, svc, "agent-diagnostic-mismatch", taskID)
|
||||
|
||||
event := bus.NewEvent(events.AgentFailed, "orchestrator", service.AgentLifecycleData{
|
||||
TaskID: taskID,
|
||||
RunID: run.ID,
|
||||
AgentID: "claude-acp",
|
||||
AgentProfileID: "agent-diagnostic-mismatch",
|
||||
SessionID: "session-diagnostic-mismatch",
|
||||
AgentExecutionID: "execution-diagnostic-mismatch",
|
||||
PromptGeneration: 1,
|
||||
EvidenceKnown: true,
|
||||
ProviderDiagnosticCandidate: true,
|
||||
ProviderDiagnosticText: "connection reset by peer",
|
||||
ErrorMessage: "rate limit exceeded",
|
||||
})
|
||||
if err := eb.Publish(ctx, events.AgentFailed, event); err != nil {
|
||||
t.Fatalf("publish: %v", err)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusFailed {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusFailed)
|
||||
}
|
||||
if refreshed.RetryCount != 0 {
|
||||
t.Fatalf("retry_count = %d, want 0", refreshed.RetryCount)
|
||||
}
|
||||
}
|
||||
|
||||
func TestAgentFailedEvent_RejectsStaleSessionEvidence(t *testing.T) {
|
||||
svc, eb := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
createTestAgent(t, svc, "ws-1", "agent-stale-session")
|
||||
taskID := "task-stale-session"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-stale-session")
|
||||
run := queueAndReadRun(t, svc, "agent-stale-session", taskID)
|
||||
svc.ExecSQL(t, `UPDATE runs SET session_id = ? WHERE id = ?`, "current-session", run.ID)
|
||||
|
||||
event := bus.NewEvent(events.AgentFailed, "orchestrator", service.AgentLifecycleData{
|
||||
TaskID: taskID,
|
||||
RunID: run.ID,
|
||||
AgentID: "claude-acp",
|
||||
AgentProfileID: "agent-stale-session",
|
||||
SessionID: "predecessor-session",
|
||||
AgentExecutionID: "execution-stale-session",
|
||||
PromptGeneration: 1,
|
||||
EvidenceKnown: true,
|
||||
ErrorMessage: "rate limit exceeded",
|
||||
})
|
||||
if err := eb.Publish(ctx, events.AgentFailed, event); err != nil {
|
||||
t.Fatalf("publish: %v", err)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusClaimed {
|
||||
t.Fatalf("run status = %q, want %q: stale event must not resolve the current claim", refreshed.Status, service.RunStatusClaimed)
|
||||
}
|
||||
}
|
||||
|
||||
// TestAgentFailedEvent_WithoutRunIDStaysTerminal covers the compatibility
|
||||
// lookup for old events. The fallback can still find the claimed run by task
|
||||
// and agent, but that event must not authorize an automatic retry because it
|
||||
// cannot prove which run produced the failure.
|
||||
func TestAgentFailedEvent_WithoutRunIDStaysTerminal(t *testing.T) {
|
||||
svc, eb := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
createTestAgent(t, svc, "ws-1", "agent-missing-run-id")
|
||||
taskID := "task-missing-run-id"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-missing-run-id")
|
||||
run := queueAndReadRun(t, svc, "agent-missing-run-id", taskID)
|
||||
|
||||
event := bus.NewEvent(events.AgentFailed, "orchestrator", service.AgentLifecycleData{
|
||||
TaskID: taskID,
|
||||
AgentID: "claude-acp",
|
||||
AgentProfileID: "agent-missing-run-id",
|
||||
SessionID: "session-missing-run-id",
|
||||
AgentExecutionID: "execution-missing-run-id",
|
||||
PromptGeneration: 1,
|
||||
EvidenceKnown: true,
|
||||
ErrorMessage: "rate limit exceeded",
|
||||
})
|
||||
if err := eb.Publish(ctx, events.AgentFailed, event); err != nil {
|
||||
t.Fatalf("publish: %v", err)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusFailed {
|
||||
t.Fatalf("run status = %q, want %q: a legacy event without run identity must stay terminal", refreshed.Status, service.RunStatusFailed)
|
||||
}
|
||||
if refreshed.RetryCount != 0 {
|
||||
t.Fatalf("retry_count = %d, want 0", refreshed.RetryCount)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_TransientRetryNeverMarksRunFailed pins Review
|
||||
// round 6's R6-1 fix: a classified-transient failure must requeue
|
||||
// straight off the row's 'claimed' status without ever passing through
|
||||
// status='failed' on the way. Before the reorder, HandleAgentFailure
|
||||
// wrote 'failed' first and requeued second, leaving a
|
||||
// claimed -> failed -> queued window: every policy cancel (task-tree
|
||||
// cancel, workspace pause, participant eviction) guards its write to
|
||||
// status IN ('queued','claimed'), so a cancel landing in that window
|
||||
// would silently no-op and this handler would then resurrect a run the
|
||||
// canceller believed it had stopped. A trigger that fails the write the
|
||||
// instant status is set to 'failed' turns that window into a test
|
||||
// failure instead of a race that only shows up in production.
|
||||
func TestHandleAgentFailure_TransientRetryNeverMarksRunFailed(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-never-failed")
|
||||
taskID := "task-never-failed"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-never-failed")
|
||||
run := queueAndReadRun(t, svc, "agent-never-failed", taskID)
|
||||
|
||||
svc.ExecSQL(t, fmt.Sprintf(`
|
||||
CREATE TRIGGER guard_no_failed_on_retry
|
||||
BEFORE UPDATE OF status ON runs
|
||||
WHEN NEW.status = 'failed' AND OLD.id = '%s'
|
||||
BEGIN
|
||||
SELECT RAISE(FAIL, 'run must not be marked failed on the retry path');
|
||||
END;
|
||||
`, run.ID))
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, transientMessage, "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v (the run was written to status=failed before requeuing)", err)
|
||||
}
|
||||
if wrote {
|
||||
t.Fatal("wrote = true, want false (retry scheduled, run not terminal)")
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusQueued {
|
||||
t.Fatalf("run status = %q, want %q", refreshed.Status, service.RunStatusQueued)
|
||||
}
|
||||
if refreshed.RetryCount != 1 {
|
||||
t.Fatalf("retry_count = %d, want 1", refreshed.RetryCount)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHandleAgentFailure_TransientRetryLostRaceFallsThroughWithoutTerminalAccounting
|
||||
// covers the other half of R6-1's fix: when the guarded requeue loses the
|
||||
// race because a concurrent writer already moved the run off 'claimed'
|
||||
// (simulating a task-tree cancel, workspace pause, or participant
|
||||
// eviction that landed first), the handler must not resurrect the run to
|
||||
// 'queued' and must not run terminal accounting (no consecutive-failure
|
||||
// increment, no auto-pause) for a run it no longer owns — it must fall
|
||||
// through to MarkRunFailed's identical 'claimed' guard, which also no-ops
|
||||
// and leaves the winning writer's terminal state untouched.
|
||||
func TestHandleAgentFailure_TransientRetryLostRaceFallsThroughWithoutTerminalAccounting(t *testing.T) {
|
||||
svc, _ := newTestServiceWithBus(t)
|
||||
ctx := context.Background()
|
||||
|
||||
createTestAgent(t, svc, "ws-1", "agent-lost-race")
|
||||
taskID := "task-lost-race"
|
||||
insertSyntheticTask(t, svc, taskID, "ws-1", "agent-lost-race")
|
||||
run := queueAndReadRun(t, svc, "agent-lost-race", taskID)
|
||||
|
||||
// Simulate a concurrent cancel winning the race and moving the run off
|
||||
// 'claimed' before this handler's guarded requeue attempt runs.
|
||||
svc.ExecSQL(t, `UPDATE runs SET status = 'cancelled', finished_at = ? WHERE id = ?`,
|
||||
time.Now().UTC(), run.ID)
|
||||
|
||||
wrote, err := svc.HandleAgentFailure(ctx, run, transientMessage, "", nil, safeTransientEvidence(run))
|
||||
if err != nil {
|
||||
t.Fatalf("handle failure: %v", err)
|
||||
}
|
||||
if wrote {
|
||||
t.Fatal("wrote = true, want false: a lost race must not run terminal accounting")
|
||||
}
|
||||
|
||||
agent, err := svc.GetAgentInstance(ctx, "agent-lost-race")
|
||||
if err != nil {
|
||||
t.Fatalf("get agent: %v", err)
|
||||
}
|
||||
if agent.ConsecutiveFailures != 0 {
|
||||
t.Fatalf("consecutive_failures = %d, want 0: the cancel already resolved this run", agent.ConsecutiveFailures)
|
||||
}
|
||||
|
||||
refreshed, err := svc.GetRun(ctx, run.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("get run: %v", err)
|
||||
}
|
||||
if refreshed.Status != service.RunStatusCancelled {
|
||||
t.Fatalf("run status = %q, want %q (the winning writer's terminal state must survive)",
|
||||
refreshed.Status, service.RunStatusCancelled)
|
||||
}
|
||||
}
|
||||
@@ -896,7 +896,10 @@ func (si *SchedulerIntegration) failUnlaunchableRun(
|
||||
"error_message": msg,
|
||||
})
|
||||
si.releaseCheckoutIfNeeded(ctx, run)
|
||||
wrote, err := si.svc.HandleAgentFailure(ctx, run, msg)
|
||||
// No lifecycle event backs a wiring fault, so there is no agent id to
|
||||
// thread — the message classifies unclassified from text alone either
|
||||
// way (TestHandleAgentFailure_UnlaunchableMessageNotRetried).
|
||||
wrote, err := si.svc.HandleAgentFailure(ctx, run, msg, "", nil)
|
||||
if err != nil {
|
||||
si.logger.Error("failed to handle agent failure for unlaunchable run",
|
||||
zap.String("run_id", run.ID), zap.Error(err))
|
||||
|
||||
@@ -152,57 +152,9 @@ func (s *Service) mutateCeilingClaim(ctx context.Context, claim *ceilingDeferred
|
||||
// the compare-and-set only; settling publishes after release because task
|
||||
// event subscribers may re-enter runtime reconciliation.
|
||||
admissionCtx, release := s.lockCeilingEntryAdmission(ctx, claim.taskID)
|
||||
var publish bool
|
||||
var result error
|
||||
func() {
|
||||
publish, result := func() (bool, error) {
|
||||
defer release()
|
||||
for attempt := 0; attempt < deferredLaunchCASRetryBudget; attempt++ {
|
||||
record, prior, err := s.repo.GetTaskDeferredLaunch(admissionCtx, claim.taskID)
|
||||
if err != nil {
|
||||
result = err
|
||||
return
|
||||
}
|
||||
current, err := models.ReadCeilingDeferral(record)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
equivalent, err := sameCeilingDeferralIdentity(current, claim.deferral)
|
||||
if err != nil {
|
||||
result = err
|
||||
return
|
||||
}
|
||||
if !equivalent {
|
||||
// Capacity observations are mutable bookkeeping. The claim id below
|
||||
// still binds the mutation to this exact in-flight owner, while the
|
||||
// stable deferral identity prevents a successor from being touched.
|
||||
return
|
||||
}
|
||||
claimID, _, ok := models.ReadCeilingLaunchClaim(record)
|
||||
if !ok || claimID != claim.id {
|
||||
return
|
||||
}
|
||||
|
||||
updated := cloneCeilingRecord(record)
|
||||
if settle {
|
||||
updated = stripCeilingRecordKeys(updated)
|
||||
} else {
|
||||
delete(updated, models.CeilingLaunchClaimKey)
|
||||
}
|
||||
stored, lostCompare, err := s.repo.SetTaskDeferredLaunchIfUnchanged(admissionCtx, claim.taskID, prior, updated)
|
||||
if err != nil {
|
||||
result = err
|
||||
return
|
||||
}
|
||||
if stored {
|
||||
publish = settle
|
||||
return
|
||||
}
|
||||
if !lostCompare {
|
||||
result = fmt.Errorf("repository reported no claim settlement")
|
||||
return
|
||||
}
|
||||
}
|
||||
result = fmt.Errorf("claim settlement compare-and-set retries exhausted")
|
||||
return s.mutateCeilingClaimLocked(admissionCtx, claim, settle)
|
||||
}()
|
||||
if result != nil {
|
||||
return result
|
||||
@@ -212,3 +164,56 @@ func (s *Service) mutateCeilingClaim(ctx context.Context, claim *ceilingDeferred
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *Service) mutateCeilingClaimLocked(
|
||||
ctx context.Context, claim *ceilingDeferredLaunchClaim, settle bool,
|
||||
) (bool, error) {
|
||||
for attempt := 0; attempt < deferredLaunchCASRetryBudget; attempt++ {
|
||||
stored, lostCompare, err := s.tryMutateCeilingClaim(ctx, claim, settle)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
if stored {
|
||||
return settle, nil
|
||||
}
|
||||
if !lostCompare {
|
||||
return false, nil
|
||||
}
|
||||
}
|
||||
return false, fmt.Errorf("claim settlement compare-and-set retries exhausted")
|
||||
}
|
||||
|
||||
func (s *Service) tryMutateCeilingClaim(
|
||||
ctx context.Context, claim *ceilingDeferredLaunchClaim, settle bool,
|
||||
) (stored, lostCompare bool, err error) {
|
||||
record, prior, err := s.repo.GetTaskDeferredLaunch(ctx, claim.taskID)
|
||||
if err != nil {
|
||||
return false, false, err
|
||||
}
|
||||
current, err := models.ReadCeilingDeferral(record)
|
||||
if err != nil {
|
||||
return false, false, nil
|
||||
}
|
||||
equivalent, err := sameCeilingDeferralIdentity(current, claim.deferral)
|
||||
if err != nil {
|
||||
return false, false, err
|
||||
}
|
||||
if !equivalent {
|
||||
// Capacity observations are mutable bookkeeping. The claim id below
|
||||
// still binds the mutation to this exact in-flight owner, while the
|
||||
// stable deferral identity prevents a successor from being touched.
|
||||
return false, false, nil
|
||||
}
|
||||
claimID, _, ok := models.ReadCeilingLaunchClaim(record)
|
||||
if !ok || claimID != claim.id {
|
||||
return false, false, nil
|
||||
}
|
||||
|
||||
updated := cloneCeilingRecord(record)
|
||||
if settle {
|
||||
updated = stripCeilingRecordKeys(updated)
|
||||
} else {
|
||||
delete(updated, models.CeilingLaunchClaimKey)
|
||||
}
|
||||
return s.repo.SetTaskDeferredLaunchIfUnchanged(ctx, claim.taskID, prior, updated)
|
||||
}
|
||||
|
||||
@@ -643,17 +643,49 @@ func (r *Repository) ClaimNextEligibleRun(ctx context.Context) (*models.Run, err
|
||||
}
|
||||
|
||||
// ScheduleRetry resets a run to queued with an incremented retry count
|
||||
// and a scheduled retry time.
|
||||
// and a scheduled retry time. session_id is cleared: every caller either
|
||||
// runs pre-launch (the run never had one) or post-start (the session it
|
||||
// had belongs to the failed attempt), and a relaunch must mint its
|
||||
// runtime credentials against the session the new attempt actually gets,
|
||||
// not a stale one from a previous attempt. error_message is cleared for
|
||||
// the same reason: a requeued run is not yet failed, so it must not carry
|
||||
// the previous attempt's error into a later successful finish.
|
||||
func (r *Repository) ScheduleRetry(ctx context.Context, runID string, retryAt time.Time, retryCount int) error {
|
||||
_, err := r.db.ExecContext(ctx, r.db.Rebind(`
|
||||
UPDATE runs
|
||||
SET status = 'queued', retry_count = ?, scheduled_retry_at = ?,
|
||||
claimed_at = NULL, finished_at = NULL
|
||||
claimed_at = NULL, finished_at = NULL, session_id = '', error_message = ''
|
||||
WHERE id = ?
|
||||
`), retryCount, retryAt, runID)
|
||||
return err
|
||||
}
|
||||
|
||||
// ScheduleRetryIfClaimed behaves like ScheduleRetry but only when the run
|
||||
// is still status='claimed', mirroring the same guard MarkRunFailed uses.
|
||||
// A caller that wants to requeue a run it has not itself moved off
|
||||
// 'claimed' must not resurrect a row a concurrent writer already
|
||||
// terminalized (a task-tree cancel, workspace pause, or participant
|
||||
// eviction) out from under it — every one of those writers targets
|
||||
// exactly the 'claimed' status this guard checks. wrote=false means a
|
||||
// concurrent writer already changed the row's status; the caller must
|
||||
// not retry again or treat the run as requeued.
|
||||
func (r *Repository) ScheduleRetryIfClaimed(ctx context.Context, runID string, retryAt time.Time, retryCount int) (bool, error) {
|
||||
res, err := r.db.ExecContext(ctx, r.db.Rebind(`
|
||||
UPDATE runs
|
||||
SET status = 'queued', retry_count = ?, scheduled_retry_at = ?,
|
||||
claimed_at = NULL, finished_at = NULL, session_id = '', error_message = ''
|
||||
WHERE id = ? AND status = 'claimed'
|
||||
`), retryCount, retryAt, runID)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
n, err := res.RowsAffected()
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
return n > 0, nil
|
||||
}
|
||||
|
||||
// CleanExpired deletes finished/failed runs older than the given time.
|
||||
func (r *Repository) CleanExpired(ctx context.Context, olderThan time.Time) (int64, error) {
|
||||
res, err := r.db.ExecContext(ctx, r.db.Rebind(`
|
||||
|
||||
@@ -482,13 +482,34 @@ func TestCreateRun_DuplicateIdempotencyKeyIsRejected(t *testing.T) {
|
||||
|
||||
// TestScheduleRetry_RequeuesAndClearsTerminalTimestamps asserts every
|
||||
// column the retry write owns, from a run that had already finished.
|
||||
// session_id is seeded to simulate a post-start failure (the run reached
|
||||
// a real launch before failing) and must come back empty: a relaunch
|
||||
// that resolved the prior attempt's session would mint its runtime JWT
|
||||
// and prompt against a session the new attempt never owns (review round
|
||||
// 1, R1-1). error_message is seeded the same way and must also come back
|
||||
// empty: a run that later finishes successfully must not still carry the
|
||||
// previous attempt's error into a status='finished' row, which the run
|
||||
// detail UI renders as an unconditional tooltip regardless of status
|
||||
// (review round 2, R2-2).
|
||||
func TestScheduleRetry_RequeuesAndClearsTerminalTimestamps(t *testing.T) {
|
||||
repo := newTestRepo(t)
|
||||
ctx := context.Background()
|
||||
now := time.Now().UTC()
|
||||
run := queueRunAt(t, repo, "retryable", "a1", now.Add(-time.Hour))
|
||||
if err := repo.UpdateRunRuntimeSnapshot(ctx, run.ID, "{}", "{}", "session-from-failed-attempt"); err != nil {
|
||||
t.Fatalf("seed session_id: %v", err)
|
||||
}
|
||||
if err := repo.SetRunErrorMessageForTest(ctx, run.ID, "connection reset by peer"); err != nil {
|
||||
t.Fatalf("seed error_message: %v", err)
|
||||
}
|
||||
setStatus(t, repo, run.ID, "failed", timePtr(now.Add(-30*time.Minute)), timePtr(now))
|
||||
untouched := queueRunAt(t, repo, "bystander", "a1", now.Add(-time.Hour))
|
||||
if err := repo.UpdateRunRuntimeSnapshot(ctx, untouched.ID, "{}", "{}", "session-still-running"); err != nil {
|
||||
t.Fatalf("seed bystander session_id: %v", err)
|
||||
}
|
||||
if err := repo.SetRunErrorMessageForTest(ctx, untouched.ID, "bystander error"); err != nil {
|
||||
t.Fatalf("seed bystander error_message: %v", err)
|
||||
}
|
||||
|
||||
retryAt := time.Date(2026, 7, 8, 9, 10, 11, 0, time.UTC)
|
||||
if err := repo.ScheduleRetry(ctx, run.ID, retryAt, 4); err != nil {
|
||||
@@ -507,6 +528,8 @@ func TestScheduleRetry_RequeuesAndClearsTerminalTimestamps(t *testing.T) {
|
||||
if got.FinishedAt != nil {
|
||||
t.Errorf("finished_at = %s, want nil", got.FinishedAt)
|
||||
}
|
||||
checkString(t, "session_id cleared", got.SessionID, "")
|
||||
checkString(t, "error_message cleared", got.ErrorMessage, "")
|
||||
checkString(t, "payload preserved", got.Payload, `{"task_id":"retryable"}`)
|
||||
|
||||
bystander := mustGetRun(t, repo, untouched.ID)
|
||||
@@ -514,6 +537,8 @@ func TestScheduleRetry_RequeuesAndClearsTerminalTimestamps(t *testing.T) {
|
||||
if bystander.ScheduledRetryAt != nil {
|
||||
t.Errorf("bystander scheduled_retry_at = %s, want nil", bystander.ScheduledRetryAt)
|
||||
}
|
||||
checkString(t, "bystander session_id untouched", bystander.SessionID, "session-still-running")
|
||||
checkString(t, "bystander error_message untouched", bystander.ErrorMessage, "bystander error")
|
||||
}
|
||||
|
||||
// TestScheduleRetry_WritesTheAttemptCountVerbatim pins that the caller
|
||||
@@ -531,6 +556,66 @@ func TestScheduleRetry_WritesTheAttemptCountVerbatim(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestScheduleRetryIfClaimed_RequeuesWhenStillClaimed asserts the guarded
|
||||
// requeue behaves exactly like ScheduleRetry — same column writes, same
|
||||
// clearing of session_id/error_message — when the run is still the
|
||||
// 'claimed' status the caller read it as.
|
||||
func TestScheduleRetryIfClaimed_RequeuesWhenStillClaimed(t *testing.T) {
|
||||
repo := newTestRepo(t)
|
||||
ctx := context.Background()
|
||||
now := time.Now().UTC()
|
||||
run := queueRunAt(t, repo, "guarded", "a1", now.Add(-time.Hour))
|
||||
setStatus(t, repo, run.ID, "claimed", timePtr(now.Add(-time.Minute)), nil)
|
||||
|
||||
wrote, err := repo.ScheduleRetryIfClaimed(ctx, run.ID, fixedRetryAt, 1)
|
||||
if err != nil {
|
||||
t.Fatalf("schedule retry if claimed: %v", err)
|
||||
}
|
||||
if !wrote {
|
||||
t.Fatal("wrote = false, want true: run was still claimed")
|
||||
}
|
||||
|
||||
got := mustGetRun(t, repo, run.ID)
|
||||
checkString(t, "status", string(got.Status), "queued")
|
||||
checkInt(t, "retry_count", got.RetryCount, 1)
|
||||
if got.ClaimedAt != nil {
|
||||
t.Errorf("claimed_at = %s, want nil", got.ClaimedAt)
|
||||
}
|
||||
if got.ScheduledRetryAt == nil || !got.ScheduledRetryAt.Equal(fixedRetryAt) {
|
||||
t.Errorf("scheduled_retry_at = %v, want %s", got.ScheduledRetryAt, fixedRetryAt)
|
||||
}
|
||||
}
|
||||
|
||||
// TestScheduleRetryIfClaimed_NoopWhenNoLongerClaimed pins the guard this
|
||||
// method exists for: a run a concurrent writer already moved off
|
||||
// 'claimed' (simulating a task-tree cancel, workspace pause, or
|
||||
// participant eviction landing between the caller's read and this write)
|
||||
// must not be resurrected to 'queued', and none of its columns may move —
|
||||
// the caller has to see wrote=false and leave the row exactly as the
|
||||
// other writer left it.
|
||||
func TestScheduleRetryIfClaimed_NoopWhenNoLongerClaimed(t *testing.T) {
|
||||
repo := newTestRepo(t)
|
||||
ctx := context.Background()
|
||||
now := time.Now().UTC()
|
||||
run := queueRunAt(t, repo, "raced", "a1", now.Add(-time.Hour))
|
||||
setStatus(t, repo, run.ID, "cancelled", timePtr(now.Add(-time.Minute)), timePtr(now))
|
||||
|
||||
wrote, err := repo.ScheduleRetryIfClaimed(ctx, run.ID, fixedRetryAt, 1)
|
||||
if err != nil {
|
||||
t.Fatalf("schedule retry if claimed: %v", err)
|
||||
}
|
||||
if wrote {
|
||||
t.Fatal("wrote = true, want false: a concurrent writer already moved the run off claimed")
|
||||
}
|
||||
|
||||
got := mustGetRun(t, repo, run.ID)
|
||||
checkString(t, "status untouched", string(got.Status), "cancelled")
|
||||
checkInt(t, "retry_count untouched", got.RetryCount, 0)
|
||||
if got.ScheduledRetryAt != nil {
|
||||
t.Errorf("scheduled_retry_at = %v, want nil (untouched)", got.ScheduledRetryAt)
|
||||
}
|
||||
}
|
||||
|
||||
// TestRecoverStale_ResetsOnlyClaimedRunsStrictlyOlderThanTheCutoff
|
||||
// exercises the staleness boundary in both directions. The cutoff is a
|
||||
// parameter, so the equality case is exact rather than clock-dependent.
|
||||
|
||||
@@ -0,0 +1,139 @@
|
||||
---
|
||||
created: 2026-09-16
|
||||
status: implemented
|
||||
requirements:
|
||||
- REQ-OFFICE-RUNTIME-001
|
||||
system_design:
|
||||
- ../../specs/office/system-design/runtime-01.md
|
||||
legacy_specs: []
|
||||
---
|
||||
|
||||
# Implementation Plan: Retry classified-transient legacy post-start failures
|
||||
|
||||
## Overview
|
||||
|
||||
Close "Hole 1" of the Beta unattended-recovery review: the legacy
|
||||
(non-routing) post-start failure path, `HandleAgentFailure`, had no retry at
|
||||
all — one transient blip counted as a full strike toward auto-pause. One
|
||||
sequential work order adds a bounded, classified-transient retry to that path
|
||||
before it can count toward `consecutive_failures`.
|
||||
|
||||
## Evidence and root cause
|
||||
|
||||
`handleAgentFailed` tries the routing tier (`tryPostStartFallback`) first and
|
||||
falls through to `HandleAgentFailure` when there is no dispatcher or no
|
||||
`ResolvedProviderID`. `HandleAgentFailure` (`office/service/failure.go`)
|
||||
incremented `consecutive_failures` immediately on every failure, with no
|
||||
classification step, so a single transient upstream blip on that fallthrough
|
||||
path auto-paused the agent at the same rate as a genuine terminal failure.
|
||||
The working routing-tier retry (`office/scheduler/routing_lifecycle.go`) and
|
||||
pre-launch retry (`office/service/retry.go`) already handle their own paths
|
||||
correctly and are unchanged.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- A bounded classified-transient retry on the legacy post-start path only,
|
||||
reusing the existing routing classifier's `ClassTransient && AutoRetryable
|
||||
&& FallbackAllowed` predicate (no second classifier).
|
||||
- A claim-guarded write so the retry can never resurrect a run the claim path
|
||||
has already cancelled for staleness.
|
||||
- Regression coverage for the new eligibility gate, the scheduled-arrival
|
||||
staleness check, agent id / `providerError.ProviderID` substitution, and the
|
||||
`claimed -> failed -> queued` race this closes.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- Hole 2 (automatic unpause) — blocked on card `497b1f63` (broken manual
|
||||
unpause); a product decision, not part of this change.
|
||||
- Hole 3 (reaper for runs stranded in `claimed`) — already shipped on `main`
|
||||
as `recoverStaleClaimedRuns`.
|
||||
- The pre-launch retry tier, the routing-tier retry policy, and the
|
||||
`blocked_provider_action_required` park — all unchanged by design.
|
||||
- `apps/web` — no frontend surface for this change.
|
||||
|
||||
## Technical approach
|
||||
|
||||
Follow the [system design](../../specs/office/system-design/runtime-01.md#legacy-post-start-transient-retry).
|
||||
Add `routingerr.(*Error).ShouldShortRetry()` (nil-safe) as the single shared
|
||||
transient predicate; `officeShortRetryAllowed` in the scheduler delegates to
|
||||
it so there is exactly one definition for both callers (`office/scheduler`
|
||||
already imports `office/service`, so the shared predicate lives in
|
||||
`routingerr`). `HandleAgentFailure` calls a new `tryLegacyTransientRetry`
|
||||
first; `MarkRunFailed` only runs when that returns false.
|
||||
|
||||
`tryLegacyTransientRetry` eligibility: `RetryCount < 2`, the existing 24h
|
||||
`retryMaxAge`, and a scheduled-arrival gate that refuses the retry (falling
|
||||
through to terminal accounting) when `age + delay` already exceeds the 2h
|
||||
`staleRunThreshold` the claim path uses to cancel stale runs — otherwise the
|
||||
run could age into the cancellation window during its own backoff and the
|
||||
failure would be silently lost instead of counted. The provider error's
|
||||
`ProviderID` (or the caller's `AgentID`) is threaded into the classifier
|
||||
before the predicate runs.
|
||||
|
||||
The write path is a new `ScheduleRetryIfClaimed` (`WHERE id=? AND
|
||||
status='claimed'`, returns affected-row count) instead of the unguarded
|
||||
`ScheduleRetry`. On success the run goes `claimed -> queued` directly and
|
||||
never visits `failed`. Zero affected rows or a DB error falls through to the
|
||||
existing `MarkRunFailed`, whose identical guard no-ops the same way — this is
|
||||
what closes the `claimed -> failed -> queued` race a plain
|
||||
claim-then-schedule ordering would leave open.
|
||||
|
||||
Backoff is 5s then 10s, capped at 2 retries, sharing the existing
|
||||
`retry_count` column with the pre-launch tier (`MaxRetryCount = 4`), so the
|
||||
combined budget can never be exceeded and no new column is needed.
|
||||
|
||||
## Tests
|
||||
|
||||
- `failure_transient_retry_test.go`: eligibility (retry-count cap, 24h
|
||||
staleness, scheduled-arrival gate), agent id / `providerError.ProviderID`
|
||||
substitution, and the classifier delegation (16 tests).
|
||||
- `runs_queue_test.go`: `TestScheduleRetryIfClaimed*` covering the
|
||||
claimed-only guard and its zero-rows fallthrough (2 tests).
|
||||
- `routingerr/short_retry_test.go`: table test for `ShouldShortRetry` across
|
||||
the classifier's boolean combinations, nil-safe (1 table, 5 cases).
|
||||
|
||||
## End-to-end evidence
|
||||
|
||||
No `apps/web` files change (`git diff --name-only origin/main...HEAD` — 13
|
||||
files, 0 under `apps/web/`), so no Playwright coverage is required. Backend
|
||||
package tests exercise the real SQLite-backed run repository, not mocks.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Retry legacy post-start transient failures](task-01-retry-legacy-post-start-failures.md)
|
||||
|
||||
## Verification results
|
||||
|
||||
Implementation completed 2026-09-16, PR
|
||||
[#3725](https://github.com/kdlbs/kandev/pull/3725).
|
||||
|
||||
- `go build ./...`: clean. `gofmt -l` over changed `.go` files: empty.
|
||||
- `make -C apps/backend lint`: "0 issues."
|
||||
- `make typecheck`: exit 0. `make lint-format`: clean.
|
||||
- `go test -tags fts5 -count=1 ./internal/office/... ./internal/runs/...
|
||||
./internal/agent/runtime/routingerr/...`: every touched package passes.
|
||||
- `python3 scripts/list-docs.py validate`: "Validated 278 decisions and 952
|
||||
specifications." `lint-spec-files.py --all`: all passed.
|
||||
- `python3 scripts/lint-architecture.py --all`: clean.
|
||||
- Full `make -C apps/backend test`: 15 pre-existing FAIL packages, none in a
|
||||
package this diff touches (standing KB clusters, unrelated to this change).
|
||||
|
||||
See the [work-order results](task-01-retry-legacy-post-start-failures.md#results)
|
||||
for the full command list and receipts.
|
||||
|
||||
## Risks
|
||||
|
||||
The retry re-drives the whole run from the top only after the lifecycle event
|
||||
identifies the exact run and proves that no output or effect was observed for
|
||||
the current invocation.
|
||||
Unknown, stale, output-producing, effectful, or diagnostically mismatched
|
||||
events fall through to terminal accounting.
|
||||
|
||||
## Documentation impact
|
||||
|
||||
`docs/specs/office/requirements/runtime.md` gained AC-OFFICE-RUNTIME-001.11;
|
||||
`docs/specs/office/system-design/runtime-01.md` § Legacy post-start transient
|
||||
retry was rewritten to match the implementation. No public-docs or operator-
|
||||
facing surface changed.
|
||||
@@ -0,0 +1,149 @@
|
||||
---
|
||||
id: "01-retry-legacy-post-start-failures"
|
||||
title: "Retry legacy post-start transient failures"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-OFFICE-RUNTIME-001
|
||||
acceptance_criteria:
|
||||
- AC-OFFICE-RUNTIME-001.11
|
||||
system_design:
|
||||
- ../../specs/office/system-design/runtime-01.md
|
||||
---
|
||||
|
||||
# Task 01: Retry legacy post-start transient failures
|
||||
|
||||
## Summary
|
||||
|
||||
Give the legacy (non-routing) post-start failure path,
|
||||
`HandleAgentFailure`, the same bounded classified-transient retry the
|
||||
routing tier and pre-launch tier already have, so one transient blip no
|
||||
longer counts as a full strike toward `consecutive_failures` and auto-pause.
|
||||
|
||||
## In scope
|
||||
|
||||
- `routingerr.(*Error).ShouldShortRetry()` as the single shared transient
|
||||
predicate, reused by both the scheduler's `officeShortRetryAllowed` and the
|
||||
new legacy-path check.
|
||||
- `tryLegacyTransientRetry` in `HandleAgentFailure`, tried before
|
||||
`MarkRunFailed`.
|
||||
- `ScheduleRetryIfClaimed` (claimed-only guarded write) so the retry can never
|
||||
resurrect a run the claim path already cancelled for staleness.
|
||||
- Agent id / `providerError.ProviderID` substitution into the classifier.
|
||||
- Lifecycle run, execution, and prompt evidence must prove a current,
|
||||
effect-safe invocation before the retry is scheduled. Unknown or mismatched
|
||||
evidence falls through to terminal accounting.
|
||||
- Regression tests for eligibility, the scheduled-arrival gate, and the
|
||||
`claimed -> failed -> queued` race.
|
||||
|
||||
## Out of scope
|
||||
|
||||
Hole 2 (auto-unpause, blocked on card `497b1f63`), Hole 3 (already shipped as
|
||||
`recoverStaleClaimedRuns`), the pre-launch retry tier, the routing-tier retry
|
||||
policy, the `blocked_provider_action_required` park, and `apps/web`.
|
||||
|
||||
## Acceptance
|
||||
|
||||
1. A classified-transient failure on the legacy post-start path (no
|
||||
dispatcher / no `ResolvedProviderID`, so `tryPostStartFallback` did not
|
||||
handle it) is retried up to 2 times at 5s then 10s before
|
||||
`consecutive_failures` increments, observed by `consecutive_failures`
|
||||
staying unchanged on the first classified-transient failure
|
||||
(AC-OFFICE-RUNTIME-001.11).
|
||||
2. The retry shares its attempt counter with the pre-launch tier (no new
|
||||
column) and is abandoned under the same 24h staleness rule, so a run that
|
||||
already exhausted pre-launch retries gets no additional post-start retry.
|
||||
3. The retry is refused — falling through to terminal accounting instead of
|
||||
being scheduled — when the run's scheduled arrival (age plus the delay
|
||||
about to be scheduled) already exceeds the 2h `staleRunThreshold` the claim
|
||||
path uses to cancel stale runs.
|
||||
4. `ScheduleRetryIfClaimed` only mutates a run whose status is still
|
||||
`claimed`; a concurrent cancellation (zero affected rows) falls through to
|
||||
the existing `MarkRunFailed` guard instead of resurrecting a cancelled run.
|
||||
|
||||
## Verification
|
||||
|
||||
Run from `apps/backend`:
|
||||
|
||||
```bash
|
||||
go test -tags fts5 -count=1 ./internal/office/... ./internal/runs/... ./internal/agent/runtime/routingerr/...
|
||||
gofmt -l $(git diff --name-only origin/main...HEAD -- '*.go')
|
||||
make lint
|
||||
make typecheck
|
||||
```
|
||||
|
||||
Repository-root document checks:
|
||||
|
||||
```bash
|
||||
python3 scripts/list-docs.py validate
|
||||
python3 scripts/lint-spec-files.py --all
|
||||
python3 scripts/lint-architecture.py --all
|
||||
```
|
||||
|
||||
## Files touched
|
||||
|
||||
- `apps/backend/internal/agent/runtime/routingerr/routingerr.go`,
|
||||
`short_retry_test.go`
|
||||
- `apps/backend/internal/office/scheduler/routing_lifecycle.go`
|
||||
- `apps/backend/internal/office/service/event_subscribers.go`,
|
||||
`event_subscribers_session_attribution_test.go`, `failure.go`,
|
||||
`failure_test.go`, `failure_transient_retry_test.go`,
|
||||
`scheduler_integration.go`
|
||||
- `apps/backend/internal/runs/repository/sqlite/runs.go`,
|
||||
`runs_queue_test.go`
|
||||
- `docs/specs/office/requirements/runtime.md`,
|
||||
`docs/specs/office/system-design/runtime-01.md`
|
||||
- This work order and `plan.md`.
|
||||
|
||||
## Dependencies
|
||||
|
||||
None. Read `apps/backend/internal/office/AGENTS.md` before touching this
|
||||
path again.
|
||||
|
||||
## Risks
|
||||
|
||||
The retry re-drives the whole run from the top only after the lifecycle event
|
||||
identifies the exact run and proves that no output or effect was observed for
|
||||
the current invocation. Events with missing or stale run identity, unknown,
|
||||
output-producing, or effectful evidence fall through to terminal accounting.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- [Requirements](../../specs/office/requirements/runtime.md#req-office-runtime-001-office-agent-runtime--error-handling-contract).
|
||||
- [Design](../../specs/office/system-design/runtime-01.md#legacy-post-start-transient-retry).
|
||||
- The existing routing-tier retry (`office/scheduler/routing_lifecycle.go`)
|
||||
and pre-launch retry (`office/service/retry.go`) as the working reference
|
||||
implementations.
|
||||
|
||||
## Results
|
||||
|
||||
Completed 2026-09-16. PR [#3725](https://github.com/kdlbs/kandev/pull/3725),
|
||||
branch `feature/office-recovery-post-d86`, head `ce8f0da06`.
|
||||
|
||||
- `go test -tags fts5 -count=1 ./internal/office/... ./internal/runs/...
|
||||
./internal/agent/runtime/routingerr/...`: all packages this diff touches
|
||||
pass, including the 16 `failure_transient_retry_test.go` cases, the 2
|
||||
`TestScheduleRetryIfClaimed*` cases, and the 5-case `ShouldShortRetry`
|
||||
table.
|
||||
- `go build ./...`: clean. `gofmt -l` over changed `.go` files: empty.
|
||||
- `make -C apps/backend lint`: "0 issues." `make typecheck`: exit 0.
|
||||
`make lint-format`: clean.
|
||||
- `python3 scripts/list-docs.py validate`: "Validated 278 decisions and 952
|
||||
specifications." `python3 scripts/lint-spec-files.py --all`: all passed.
|
||||
- `python3 scripts/lint-architecture.py --all`: clean.
|
||||
- Full `make -C apps/backend test`: 15 pre-existing FAIL packages, zero in any
|
||||
package this diff touches (standing KB clusters, unrelated).
|
||||
- Two additional pre-existing failures independently reproduced against a
|
||||
scratch worktree at bare `origin/main`
|
||||
(`TestMigrate_PriorityIdempotent`, `scripts/pr-await.test.sh`'s
|
||||
strict-deadline case) and excluded as not caused by this change.
|
||||
|
||||
No live instance, deployment, or database migration was touched. Production
|
||||
code matches the rewritten system design section exactly; no subagents were
|
||||
used for the implementation.
|
||||
@@ -9,19 +9,19 @@ owners:
|
||||
|
||||
## Overview
|
||||
|
||||
When an agent run fails mid-turn (invalid model, auth failure, malformed response, transient upstream), the failed wakeup is terminal. The workflow engine can still evaluate a configured `on_agent_error` action and queue a separate coordinator run for escalation.
|
||||
When an agent run fails mid-turn (invalid model, auth failure, malformed response, transient upstream), the failed wakeup is terminal — except a **classified-transient** failure on the legacy (non-routing) post-start path, which is retried a bounded number of times before it counts as a terminal failure (AC-OFFICE-RUNTIME-001.11). The workflow engine can still evaluate a configured `on_agent_error` action and queue a separate coordinator run for escalation.
|
||||
|
||||
## Requirements
|
||||
|
||||
### REQ-OFFICE-RUNTIME-001: Office Agent Runtime — Error Handling Contract
|
||||
|
||||
**Intent:** When an agent run fails mid-turn (invalid model, auth failure, malformed response, transient upstream), the failed agent run is terminal. A workflow can use `on_agent_error` to queue a separate coordinator run for escalation.
|
||||
**Intent:** When an agent run fails mid-turn (invalid model, auth failure, malformed response, transient upstream), the failed agent run is terminal, except the bounded classified-transient retry carved out by AC-OFFICE-RUNTIME-001.11. A workflow can use `on_agent_error` to queue a separate coordinator run for escalation.
|
||||
|
||||
#### Acceptance criteria
|
||||
|
||||
- **AC-OFFICE-RUNTIME-001.1:** No automatic retry of the failed agent run. Every adapter error is terminal for the wakeup that produced it.
|
||||
- **AC-OFFICE-RUNTIME-001.1:** No automatic retry of the failed agent run, except the bounded classified-transient retry in AC-OFFICE-RUNTIME-001.11. Every other adapter error is terminal for the wakeup that produced it.
|
||||
- **AC-OFFICE-RUNTIME-001.2:** The wakeup row is stamped with `status = failed` and `error_message`. The failure path does not queue another wakeup for the same failed agent. A configured `on_agent_error` action can queue a separate coordinator run.
|
||||
- **AC-OFFICE-RUNTIME-001.3:** Re-runs of the failed agent happen only via explicit user action: **Resume session** in chat, **Mark fixed** on an inbox entry, or task reassignment to a different agent. A coordinator run from `on_agent_error` is escalation, not a retry.
|
||||
- **AC-OFFICE-RUNTIME-001.3:** Re-runs of the failed agent happen only via explicit user action — **Resume session** in chat, **Mark fixed** on an inbox entry, or task reassignment to a different agent — except the automatic bounded retry in AC-OFFICE-RUNTIME-001.11. A coordinator run from `on_agent_error` is escalation, not a retry.
|
||||
- **AC-OFFICE-RUNTIME-001.4:** **`agent_run_failed`** - one entry per failed (task, agent) wakeup while the agent is below threshold. Title: "<agent> failed on <task>". Action: **Mark fixed** -> dismiss + retry.
|
||||
- **AC-OFFICE-RUNTIME-001.5:** **`agent_paused_after_failures`** - one entry per auto-paused agent. Title: "<agent> auto-paused after <N> failures (tasks A, B, C)". Action: **Mark fixed** -> unpause + retry the affected tasks.
|
||||
- **AC-OFFICE-RUNTIME-001.6:** Changing `assignee_agent_instance_id` on a task fires the existing reactivity pipeline, which queues a fresh `task_assigned` wakeup for the new agent.
|
||||
@@ -29,6 +29,7 @@ When an agent run fails mid-turn (invalid model, auth failure, malformed respons
|
||||
- **AC-OFFICE-RUNTIME-001.8:** Any per-task `agent_run_failed` inbox entry tied to the old (task, agent) auto-dismisses - the failure is no longer actionable on this task.
|
||||
- **AC-OFFICE-RUNTIME-001.9:** If a second auto-pause replaces the agent's `pause_reason` after **Mark fixed** on `agent_paused_after_failures` has read it but before its clear commits, the clear is refused and the call aborts instead of adopting the newer reason: `consecutive_failures` is not reset and no wakeup is re-queued.
|
||||
- **AC-OFFICE-RUNTIME-001.10:** A **Mark fixed** call aborted under AC-OFFICE-RUNTIME-001.9 does not dismiss the inbox entry - it stays actionable for the still-paused agent, and is dismissed only once a clear against the then-current `pause_reason` succeeds.
|
||||
- **AC-OFFICE-RUNTIME-001.11:** A post-start failure on the legacy (`HandleAgentFailure`) path that classifies transient (the shared routing classifier's `ClassTransient && AutoRetryable && FallbackAllowed` predicate) is retried up to 2 times, at 5s then 10s, before it counts toward `consecutive_failures` or auto-pause — observed by `consecutive_failures` staying unchanged on the first classified-transient failure. This shares its attempt counter with the pre-launch retry tier (no separate column) and is abandoned under the same 24h staleness rule, so a run that already exhausted pre-launch retries gets no additional post-start retry. The retry is additionally refused — falling through to terminal accounting instead of being scheduled — when the run's **scheduled arrival** (`time.Since(run.requested_at)` plus the 5s/10s delay this attempt is about to schedule) already exceeds the 2h `staleRunThreshold` the claim path uses to cancel stale runs: the run cannot be claimed before that delay elapses, so testing only its current age would let it age into the cancellation window during its own backoff, silently losing the failure instead of counting it. A narrower tick-width residual remains — the claim path only re-evaluates a run at the scheduler's next poll after `scheduled_retry_at`, not exactly at it — and is deliberately not closed. The retry is scheduled only when the lifecycle event carries the exact run identity plus current-invocation evidence with no observed output or effect; unknown, stale, output-producing, effectful, or diagnostically mismatched failures fall through to terminal accounting. See [runtime-01.md § Legacy post-start transient retry](../system-design/runtime-01.md#legacy-post-start-transient-retry).
|
||||
|
||||
## System design
|
||||
|
||||
|
||||
@@ -65,10 +65,20 @@ A follow-up spec can introduce per-adapter classification once we have real usag
|
||||
|
||||
### Retry policy
|
||||
|
||||
- No automatic retry. Every adapter error is terminal for the wakeup that produced it.
|
||||
- The wakeup row is stamped with `status = failed` and `error_message`. No follow-up wakeup is queued from the failure path.
|
||||
- Re-runs only happen via explicit user action: **Resume session** in chat, **Mark fixed** on an inbox entry, or task reassignment to a different agent.
|
||||
- A terminal failure still stamps `office_agent_runtime.last_run_finished_at` — `HandleAgentFailure` calls the same `Service.stampRunFinished` helper the completed/stopped paths use. Cooldown is a run-pacing mechanism, not a success tracker: a below-threshold failing agent is paced by `cooldown_sec` on its next heartbeat-driven fire exactly as it would be after a success, so a flapping provider or a misconfigured agent cannot retry back-to-back with zero pacing and burn cost/tokens before hitting auto-pause. This applies uniformly across every task assigned to the agent, not just the one that just failed - `cooldown_sec` gates the agent's next heartbeat-triggered fire, regardless of which task last failed. It does not gate task assignment or manual resume: those go through the runs scheduler's `ClaimNextRun`/`ProcessRunGuard` path, which does not consult `last_run_finished_at`. The consecutive-failure counter and auto-pause threshold (below) are the separate, existing mechanism for stopping a chronically broken agent altogether; cooldown only paces the failures that happen before that threshold is reached.
|
||||
- No automatic retry, except the bounded legacy-path transient retry below. Every other adapter error is terminal for the wakeup that produced it.
|
||||
- The wakeup row is stamped with `status = failed` and `error_message`. No follow-up wakeup is queued from the failure path, other than the classified-transient retry's own `scheduled_retry_at`.
|
||||
- Re-runs only happen via explicit user action — **Resume session** in chat, **Mark fixed** on an inbox entry, or task reassignment to a different agent — except the automatic bounded retry below.
|
||||
- A terminal failure still stamps `office_agent_runtime.last_run_finished_at` — `HandleAgentFailure` calls the same `Service.stampRunFinished` helper the completed/stopped paths use. Cooldown is a run-pacing mechanism, not a success tracker: a below-threshold failing agent is paced by `cooldown_sec` on its next heartbeat-driven fire exactly as it would be after a success, so a flapping provider or a misconfigured agent cannot retry back-to-back with zero pacing and burn cost/tokens before hitting auto-pause. This applies uniformly across every task assigned to the agent, not just the one that just failed - `cooldown_sec` gates the agent's next heartbeat-triggered fire, regardless of which task last failed. It does not gate task assignment or manual resume: those go through the runs scheduler's `ClaimNextRun`/`ProcessRunGuard` path, which does not consult `last_run_finished_at`. The consecutive-failure counter and auto-pause threshold (below) are the separate, existing mechanism for stopping a chronically broken agent altogether; cooldown only paces the failures that happen before that threshold is reached. The legacy-path transient retry below deliberately skips this stamp while it is retrying: it is not a success-tracking event, and pacing is already bounded by its own fixed 5s/10s schedule rather than the heartbeat cadence `cooldown_sec` governs.
|
||||
|
||||
### Legacy post-start transient retry
|
||||
|
||||
AC-OFFICE-RUNTIME-001.11 carves a narrow exception out of the "every error is terminal" policy above, closing Beta gap AC-4.2: a post-start failure on the legacy (`HandleAgentFailure`) path — the one `tryPostStartFallback`'s routing-tier retry (see the amendment note above) does not reach, because no routing dispatcher is wired or the run has no `ResolvedProviderID` — is retried instead of counted toward auto-pause, but only when it classifies **transient** by the same shared predicate the routing tier uses (`(*routingerr.Error).ShouldShortRetry`: `ClassTransient && AutoRetryable && FallbackAllowed`), reused rather than reimplemented.
|
||||
|
||||
Mechanism: `HandleAgentFailure` classifies the failure (preferring the structured `ProviderError.Message` over the bare error string, since a bare adapter string like `"Overloaded"` or a raw 429 rarely classifies on its own) **before** calling `MarkRunFailed` at all, not after. Classification passes the underlying agent id (for example `"claude-acp"`) as the provider id, substituting `ProviderError.ProviderID` only when the agent id itself has no provider rules and the provider id does — the same resolution `classifyKanbanFailure` uses, mirrored rather than reimplemented, because provider rules are keyed by agent id and a raw stderr rate-limit string only matches its provider's rule set when the right id is threaded through. `ProviderError.ResetAt` is not used on this path: unlike the routing tier's short-retry delay, the fixed 5s/10s backoff below does not shorten for an imminent reset window. On a classified-transient hit, bounded by `run.RetryCount < 2` and the same 24h `retryMaxAge` staleness rule the pre-launch tier uses, it releases the task checkout, clears the agent's working flag, and calls `ScheduleRetryIfClaimed` at a fixed 5s then 10s delay — a requeue write guarded to `status = 'claimed'`, mirroring `MarkRunFailed`'s own guard, so it only succeeds while the run is still the row this handler read. On success it returns early so `MarkRunFailed`, `recordTerminalShape`, `stampRunFinished`, `IncrementAgentConsecutiveFailures`, `publishRunFailed`, and `autoPauseAgent` never run for that attempt — the run goes `claimed -> queued` directly and never visits `status = 'failed'`. If the guarded requeue writes zero rows, a concurrent writer (a task-tree cancel, workspace pause, or participant eviction — each guarded to `status IN ('queued','claimed')`) already moved the run off `claimed` first; the handler falls through to `MarkRunFailed`, whose identical guard no-ops the same way, leaving the concurrent writer's terminal state untouched (Review round 6, R6-1: classifying before any write closes the `claimed -> failed -> queued` window a cancel's guard could not see into). `RetryCount` is shared with the pre-launch tier rather than a new column, so a run that already burned pre-launch retries gets no post-start retry — under-retrying rather than ever exceeding the combined budget. `ScheduleRetryIfClaimed` also clears `session_id` and `error_message`: the row may already carry the session id and error text of the failed attempt, and a relaunch must resolve the session the new attempt actually gets and not still be flagged with an error once it finishes successfully.
|
||||
|
||||
The retry additionally checks the run's **scheduled arrival**, not just its current age: `time.Since(run.RequestedAt) + delay > staleRunThreshold` (2h, `scheduler_staleness.go`), where `delay` is the same 5s/10s backoff this attempt is about to schedule — and refuses to schedule when that would already be true, falling through to terminal accounting instead. This is a second, tighter gate than the 24h `retryMaxAge` above: `evaluateRunStaleness` cancels any claimed run with `RetryCount > 0` once it is older than `staleRunThreshold`, and scheduling this retry always leaves `RetryCount > 0`. The run cannot be claimed before `ScheduleRetry`'s `retry_at` elapses, so testing the age the run has *now* rather than the age it will have on arrival lets a run age into the cancellation window during its own backoff, silently converting a scheduled retry into an `execution_too_old` cancellation instead of counting the failure (no `consecutive_failures` increment, no `agent_run_failed` inbox row). An Office agent session running past 2h is routine, not an edge case, so this band is reachable in practice. A narrower residual remains even with the delay folded in: the run is claimed at the first scheduler tick *after* `scheduled_retry_at`, not exactly at it, so a tick-width window (bounded by the scheduler's poll interval) can still let `evaluateRunStaleness` cancel a retry whose scheduled arrival landed just inside the threshold. This residual is deliberately not closed — it is orders of magnitude narrower than the backoff-width gap it replaces and would require coupling this gate to the scheduler's poll cadence.
|
||||
|
||||
This retry re-drives the whole run from the top only when the lifecycle event carries the exact run identity and current-invocation evidence: a non-empty execution ID, a non-zero prompt generation, known evidence, and no observed output or effect. A missing or mismatched run or session identity, unknown evidence, output, effect, or a provider diagnostic that does not match the terminal failure falls through to terminal accounting. This keeps the Office path aligned with the shared provider-recovery contract while preserving the existing 5s and 10s retry policy.
|
||||
|
||||
### Inbox surfacing
|
||||
|
||||
@@ -91,11 +101,8 @@ For `agent_run_failed`:
|
||||
3. Re-queue a wakeup for that (task, agent) with reason `manual_resume_after_failure`.
|
||||
|
||||
For `agent_paused_after_failures`:
|
||||
1. Compare-and-set clear of the agent's `pause_reason`, reset `consecutive_failures` to zero. (Counter reset is the user saying "I fixed the cause; start over".) The service reads the expected reason at the start of the request. If a second auto-pause replaces it before the conditional write, the clear is refused and the call aborts rather than adopting the newer reason (`AC-OFFICE-RUNTIME-001.9`).
|
||||
2. Insert into `inbox_dismissals`, only after step 1 succeeds. An aborted clear leaves the entry actionable for the still-paused agent (`AC-OFFICE-RUNTIME-001.10`).
|
||||
3. Re-queue wakeups for every (task, agent) listed on the entry with reason `manual_resume_after_failure`.
|
||||
|
||||
See [part 2](runtime-02.md#migrated-source-detail) for the compare-and-set scenario detail.
|
||||
See [part 2](runtime-02.md#migrated-source-detail) for the compare-and-set clear,
|
||||
dismissal ordering, and retry scenario detail.
|
||||
|
||||
There is no separate "Dismiss without retry" affordance. If the user wants to silence the inbox without re-running the task, they reassign the task to a different agent - that's the explicit recovery path described below.
|
||||
|
||||
@@ -274,10 +281,9 @@ Recovery and runtime-action authorization splits across three actors: human user
|
||||
|
||||
- **Adapter error event published**: session -> `FAILED`, wakeup -> `failed`, agent process and agentctl torn down, `executors_running` preserved, `consecutive_failures++`, inbox entry created or upgraded to `agent_paused_after_failures` on threshold.
|
||||
- **Provider returns malformed response**: treated as terminal like any other adapter error. Raw payload preserved in the chat error entry and wakeup `error_message`.
|
||||
- **Transient network failure**: treated as terminal in v1. User clicks Resume / Mark fixed to retry. Future classifier may auto-retry.
|
||||
- **Transient network failure**: on the legacy post-start path, classified-transient failures auto-retry up to 2 times (5s/10s) per [Legacy post-start transient retry](#legacy-post-start-transient-retry) before falling back to terminal; any other transient failure (pre-launch, or already exhausted its retry budget) is terminal and the user clicks Resume / Mark fixed to retry.
|
||||
- **Auto-paused agent's wakeup fires**: scheduler refuses to claim it; behaviour identical to a manually-paused agent.
|
||||
- **Reassignment during failure**: cancels the (task, old agent) wakeup via the existing staleness check; auto-dismisses `agent_run_failed` for (task, old agent). Does not unpause the old agent.
|
||||
- **Second auto-pause lands during Mark fixed**: the compare-and-set clear on `pause_reason` is refused; the call aborts instead of retrying against the newer reason. `consecutive_failures` is unchanged, no wakeup is queued, and the inbox entry is not dismissed - the newer pause remains actionable.
|
||||
|
||||
## Persistence guarantees
|
||||
|
||||
@@ -293,6 +299,6 @@ Recovery and runtime-action authorization splits across three actors: human user
|
||||
- **Agent JWT signing key**: process-local. Defaults to a random 32-byte key when the `KANDEV_AGENT_JWT_KEY` config is empty, in which case all in-flight JWTs are invalidated on restart and agents must mint fresh tokens on the next prompt cycle. Configurable for stable cross-restart JWTs.
|
||||
- **`affected_tasks` snapshot on `agent_paused_after_failures`**: NOT recomputed when listed tasks are reassigned away. The snapshot is intentionally point-in-time so the user resolves the pause entry as a whole.
|
||||
- **`office_agent_runtime.last_run_finished_at`**: durable. Stamped on every terminal run outcome for the agent, including failure - not just completion/stop - so the heartbeat cooldown gate paces the agent's next fire regardless of why the previous run ended. See [`office/agents.md`](../requirements/agents.md#persistence-guarantees) persistence guarantees for the gate itself.
|
||||
- **`scheduled_retry` runs**: not produced in v1. Every adapter error is terminal; the wakeup is stamped `failed` with no follow-up scheduled. The `scheduled_retry` status is reserved for a future classifier.
|
||||
- **`scheduled_retry` runs**: not produced by the general v1 classification policy — every other adapter error is terminal, stamped `failed` with no follow-up scheduled. The one exception is [Legacy post-start transient retry](#legacy-post-start-transient-retry): the run is requeued (`status = queued`, `scheduled_retry_at` set, `session_id` and `error_message` cleared) via a `status = 'claimed'`-guarded write, rather than marked `failed` at all, so a successful retry never reaches this status literally named `scheduled_retry`. A fuller per-adapter classifier producing that status remains future work.
|
||||
|
||||
See also: [Dynamic Agent Routing](../../agents/requirements/dynamic-agent-routing.md) for provider routing and [`office/scheduler.md`](../requirements/scheduler.md) for wakeup queue semantics and the staleness check that cancels superseded wakeups.
|
||||
|
||||
@@ -141,4 +141,3 @@ One line per case; the cited AC is authoritative for the expected value.
|
||||
that no fixture reproduces, so there is nothing to assert against. This case
|
||||
exists so the requirement is discharged explicitly rather than by silence
|
||||
(002.6).
|
||||
|
||||
|
||||
Reference in New Issue
Block a user