fix: preserve Kubernetes runtimes after recoverable agent failures (#3824)
This commit is contained in:
@@ -644,7 +644,7 @@ jobs:
|
|||||||
./internal/workflowsync
|
./internal/workflowsync
|
||||||
./internal/worktree
|
./internal/worktree
|
||||||
)
|
)
|
||||||
go test -v -race "${POSTGRES_PACKAGES[@]}"
|
go test -v -race -timeout 15m "${POSTGRES_PACKAGES[@]}"
|
||||||
|
|
||||||
postgres-18:
|
postgres-18:
|
||||||
name: Backend Postgres 18
|
name: Backend Postgres 18
|
||||||
|
|||||||
@@ -8,6 +8,13 @@ import (
|
|||||||
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
// Recovery stop reasons preserve runtime-specific teardown semantics through
|
||||||
|
// the public runtime boundary.
|
||||||
|
const (
|
||||||
|
StopReasonRecoverableAgentFailure = lifecycle.StopReasonRecoverableAgentFailure
|
||||||
|
StopReasonAgentBootstrapFailed = lifecycle.StopReasonAgentBootstrapFailed
|
||||||
|
)
|
||||||
|
|
||||||
// New returns a Runtime backed by the supplied Backend (typically a
|
// New returns a Runtime backed by the supplied Backend (typically a
|
||||||
// *lifecycle.Manager).
|
// *lifecycle.Manager).
|
||||||
func New(backend Backend) Runtime {
|
func New(backend Backend) Runtime {
|
||||||
|
|||||||
@@ -1151,7 +1151,7 @@ func (m *Manager) StopAgentWithReason(ctx context.Context, executionID string, r
|
|||||||
}
|
}
|
||||||
backendForce := force
|
backendForce := force
|
||||||
stopCtx := ctx
|
stopCtx := ctx
|
||||||
if shouldPreserveFailedKubernetesResume(execution, reason) {
|
if shouldPreserveKubernetesRuntime(execution, reason) {
|
||||||
backendForce = false
|
backendForce = false
|
||||||
var cancelStop context.CancelFunc
|
var cancelStop context.CancelFunc
|
||||||
stopCtx, cancelStop = kubernetesDurableContext(ctx)
|
stopCtx, cancelStop = kubernetesDurableContext(ctx)
|
||||||
@@ -1247,11 +1247,6 @@ func (m *Manager) StopAgentWithReason(ctx context.Context, executionID string, r
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func shouldPreserveFailedKubernetesResume(execution *AgentExecution, reason string) bool {
|
|
||||||
return execution != nil && execution.RuntimeName == executor.NameKubernetes &&
|
|
||||||
execution.isResumedSession && reason == StopReasonAgentBootstrapFailed
|
|
||||||
}
|
|
||||||
|
|
||||||
// detachAgentExecution implements the AC-EXECUTORS-SURVIVAL survivable-detach
|
// detachAgentExecution implements the AC-EXECUTORS-SURVIVAL survivable-detach
|
||||||
// branch of StopAgentWithReason (design 01/02 kill-path #4): when the
|
// branch of StopAgentWithReason (design 01/02 kill-path #4): when the
|
||||||
// agent-survival capability is enabled and the stop reason is graceful
|
// agent-survival capability is enabled and the stop reason is graceful
|
||||||
|
|||||||
+159
@@ -0,0 +1,159 @@
|
|||||||
|
package lifecycle
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
|
||||||
|
"github.com/kandev/kandev/internal/agent/executor"
|
||||||
|
agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl"
|
||||||
|
"github.com/kandev/kandev/internal/agentruntime"
|
||||||
|
"github.com/kandev/kandev/internal/secrets"
|
||||||
|
"github.com/kandev/kandev/internal/task/models"
|
||||||
|
)
|
||||||
|
|
||||||
|
// @covers AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.1
|
||||||
|
func TestKubernetesRecoverableFailurePreservesRuntimeAndSecrets(t *testing.T) {
|
||||||
|
log := newTestRegistryLogger()
|
||||||
|
stopRequests := make(chan string, 1)
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
stopRequests <- r.Method + " " + r.URL.Path
|
||||||
|
w.Header().Set("Content-Type", "application/json")
|
||||||
|
_, _ = w.Write([]byte(`{"success":true}`))
|
||||||
|
}))
|
||||||
|
t.Cleanup(server.Close)
|
||||||
|
host, port := parseHTTPURL(t, server.URL)
|
||||||
|
|
||||||
|
executorRegistry := NewExecutorRegistry(log)
|
||||||
|
backend := &stopTrackingExecutor{MockExecutor: MockExecutor{name: executor.NameKubernetes}}
|
||||||
|
executorRegistry.Register(backend)
|
||||||
|
mgr := NewManager(newTestRegistry(), &MockEventBus{}, executorRegistry, nil, nil, nil, ExecutorFallbackWarn, "", log)
|
||||||
|
cleanupManagerStopCh(t, mgr)
|
||||||
|
|
||||||
|
secretStore := newInMemorySecretStore()
|
||||||
|
for _, id := range []string{"runtime-auth", "runtime-bootstrap"} {
|
||||||
|
require.NoError(t, secretStore.Create(context.Background(), &secrets.SecretWithValue{
|
||||||
|
Secret: secrets.Secret{ID: id, Name: id}, Value: "secret",
|
||||||
|
}))
|
||||||
|
}
|
||||||
|
mgr.SetSecretStore(secretStore)
|
||||||
|
require.NoError(t, mgr.executionStore.Add(&AgentExecution{
|
||||||
|
ID: "execution-1", TaskID: "task-1", SessionID: "session-1",
|
||||||
|
RuntimeName: executor.NameKubernetes, isResumedSession: false,
|
||||||
|
agentctl: agentctl.NewClient(host, port, log),
|
||||||
|
metadata: map[string]interface{}{
|
||||||
|
MetadataKeyAuthTokenSecret: "runtime-auth",
|
||||||
|
MetadataKeyBootstrapNonceSecret: "runtime-bootstrap",
|
||||||
|
},
|
||||||
|
}))
|
||||||
|
|
||||||
|
ctx, cancel := context.WithCancel(context.Background())
|
||||||
|
cancel()
|
||||||
|
err := mgr.StopAgentWithReason(ctx, "execution-1", StopReasonRecoverableAgentFailure, true)
|
||||||
|
|
||||||
|
require.NoError(t, err)
|
||||||
|
select {
|
||||||
|
case request := <-stopRequests:
|
||||||
|
require.Equal(t, "POST /api/v1/stop", request)
|
||||||
|
case <-time.After(time.Second):
|
||||||
|
t.Fatal("the failed process was not stopped before its Kubernetes runtime was retained")
|
||||||
|
}
|
||||||
|
require.Equal(t, []bool{false}, backend.forces,
|
||||||
|
"failed resume cleanup must only close process-local Kubernetes connections")
|
||||||
|
require.Len(t, secretStore.store, 2, "retained runtime secrets must remain available for retry")
|
||||||
|
_, exists := mgr.executionStore.Get("execution-1")
|
||||||
|
require.False(t, exists, "failed execution must release its in-memory session slot")
|
||||||
|
}
|
||||||
|
|
||||||
|
// @covers AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.1
|
||||||
|
// @covers AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.4
|
||||||
|
func TestKubernetesRecoverableFailureStopPolicy(t *testing.T) {
|
||||||
|
for _, tc := range []struct {
|
||||||
|
name string
|
||||||
|
runtime executor.Name
|
||||||
|
resumed bool
|
||||||
|
reason string
|
||||||
|
preserve bool
|
||||||
|
}{
|
||||||
|
{"fresh established session", executor.NameKubernetes, false, StopReasonRecoverableAgentFailure, true},
|
||||||
|
{"resumed established session", executor.NameKubernetes, true, StopReasonRecoverableAgentFailure, true},
|
||||||
|
{"archive remains destructive", executor.NameKubernetes, true, StopReasonTaskArchived, false},
|
||||||
|
{"delete remains destructive", executor.NameKubernetes, true, StopReasonTaskDeleted, false},
|
||||||
|
{"explicit force remains destructive", executor.NameKubernetes, true, "", false},
|
||||||
|
{"fresh bootstrap rollback", executor.NameKubernetes, false, StopReasonAgentBootstrapFailed, false},
|
||||||
|
{"standalone unchanged", executor.NameStandalone, false, StopReasonRecoverableAgentFailure, false},
|
||||||
|
} {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
log := newTestRegistryLogger()
|
||||||
|
registry := NewExecutorRegistry(log)
|
||||||
|
backend := &stopTrackingExecutor{MockExecutor: MockExecutor{name: tc.runtime}}
|
||||||
|
registry.Register(backend)
|
||||||
|
mgr := NewManager(newTestRegistry(), &MockEventBus{}, registry, nil, nil, nil, ExecutorFallbackWarn, "", log)
|
||||||
|
cleanupManagerStopCh(t, mgr)
|
||||||
|
store := newInMemorySecretStore()
|
||||||
|
mgr.SetSecretStore(store)
|
||||||
|
metadata := map[string]interface{}{
|
||||||
|
MetadataKeyAuthTokenSecret: "retained-auth", MetadataKeyBootstrapNonceSecret: "retained-bootstrap",
|
||||||
|
}
|
||||||
|
for _, id := range []string{"retained-auth", "retained-bootstrap"} {
|
||||||
|
require.NoError(t, store.Create(ctx, &secrets.SecretWithValue{Secret: secrets.Secret{ID: id, Name: id}, Value: id + "-value"}))
|
||||||
|
}
|
||||||
|
execution := &AgentExecution{ID: "execution-1", TaskID: "task-1", SessionID: "session-1",
|
||||||
|
RuntimeName: tc.runtime, isResumedSession: tc.resumed, metadata: metadata}
|
||||||
|
require.NoError(t, mgr.executionStore.Add(execution))
|
||||||
|
require.NoError(t, mgr.StopAgentWithReason(ctx, execution.ID, tc.reason, true))
|
||||||
|
require.Equal(t, []bool{!tc.preserve}, backend.forces)
|
||||||
|
for _, id := range []string{"retained-auth", "retained-bootstrap"} {
|
||||||
|
value, err := store.Reveal(ctx, id)
|
||||||
|
if tc.preserve || tc.runtime != executor.NameKubernetes {
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, id+"-value", value)
|
||||||
|
} else {
|
||||||
|
require.ErrorIs(t, err, secrets.ErrNotFound)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
require.Equal(t, metadata, execution.MetadataSnapshot())
|
||||||
|
_, exists := mgr.executionStore.Get(execution.ID)
|
||||||
|
require.False(t, exists)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// @covers AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.3
|
||||||
|
func TestKubernetesRecoverableFailureCleanupCannotStopReplacement(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
log := newTestRegistryLogger()
|
||||||
|
registry := NewExecutorRegistry(log)
|
||||||
|
backend := &stopTrackingExecutor{MockExecutor: MockExecutor{name: executor.NameKubernetes}}
|
||||||
|
registry.Register(backend)
|
||||||
|
mgr := NewManager(newTestRegistry(), &MockEventBus{}, registry, nil, nil, nil, ExecutorFallbackWarn, "", log)
|
||||||
|
cleanupManagerStopCh(t, mgr)
|
||||||
|
store := newInMemorySecretStore()
|
||||||
|
mgr.SetSecretStore(store)
|
||||||
|
require.NoError(t, store.Create(ctx, &secrets.SecretWithValue{
|
||||||
|
Secret: secrets.Secret{ID: "shared-runtime-auth", Name: "auth"}, Value: "retained-token",
|
||||||
|
}))
|
||||||
|
old := &AgentExecution{ID: "old-execution", SessionID: "session", RuntimeName: executor.NameKubernetes,
|
||||||
|
metadata: map[string]interface{}{MetadataKeyAuthTokenSecret: "shared-runtime-auth"}}
|
||||||
|
require.NoError(t, mgr.executionStore.Add(old))
|
||||||
|
require.NoError(t, mgr.StopAgentWithReason(ctx, old.ID, StopReasonRecoverableAgentFailure, true))
|
||||||
|
mgr.SetExecutorRunningWriter(&restartKubernetesInventoryStore{rows: []*models.ExecutorRunning{{
|
||||||
|
AgentExecutionID: old.ID, SessionID: old.SessionID, Runtime: agentruntime.RuntimeKubernetes,
|
||||||
|
}}})
|
||||||
|
replacement := &AgentExecution{ID: "replacement", SessionID: "session", RuntimeName: executor.NameKubernetes,
|
||||||
|
metadata: old.MetadataSnapshot(), isResumedSession: true}
|
||||||
|
require.NoError(t, mgr.executionStore.Add(replacement))
|
||||||
|
require.ErrorIs(t, mgr.StopAgentWithReason(ctx, old.ID, StopReasonRecoverableAgentFailure, true), ErrExecutionNotFound)
|
||||||
|
current, exists := mgr.executionStore.GetBySessionID("session")
|
||||||
|
require.True(t, exists)
|
||||||
|
require.Same(t, replacement, current)
|
||||||
|
require.Equal(t, 1, backend.stopCalls)
|
||||||
|
token, err := mgr.resolveLaunchAuthToken(ctx, &LaunchRequest{ExecutorType: "k8s"}, replacement.MetadataSnapshot())
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, "retained-token", token)
|
||||||
|
}
|
||||||
@@ -32,6 +32,11 @@ func (m *Manager) stopPersistedKubernetesExecution(
|
|||||||
reason string,
|
reason string,
|
||||||
force bool,
|
force bool,
|
||||||
) (bool, error) {
|
) (bool, error) {
|
||||||
|
// Recoverable teardown only releases a tracked execution. Its retained
|
||||||
|
// inventory must not turn a duplicate stop into destructive remote cleanup.
|
||||||
|
if reason == StopReasonRecoverableAgentFailure {
|
||||||
|
return false, nil
|
||||||
|
}
|
||||||
if !force && !shouldRunExecutorCleanup(reason) {
|
if !force && !shouldRunExecutorCleanup(reason) {
|
||||||
return false, nil
|
return false, nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,17 @@
|
|||||||
|
package lifecycle
|
||||||
|
|
||||||
|
import "github.com/kandev/kandev/internal/agent/executor"
|
||||||
|
|
||||||
|
// StopReasonRecoverableAgentFailure stops a failed agent while retaining the
|
||||||
|
// Kubernetes runtime and credentials needed to resume its session.
|
||||||
|
const StopReasonRecoverableAgentFailure = "recoverable agent failure"
|
||||||
|
|
||||||
|
// shouldPreserveKubernetesRuntime retains established Kubernetes resources for
|
||||||
|
// recovery while keeping fresh bootstrap rollback and explicit cleanup destructive.
|
||||||
|
func shouldPreserveKubernetesRuntime(execution *AgentExecution, reason string) bool {
|
||||||
|
if execution == nil || execution.RuntimeName != executor.NameKubernetes {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
return reason == StopReasonRecoverableAgentFailure ||
|
||||||
|
(execution.isResumedSession && reason == StopReasonAgentBootstrapFailed)
|
||||||
|
}
|
||||||
@@ -974,7 +974,9 @@ func (s *Service) runDetachedDynamicSuccessorLaunch(
|
|||||||
errMsg = "dynamic successor launch failed"
|
errMsg = "dynamic successor launch failed"
|
||||||
}
|
}
|
||||||
s.finalizeAutomationRun(failureCtx, data.TaskID, false, errMsg)
|
s.finalizeAutomationRun(failureCtx, data.TaskID, false, errMsg)
|
||||||
s.handleRecoverableFailureLocked(failureCtx, data)
|
if dispatch := s.handleRecoverableFailureLockedState(failureCtx, data, agentruntime.StopReasonRecoverableAgentFailure); dispatch != nil {
|
||||||
|
s.startAgentFailureRecovery(dispatch)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Service) resetDynamicSuccessorWorkers() {
|
func (s *Service) resetDynamicSuccessorWorkers() {
|
||||||
|
|||||||
@@ -2403,7 +2403,7 @@ func (s *Service) finishAgentCompletedTurn(
|
|||||||
func (s *Service) handleAgentFailed(ctx context.Context, data watcher.AgentEventData) {
|
func (s *Service) handleAgentFailed(ctx context.Context, data watcher.AgentEventData) {
|
||||||
if data.SessionID == "" {
|
if data.SessionID == "" {
|
||||||
if dispatch := s.handleAgentFailedLocked(ctx, data); dispatch != nil {
|
if dispatch := s.handleAgentFailedLocked(ctx, data); dispatch != nil {
|
||||||
dispatch()
|
s.startAgentFailureRecovery(dispatch)
|
||||||
}
|
}
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -2442,11 +2442,13 @@ func (s *Service) handleAgentFailed(ctx context.Context, data watcher.AgentEvent
|
|||||||
lock.Unlock()
|
lock.Unlock()
|
||||||
release()
|
release()
|
||||||
if dispatch != nil {
|
if dispatch != nil {
|
||||||
dispatch()
|
s.startAgentFailureRecovery(dispatch)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Service) handleAgentFailedLocked(ctx context.Context, data watcher.AgentEventData) func() {
|
// handleAgentFailedLocked reconciles a failure under the session guard and returns
|
||||||
|
// recovery work that must run after that guard is released.
|
||||||
|
func (s *Service) handleAgentFailedLocked(ctx context.Context, data watcher.AgentEventData) func(context.Context) {
|
||||||
data = s.withPromptAttemptEvidence(data)
|
data = s.withPromptAttemptEvidence(data)
|
||||||
defer s.clearPromptAttemptEvidence(data.SessionID, data.AgentExecutionID, data.PromptGeneration)
|
defer s.clearPromptAttemptEvidence(data.SessionID, data.AgentExecutionID, data.PromptGeneration)
|
||||||
s.logger.Warn("handling agent failed",
|
s.logger.Warn("handling agent failed",
|
||||||
@@ -2499,7 +2501,7 @@ func (s *Service) handleAgentFailedLocked(ctx context.Context, data watcher.Agen
|
|||||||
|
|
||||||
// Make all agent CLI failures recoverable — let the user choose to resume or start fresh.
|
// Make all agent CLI failures recoverable — let the user choose to resume or start fresh.
|
||||||
if data.SessionID != "" {
|
if data.SessionID != "" {
|
||||||
return s.handleRecoverableFailureLockedState(ctx, data)
|
return s.handleRecoverableFailureLockedState(ctx, data, lifecycle.StopReasonRecoverableAgentFailure)
|
||||||
}
|
}
|
||||||
|
|
||||||
// No session — fall back to scheduler retry + task to REVIEW unless another
|
// No session — fall back to scheduler retry + task to REVIEW unless another
|
||||||
@@ -2568,8 +2570,9 @@ const (
|
|||||||
)
|
)
|
||||||
|
|
||||||
type executionTeardownClaim struct {
|
type executionTeardownClaim struct {
|
||||||
intent executionTeardownIntent
|
intent executionTeardownIntent
|
||||||
expiresAt time.Time
|
expiresAt time.Time
|
||||||
|
cleanupCompleted bool
|
||||||
}
|
}
|
||||||
|
|
||||||
// claimExecutionTeardown accepts the first teardown intent for one concrete
|
// claimExecutionTeardown accepts the first teardown intent for one concrete
|
||||||
@@ -2790,8 +2793,8 @@ func (s *Service) clearResumeToken(ctx context.Context, sessionID string) error
|
|||||||
// resume the agent session or start fresh.
|
// resume the agent session or start fresh.
|
||||||
func (s *Service) handleRecoverableFailure(ctx context.Context, data watcher.AgentEventData) {
|
func (s *Service) handleRecoverableFailure(ctx context.Context, data watcher.AgentEventData) {
|
||||||
if data.SessionID == "" {
|
if data.SessionID == "" {
|
||||||
if dispatch := s.handleRecoverableFailureLockedState(ctx, data); dispatch != nil {
|
if dispatch := s.handleRecoverableFailureLockedState(ctx, data, lifecycle.StopReasonRecoverableAgentFailure); dispatch != nil {
|
||||||
dispatch()
|
s.startAgentFailureRecovery(dispatch)
|
||||||
}
|
}
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@@ -2813,21 +2816,11 @@ func (s *Service) handleRecoverableFailure(ctx context.Context, data watcher.Age
|
|||||||
zap.String("session_id", data.SessionID),
|
zap.String("session_id", data.SessionID),
|
||||||
zap.Error(err))
|
zap.Error(err))
|
||||||
}
|
}
|
||||||
dispatch := s.handleRecoverableFailureLockedState(ctx, data)
|
dispatch := s.handleRecoverableFailureLockedState(ctx, data, lifecycle.StopReasonRecoverableAgentFailure)
|
||||||
lock.Unlock()
|
lock.Unlock()
|
||||||
release()
|
release()
|
||||||
if dispatch != nil {
|
if dispatch != nil {
|
||||||
dispatch()
|
s.startAgentFailureRecovery(dispatch)
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// handleRecoverableFailureLocked performs recovery side effects and dispatches
|
|
||||||
// the workflow trigger synchronously. Production callers that already hold the
|
|
||||||
// session guard must use handleRecoverableFailureLockedState and invoke the
|
|
||||||
// returned dispatch after releasing that guard.
|
|
||||||
func (s *Service) handleRecoverableFailureLocked(ctx context.Context, data watcher.AgentEventData) {
|
|
||||||
if dispatch := s.handleRecoverableFailureLockedState(ctx, data); dispatch != nil {
|
|
||||||
dispatch()
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -2835,7 +2828,7 @@ func (s *Service) handleRecoverableFailureLocked(ctx context.Context, data watch
|
|||||||
// session's cancelInFlight guard is held and returns the workflow dispatch to
|
// session's cancelInFlight guard is held and returns the workflow dispatch to
|
||||||
// run after the guard is released. Deletion uses the same guard, so an active
|
// run after the guard is released. Deletion uses the same guard, so an active
|
||||||
// error event cannot publish after the deleted-session inactive event.
|
// error event cannot publish after the deleted-session inactive event.
|
||||||
func (s *Service) handleRecoverableFailureLockedState(ctx context.Context, data watcher.AgentEventData) func() {
|
func (s *Service) handleRecoverableFailureLockedState(ctx context.Context, data watcher.AgentEventData, stopReason string) func(context.Context) {
|
||||||
completionFollowUp := false
|
completionFollowUp := false
|
||||||
if session, err := s.repo.GetTaskSession(ctx, data.SessionID); err == nil && session != nil {
|
if session, err := s.repo.GetTaskSession(ctx, data.SessionID); err == nil && session != nil {
|
||||||
completionFollowUp = models.IsCompletionFollowUpSession(session.Metadata)
|
completionFollowUp = models.IsCompletionFollowUpSession(session.Metadata)
|
||||||
@@ -2910,19 +2903,17 @@ func (s *Service) handleRecoverableFailureLockedState(ctx context.Context, data
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Clean up the agent execution.
|
// Callers run teardown and workflow dispatch asynchronously after releasing
|
||||||
go s.cleanupAgentExecution(data.AgentExecutionID, data.TaskID, data.SessionID)
|
// cancelInFlight: lifecycle publishers may still hold their prompt lock.
|
||||||
|
// A workflow restart must observe the failed execution's released slot.
|
||||||
// The caller runs this after releasing cancelInFlight. A direct
|
return func(workerCtx context.Context) {
|
||||||
// auto_start_agent callback can start the same session and reacquire that
|
if !s.cleanupAgentExecutionWithReason(workerCtx, data.AgentExecutionID, data.TaskID, data.SessionID,
|
||||||
// guard, so dispatching while it is held would deadlock. Panic recovery is
|
stopReason) {
|
||||||
// the recovered wrapper's job, not this one's — routes R2-R5 reach this
|
return
|
||||||
// closure with no recover above them on the stack.
|
}
|
||||||
if completionFollowUp {
|
if !completionFollowUp && workerCtx.Err() == nil {
|
||||||
return func() {}
|
s.dispatchKanbanAgentErrorTriggerRecovered(workerCtx, data)
|
||||||
}
|
}
|
||||||
return func() {
|
|
||||||
s.dispatchKanbanAgentErrorTriggerRecovered(context.WithoutCancel(ctx), data)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -3465,14 +3456,14 @@ func (s *Service) handleAgentStartFailed(ctx context.Context, taskID, sessionID,
|
|||||||
}
|
}
|
||||||
var unlockGuard func()
|
var unlockGuard func()
|
||||||
var releaseGuard func()
|
var releaseGuard func()
|
||||||
var dispatch func()
|
var dispatch func(context.Context)
|
||||||
defer func() {
|
defer func() {
|
||||||
if unlockGuard != nil {
|
if unlockGuard != nil {
|
||||||
unlockGuard()
|
unlockGuard()
|
||||||
releaseGuard()
|
releaseGuard()
|
||||||
}
|
}
|
||||||
if dispatch != nil {
|
if dispatch != nil {
|
||||||
dispatch()
|
s.startAgentFailureRecovery(dispatch)
|
||||||
}
|
}
|
||||||
}()
|
}()
|
||||||
if sessionID != "" {
|
if sessionID != "" {
|
||||||
@@ -3521,7 +3512,7 @@ func (s *Service) handleAgentStartFailed(ctx context.Context, taskID, sessionID,
|
|||||||
zap.String("task_id", taskID),
|
zap.String("task_id", taskID),
|
||||||
zap.String("session_id", sessionID),
|
zap.String("session_id", sessionID),
|
||||||
zap.String("agent_execution_id", agentExecutionID))
|
zap.String("agent_execution_id", agentExecutionID))
|
||||||
dispatch = s.handleRecoverableFailureLockedState(ctx, failureData)
|
dispatch = s.handleRecoverableFailureLockedState(ctx, failureData, lifecycle.StopReasonAgentBootstrapFailed)
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -3542,7 +3533,7 @@ func (s *Service) handleAgentStartFailed(ctx context.Context, taskID, sessionID,
|
|||||||
s.logger.Info("agent start failure is auth error, treating as recoverable",
|
s.logger.Info("agent start failure is auth error, treating as recoverable",
|
||||||
zap.String("task_id", taskID),
|
zap.String("task_id", taskID),
|
||||||
zap.String("session_id", sessionID))
|
zap.String("session_id", sessionID))
|
||||||
dispatch = s.handleRecoverableFailureLockedState(ctx, failureData)
|
dispatch = s.handleRecoverableFailureLockedState(ctx, failureData, lifecycle.StopReasonAgentBootstrapFailed)
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -3771,30 +3762,5 @@ func (s *Service) handleAgentStoppedLocked(ctx context.Context, data watcher.Age
|
|||||||
// the agent reaches a terminal state (completed/failed). This runs in a goroutine
|
// the agent reaches a terminal state (completed/failed). This runs in a goroutine
|
||||||
// so it doesn't block the event handler.
|
// so it doesn't block the event handler.
|
||||||
func (s *Service) cleanupAgentExecution(executionID, taskID, sessionID string) {
|
func (s *Service) cleanupAgentExecution(executionID, taskID, sessionID string) {
|
||||||
if executionID == "" {
|
s.cleanupAgentExecutionWithReason(context.Background(), executionID, taskID, sessionID, "agent completed")
|
||||||
return
|
|
||||||
}
|
|
||||||
if !s.claimForcedExecutionCleanup(sessionID, executionID) {
|
|
||||||
s.logger.Debug("skipping duplicate execution teardown",
|
|
||||||
zap.String("execution_id", executionID),
|
|
||||||
zap.String("task_id", taskID),
|
|
||||||
zap.String("session_id", sessionID))
|
|
||||||
return
|
|
||||||
}
|
|
||||||
ctx := context.Background()
|
|
||||||
if !s.isExecutionCompleted(sessionID, executionID) {
|
|
||||||
// Direct crash/forced-cleanup callers may reach this boundary without a
|
|
||||||
// preceding lifecycle event. Preserve an existing completed marker (and
|
|
||||||
// its allowed terminal stream), otherwise tombstone trailing frames.
|
|
||||||
s.markExecutionFailed(sessionID, executionID)
|
|
||||||
}
|
|
||||||
// Defensive terminal-boundary retirement. Normal lifecycle events run this
|
|
||||||
// first; the repeated forced-cleanup call is idempotent.
|
|
||||||
s.retireExecutionActivityAndPublish(ctx, taskID, sessionID, executionID)
|
|
||||||
if err := s.executor.StopExecution(ctx, executionID, "agent completed", true); err != nil {
|
|
||||||
s.logger.Debug("agent execution cleanup after terminal state",
|
|
||||||
zap.String("execution_id", executionID),
|
|
||||||
zap.String("task_id", taskID),
|
|
||||||
zap.Error(err))
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -52,6 +52,8 @@ func newAgentErrorTransientTestService(
|
|||||||
// goroutine race — the marker's value is what's contractual, not the
|
// goroutine race — the marker's value is what's contractual, not the
|
||||||
// interleaving that produces it. ---
|
// interleaving that produces it. ---
|
||||||
|
|
||||||
|
// TestDispatchKanbanAgentErrorTrigger_ConcurrentCancelDoesNotLeakMarker verifies
|
||||||
|
// that a user cancellation cannot suppress a separately owned retry failure.
|
||||||
func TestDispatchKanbanAgentErrorTrigger_ConcurrentCancelDoesNotLeakMarker(t *testing.T) {
|
func TestDispatchKanbanAgentErrorTrigger_ConcurrentCancelDoesNotLeakMarker(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
repo := setupTestRepo(t)
|
repo := setupTestRepo(t)
|
||||||
@@ -73,6 +75,7 @@ func TestDispatchKanbanAgentErrorTrigger_ConcurrentCancelDoesNotLeakMarker(t *te
|
|||||||
if !svc.CancelTransientRetry(ctx, "t1", "s1") {
|
if !svc.CancelTransientRetry(ctx, "t1", "s1") {
|
||||||
t.Fatal("CancelTransientRetry = false, want true (a loop was active)")
|
t.Fatal("CancelTransientRetry = false, want true (a loop was active)")
|
||||||
}
|
}
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
if decisions.clearCalls != 0 {
|
if decisions.clearCalls != 0 {
|
||||||
t.Fatalf("cancel's own delivery clearCalls = %d, want 0 (AC-A8 suppression)", decisions.clearCalls)
|
t.Fatalf("cancel's own delivery clearCalls = %d, want 0 (AC-A8 suppression)", decisions.clearCalls)
|
||||||
}
|
}
|
||||||
@@ -80,6 +83,7 @@ func TestDispatchKanbanAgentErrorTrigger_ConcurrentCancelDoesNotLeakMarker(t *te
|
|||||||
// The claimed timer still reaches R4 on its own, unaffected context. Its
|
// The claimed timer still reaches R4 on its own, unaffected context. Its
|
||||||
// event must not carry the cancel's UserInitiated marker.
|
// event must not carry the cancel's UserInitiated marker.
|
||||||
svc.retryTransientPrompt(ctx, "t1", "s1", "exec-1")
|
svc.retryTransientPrompt(ctx, "t1", "s1", "exec-1")
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
|
|
||||||
if decisions.clearCalls != 1 {
|
if decisions.clearCalls != 1 {
|
||||||
t.Fatalf("timer's own delivery clearCalls = %d, want 1 (AC-A8's marker must not leak into R4)", decisions.clearCalls)
|
t.Fatalf("timer's own delivery clearCalls = %d, want 1 (AC-A8's marker must not leak into R4)", decisions.clearCalls)
|
||||||
@@ -100,6 +104,8 @@ func TestDispatchKanbanAgentErrorTrigger_ConcurrentCancelDoesNotLeakMarker(t *te
|
|||||||
// step1 the handler observed on entry. Reads the task fresh, after the
|
// step1 the handler observed on entry. Reads the task fresh, after the
|
||||||
// decision that used the earlier snapshot. ---
|
// decision that used the earlier snapshot. ---
|
||||||
|
|
||||||
|
// TestDispatchKanbanAgentErrorTrigger_ReadsPostReconciliationStep verifies that
|
||||||
|
// recovery dispatch uses the workflow step persisted by failure reconciliation.
|
||||||
func TestDispatchKanbanAgentErrorTrigger_ReadsPostReconciliationStep(t *testing.T) {
|
func TestDispatchKanbanAgentErrorTrigger_ReadsPostReconciliationStep(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
repo := setupTestRepo(t)
|
repo := setupTestRepo(t)
|
||||||
@@ -126,6 +132,7 @@ func TestDispatchKanbanAgentErrorTrigger_ReadsPostReconciliationStep(t *testing.
|
|||||||
svc.handleAgentFailed(ctx, watcher.AgentEventData{
|
svc.handleAgentFailed(ctx, watcher.AgentEventData{
|
||||||
TaskID: "t1", SessionID: "s1", AgentExecutionID: "exec-1", ErrorMessage: "agent crashed",
|
TaskID: "t1", SessionID: "s1", AgentExecutionID: "exec-1", ErrorMessage: "agent crashed",
|
||||||
})
|
})
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
|
|
||||||
task, err := repo.GetTask(ctx, "t1")
|
task, err := repo.GetTask(ctx, "t1")
|
||||||
if err != nil {
|
if err != nil {
|
||||||
|
|||||||
@@ -10,13 +10,8 @@ import (
|
|||||||
wfmodels "github.com/kandev/kandev/internal/workflow/models"
|
wfmodels "github.com/kandev/kandev/internal/workflow/models"
|
||||||
)
|
)
|
||||||
|
|
||||||
// A panicking dispatch does not take the process down. Routes R2-R5 never
|
// A panicking callback is recovered by the asynchronous recovery worker.
|
||||||
// reach the event bus's own recover-and-log wrapper, and this dispatch now
|
// Inject through the registered callback to exercise the real dispatch boundary.
|
||||||
// runs synchronously (after the session guard is released, not on a
|
|
||||||
// goroutine of its own), so nothing above it on the call stack recovers a
|
|
||||||
// panicking callback unless dispatchKanbanAgentErrorTriggerRecovered does.
|
|
||||||
// The panic is injected through a registered callback's Execute, the only
|
|
||||||
// available seam — not a stub engine.
|
|
||||||
func TestDispatchKanbanAgentErrorTrigger_RecoversFromPanickingCallback(t *testing.T) {
|
func TestDispatchKanbanAgentErrorTrigger_RecoversFromPanickingCallback(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
repo := setupTestRepo(t)
|
repo := setupTestRepo(t)
|
||||||
@@ -48,6 +43,7 @@ func TestDispatchKanbanAgentErrorTrigger_RecoversFromPanickingCallback(t *testin
|
|||||||
t.Fatal("handleAgentFailed did not return after a panicking callback: the panic escaped recovery")
|
t.Fatal("handleAgentFailed did not return after a panicking callback: the panic escaped recovery")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
errs := filterLogs(logs, msgAgentErrorDispatchPanicked)
|
errs := filterLogs(logs, msgAgentErrorDispatchPanicked)
|
||||||
if len(errs) != 1 {
|
if len(errs) != 1 {
|
||||||
t.Fatalf("got %d %q ERROR record(s), want 1 (all: %+v)", len(errs), msgAgentErrorDispatchPanicked, logs.All())
|
t.Fatalf("got %d %q ERROR record(s), want 1 (all: %+v)", len(errs), msgAgentErrorDispatchPanicked, logs.All())
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import (
|
|||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
|
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
||||||
"github.com/kandev/kandev/internal/agent/runtime/routingerr"
|
"github.com/kandev/kandev/internal/agent/runtime/routingerr"
|
||||||
"github.com/kandev/kandev/internal/orchestrator/watcher"
|
"github.com/kandev/kandev/internal/orchestrator/watcher"
|
||||||
"github.com/kandev/kandev/internal/task/models"
|
"github.com/kandev/kandev/internal/task/models"
|
||||||
@@ -21,6 +22,8 @@ import (
|
|||||||
// asserts the payload the engine receives (AC-D1/D2/D3), per this spec's
|
// asserts the payload the engine receives (AC-D1/D2/D3), per this spec's
|
||||||
// Verification text for AC-A1/A2. ---
|
// Verification text for AC-A1/A2. ---
|
||||||
|
|
||||||
|
// TestDispatchKanbanAgentErrorTrigger_R1BusDrivenFailureDispatches verifies that
|
||||||
|
// bus-delivered agent failures reach workflow recovery after cleanup.
|
||||||
func TestDispatchKanbanAgentErrorTrigger_R1BusDrivenFailureDispatches(t *testing.T) {
|
func TestDispatchKanbanAgentErrorTrigger_R1BusDrivenFailureDispatches(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
repo := setupTestRepo(t)
|
repo := setupTestRepo(t)
|
||||||
@@ -39,6 +42,7 @@ func TestDispatchKanbanAgentErrorTrigger_R1BusDrivenFailureDispatches(t *testing
|
|||||||
TaskID: "t1", SessionID: "s1", AgentExecutionID: "exec-1", ErrorMessage: "agent crashed",
|
TaskID: "t1", SessionID: "s1", AgentExecutionID: "exec-1", ErrorMessage: "agent crashed",
|
||||||
})
|
})
|
||||||
|
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
if decisions.clearCalls != 1 {
|
if decisions.clearCalls != 1 {
|
||||||
t.Fatalf("clearCalls = %d, want 1 (R1's bus-driven entry point must reach the real dispatch)", decisions.clearCalls)
|
t.Fatalf("clearCalls = %d, want 1 (R1's bus-driven entry point must reach the real dispatch)", decisions.clearCalls)
|
||||||
}
|
}
|
||||||
@@ -53,6 +57,8 @@ func TestDispatchKanbanAgentErrorTrigger_R1BusDrivenFailureDispatches(t *testing
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestDispatchKanbanAgentErrorTrigger_R2ManagedRuntimeNpmFailureDispatches verifies
|
||||||
|
// workflow recovery for managed-runtime package resolution failures.
|
||||||
func TestDispatchKanbanAgentErrorTrigger_R2ManagedRuntimeNpmFailureDispatches(t *testing.T) {
|
func TestDispatchKanbanAgentErrorTrigger_R2ManagedRuntimeNpmFailureDispatches(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
repo := setupTestRepo(t)
|
repo := setupTestRepo(t)
|
||||||
@@ -76,6 +82,7 @@ func TestDispatchKanbanAgentErrorTrigger_R2ManagedRuntimeNpmFailureDispatches(t
|
|||||||
if !handled {
|
if !handled {
|
||||||
t.Fatal("expected handled=true for a managed npm runtime startup failure")
|
t.Fatal("expected handled=true for a managed npm runtime startup failure")
|
||||||
}
|
}
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
if decisions.clearCalls != 1 {
|
if decisions.clearCalls != 1 {
|
||||||
t.Fatalf("clearCalls = %d, want 1 (R2's npm-resolution branch must reach the real dispatch)", decisions.clearCalls)
|
t.Fatalf("clearCalls = %d, want 1 (R2's npm-resolution branch must reach the real dispatch)", decisions.clearCalls)
|
||||||
}
|
}
|
||||||
@@ -91,6 +98,8 @@ func TestDispatchKanbanAgentErrorTrigger_R2ManagedRuntimeNpmFailureDispatches(t
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestDispatchKanbanAgentErrorTrigger_R3AuthErrorDispatches verifies workflow
|
||||||
|
// recovery for authentication failures during agent startup.
|
||||||
func TestDispatchKanbanAgentErrorTrigger_R3AuthErrorDispatches(t *testing.T) {
|
func TestDispatchKanbanAgentErrorTrigger_R3AuthErrorDispatches(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
repo := setupTestRepo(t)
|
repo := setupTestRepo(t)
|
||||||
@@ -112,6 +121,7 @@ func TestDispatchKanbanAgentErrorTrigger_R3AuthErrorDispatches(t *testing.T) {
|
|||||||
if !handled {
|
if !handled {
|
||||||
t.Fatal("expected handled=true for an auth-error start failure")
|
t.Fatal("expected handled=true for an auth-error start failure")
|
||||||
}
|
}
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
if decisions.clearCalls != 1 {
|
if decisions.clearCalls != 1 {
|
||||||
t.Fatalf("clearCalls = %d, want 1 (R3's auth-error branch must reach the real dispatch)", decisions.clearCalls)
|
t.Fatalf("clearCalls = %d, want 1 (R3's auth-error branch must reach the real dispatch)", decisions.clearCalls)
|
||||||
}
|
}
|
||||||
@@ -127,6 +137,8 @@ func TestDispatchKanbanAgentErrorTrigger_R3AuthErrorDispatches(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestDispatchKanbanAgentErrorTrigger_R4NoCachedPromptDispatches verifies that
|
||||||
|
// a retry without a cached prompt falls back to workflow recovery.
|
||||||
func TestDispatchKanbanAgentErrorTrigger_R4NoCachedPromptDispatches(t *testing.T) {
|
func TestDispatchKanbanAgentErrorTrigger_R4NoCachedPromptDispatches(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
repo := setupTestRepo(t)
|
repo := setupTestRepo(t)
|
||||||
@@ -148,6 +160,7 @@ func TestDispatchKanbanAgentErrorTrigger_R4NoCachedPromptDispatches(t *testing.T
|
|||||||
svc.scheduleTransientRetry("t1", "s1", "", 1, time.Hour)
|
svc.scheduleTransientRetry("t1", "s1", "", 1, time.Hour)
|
||||||
svc.retryTransientPrompt(ctx, "t1", "s1", "")
|
svc.retryTransientPrompt(ctx, "t1", "s1", "")
|
||||||
|
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
if decisions.clearCalls != 1 {
|
if decisions.clearCalls != 1 {
|
||||||
t.Fatalf("clearCalls = %d, want 1 (R4's no-cached-prompt path must reach the real dispatch)", decisions.clearCalls)
|
t.Fatalf("clearCalls = %d, want 1 (R4's no-cached-prompt path must reach the real dispatch)", decisions.clearCalls)
|
||||||
}
|
}
|
||||||
@@ -163,7 +176,28 @@ func TestDispatchKanbanAgentErrorTrigger_R4NoCachedPromptDispatches(t *testing.T
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestDispatchKanbanAgentErrorTrigger_R5SynchronousPromptErrorDispatches verifies
|
||||||
|
// that a failed replacement prompt dispatches recovery only after completed teardown.
|
||||||
func TestDispatchKanbanAgentErrorTrigger_R5SynchronousPromptErrorDispatches(t *testing.T) {
|
func TestDispatchKanbanAgentErrorTrigger_R5SynchronousPromptErrorDispatches(t *testing.T) {
|
||||||
|
for _, tc := range []struct {
|
||||||
|
name string
|
||||||
|
stopErr error
|
||||||
|
wantDispatch int
|
||||||
|
}{
|
||||||
|
{"stopped", nil, 1},
|
||||||
|
{"already absent", lifecycle.ErrExecutionNotFound, 1},
|
||||||
|
{"stop failed", errors.New("runtime stop failed"), 0},
|
||||||
|
} {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
testSynchronousPromptFailureDispatch(t, tc.stopErr, tc.wantDispatch)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// testSynchronousPromptFailureDispatch exercises workflow recovery after the
|
||||||
|
// transient retry owns teardown and its replacement prompt fails synchronously.
|
||||||
|
func testSynchronousPromptFailureDispatch(t *testing.T, stopErr error, wantDispatch int) {
|
||||||
|
t.Helper()
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
repo := setupTestRepo(t)
|
repo := setupTestRepo(t)
|
||||||
seedSession(t, repo, "t1", "s1", "step1")
|
seedSession(t, repo, "t1", "s1", "step1")
|
||||||
@@ -183,8 +217,9 @@ func TestDispatchKanbanAgentErrorTrigger_R5SynchronousPromptErrorDispatches(t *t
|
|||||||
Events: wfmodels.StepEvents{OnAgentError: []wfmodels.GenericAction{{Type: wfmodels.GenericActionClearDecisions}}},
|
Events: wfmodels.StepEvents{OnAgentError: []wfmodels.GenericAction{{Type: wfmodels.GenericActionClearDecisions}}},
|
||||||
}
|
}
|
||||||
agentMgr := &mockAgentManager{
|
agentMgr := &mockAgentManager{
|
||||||
repoForExecutionLookup: repo,
|
repoForExecutionLookup: repo,
|
||||||
promptErr: errors.New("session rejected prompt synchronously"),
|
promptErr: errors.New("session rejected prompt synchronously"),
|
||||||
|
stopAgentWithReasonFunc: func(context.Context, string, string, bool) error { return stopErr },
|
||||||
}
|
}
|
||||||
decisions := &spyDecisionStore{}
|
decisions := &spyDecisionStore{}
|
||||||
svc, logs := newAgentErrorTransientTestService(t, repo, stepGetter, agentMgr, func(s *Service) {
|
svc, logs := newAgentErrorTransientTestService(t, repo, stepGetter, agentMgr, func(s *Service) {
|
||||||
@@ -198,14 +233,18 @@ func TestDispatchKanbanAgentErrorTrigger_R5SynchronousPromptErrorDispatches(t *t
|
|||||||
|
|
||||||
svc.retryTransientPrompt(ctx, "t1", "s1", "exec-1")
|
svc.retryTransientPrompt(ctx, "t1", "s1", "exec-1")
|
||||||
|
|
||||||
if decisions.clearCalls != 1 {
|
waitForFailureRecovery(t, svc)
|
||||||
t.Fatalf("clearCalls = %d, want 1 (R5's synchronous PromptTask failure must reach the real dispatch)", decisions.clearCalls)
|
if decisions.clearCalls != wantDispatch {
|
||||||
|
t.Fatalf("clearCalls = %d, want %d after transient teardown", decisions.clearCalls, wantDispatch)
|
||||||
}
|
}
|
||||||
if got := filterLogs(logs, msgAgentErrorDispatched); len(got) != 1 {
|
if got := filterLogs(logs, msgAgentErrorDispatched); len(got) != wantDispatch {
|
||||||
t.Fatalf("got %d dispatch INFO records, want 1", len(got))
|
t.Fatalf("got %d dispatch INFO records, want %d", len(got), wantDispatch)
|
||||||
}
|
}
|
||||||
if len(*captured) != 1 {
|
if len(*captured) != wantDispatch {
|
||||||
t.Fatalf("got %d payload(s), want 1", len(*captured))
|
t.Fatalf("got %d payload(s), want %d", len(*captured), wantDispatch)
|
||||||
|
}
|
||||||
|
if wantDispatch == 0 {
|
||||||
|
return
|
||||||
}
|
}
|
||||||
wantMsg := "Automatic provider retry could not be started. Resume or start fresh to continue."
|
wantMsg := "Automatic provider retry could not be started. Resume or start fresh to continue."
|
||||||
if got := (*captured)[0]; got.FailedSessionID != "s1" || got.ErrorMessage != wantMsg {
|
if got := (*captured)[0]; got.FailedSessionID != "s1" || got.ErrorMessage != wantMsg {
|
||||||
|
|||||||
@@ -30,10 +30,8 @@ func newAgentErrorTestService(
|
|||||||
t *testing.T, repo *sqliterepo.Repository, stepGetter *mockStepGetter, configure func(*Service),
|
t *testing.T, repo *sqliterepo.Repository, stepGetter *mockStepGetter, configure func(*Service),
|
||||||
) (*Service, *observer.ObservedLogs) {
|
) (*Service, *observer.ObservedLogs) {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
// handleRecoverableFailureLocked's last-but-one step (before this card's
|
// Recovery stops the failed execution before dispatching workflow actions.
|
||||||
// dispatch) fires a background cleanupAgentExecution that dereferences
|
// Use the fixture that wires svc.executor for that cleanup boundary.
|
||||||
// svc.executor — createTestServiceWithScheduler is the fixture that wires
|
|
||||||
// one, unlike the bare createTestService used elsewhere in this package.
|
|
||||||
agentMgr := &mockAgentManager{repoForExecutionLookup: repo}
|
agentMgr := &mockAgentManager{repoForExecutionLookup: repo}
|
||||||
svc := createTestServiceWithScheduler(repo, stepGetter, newMockTaskRepo(), agentMgr)
|
svc := createTestServiceWithScheduler(repo, stepGetter, newMockTaskRepo(), agentMgr)
|
||||||
core, logs := observer.New(zapcore.DebugLevel)
|
core, logs := observer.New(zapcore.DebugLevel)
|
||||||
@@ -46,6 +44,7 @@ func newAgentErrorTestService(
|
|||||||
configure(svc)
|
configure(svc)
|
||||||
}
|
}
|
||||||
svc.initWorkflowEngine()
|
svc.initWorkflowEngine()
|
||||||
|
t.Cleanup(svc.stopDynamicSuccessorWorkers)
|
||||||
return svc, logs
|
return svc, logs
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -459,6 +458,8 @@ func (r *agentErrorTaskLoadErrorRepo) GetTask(_ context.Context, _ string) (*mod
|
|||||||
|
|
||||||
// --- AC-A5/A7/A8/F2/F3/F4/F5/F6/B5: the guard sequence. ---
|
// --- AC-A5/A7/A8/F2/F3/F4/F5/F6/B5: the guard sequence. ---
|
||||||
|
|
||||||
|
// TestDispatchKanbanAgentErrorTrigger_Guards verifies dispatch suppression for
|
||||||
|
// user cancellation and dispatch eligibility for recoverable agent failures.
|
||||||
func TestDispatchKanbanAgentErrorTrigger_Guards(t *testing.T) {
|
func TestDispatchKanbanAgentErrorTrigger_Guards(t *testing.T) {
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
|
|
||||||
@@ -501,6 +502,7 @@ func TestDispatchKanbanAgentErrorTrigger_Guards(t *testing.T) {
|
|||||||
if !svc.CancelTransientRetry(ctx, "t1", "s1") {
|
if !svc.CancelTransientRetry(ctx, "t1", "s1") {
|
||||||
t.Fatal("CancelTransientRetry = false, want true (a loop was active)")
|
t.Fatal("CancelTransientRetry = false, want true (a loop was active)")
|
||||||
}
|
}
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
if decisions.clearCalls != 0 {
|
if decisions.clearCalls != 0 {
|
||||||
t.Errorf("clearCalls = %d, want 0 (a user cancel must not dispatch on_agent_error)", decisions.clearCalls)
|
t.Errorf("clearCalls = %d, want 0 (a user cancel must not dispatch on_agent_error)", decisions.clearCalls)
|
||||||
}
|
}
|
||||||
@@ -518,6 +520,7 @@ func TestDispatchKanbanAgentErrorTrigger_Guards(t *testing.T) {
|
|||||||
svc, _ := newAgentErrorTestService(t, repo, stepGetter, func(s *Service) { s.engineDecisions = decisions })
|
svc, _ := newAgentErrorTestService(t, repo, stepGetter, func(s *Service) { s.engineDecisions = decisions })
|
||||||
|
|
||||||
svc.handleRecoverableFailure(ctx, watcher.AgentEventData{TaskID: "t1", SessionID: "s1", AgentExecutionID: "exec-1"})
|
svc.handleRecoverableFailure(ctx, watcher.AgentEventData{TaskID: "t1", SessionID: "s1", AgentExecutionID: "exec-1"})
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
|
|
||||||
if decisions.clearCalls != 1 {
|
if decisions.clearCalls != 1 {
|
||||||
t.Errorf("clearCalls = %d, want 1 (a non-user-initiated recoverable failure must dispatch)", decisions.clearCalls)
|
t.Errorf("clearCalls = %d, want 1 (a non-user-initiated recoverable failure must dispatch)", decisions.clearCalls)
|
||||||
|
|||||||
@@ -0,0 +1,252 @@
|
|||||||
|
package orchestrator
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"errors"
|
||||||
|
"fmt"
|
||||||
|
"sync"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
||||||
|
"github.com/kandev/kandev/internal/agent/runtime/routingerr"
|
||||||
|
"github.com/kandev/kandev/internal/orchestrator/watcher"
|
||||||
|
"github.com/kandev/kandev/internal/task/models"
|
||||||
|
"github.com/kandev/kandev/internal/workflow/engine"
|
||||||
|
wfmodels "github.com/kandev/kandev/internal/workflow/models"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
|
)
|
||||||
|
|
||||||
|
// Tests of workflow outcomes can run the deferred failure work synchronously
|
||||||
|
// when they do not hold the session guard or publish from a lifecycle callback.
|
||||||
|
func (s *Service) handleRecoverableFailureLocked(ctx context.Context, data watcher.AgentEventData) {
|
||||||
|
if dispatch := s.handleRecoverableFailureLockedState(ctx, data, lifecycle.StopReasonRecoverableAgentFailure); dispatch != nil {
|
||||||
|
dispatch(ctx)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// @covers AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.1
|
||||||
|
func TestKubernetesRecoverableFailurePreservesResume(t *testing.T) {
|
||||||
|
repo := setupTestRepo(t)
|
||||||
|
seedSession(t, repo, "task-recovery", "session-recovery", "step1")
|
||||||
|
stopped := make(chan stopAgentCall, 1)
|
||||||
|
svc, _ := newAgentErrorTestService(t, repo, newMockStepGetter(), func(s *Service) {
|
||||||
|
manager := s.agentManager.(*mockAgentManager)
|
||||||
|
manager.stopAgentWithReasonFunc = func(_ context.Context, id, reason string, force bool) error {
|
||||||
|
stopped <- stopAgentCall{ExecutionID: id, Reason: reason, Force: force}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
})
|
||||||
|
svc.handleRecoverableFailure(context.Background(), watcher.AgentEventData{
|
||||||
|
TaskID: "task-recovery", SessionID: "session-recovery", AgentExecutionID: "execution-recovery", ErrorMessage: "Internal error",
|
||||||
|
})
|
||||||
|
select {
|
||||||
|
case call := <-stopped:
|
||||||
|
require.Equal(t, "execution-recovery", call.ExecutionID)
|
||||||
|
require.Equal(t, "recoverable agent failure", call.Reason)
|
||||||
|
case <-time.After(5 * time.Second):
|
||||||
|
t.Fatal("recoverable failure did not stop its failed execution")
|
||||||
|
}
|
||||||
|
session, err := repo.GetTaskSession(context.Background(), "session-recovery")
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.Equal(t, models.TaskSessionStateWaitingForInput, session.State)
|
||||||
|
}
|
||||||
|
|
||||||
|
// @covers AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.3
|
||||||
|
func TestKubernetesRecoverableFailureCleanupBeforeWorkflowResume(t *testing.T) {
|
||||||
|
repo := setupTestRepo(t)
|
||||||
|
seedSession(t, repo, "t1", "s1", "step1")
|
||||||
|
steps := newMockStepGetter()
|
||||||
|
steps.steps["step1"] = &wfmodels.WorkflowStep{
|
||||||
|
ID: "step1", WorkflowID: "wf1", Name: "Step 1", Position: 0,
|
||||||
|
Events: wfmodels.StepEvents{OnAgentError: []wfmodels.GenericAction{{Type: wfmodels.GenericActionAutoStartAgent}}},
|
||||||
|
}
|
||||||
|
stopEntered := make(chan struct{})
|
||||||
|
releaseStop := make(chan struct{})
|
||||||
|
var releaseOnce sync.Once
|
||||||
|
unblock := func() { releaseOnce.Do(func() { close(releaseStop) }) }
|
||||||
|
defer unblock()
|
||||||
|
svc, _ := newAgentErrorTestService(t, repo, steps, func(s *Service) {
|
||||||
|
s.agentManager.(*mockAgentManager).stopAgentWithReasonFunc = func(context.Context, string, string, bool) error {
|
||||||
|
close(stopEntered)
|
||||||
|
<-releaseStop
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
})
|
||||||
|
callback := &guardReacquiringAgentErrorCallback{svc: svc, done: make(chan struct{})}
|
||||||
|
registry := engine.MapRegistry{engine.ActionAutoStartAgent: callback}
|
||||||
|
workflowEngine := engine.New(svc.workflowStore, registry)
|
||||||
|
svc.workflowEngine = workflowEngine
|
||||||
|
svc.agentErrorDeps.Store(&agentErrorDispatchDeps{engine: workflowEngine, registry: registry, store: svc.workflowStore})
|
||||||
|
finished := make(chan struct{})
|
||||||
|
go func() {
|
||||||
|
svc.handleRecoverableFailure(context.Background(), watcher.AgentEventData{
|
||||||
|
TaskID: "t1", SessionID: "s1", AgentExecutionID: "exec-1", ErrorMessage: "Internal error",
|
||||||
|
})
|
||||||
|
close(finished)
|
||||||
|
}()
|
||||||
|
awaitFailureSignal(t, stopEntered)
|
||||||
|
awaitFailureSignal(t, finished)
|
||||||
|
// A duplicate event must not dispatch while the first stop still owns cleanup.
|
||||||
|
svc.handleRecoverableFailure(context.Background(), watcher.AgentEventData{
|
||||||
|
TaskID: "t1", SessionID: "s1", AgentExecutionID: "exec-1", ErrorMessage: "Internal error",
|
||||||
|
})
|
||||||
|
select {
|
||||||
|
case <-callback.done:
|
||||||
|
t.Error("workflow resume ran before failed execution cleanup finished")
|
||||||
|
default:
|
||||||
|
}
|
||||||
|
unblock()
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
|
select {
|
||||||
|
case <-callback.done:
|
||||||
|
default:
|
||||||
|
t.Fatal("workflow resume did not run after cleanup")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// @covers AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.3
|
||||||
|
func TestKubernetesRecoverableFailureDoesNotResumeAfterCleanupError(t *testing.T) {
|
||||||
|
svc, resumed := recoveryWorkflowFixture(t)
|
||||||
|
stopErr := errors.New("runtime stop failed")
|
||||||
|
stops := 0
|
||||||
|
svc.agentManager.(*mockAgentManager).stopAgentWithReasonFunc = func(context.Context, string, string, bool) error {
|
||||||
|
stops++
|
||||||
|
return stopErr
|
||||||
|
}
|
||||||
|
data := watcher.AgentEventData{TaskID: "t1", SessionID: "s1", AgentExecutionID: "exec-1", ErrorMessage: "Internal error"}
|
||||||
|
svc.handleRecoverableFailure(context.Background(), data)
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
|
select {
|
||||||
|
case <-resumed:
|
||||||
|
t.Fatal("automatic Resume ran after failed cleanup")
|
||||||
|
default:
|
||||||
|
}
|
||||||
|
stopErr = nil
|
||||||
|
svc.handleRecoverableFailure(context.Background(), data)
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
|
require.Equal(t, 2, stops, "failed cleanup must remain retryable")
|
||||||
|
select {
|
||||||
|
case <-resumed:
|
||||||
|
default:
|
||||||
|
t.Fatal("successful cleanup retry did not dispatch Resume")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// @covers AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.3
|
||||||
|
func TestKubernetesRecoverableFailureShutdownOwnsCleanup(t *testing.T) {
|
||||||
|
svc, resumed := recoveryWorkflowFixture(t)
|
||||||
|
entered, release, drained := make(chan struct{}), make(chan struct{}), make(chan struct{})
|
||||||
|
var once sync.Once
|
||||||
|
unblock := func() { once.Do(func() { close(release) }) }
|
||||||
|
defer unblock()
|
||||||
|
stops := 0
|
||||||
|
svc.agentManager.(*mockAgentManager).stopAgentWithReasonFunc = func(context.Context, string, string, bool) error {
|
||||||
|
stops++
|
||||||
|
if stops == 1 {
|
||||||
|
close(entered)
|
||||||
|
}
|
||||||
|
<-release
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
data := watcher.AgentEventData{TaskID: "t1", SessionID: "s1", AgentExecutionID: "exec-1", ErrorMessage: "Internal error"}
|
||||||
|
svc.handleRecoverableFailure(context.Background(), data)
|
||||||
|
awaitFailureSignal(t, entered)
|
||||||
|
go func() { svc.stopDynamicSuccessorWorkers(); close(drained) }()
|
||||||
|
awaitFailureSignal(t, svc.dynamicSuccessorCtx.Done())
|
||||||
|
select {
|
||||||
|
case <-drained:
|
||||||
|
t.Error("shutdown returned while recovery cleanup was still running")
|
||||||
|
default:
|
||||||
|
}
|
||||||
|
unblock()
|
||||||
|
awaitFailureSignal(t, drained)
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
|
select {
|
||||||
|
case <-resumed:
|
||||||
|
t.Error("automatic Resume ran after shutdown started")
|
||||||
|
default:
|
||||||
|
}
|
||||||
|
data.AgentExecutionID = "exec-after-shutdown"
|
||||||
|
svc.handleRecoverableFailure(context.Background(), data)
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
|
require.Equal(t, 1, stops, "stopped service must reject detached recovery work")
|
||||||
|
}
|
||||||
|
|
||||||
|
// recoveryWorkflowFixture wires a real workflow whose Resume callback signals
|
||||||
|
// completion after reacquiring the session guard.
|
||||||
|
func recoveryWorkflowFixture(t *testing.T) (*Service, <-chan struct{}) {
|
||||||
|
t.Helper()
|
||||||
|
repo := setupTestRepo(t)
|
||||||
|
seedSession(t, repo, "t1", "s1", "step1")
|
||||||
|
steps := newMockStepGetter()
|
||||||
|
steps.steps["step1"] = &wfmodels.WorkflowStep{
|
||||||
|
ID: "step1", WorkflowID: "wf1", Name: "Step 1", Position: 0,
|
||||||
|
Events: wfmodels.StepEvents{OnAgentError: []wfmodels.GenericAction{{Type: wfmodels.GenericActionAutoStartAgent}}},
|
||||||
|
}
|
||||||
|
svc, _ := newAgentErrorTestService(t, repo, steps, nil)
|
||||||
|
callback := &guardReacquiringAgentErrorCallback{svc: svc, done: make(chan struct{})}
|
||||||
|
registry := engine.MapRegistry{engine.ActionAutoStartAgent: callback}
|
||||||
|
workflowEngine := engine.New(svc.workflowStore, registry)
|
||||||
|
svc.workflowEngine = workflowEngine
|
||||||
|
svc.agentErrorDeps.Store(&agentErrorDispatchDeps{engine: workflowEngine, registry: registry, store: svc.workflowStore})
|
||||||
|
return svc, callback.done
|
||||||
|
}
|
||||||
|
|
||||||
|
// waitForFailureRecovery drains recovery workers after the producer has
|
||||||
|
// scheduled them, synchronizing subsequent reads of test spies and logs.
|
||||||
|
func waitForFailureRecovery(t *testing.T, svc *Service) {
|
||||||
|
t.Helper()
|
||||||
|
done := make(chan struct{})
|
||||||
|
go func() { svc.dynamicSuccessorWorkers.Wait(); close(done) }()
|
||||||
|
awaitFailureSignal(t, done)
|
||||||
|
}
|
||||||
|
|
||||||
|
// awaitFailureSignal bounds real-time synchronization without simulated time
|
||||||
|
// around SQLite-backed repository operations.
|
||||||
|
func awaitFailureSignal(t *testing.T, done <-chan struct{}) {
|
||||||
|
t.Helper()
|
||||||
|
select {
|
||||||
|
case <-done:
|
||||||
|
case <-time.After(5 * time.Second):
|
||||||
|
t.Fatal("timed out waiting for failure recovery")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// @covers AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.4
|
||||||
|
func TestKubernetesRecoverableFailureStartupUsesBootstrapCleanup(t *testing.T) {
|
||||||
|
for _, fromResume := range []bool{false, true} {
|
||||||
|
for _, failure := range []struct {
|
||||||
|
name string
|
||||||
|
err error
|
||||||
|
}{
|
||||||
|
{"auth", errors.New("authentication required: please log in")},
|
||||||
|
{"npm", &routingerr.ManagedRuntimeStartupError{Code: routingerr.CodeManagedRuntimeNpmResolution, Details: "npm error code ETARGET"}},
|
||||||
|
} {
|
||||||
|
t.Run(fmt.Sprintf("%s/resume=%t", failure.name, fromResume), func(t *testing.T) {
|
||||||
|
svc, _ := recoveryWorkflowFixture(t)
|
||||||
|
var reason string
|
||||||
|
svc.agentManager.(*mockAgentManager).stopAgentWithReasonFunc = func(_ context.Context, _, stopReason string, _ bool) error {
|
||||||
|
reason = stopReason
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
require.True(t, svc.handleAgentStartFailed(context.Background(), "t1", "s1", "exec-1", failure.err, fromResume))
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
|
require.Equal(t, lifecycle.StopReasonAgentBootstrapFailed, reason)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// @covers AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.3
|
||||||
|
func TestKubernetesRecoverableFailureAlreadyStoppedAllowsRecovery(t *testing.T) {
|
||||||
|
svc, resumed := recoveryWorkflowFixture(t)
|
||||||
|
svc.agentManager.(*mockAgentManager).stopAgentWithReasonFunc = func(context.Context, string, string, bool) error {
|
||||||
|
return fmt.Errorf("execution already removed: %w", lifecycle.ErrExecutionNotFound)
|
||||||
|
}
|
||||||
|
svc.handleRecoverableFailure(context.Background(), watcher.AgentEventData{
|
||||||
|
TaskID: "t1", SessionID: "s1", AgentExecutionID: "exec-1", ErrorMessage: "Internal error",
|
||||||
|
})
|
||||||
|
waitForFailureRecovery(t, svc)
|
||||||
|
awaitFailureSignal(t, resumed)
|
||||||
|
}
|
||||||
@@ -9,6 +9,7 @@ import (
|
|||||||
|
|
||||||
"go.uber.org/zap"
|
"go.uber.org/zap"
|
||||||
|
|
||||||
|
agentruntime "github.com/kandev/kandev/internal/agent/runtime"
|
||||||
"github.com/kandev/kandev/internal/agent/runtime/routingerr"
|
"github.com/kandev/kandev/internal/agent/runtime/routingerr"
|
||||||
"github.com/kandev/kandev/internal/orchestrator/watcher"
|
"github.com/kandev/kandev/internal/orchestrator/watcher"
|
||||||
"github.com/kandev/kandev/internal/task/models"
|
"github.com/kandev/kandev/internal/task/models"
|
||||||
@@ -530,11 +531,14 @@ func (s *Service) retryTransientPrompt(ctx context.Context, taskID, sessionID, e
|
|||||||
s.resetTransientRetry(sessionID)
|
s.resetTransientRetry(sessionID)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if err := s.stopTransientRetryExecution(ctx, execID); err != nil {
|
claim, claimed := s.executionTeardownClaimFor(sessionID, execID)
|
||||||
|
if err := s.stopTransientRetryExecution(ctx, execID); err != nil && !agentruntime.IsNotFound(err) {
|
||||||
s.logger.Debug("failed to stop failed execution before transient retry",
|
s.logger.Debug("failed to stop failed execution before transient retry",
|
||||||
zap.String("session_id", sessionID),
|
zap.String("session_id", sessionID),
|
||||||
zap.String("execution_id", execID),
|
zap.String("execution_id", execID),
|
||||||
zap.Error(err))
|
zap.Error(err))
|
||||||
|
} else if claimed {
|
||||||
|
s.completeExecutionTeardownClaim(sessionID, execID, claim)
|
||||||
}
|
}
|
||||||
// handleAgentFailed terminal-marked this exact execution before the
|
// handleAgentFailed terminal-marked this exact execution before the
|
||||||
// retry was scheduled, so no later frame may reclaim activity even when
|
// retry was scheduled, so no later frame may reclaim activity even when
|
||||||
|
|||||||
@@ -0,0 +1,85 @@
|
|||||||
|
package orchestrator
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
|
||||||
|
agentruntime "github.com/kandev/kandev/internal/agent/runtime"
|
||||||
|
|
||||||
|
"go.uber.org/zap"
|
||||||
|
)
|
||||||
|
|
||||||
|
// cleanupAgentExecutionWithReason reports whether teardown is complete and
|
||||||
|
// workflow recovery may proceed, including a previously completed exact claim.
|
||||||
|
func (s *Service) cleanupAgentExecutionWithReason(ctx context.Context, executionID, taskID, sessionID, reason string) bool {
|
||||||
|
if ctx.Err() != nil {
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
if executionID == "" {
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
if !s.claimForcedExecutionCleanup(sessionID, executionID) {
|
||||||
|
s.logger.Debug("skipping duplicate execution teardown",
|
||||||
|
zap.String("execution_id", executionID),
|
||||||
|
zap.String("task_id", taskID),
|
||||||
|
zap.String("session_id", sessionID))
|
||||||
|
claim, ok := s.executionTeardownClaimFor(sessionID, executionID)
|
||||||
|
return ok && claim.cleanupCompleted
|
||||||
|
}
|
||||||
|
claimKey := terminalExecutionKey(sessionID, executionID)
|
||||||
|
claim, claimed := s.executionTeardownClaimFor(sessionID, executionID)
|
||||||
|
if !s.isExecutionCompleted(sessionID, executionID) {
|
||||||
|
// Direct crash/forced-cleanup callers may reach this boundary without a
|
||||||
|
// preceding lifecycle event. Preserve an existing completed marker (and
|
||||||
|
// its allowed terminal stream), otherwise tombstone trailing frames.
|
||||||
|
s.markExecutionFailed(sessionID, executionID)
|
||||||
|
}
|
||||||
|
// Defensive terminal-boundary retirement. Normal lifecycle events run this
|
||||||
|
// first; the repeated forced-cleanup call is idempotent.
|
||||||
|
s.retireExecutionActivityAndPublish(ctx, taskID, sessionID, executionID)
|
||||||
|
if err := s.executor.StopExecution(ctx, executionID, reason, true); err != nil && !agentruntime.IsNotFound(err) {
|
||||||
|
s.logger.Debug("agent execution cleanup after terminal state",
|
||||||
|
zap.String("execution_id", executionID),
|
||||||
|
zap.String("task_id", taskID),
|
||||||
|
zap.Error(err))
|
||||||
|
if claimed && (reason == agentruntime.StopReasonRecoverableAgentFailure || reason == agentruntime.StopReasonAgentBootstrapFailed) {
|
||||||
|
// A failed teardown remains retryable without releasing a newer claim.
|
||||||
|
s.executionTeardownClaims.CompareAndDelete(claimKey, claim)
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
if claimed {
|
||||||
|
s.completeExecutionTeardownClaim(sessionID, executionID, claim)
|
||||||
|
}
|
||||||
|
return true
|
||||||
|
}
|
||||||
|
|
||||||
|
// startAgentFailureRecovery shares shutdown ownership with dynamic fallback
|
||||||
|
// launches. The worker must leave the failure publisher before stopping an
|
||||||
|
// execution, because that publisher can still hold the lifecycle prompt lock.
|
||||||
|
func (s *Service) startAgentFailureRecovery(recoverFailure func(context.Context)) {
|
||||||
|
s.dynamicSuccessorMu.Lock()
|
||||||
|
if s.dynamicSuccessorStopped {
|
||||||
|
s.dynamicSuccessorMu.Unlock()
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if s.dynamicSuccessorCtx == nil {
|
||||||
|
s.dynamicSuccessorCtx, s.dynamicSuccessorCancel = context.WithCancel(context.Background())
|
||||||
|
}
|
||||||
|
workerCtx := s.dynamicSuccessorCtx
|
||||||
|
s.dynamicSuccessorWorkers.Add(1)
|
||||||
|
s.dynamicSuccessorMu.Unlock()
|
||||||
|
go func() {
|
||||||
|
defer s.dynamicSuccessorWorkers.Done()
|
||||||
|
ctx, cancel := context.WithTimeout(workerCtx, dynamicSuccessorDetachedTimeout)
|
||||||
|
defer cancel()
|
||||||
|
recoverFailure(ctx)
|
||||||
|
}()
|
||||||
|
}
|
||||||
|
|
||||||
|
// completeExecutionTeardownClaim records successful cleanup only while the
|
||||||
|
// caller still owns the same claim, preserving any newer teardown owner.
|
||||||
|
func (s *Service) completeExecutionTeardownClaim(sessionID, executionID string, claim executionTeardownClaim) {
|
||||||
|
completed := claim
|
||||||
|
completed.cleanupCompleted = true
|
||||||
|
s.executionTeardownClaims.CompareAndSwap(terminalExecutionKey(sessionID, executionID), claim, completed)
|
||||||
|
}
|
||||||
@@ -1470,7 +1470,7 @@ type Service struct {
|
|||||||
ciAutomationStopped bool
|
ciAutomationStopped bool
|
||||||
ciAutomationWorkers sync.WaitGroup
|
ciAutomationWorkers sync.WaitGroup
|
||||||
|
|
||||||
// dynamicSuccessorWorkers owns the detached dynamic fallback launches. The
|
// dynamicSuccessorWorkers owns detached dynamic launches and failure recovery. The
|
||||||
// launch has to leave the agent.failed dispatch to avoid the prompt
|
// launch has to leave the agent.failed dispatch to avoid the prompt
|
||||||
// lifecycle deadlock, but a detached goroutine must still stop mutating
|
// lifecycle deadlock, but a detached goroutine must still stop mutating
|
||||||
// session state once Stop begins, so it runs under a service-owned context
|
// session state once Stop begins, so it runs under a service-owned context
|
||||||
|
|||||||
@@ -36,6 +36,7 @@ const BACKEND_DIR = path.join(REPO_ROOT, "apps/backend");
|
|||||||
const WEB_DIST_DIR = path.join(REPO_ROOT, "apps/web/dist");
|
const WEB_DIST_DIR = path.join(REPO_ROOT, "apps/web/dist");
|
||||||
const CONTROL_NAMESPACE = "kandev-e2e-control";
|
const CONTROL_NAMESPACE = "kandev-e2e-control";
|
||||||
const WORKLOAD_NAMESPACE = "kandev-e2e-workloads";
|
const WORKLOAD_NAMESPACE = "kandev-e2e-workloads";
|
||||||
|
const WORKLOAD_SERVICE_ACCOUNT = "kandev-workload";
|
||||||
const HOST_SERVICE_ACCOUNT = "kandev-host";
|
const HOST_SERVICE_ACCOUNT = "kandev-host";
|
||||||
const IN_CLUSTER_SERVICE_ACCOUNT = "kandev-in-cluster";
|
const IN_CLUSTER_SERVICE_ACCOUNT = "kandev-in-cluster";
|
||||||
const RESTRICTED_SERVICE_ACCOUNT = "kandev-restricted";
|
const RESTRICTED_SERVICE_ACCOUNT = "kandev-restricted";
|
||||||
@@ -277,6 +278,7 @@ function runKubectl(
|
|||||||
).trim();
|
).trim();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Create a fixture-owned workload identity independently of the namespace controller. */
|
||||||
function workloadRBAC(): string {
|
function workloadRBAC(): string {
|
||||||
return `apiVersion: v1
|
return `apiVersion: v1
|
||||||
kind: Namespace
|
kind: Namespace
|
||||||
@@ -290,6 +292,12 @@ metadata:
|
|||||||
---
|
---
|
||||||
apiVersion: v1
|
apiVersion: v1
|
||||||
kind: ServiceAccount
|
kind: ServiceAccount
|
||||||
|
metadata:
|
||||||
|
name: ${WORKLOAD_SERVICE_ACCOUNT}
|
||||||
|
namespace: ${WORKLOAD_NAMESPACE}
|
||||||
|
---
|
||||||
|
apiVersion: v1
|
||||||
|
kind: ServiceAccount
|
||||||
metadata:
|
metadata:
|
||||||
name: ${HOST_SERVICE_ACCOUNT}
|
name: ${HOST_SERVICE_ACCOUNT}
|
||||||
namespace: ${CONTROL_NAMESPACE}
|
namespace: ${CONTROL_NAMESPACE}
|
||||||
@@ -417,6 +425,7 @@ function serviceAccountKubeconfig(
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Render workload Pods with the fixture-owned identity and optional image or placement overrides. */
|
||||||
function podTemplate(image: string, overrides: PodTemplateOverrides = {}): string {
|
function podTemplate(image: string, overrides: PodTemplateOverrides = {}): string {
|
||||||
const nodeSelector = overrides.nodeSelector
|
const nodeSelector = overrides.nodeSelector
|
||||||
? `\n nodeSelector:\n${Object.entries(overrides.nodeSelector)
|
? `\n nodeSelector:\n${Object.entries(overrides.nodeSelector)
|
||||||
@@ -427,6 +436,7 @@ function podTemplate(image: string, overrides: PodTemplateOverrides = {}): strin
|
|||||||
kind: PodTemplate
|
kind: PodTemplate
|
||||||
template:
|
template:
|
||||||
spec:${nodeSelector}
|
spec:${nodeSelector}
|
||||||
|
serviceAccountName: ${WORKLOAD_SERVICE_ACCOUNT}
|
||||||
containers:
|
containers:
|
||||||
- name: kandev-agent
|
- name: kandev-agent
|
||||||
image: ${overrides.image ?? image}
|
image: ${overrides.image ?? image}
|
||||||
|
|||||||
@@ -92,7 +92,10 @@ test("proves Kubernetes API compatibility and an agentctl round-trip", async ({
|
|||||||
JSON.stringify(exactPod.spec ?? {}).includes(credential),
|
JSON.stringify(exactPod.spec ?? {}).includes(credential),
|
||||||
"agent Pod spec contains host kubeconfig credential: false expected",
|
"agent Pod spec contains host kubeconfig credential: false expected",
|
||||||
).toBe(false);
|
).toBe(false);
|
||||||
expect(exactPod.spec?.automountServiceAccountToken).toBe(false);
|
expect(exactPod.spec).toMatchObject({
|
||||||
|
serviceAccountName: "kandev-workload",
|
||||||
|
automountServiceAccountToken: false,
|
||||||
|
});
|
||||||
expect(
|
expect(
|
||||||
kubernetesContainerLogsContainCredential(cluster, exactPod, credential),
|
kubernetesContainerLogsContainCredential(cluster, exactPod, credential),
|
||||||
"agent Pod logs contain host kubeconfig credential: false expected",
|
"agent Pod logs contain host kubeconfig credential: false expected",
|
||||||
|
|||||||
@@ -0,0 +1,111 @@
|
|||||||
|
import fs from "node:fs";
|
||||||
|
import { test, expect, kubernetesProfileConfig } from "../../fixtures/kubernetes-test-base";
|
||||||
|
import {
|
||||||
|
execInKubernetesPod,
|
||||||
|
waitForKubernetesPod,
|
||||||
|
waitForKubernetesPVC,
|
||||||
|
waitForKubernetesResourceAbsent,
|
||||||
|
waitForTaskSessionState,
|
||||||
|
} from "../../helpers/kubernetes";
|
||||||
|
import { waitForAgentMessage, waitForSessionDone } from "../../helpers/session";
|
||||||
|
import { SessionPage } from "../../pages/session-page";
|
||||||
|
|
||||||
|
// Repeated real-cluster setup can exhaust the job before failure artifacts are written.
|
||||||
|
test.describe.configure({ retries: 0 });
|
||||||
|
|
||||||
|
// @covers AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.1 through .4
|
||||||
|
for (const restart of [false, true]) {
|
||||||
|
const title = restart
|
||||||
|
? "preserves Kubernetes workspace after recoverable agent error and backend restart"
|
||||||
|
: "preserves Kubernetes workspace after recoverable agent error";
|
||||||
|
test(title, async ({ apiClient, seedData, cluster, backend, testPage }) => {
|
||||||
|
test.setTimeout(360_000);
|
||||||
|
const profile = await apiClient.createExecutorProfile(seedData.executorId, {
|
||||||
|
name: "Recoverable failure managed workspace",
|
||||||
|
config: kubernetesProfileConfig(cluster, {
|
||||||
|
"workspace.mode": "managed_pvc",
|
||||||
|
"workspace.size": "1Gi",
|
||||||
|
"workspace.access_modes": JSON.stringify(["ReadWriteOnce"]),
|
||||||
|
}),
|
||||||
|
prepare_script: "",
|
||||||
|
cleanup_script: "",
|
||||||
|
env_vars: [],
|
||||||
|
});
|
||||||
|
try {
|
||||||
|
await apiClient.saveUserSettings({ prevent_auto_start_agent_on_open: true });
|
||||||
|
const task = await apiClient.createTaskWithAgent(
|
||||||
|
seedData.workspaceId,
|
||||||
|
"Kubernetes failure recovery",
|
||||||
|
seedData.agentProfileId,
|
||||||
|
{
|
||||||
|
description: "/e2e:simple-message",
|
||||||
|
workflow_id: seedData.workflowId,
|
||||||
|
workflow_step_id: seedData.startStepId,
|
||||||
|
executor_id: seedData.executorId,
|
||||||
|
executor_profile_id: profile.id,
|
||||||
|
},
|
||||||
|
);
|
||||||
|
const sessionId = task.session_id!;
|
||||||
|
expect(sessionId).toBeTruthy();
|
||||||
|
await waitForSessionDone(apiClient, task.id, sessionId, "Initial Kubernetes turn");
|
||||||
|
const pod = await waitForKubernetesPod(cluster, task.id, sessionId);
|
||||||
|
const claim = await waitForKubernetesPVC(cluster, task.id, sessionId);
|
||||||
|
execInKubernetesPod(cluster, pod.metadata.name, [
|
||||||
|
"/bin/sh",
|
||||||
|
"-c",
|
||||||
|
"printf retained > /workspace/recovery-sentinel",
|
||||||
|
]);
|
||||||
|
const session = new SessionPage(testPage);
|
||||||
|
const logOffset = fs.readFileSync(backend.logPath, "utf8").length;
|
||||||
|
await apiClient.addUserMessage(task.id, sessionId, "/transport-lost");
|
||||||
|
await waitForTaskSessionState(apiClient, task.id, sessionId, "WAITING_FOR_INPUT");
|
||||||
|
await expect
|
||||||
|
.poll(
|
||||||
|
() =>
|
||||||
|
fs
|
||||||
|
.readFileSync(backend.logPath, "utf8")
|
||||||
|
.slice(logOffset)
|
||||||
|
.split("\n")
|
||||||
|
.some(
|
||||||
|
(line) =>
|
||||||
|
line.includes(task.id) &&
|
||||||
|
line.includes("agent stopped and removed from tracking"),
|
||||||
|
),
|
||||||
|
{
|
||||||
|
timeout: 30_000,
|
||||||
|
message: "Waiting for failed execution cleanup before checking retention",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.toBe(true);
|
||||||
|
expect((await waitForKubernetesPod(cluster, task.id, sessionId)).metadata.uid).toBe(
|
||||||
|
pod.metadata.uid,
|
||||||
|
);
|
||||||
|
expect((await waitForKubernetesPVC(cluster, task.id, sessionId)).metadata.uid).toBe(
|
||||||
|
claim.metadata.uid,
|
||||||
|
);
|
||||||
|
if (restart) await backend.restart();
|
||||||
|
await testPage.goto(`/t/${task.id}`);
|
||||||
|
await session.waitForLoad();
|
||||||
|
await expect(session.recoveryResumeButton()).toBeVisible({ timeout: 30_000 });
|
||||||
|
await session.recoveryResumeButton().click({ timeout: 30_000 });
|
||||||
|
await session.waitForChatIdle({ timeout: 90_000, requireEditable: true });
|
||||||
|
await waitForTaskSessionState(apiClient, task.id, sessionId, "WAITING_FOR_INPUT", 90_000);
|
||||||
|
const reply = restart ? "recovered-after-restart" : "recovered-after-failure";
|
||||||
|
await apiClient.addUserMessage(task.id, sessionId, `e2e:message("${reply}")`);
|
||||||
|
await waitForAgentMessage(apiClient, sessionId, reply);
|
||||||
|
await waitForSessionDone(apiClient, task.id, sessionId, "Recovered Kubernetes turn");
|
||||||
|
expect((await waitForKubernetesPod(cluster, task.id, sessionId)).metadata.uid).toBe(
|
||||||
|
pod.metadata.uid,
|
||||||
|
);
|
||||||
|
expect(
|
||||||
|
execInKubernetesPod(cluster, pod.metadata.name, ["cat", "/workspace/recovery-sentinel"]),
|
||||||
|
).toBe("retained");
|
||||||
|
await apiClient.archiveTask(task.id);
|
||||||
|
await waitForKubernetesResourceAbsent(cluster, "pod", pod.metadata.name);
|
||||||
|
await waitForKubernetesResourceAbsent(cluster, "persistentvolumeclaim", claim.metadata.name);
|
||||||
|
} finally {
|
||||||
|
await apiClient.saveUserSettings({ prevent_auto_start_agent_on_open: false });
|
||||||
|
await apiClient.deleteExecutorProfile(profile.id);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
}
|
||||||
@@ -0,0 +1,176 @@
|
|||||||
|
---
|
||||||
|
created: 2026-09-19
|
||||||
|
status: blocked
|
||||||
|
requirements:
|
||||||
|
- REQ-EXECUTORS-K8S-FAILURE-RECOVERY-001
|
||||||
|
system_design:
|
||||||
|
- ../../specs/executors/system-design/kubernetes-failure-recovery.md
|
||||||
|
legacy_specs:
|
||||||
|
- ../../specs/kubernetes-executor/spec.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# Implementation Plan: Kubernetes Recoverable Failure Cleanup
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
|
||||||
|
Preserve the runtime needed by Resume after a recoverable Kubernetes agent
|
||||||
|
failure. One sequential work order covers the cross-boundary regression and fix.
|
||||||
|
Implementation was authorized by the user after the design handoff.
|
||||||
|
|
||||||
|
## Evidence and root cause
|
||||||
|
|
||||||
|
Read-only backend diagnostic bundle `84da3c3c3d03360d63cbfda4f5e1e579`
|
||||||
|
reported ready with no warnings or lost log entries. Running version:
|
||||||
|
`v0.94.0-247-ge3ed7bfd5`; inspected checkout: `794773dd6`.
|
||||||
|
Affected task: `73dca427-4819-4ac5-9b44-4ac65a53a486`, session:
|
||||||
|
`c4a5774f-a68e-4f16-91f3-286901e75f8b`.
|
||||||
|
|
||||||
|
Retained `backend-logs.log` timestamps on 2026-09-19, UTC+01:00:
|
||||||
|
|
||||||
|
1. 13:10:41: backend shutdown stops initial Kubernetes execution `30c4651b...`
|
||||||
|
without force. At 13:19:27, execution `3767871c...` resumes successfully.
|
||||||
|
2. 14:30:03: agent reports `Internal error`; orchestrator handles it as
|
||||||
|
recoverable, then stops execution with `reason=agent completed, force=true`.
|
||||||
|
3. 14:30:04: stop finishes successfully. At 14:31:11 idle reclaim preserves
|
||||||
|
the runtime row for resume.
|
||||||
|
4. 14:35:58: relaunch fails resolving `env_secret_id_AGENTCTL_AUTH_TOKEN`,
|
||||||
|
referencing the original `kandev-runtime:30c4651b-7325-4e22-b2e4-eb8d1363ebd6:agentctl-auth`.
|
||||||
|
|
||||||
|
Source trace: `handleRecoverableFailureLockedState` ->
|
||||||
|
`cleanupAgentExecution` -> forced `StopExecution` ->
|
||||||
|
`StopAgentWithReason` -> `KubernetesExecutor.StopInstance` -> exact resource
|
||||||
|
deletion and `deleteKubernetesRuntimeSecrets`. The old resource-instance ID is
|
||||||
|
expected across resume; its reuse alone is not the defect. Destructive cleanup
|
||||||
|
while promising recovery is the defect.
|
||||||
|
|
||||||
|
The bundle proves the cleanup request and subsequent missing secret. It does
|
||||||
|
not establish the PVC storage mode, current cluster state, or recoverability of
|
||||||
|
files. The original provider `Internal error` remains unexplained by these logs.
|
||||||
|
No live runtime or database was modified during diagnosis.
|
||||||
|
|
||||||
|
## Scope
|
||||||
|
|
||||||
|
### In scope
|
||||||
|
|
||||||
|
Recoverable established-session failure teardown, exact-execution concurrency,
|
||||||
|
credential/resource retention, and real error-to-Resume verification.
|
||||||
|
|
||||||
|
### Out of scope
|
||||||
|
|
||||||
|
Repairing this historical session's data, masking missing secrets, changing
|
||||||
|
provider routing, fresh launch rollback, or changing other runtimes. UI cleanup
|
||||||
|
is sibling task `e8757499-cfd8-4715-8b04-81fb7c1f3749`, created without a parent.
|
||||||
|
|
||||||
|
## Technical approach
|
||||||
|
|
||||||
|
Implement the [design](../../specs/executors/system-design/kubernetes-failure-recovery.md)
|
||||||
|
using a specific recoverable-failure stop reason and lifecycle-owned Kubernetes
|
||||||
|
retention. Keep generic forced cleanup and explicit archive/delete behavior.
|
||||||
|
No schema or public API changes. Reuse existing teardown claims rather than
|
||||||
|
introducing a second cleanup coordinator.
|
||||||
|
|
||||||
|
## Tests
|
||||||
|
|
||||||
|
New `event_handlers_kubernetes_recovery_test.go` in orchestrator:
|
||||||
|
`TestKubernetesRecoverableFailurePreservesResume`, covering criteria .1-.2;
|
||||||
|
`TestKubernetesRecoverableFailureCleanupBeforeWorkflowResume`, covering .3,
|
||||||
|
including publisher return and immediate workflow-triggered resume. Duplicate
|
||||||
|
cleanup with persisted inventory is covered at the lifecycle boundary.
|
||||||
|
|
||||||
|
New `manager_kubernetes_recoverable_failure_test.go` in lifecycle:
|
||||||
|
`TestKubernetesRecoverableFailurePreservesRuntimeAndSecrets`, covering .1-.2;
|
||||||
|
destructive-stop and non-Kubernetes controls for .4; existing auth resolution
|
||||||
|
tests for .5. Use fresh and resumed established executions in the matrix.
|
||||||
|
|
||||||
|
## E2E tests
|
||||||
|
|
||||||
|
Add `apps/web/e2e/tests/kubernetes/kubernetes-failure-recovery.spec.ts` in
|
||||||
|
`containers` with `preserves Kubernetes workspace after recoverable agent error`.
|
||||||
|
Use the existing mock error scenario, a sentinel file, existing Resume control,
|
||||||
|
exact Pod/PVC identities, restart, and archive assertions. Covers .1-.4.
|
||||||
|
No component markup or mobile composition changes are planned.
|
||||||
|
|
||||||
|
## Work orders
|
||||||
|
|
||||||
|
- [ ] [Task 01: Preserve Kubernetes recoverable runtimes](task-01-preserve-recoverable-runtime.md)
|
||||||
|
|
||||||
|
## Verification results
|
||||||
|
|
||||||
|
Initial orchestrator and lifecycle regression failures reproduced destructive
|
||||||
|
cleanup. A blocked-stop regression reproduced workflow Resume preceding cleanup;
|
||||||
|
a publisher-return assertion reproduced synchronous cleanup blocking publication.
|
||||||
|
The retained-inventory duplicate regression reproduced fallback into destructive
|
||||||
|
persisted cleanup. Those focused tests now pass. The existing dynamic successor
|
||||||
|
failure test exposed a session-guard deadlock; its caller now defers cleanup.
|
||||||
|
Final race-enabled tests passed for both packages, including a rerun after the
|
||||||
|
last production change. Kind verification is blocked by local fixture setup:
|
||||||
|
|
||||||
|
- Initial managed build was rejected as stale after a production edit. Rebuilt
|
||||||
|
the required Linux artifacts and reused the completed Vite/plugin builds.
|
||||||
|
- First refreshed run failed before the test body: `kind load docker-image`
|
||||||
|
exceeded the fixture's 180-second timeout.
|
||||||
|
- One retry with `E2E_DEBUG=1` reused image layers but failed creating the Kind
|
||||||
|
node: no log line matched `Reached target .*Multi-User System.*|detected cgroup v1`.
|
||||||
|
- Both attempts discovered one test. Neither executed the recovery assertions.
|
||||||
|
Fixture cleanup removed their owned clusters and image tags. Do not count
|
||||||
|
either run as evidence for Pod/PVC retention or backend-restart recovery.
|
||||||
|
|
||||||
|
Focused Go race checks, targeted Go lint (0 issues), ESLint, Prettier,
|
||||||
|
architecture lint, and documentation
|
||||||
|
validation passed. The remaining gate is a successful Kind scenario on a
|
||||||
|
working local/CI container runner. The user explicitly requested a ready PR
|
||||||
|
with this known integration-verification blocker disclosed.
|
||||||
|
Read-only lifecycle trace: confirmed the destructive failure-cleanup path.
|
||||||
|
`python3 scripts/list-docs.py validate`: passed (291 decisions, 1040 specifications).
|
||||||
|
`python3 scripts/lint-spec-files.test.py`: passed (36 tests).
|
||||||
|
`python3 scripts/lint-spec-files.py --all`: passed.
|
||||||
|
`git diff --check`: passed.
|
||||||
|
|
||||||
|
## Risks
|
||||||
|
|
||||||
|
- Already-deleted credentials and managed workspace data cannot be restored by
|
||||||
|
this patch. Current affected-session recovery requires separate evidence.
|
||||||
|
- Late cleanup can race immediate Resume; preserving secrets alone is insufficient.
|
||||||
|
- A global change to force cleanup can leak runtimes or weaken explicit cleanup.
|
||||||
|
- Kind validation requires Docker and disposable resources; never use the
|
||||||
|
affected live cluster for reproduction.
|
||||||
|
|
||||||
|
|
||||||
|
## PR review remediation
|
||||||
|
|
||||||
|
- Cleanup failures now block workflow recovery and release their exact claim for
|
||||||
|
retry. Completed teardown claims allow deferred workflow redelivery without
|
||||||
|
repeating a stop; in-flight duplicates cannot bypass cleanup.
|
||||||
|
- Recovery workers share service shutdown ownership with dynamic successors.
|
||||||
|
Cancellation suppresses workflow Resume, and shutdown drains tracked workers.
|
||||||
|
- Authentication and managed-NPM startup failures use bootstrap cleanup, keeping
|
||||||
|
fresh-launch rollback destructive and resumed-bootstrap retention intact.
|
||||||
|
- Async workflow route, cancellation, reconciliation, and panic tests now await
|
||||||
|
worker completion. SQLite-backed recovery tests use bounded real-time signals.
|
||||||
|
- Full affected-package race suites passed:
|
||||||
|
`go test -race ./internal/orchestrator ./internal/agent/runtime/lifecycle ./internal/orchestrator/executor -count=1 -timeout=15m`.
|
||||||
|
This includes every backend CI assertion that failed on the opening head.
|
||||||
|
- Container CI reached the new test and exposed an invalid failure injection:
|
||||||
|
`/e2e:error` emits text but does not fail ACP. The test now uses the existing
|
||||||
|
`/transport-lost` prompt error in separate fresh-session cases with and without
|
||||||
|
backend restart. A retries-disabled local run used rebuilt backend, Linux
|
||||||
|
helpers, and plugin artifacts, but again hit the Kind image-load timeout before
|
||||||
|
entering either assertion path. The run was interrupted during fixture cleanup;
|
||||||
|
CI's Kubernetes runner remains the integration gate. Docker also reported that
|
||||||
|
it could not immediately kill the owned Kind node; it subsequently exited and
|
||||||
|
was removed explicitly. The owned image was already removed by fixture cleanup.
|
||||||
|
No recovery assertion is counted as passing from this local attempt.
|
||||||
|
|
||||||
|
Architecture lint rejected direct lifecycle imports in new callers; stop reasons
|
||||||
|
and missing-execution checks now use the public runtime facade. After that
|
||||||
|
adjustment, race tests for Kubernetes recovery, dynamic failure, all agent-error
|
||||||
|
workflow routes, runtime missing-execution handling, and stop paths passed across
|
||||||
|
orchestrator, runtime, and lifecycle. Architecture lint passed.
|
||||||
|
|
||||||
|
A subsequent review exposed the same already-absent outcome in transient retry
|
||||||
|
teardown. The R5 workflow-route regression failed for a missing execution, then
|
||||||
|
passed after the transient path adopted the runtime facade's `IsNotFound` check.
|
||||||
|
Success, already-absent, and real stop-error controls now run together. The race
|
||||||
|
command `go test -race ./internal/orchestrator -run 'TestDispatchKanbanAgentErrorTrigger|TestKubernetesRecoverableFailure|Test.*TransientRetry' -count=1 -timeout=180s`
|
||||||
|
passed. Cleanup ownership and synchronization helpers also received explanatory
|
||||||
|
comments in response to the standalone documentation-coverage suggestion.
|
||||||
+174
@@ -0,0 +1,174 @@
|
|||||||
|
---
|
||||||
|
id: "01-preserve-recoverable-runtime"
|
||||||
|
title: "Preserve Kubernetes recoverable runtimes"
|
||||||
|
status: blocked
|
||||||
|
wave: 1
|
||||||
|
depends_on: []
|
||||||
|
plan: "plan.md"
|
||||||
|
requirements:
|
||||||
|
- REQ-EXECUTORS-K8S-FAILURE-RECOVERY-001
|
||||||
|
acceptance_criteria:
|
||||||
|
- AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.1
|
||||||
|
- AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.2
|
||||||
|
- AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.3
|
||||||
|
- AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.4
|
||||||
|
- AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.5
|
||||||
|
system_design:
|
||||||
|
- ../../specs/executors/system-design/kubernetes-failure-recovery.md
|
||||||
|
---
|
||||||
|
|
||||||
|
# Task 01: Preserve Kubernetes Recoverable Runtimes
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
Separate recoverable agent-failure teardown from destructive runtime cleanup.
|
||||||
|
Keep existing Kubernetes resources and secret values available to Resume while
|
||||||
|
stopping the failed agent and releasing its local execution slot.
|
||||||
|
|
||||||
|
## In scope
|
||||||
|
|
||||||
|
- Add the orchestrator regression first; prove the existing handler reaches
|
||||||
|
destructive Kubernetes cleanup. Cover fresh and resumed established sessions.
|
||||||
|
- Route recoverable failure through a distinct stop reason, preserving existing
|
||||||
|
exact-execution claims, stream retirement, and non-Kubernetes behavior.
|
||||||
|
- Apply Kubernetes retention at the lifecycle boundary and prove secret values,
|
||||||
|
references, inventory, and remote resources survive.
|
||||||
|
- Block cleanup in a deterministic test while a replacement is requested;
|
||||||
|
cover duplicate events and synchronous workflow restart without deadlock or
|
||||||
|
predecessor teardown affecting the replacement.
|
||||||
|
- Add the Kind scenario named in the plan and verify archive cleanup afterward.
|
||||||
|
|
||||||
|
## Out of scope
|
||||||
|
|
||||||
|
Live data mutation, recovery-token regeneration, UI redesign, new schema, broad
|
||||||
|
cleanup refactoring, and unexplained provider failures.
|
||||||
|
|
||||||
|
## Acceptance
|
||||||
|
|
||||||
|
1. The regression fails before the correction because recoverable failure
|
||||||
|
requests destructive Kubernetes teardown, and passes after it.
|
||||||
|
2. Recovery retains the same workspace and credentials across retry/restart;
|
||||||
|
late or duplicate failures cannot damage the replacement execution.
|
||||||
|
3. Explicit destructive cleanup, missing-secret fail-closed behavior, and
|
||||||
|
existing non-Kubernetes teardown remain covered and passing.
|
||||||
|
|
||||||
|
## Verification
|
||||||
|
|
||||||
|
From the repository root; install workspace dependencies before the first pnpm
|
||||||
|
command if this worktree has not been installed. Docker is required for Kind.
|
||||||
|
|
||||||
|
```bash
|
||||||
|
(cd apps && pnpm install --frozen-lockfile)
|
||||||
|
(cd apps/backend && go test -race ./internal/orchestrator -run 'Test(KubernetesRecoverableFailure|HandleRecoverableFailure|HandleAgentFailed|HandleAgentStartFailed|RunDetachedDynamicSuccessorLaunch|ClaimForcedExecutionCleanup|RegisterExecutionStopOwner|HandleAgentStopped)' -count=1)
|
||||||
|
(cd apps/backend && go test -race ./internal/agent/runtime/lifecycle -run 'Test.*(Kubernetes.*(Failure|Stop|Cleanup|Resume)|ResolveLaunchAuthToken|PersistedKubernetesInventory)' -count=1)
|
||||||
|
(cd apps/web && KANDEV_E2E_CONTAINERS=1 pnpm e2e:run --host --project containers tests/kubernetes/kubernetes-failure-recovery.spec.ts)
|
||||||
|
python3 scripts/list-docs.py validate
|
||||||
|
python3 scripts/lint-spec-files.py --all
|
||||||
|
git diff --check
|
||||||
|
```
|
||||||
|
|
||||||
|
## Files likely touched
|
||||||
|
|
||||||
|
- `apps/backend/internal/orchestrator/event_handlers_agent.go`
|
||||||
|
- `apps/backend/internal/orchestrator/dynamic_launch.go`
|
||||||
|
- `apps/backend/internal/orchestrator/event_handlers_kubernetes_recovery_test.go` (new)
|
||||||
|
- `apps/backend/internal/orchestrator/execution_teardown_ownership_test.go`
|
||||||
|
- `apps/backend/internal/agent/runtime/lifecycle/manager_interaction.go`
|
||||||
|
- `apps/backend/internal/agent/runtime/lifecycle/manager_kubernetes_restart_cleanup.go`
|
||||||
|
- `apps/backend/internal/agent/runtime/lifecycle/recovery_stop.go` (recoverable stop reason)
|
||||||
|
- `apps/backend/internal/orchestrator/execution_failure_cleanup.go`
|
||||||
|
- `apps/backend/internal/agent/runtime/lifecycle/manager_kubernetes_recoverable_failure_test.go` (new)
|
||||||
|
- `apps/backend/internal/agent/runtime/lifecycle/manager_kubernetes_resume_bootstrap_cleanup_test.go`
|
||||||
|
- `apps/web/e2e/tests/kubernetes/kubernetes-failure-recovery.spec.ts`
|
||||||
|
|
||||||
|
## Dependencies
|
||||||
|
|
||||||
|
None. Execute after the explicit implementation request.
|
||||||
|
|
||||||
|
## Risks
|
||||||
|
|
||||||
|
The failure handler dispatches workflow callbacks that can immediately resume.
|
||||||
|
Respect existing cancellation/teardown ordering; do not hold a session guard
|
||||||
|
across a callback that needs the same guard. Do not change the shared helper's
|
||||||
|
default behavior for terminal or orphan callers.
|
||||||
|
|
||||||
|
## Parallelism
|
||||||
|
|
||||||
|
`sequential`
|
||||||
|
|
||||||
|
## Inputs
|
||||||
|
|
||||||
|
- [Requirements](../../specs/executors/requirements/kubernetes-failure-recovery.md)
|
||||||
|
- [System design](../../specs/executors/system-design/kubernetes-failure-recovery.md)
|
||||||
|
- [Evidence and source trace](plan.md#evidence-and-root-cause)
|
||||||
|
- Existing `newAgentErrorTestService`, teardown-claim tests, failed-resume
|
||||||
|
bootstrap retention tests, and Kubernetes Kind fixtures.
|
||||||
|
|
||||||
|
## Results
|
||||||
|
|
||||||
|
Implementation complete; integration verification blocked. The destructive-stop, cleanup ordering, publisher
|
||||||
|
return, and persisted-inventory duplicate regressions were observed failing and
|
||||||
|
then passing. Final checks include the existing dynamic successor failure tests,
|
||||||
|
which exposed and drove correction of another guarded cleanup caller.
|
||||||
|
Focused non-race regressions plus dynamic failure tests passed after the final
|
||||||
|
production edit. Race-enabled orchestrator/lifecycle checks passed, including
|
||||||
|
recoverable failures, event handling, teardown claims, Kubernetes cleanup and
|
||||||
|
resume controls, persisted cleanup, and auth resolution. A final race-enabled
|
||||||
|
rerun of `TestKubernetesRecoverableFailure|TestRunDetachedDynamicSuccessorLaunch`
|
||||||
|
passed for both packages.
|
||||||
|
|
||||||
|
Kind verification was attempted twice against refreshed artifacts. The first
|
||||||
|
failed during image loading (180-second fixture timeout); a debug retry failed
|
||||||
|
while creating the Kind node (missing expected systemd boot log). Neither
|
||||||
|
entered the test body. Owned clusters and image tags were cleaned up. Rerun the
|
||||||
|
same managed `containers` command on a working Kind runner before completing
|
||||||
|
this work order. The user explicitly authorized committing and opening a ready
|
||||||
|
PR with the unresolved integration verification disclosed.
|
||||||
|
|
||||||
|
The shared Go build cache lost entries during the first race attempt. Successful
|
||||||
|
runs used `GOCACHE=/tmp/kandev-recovery-go-cache`; the failures were build errors,
|
||||||
|
not test assertions.
|
||||||
|
|
||||||
|
Targeted Go lint passed with 0 issues. The E2E file passed ESLint and Prettier.
|
||||||
|
Architecture and documentation validators passed. Final Docker inspection
|
||||||
|
confirmed no owned test containers or runtime image tags remained.
|
||||||
|
|
||||||
|
|
||||||
|
## PR review remediation
|
||||||
|
|
||||||
|
- Cleanup failures now block workflow recovery and release their exact claim for
|
||||||
|
retry. Completed teardown claims allow deferred workflow redelivery without
|
||||||
|
repeating a stop; in-flight duplicates cannot bypass cleanup.
|
||||||
|
- Recovery workers share service shutdown ownership with dynamic successors.
|
||||||
|
Cancellation suppresses workflow Resume, and shutdown drains tracked workers.
|
||||||
|
- Authentication and managed-NPM startup failures use bootstrap cleanup, keeping
|
||||||
|
fresh-launch rollback destructive and resumed-bootstrap retention intact.
|
||||||
|
- Async workflow route, cancellation, reconciliation, and panic tests now await
|
||||||
|
worker completion. SQLite-backed recovery tests use bounded real-time signals.
|
||||||
|
- Full affected-package race suites passed:
|
||||||
|
`go test -race ./internal/orchestrator ./internal/agent/runtime/lifecycle ./internal/orchestrator/executor -count=1 -timeout=15m`.
|
||||||
|
This includes every backend CI assertion that failed on the opening head.
|
||||||
|
- Container CI reached the new test and exposed an invalid failure injection:
|
||||||
|
`/e2e:error` emits text but does not fail ACP. The test now uses the existing
|
||||||
|
`/transport-lost` prompt error in separate fresh-session cases with and without
|
||||||
|
backend restart. A retries-disabled local run used rebuilt backend, Linux
|
||||||
|
helpers, and plugin artifacts, but again hit the Kind image-load timeout before
|
||||||
|
entering either assertion path. The run was interrupted during fixture cleanup;
|
||||||
|
CI's Kubernetes runner remains the integration gate. Docker also reported that
|
||||||
|
it could not immediately kill the owned Kind node; it subsequently exited and
|
||||||
|
was removed explicitly. The owned image was already removed by fixture cleanup.
|
||||||
|
No recovery assertion is counted as passing from this local attempt.
|
||||||
|
|
||||||
|
Architecture lint rejected direct lifecycle imports in new callers; stop reasons
|
||||||
|
and missing-execution checks now use the public runtime facade. After that
|
||||||
|
adjustment, race tests for Kubernetes recovery, dynamic failure, all agent-error
|
||||||
|
workflow routes, runtime missing-execution handling, and stop paths passed across
|
||||||
|
orchestrator, runtime, and lifecycle. Architecture lint passed.
|
||||||
|
|
||||||
|
A subsequent review exposed the same already-absent outcome in transient retry
|
||||||
|
teardown. The R5 workflow-route regression failed for a missing execution, then
|
||||||
|
passed after the transient path adopted the runtime facade's `IsNotFound` check.
|
||||||
|
Success, already-absent, and real stop-error controls now run together. The race
|
||||||
|
command `go test -race ./internal/orchestrator -run 'TestDispatchKanbanAgentErrorTrigger|TestKubernetesRecoverableFailure|Test.*TransientRetry' -count=1 -timeout=180s`
|
||||||
|
passed. Cleanup ownership and synchronization helpers also received explanatory
|
||||||
|
comments in response to the standalone documentation-coverage suggestion.
|
||||||
@@ -354,6 +354,12 @@ Current connection configuration and recorded workload configuration serve diffe
|
|||||||
|
|
||||||
Ordinary Stop and backend shutdown close local clients and forwards but preserve the Pod and workspace. Agent or main-container restart keeps the Pod volumes; Kandev performs a new nonce handshake and local port-forward. If a Pod disappears, managed or existing PVC storage can support a replacement Pod after identity checks; `emptyDir` cannot.
|
Ordinary Stop and backend shutdown close local clients and forwards but preserve the Pod and workspace. Agent or main-container restart keeps the Pod volumes; Kandev performs a new nonce handshake and local port-forward. If a Pod disappears, managed or existing PVC storage can support a replacement Pod after identity checks; `emptyDir` cannot.
|
||||||
|
|
||||||
|
A recoverable agent error also preserves the established Pod, workspace, and
|
||||||
|
recovery credentials. Use **Resume** to continue that session. If its retained
|
||||||
|
credentials or storage are missing, recovery fails rather than silently creating
|
||||||
|
an empty replacement workspace. A software update cannot restore resources that
|
||||||
|
were already deleted.
|
||||||
|
|
||||||
Archive/delete terminal cleanup and explicit force cleanup are destructive. Before deletion, Kandev verifies the recorded namespace, name, UID, and complete standard plus `kandev.ai/*` ownership-label set. It then deletes the exact Pod with UID/resource-version preconditions and deletes a PVC only when inventory proves Kandev created that managed claim. Existing claims are never deleted. A missing object is idempotent; an inventory read error, same-name replacement, UID mismatch, missing label, changed label, extra `kandev.ai/*` label, or mismatched create nonce fails closed without deleting the ambiguous object.
|
Archive/delete terminal cleanup and explicit force cleanup are destructive. Before deletion, Kandev verifies the recorded namespace, name, UID, and complete standard plus `kandev.ai/*` ownership-label set. It then deletes the exact Pod with UID/resource-version preconditions and deletes a PVC only when inventory proves Kandev created that managed claim. Existing claims are never deleted. A missing object is idempotent; an inventory read error, same-name replacement, UID mismatch, missing label, changed label, extra `kandev.ai/*` label, or mismatched create nonce fails closed without deleting the ambiguous object.
|
||||||
|
|
||||||
Kandev blocks deleting an executor, or changing an executor into or out of Kubernetes, while runtime inventory still refers to it. Finish normal session cleanup first. Profile deletion does not rewrite or destroy a retained workload because recovery owns the recorded profile ID and snapshot.
|
Kandev blocks deleting an executor, or changing an executor into or out of Kubernetes, while runtime inventory still refers to it. Finish normal session cleanup first. Profile deletion does not rewrite or destroy a retained workload because recovery owns the recorded profile ID and snapshot.
|
||||||
|
|||||||
@@ -0,0 +1,47 @@
|
|||||||
|
---
|
||||||
|
status: draft
|
||||||
|
system: executors
|
||||||
|
created: 2026-09-19
|
||||||
|
owners:
|
||||||
|
- kandev
|
||||||
|
---
|
||||||
|
|
||||||
|
# Kubernetes Failure Recovery
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
|
||||||
|
A failed agent turn must not destroy the Kubernetes workspace that the user is
|
||||||
|
offered to resume. This document defines the recoverable agent-failure case
|
||||||
|
alongside the [Kubernetes foundation](../../kubernetes-executor/spec.md).
|
||||||
|
The executor system owns resource retention; task state remains task-owned.
|
||||||
|
|
||||||
|
## Requirements
|
||||||
|
|
||||||
|
### REQ-EXECUTORS-K8S-FAILURE-RECOVERY-001: Retained recovery resources
|
||||||
|
|
||||||
|
**Intent:** Keep an established Kubernetes session usable after an agent error.
|
||||||
|
|
||||||
|
#### Acceptance criteria
|
||||||
|
|
||||||
|
- **AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.1:** When an established Kubernetes
|
||||||
|
session encounters a recoverable agent failure, stopping its failed execution
|
||||||
|
shall preserve its Pod, workspace, and credentials needed for recovery.
|
||||||
|
- **AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.2:** After that failure, Resume shall
|
||||||
|
reconnect to the retained session and preserve workspace changes, including
|
||||||
|
after a backend restart. Cleanup shall not leave recovery referencing
|
||||||
|
credentials that cleanup itself removed.
|
||||||
|
- **AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.3:** A delayed or repeated failure for
|
||||||
|
an earlier execution shall not stop a newer execution or delete its resources.
|
||||||
|
- **AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.4:** Explicit archive, delete, and
|
||||||
|
destructive cleanup shall retain their existing ownership verification and
|
||||||
|
cleanup behavior. Existing operator-owned claims shall remain untouched.
|
||||||
|
- **AC-EXECUTORS-K8S-FAILURE-RECOVERY-001.5:** Missing or inaccessible retained
|
||||||
|
credentials shall remain an actionable recovery failure; the system shall not
|
||||||
|
silently substitute credentials, adopt another workload, or recreate an empty
|
||||||
|
workspace as if recovery succeeded.
|
||||||
|
|
||||||
|
## Exclusions
|
||||||
|
|
||||||
|
Restoring already-deleted data, changing provider error classification, changing
|
||||||
|
other executors' teardown behavior, and redesigning error presentation are outside
|
||||||
|
this contract. Fresh launch rollback remains governed by the foundation.
|
||||||
@@ -0,0 +1,85 @@
|
|||||||
|
---
|
||||||
|
status: draft
|
||||||
|
system: executors
|
||||||
|
requirements:
|
||||||
|
- REQ-EXECUTORS-K8S-FAILURE-RECOVERY-001
|
||||||
|
---
|
||||||
|
|
||||||
|
# Kubernetes Failure Recovery Design
|
||||||
|
|
||||||
|
## Boundary
|
||||||
|
|
||||||
|
Implements [retained failure recovery](../requirements/kubernetes-failure-recovery.md)
|
||||||
|
within the existing [resource ownership decision](../../../decisions/2026-08-24-kubernetes-executor-resource-ownership.md).
|
||||||
|
No new storage, credential rotation, public endpoint, or resource ownership rule
|
||||||
|
is needed. The foundation's inventory and identity checks remain authoritative.
|
||||||
|
|
||||||
|
## Failure teardown
|
||||||
|
|
||||||
|
`handleRecoverableFailureLockedState` currently schedules
|
||||||
|
`cleanupAgentExecution`, which calls `StopExecution` with reason `agent completed`
|
||||||
|
and `force=true`. Kubernetes interprets force as destructive Pod/PVC teardown;
|
||||||
|
`StopAgentWithReason` subsequently deletes authentication and bootstrap secrets.
|
||||||
|
The recoverable task state and retained inventory then disagree with reality.
|
||||||
|
|
||||||
|
Use a distinct semantic reason for recoverable agent-failure teardown. Keep the
|
||||||
|
existing exact-execution ownership claim and activity retirement. Do not globally
|
||||||
|
change the shared cleanup helper's force setting: it also serves terminal,
|
||||||
|
orphan, and stale-resume callers.
|
||||||
|
|
||||||
|
At the lifecycle boundary, use the recorded execution runtime to make this
|
||||||
|
specific reason non-destructive for Kubernetes, whether the established
|
||||||
|
execution originated from a fresh launch or a resumed session. Stop the failed
|
||||||
|
agent through agentctl with a bounded cleanup context, close local connections,
|
||||||
|
and release the in-memory execution slot. Preserve authoritative runtime
|
||||||
|
inventory and both secret references and values. Other runtimes retain their
|
||||||
|
existing force behavior. Explicit destructive reasons retain priority.
|
||||||
|
|
||||||
|
The existing failed-resume-bootstrap retention path in
|
||||||
|
`manager_kubernetes_resume_bootstrap_cleanup_test.go` provides the nearest
|
||||||
|
pattern. Startup authentication and managed-NPM failures carry the bootstrap
|
||||||
|
stop reason, so resumed startup retains resources while fresh-launch rollback
|
||||||
|
remains destructive.
|
||||||
|
|
||||||
|
## Concurrency and failure handling
|
||||||
|
|
||||||
|
Retain the exact `(session, execution)` teardown claim and stale-event guards.
|
||||||
|
Failure cleanup and a subsequent resume must be ordered so predecessor cleanup
|
||||||
|
cannot tear down the newly attached execution or its shared retained Pod. Cover
|
||||||
|
this ordering with a blocked-stop test, including synchronous `on_agent_error`
|
||||||
|
restart. Run cleanup and its subsequent workflow callback together after releasing
|
||||||
|
the session guard, on a service-owned worker: lifecycle failure publication can
|
||||||
|
hold the prompt lock until its subscribers return. Do not add a session-wide
|
||||||
|
lock across callbacks that reacquire it. A repeated recoverable stop for an
|
||||||
|
execution no longer tracked in memory must not enter destructive persisted
|
||||||
|
Kubernetes cleanup.
|
||||||
|
|
||||||
|
Keep secret lookup failures fail-closed in `resolveLaunchAuthToken`.
|
||||||
|
Regenerating a secret would not restore a deleted PVC or authenticate to an
|
||||||
|
already-running agentctl. Do not silently migrate broken historical sessions.
|
||||||
|
Cleanup errors release only the exact failed teardown claim so a later attempt
|
||||||
|
can retry; they suppress workflow dispatch. A successful teardown records
|
||||||
|
completion on its claim, allowing workflow redelivery without repeating cleanup.
|
||||||
|
An in-flight duplicate cannot dispatch ahead of the owning cleanup. An already
|
||||||
|
absent execution is a successful cleanup outcome.
|
||||||
|
|
||||||
|
Recovery workers share the dynamic-successor cancellation context and wait group.
|
||||||
|
Shutdown rejects new workers, cancels active work, and drains within the existing
|
||||||
|
shutdown bound. Workflow recovery checks cancellation after cleanup and receives
|
||||||
|
the worker context, so a stop finishing during shutdown cannot start Resume.
|
||||||
|
|
||||||
|
## Validation
|
||||||
|
|
||||||
|
Map criteria .1-.3 to orchestrator failure-handler, teardown ownership, and
|
||||||
|
lifecycle tests. Verify .4 with existing exact Kubernetes cleanup tests plus
|
||||||
|
explicit destructive-stop controls. Verify .5 with secret-store missing/error
|
||||||
|
tests. A Kind scenario must write a workspace sentinel, inject an agent error,
|
||||||
|
resume, and prove unchanged Pod/PVC identity and sentinel contents. Repeat after
|
||||||
|
a backend restart; archive at the end to prove cleanup remains available.
|
||||||
|
|
||||||
|
No rendered interface changes are required. The existing Resume control is the
|
||||||
|
user-visible entry point. The separate error-UI task owns presentation changes.
|
||||||
|
|
||||||
|
## Implementation plan
|
||||||
|
|
||||||
|
[Kubernetes recoverable failure cleanup](../../../plans/kubernetes-recoverable-failure-cleanup/plan.md)
|
||||||
Reference in New Issue
Block a user