diff --git a/apps/backend/internal/agent/runtime/lifecycle/activity_test.go b/apps/backend/internal/agent/runtime/lifecycle/activity_test.go index 0704959ee3..180de9bc92 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/activity_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/activity_test.go @@ -142,30 +142,128 @@ func TestStartAgentProcessFailureReleasesTrackedExecutionActivity(t *testing.T) } } -func TestInitialPromptWaitFailureRetainsExecutionActivity(t *testing.T) { +func TestInitialPromptFailureMarksExecutionFailedAndReleasesActivity(t *testing.T) { manager := newTestManager(t) coordinator := activity.NewCoordinator(activity.Options{}) manager.SetActivityCoordinator(coordinator) + execution := &AgentExecution{ + ID: "execution-prompt-failure", + TaskID: "task-prompt-failure", + SessionID: "session-prompt-failure", + Status: v1.AgentStatusRunning, + } + if err := manager.executionStore.Add(execution); err != nil { + t.Fatalf("Add execution: %v", err) + } lease, err := coordinator.AcquireTask(context.Background(), activity.KindExecutionRunning) if err != nil { t.Fatal(err) } - manager.trackActivity(executionActivityKey("execution-prompt-wait"), lease) + manager.trackActivity(executionActivityKey(execution.ID), lease) - manager.sessionManager.SetInitialPromptFailureHandler(func(executionID string) { - if executionID != "execution-prompt-wait" { - t.Errorf("initial prompt failure execution ID = %q", executionID) - } + if manager.sessionManager.initialPromptFailure == nil { + t.Fatal("NewManager did not wire the initial prompt failure handler") + } + manager.sessionManager.initialPromptFailure(InitialPromptFailure{ + ExecutionID: execution.ID, + SessionID: execution.SessionID, + Err: errors.New("attachment materialization failed"), }) - manager.sessionManager.initialPromptFailure("execution-prompt-wait") + if execution.Status != v1.AgentStatusFailed { + t.Fatalf("execution status = %q, want %q", execution.Status, v1.AgentStatusFailed) + } + if execution.ErrorMessage != "initial prompt delivery failed" { + t.Fatalf("execution error = %q, want safe prompt failure", execution.ErrorMessage) + } maintenance, _, err := coordinator.TryAcquireMaintenance(context.Background(), 0) if maintenance != nil { maintenance.Release() } - if !errors.Is(err, activity.ErrBusy) { - t.Fatalf("maintenance error = %v, want ErrBusy while execution may still run", err) + if err != nil { + t.Fatalf("maintenance after prompt failure: %v", err) + } +} + +func TestInitialPromptFailureCannotFailReplacementExecution(t *testing.T) { + manager := newTestManager(t) + original := &AgentExecution{ + ID: "execution-original", TaskID: "task-1", SessionID: "session-1", + Status: v1.AgentStatusRunning, + } + replacement := &AgentExecution{ + ID: "execution-replacement", TaskID: "task-1", SessionID: "session-1", + Status: v1.AgentStatusRunning, + } + if err := manager.executionStore.Add(original); err != nil { + t.Fatalf("Add original execution: %v", err) + } + manager.executionStore.Remove(original.ID) + if err := manager.executionStore.Add(replacement); err != nil { + t.Fatalf("Add replacement execution: %v", err) + } + + manager.sessionManager.initialPromptFailure(InitialPromptFailure{ + ExecutionID: original.ID, + SessionID: original.SessionID, + Err: errors.New("late prompt failure"), + }) + + if original.Status != v1.AgentStatusRunning { + t.Fatalf("removed original status = %q, want %q", original.Status, v1.AgentStatusRunning) + } + if replacement.Status != v1.AgentStatusRunning { + t.Fatalf("replacement status = %q, want %q", replacement.Status, v1.AgentStatusRunning) + } +} + +func TestInitialPromptFailureCannotFailSuccessorPrompt(t *testing.T) { + manager := newTestManager(t) + execution := &AgentExecution{ + ID: "execution-successor", TaskID: "task-1", SessionID: "session-1", + Status: v1.AgentStatusRunning, + } + if err := manager.executionStore.Add(execution); err != nil { + t.Fatalf("Add execution: %v", err) + } + if _, err := manager.BeginPrompt(execution.ID); err != nil { + t.Fatalf("begin initial prompt: %v", err) + } + if _, err := manager.BeginPrompt(execution.ID); err != nil { + t.Fatalf("begin successor prompt: %v", err) + } + + manager.sessionManager.initialPromptFailure(InitialPromptFailure{ + ExecutionID: execution.ID, + SessionID: execution.SessionID, + PromptGeneration: 1, + Err: errors.New("late initial failure"), + }) + + if execution.Status != v1.AgentStatusRunning { + t.Fatalf("successor status = %q, want %q", execution.Status, v1.AgentStatusRunning) + } +} + +func TestInitialPromptFailureDuringShutdownStopsExecution(t *testing.T) { + manager := newTestManager(t) + execution := &AgentExecution{ + ID: "execution-shutdown", TaskID: "task-1", SessionID: "session-1", + Status: v1.AgentStatusRunning, + } + if err := manager.executionStore.Add(execution); err != nil { + t.Fatalf("Add execution: %v", err) + } + manager.shuttingDown.Store(true) + + manager.sessionManager.initialPromptFailure(InitialPromptFailure{ + ExecutionID: execution.ID, + SessionID: execution.SessionID, + Err: errors.New("transport stopped"), + }) + + if execution.Status != v1.AgentStatusStopped { + t.Fatalf("shutdown status = %q, want %q", execution.Status, v1.AgentStatusStopped) } - manager.releaseActivity(executionActivityKey("execution-prompt-wait")) } func TestReleaseCancelsPendingExecutionActivityAcquire(t *testing.T) { diff --git a/apps/backend/internal/agent/runtime/lifecycle/manager.go b/apps/backend/internal/agent/runtime/lifecycle/manager.go index 90920ef93d..5b333aad36 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/manager.go +++ b/apps/backend/internal/agent/runtime/lifecycle/manager.go @@ -334,6 +334,7 @@ func NewManager( // Set session manager dependencies for full orchestration sessionManager.SetDependencies(eventPublisher, mgr.streamManager, executionStore, historyManager) sessionManager.SetPromptStarter(mgr.BeginPrompt) + sessionManager.SetInitialPromptFailureHandler(mgr.handleInitialPromptFailure) mgr.pollAggregator = newWorkspacePollAggregator(mgr) @@ -344,6 +345,28 @@ func NewManager( return mgr } +func (m *Manager) handleInitialPromptFailure(failure InitialPromptFailure) { + execution, exists := m.executionStore.Get(failure.ExecutionID) + if !exists { + m.logger.Debug("ignoring stale initial prompt delivery failure", + zap.String("execution_id", failure.ExecutionID), + zap.Uint64("prompt_generation", failure.PromptGeneration)) + return + } + settled := m.handleErrorEvent(execution, agentctl.AgentEvent{ + Type: "error", + Error: "initial prompt delivery failed", + SessionID: failure.SessionID, + PromptGeneration: failure.PromptGeneration, + TurnID: failure.TurnID, + }) + if !settled { + m.logger.Debug("ignoring superseded initial prompt delivery failure", + zap.String("execution_id", failure.ExecutionID), + zap.Uint64("prompt_generation", failure.PromptGeneration)) + } +} + // HandleSessionMode routes a session-level mode transition (from the gateway // hub) into the per-workspace aggregator, which pushes the resulting // workspace-effective mode to agentctl. See manager_subscription.go. diff --git a/apps/backend/internal/agent/runtime/lifecycle/session.go b/apps/backend/internal/agent/runtime/lifecycle/session.go index 19adcae847..46839c8814 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/session.go +++ b/apps/backend/internal/agent/runtime/lifecycle/session.go @@ -56,7 +56,7 @@ type SessionManager struct { streamManager *StreamManager executionStore *ExecutionStore promptStarter func(executionID string) (uint64, error) - initialPromptFailure func(executionID string) + initialPromptFailure func(failure InitialPromptFailure) historyManager *SessionHistoryManager attachmentReader AttachmentReader stopCh <-chan struct{} // For graceful shutdown coordination @@ -72,6 +72,21 @@ type SessionManager struct { beforePromptDispatchHook func() } +// InitialPromptFailure captures the immutable prompt identity at the point +// where initial delivery fails, before a successor can begin. +type InitialPromptFailure struct { + ExecutionID string + SessionID string + PromptGeneration uint64 + TurnID string + Err error +} + +type sendPromptCallbacks struct { + onDispatched func() + onFailure func(InitialPromptFailure) +} + // NewSessionManager creates a new SessionManager func NewSessionManager(log *logger.Logger, stopCh <-chan struct{}) *SessionManager { return &SessionManager{ @@ -99,7 +114,7 @@ func (sm *SessionManager) SetPromptStarter(starter func(executionID string) (uin sm.promptStarter = starter } -func (sm *SessionManager) SetInitialPromptFailureHandler(handler func(executionID string)) { +func (sm *SessionManager) SetInitialPromptFailureHandler(handler func(failure InitialPromptFailure)) { sm.initialPromptFailure = handler } @@ -901,7 +916,16 @@ func (sm *SessionManager) dispatchInitialPrompt(ctx context.Context, execution * go func() { promptCtx, cancel := appctx.Detached(ctx, sm.stopCh, 0) defer cancel() - _, err := sm.SendPrompt(promptCtx, execution, effectivePrompt, false, acpAttachments, false) + _, err := sm.sendPrompt( + promptCtx, + execution, + effectivePrompt, + false, + acpAttachments, + false, + sendPromptCallbacks{onFailure: sm.initialPromptFailure}, + false, + ) if err != nil { // During graceful shutdown the agent subprocess is terminated, // so an in-flight initial prompt fails with a transport death @@ -917,9 +941,6 @@ func (sm *SessionManager) dispatchInitialPrompt(ctx context.Context, execution * zap.String("execution_id", execution.ID), zap.Error(err)) } - if sm.initialPromptFailure != nil { - sm.initialPromptFailure(execution.ID) - } } }() case sm.shouldInjectResumeContext(agentConfig, execution.SessionID): @@ -1131,7 +1152,7 @@ func (sm *SessionManager) SendPrompt( attachments []v1.MessageAttachment, dispatchOnly bool, ) (*PromptResult, error) { - return sm.sendPrompt(ctx, execution, prompt, validateStatus, attachments, dispatchOnly, nil, false) + return sm.sendPrompt(ctx, execution, prompt, validateStatus, attachments, dispatchOnly, sendPromptCallbacks{}, false) } // SendPromptWithDispatchCallback reports the point at which agentctl accepted @@ -1145,7 +1166,16 @@ func (sm *SessionManager) SendPromptWithDispatchCallback( dispatchOnly bool, onDispatched func(), ) (*PromptResult, error) { - return sm.sendPrompt(ctx, execution, prompt, validateStatus, attachments, dispatchOnly, onDispatched, false) + return sm.sendPrompt( + ctx, + execution, + prompt, + validateStatus, + attachments, + dispatchOnly, + sendPromptCallbacks{onDispatched: onDispatched}, + false, + ) } // activePromptGeneration returns the dispatched, in-flight prompt generation a @@ -1220,7 +1250,16 @@ func (sm *SessionManager) SendPromptSteerWithDispatchCallback( return nil, err } if !steered { - return sm.sendPrompt(ctx, execution, prompt, validateStatus, attachments, dispatchOnly, onDispatched, false) + return sm.sendPrompt( + ctx, + execution, + prompt, + validateStatus, + attachments, + dispatchOnly, + sendPromptCallbacks{onDispatched: onDispatched}, + false, + ) } if onDispatched != nil { onDispatched() @@ -1339,11 +1378,13 @@ func (sm *SessionManager) sendPrompt( validateStatus bool, attachments []v1.MessageAttachment, dispatchOnly bool, - onDispatched func(), + callbacks sendPromptCallbacks, steer bool, ) (*PromptResult, error) { if execution.agentctl == nil { - return nil, fmt.Errorf("execution %q has no agentctl client", execution.ID) + err := fmt.Errorf("execution %q has no agentctl client", execution.ID) + sm.reportPromptFailure(execution, 0, err, callbacks.onFailure) + return nil, err } // Agentctl acknowledges prompt dispatch before the adapter's session/prompt @@ -1378,22 +1419,53 @@ func (sm *SessionManager) sendPrompt( preparedCtx, effectivePrompt, promptGeneration, err := sm.preparePrompt(ctx, execution, prompt, validateStatus, attachments) if err != nil { + sm.reportPromptFailure(execution, promptGeneration, err, callbacks.onFailure) return nil, err } materializedAttachments, err := sm.materializeAttachments(preparedCtx, execution, attachments) if err != nil { + sm.reportPromptFailure(execution, promptGeneration, err, callbacks.onFailure) return nil, err } if sm.beforePromptDispatchHook != nil { sm.beforePromptDispatchHook() } if err := sm.triggerPrompt(preparedCtx, execution, effectivePrompt, materializedAttachments, promptGeneration, steer); err != nil { + sm.reportPromptFailure(execution, promptGeneration, err, callbacks.onFailure) return nil, err } // The generation is now accepted by agentctl and in flight, so a concurrent // steer may reuse it. (The steer path never marks — it reuses, not owns.) sm.markPromptDispatched(execution, promptGeneration) - return sm.finishAcceptedPrompt(preparedCtx, execution, dispatchOnly, onDispatched, promptGeneration) + result, err := sm.finishAcceptedPrompt( + preparedCtx, + execution, + dispatchOnly, + callbacks.onDispatched, + promptGeneration, + ) + if err != nil { + sm.reportPromptFailure(execution, promptGeneration, err, callbacks.onFailure) + } + return result, err +} + +func (sm *SessionManager) reportPromptFailure( + execution *AgentExecution, + promptGeneration uint64, + err error, + handler func(InitialPromptFailure), +) { + if handler == nil { + return + } + handler(InitialPromptFailure{ + ExecutionID: execution.ID, + SessionID: execution.SessionID, + PromptGeneration: promptGeneration, + TurnID: execution.promptTurnIDSnapshot(), + Err: err, + }) } // materializeAttachments resolves claimed backend descriptors into the active diff --git a/apps/backend/internal/agent/runtime/lifecycle/session_attachments_test.go b/apps/backend/internal/agent/runtime/lifecycle/session_attachments_test.go index e79c8d130c..4a51154128 100644 --- a/apps/backend/internal/agent/runtime/lifecycle/session_attachments_test.go +++ b/apps/backend/internal/agent/runtime/lifecycle/session_attachments_test.go @@ -2,6 +2,7 @@ package lifecycle import ( "context" + "errors" "io" "net/http" "net/http/httptest" @@ -9,12 +10,52 @@ import ( "strconv" "strings" "testing" + "time" agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl" "github.com/kandev/kandev/internal/common/logger" v1 "github.com/kandev/kandev/pkg/api/v1" ) +func TestDispatchInitialPromptReportsDeliveryFailure(t *testing.T) { + agentConfig, ok := newTestRegistry().Get("claude-acp") + if !ok { + t.Fatal("claude-acp test agent is not registered") + } + sm := NewSessionManager(logger.Default(), nil) + wantErr := "has no agentctl client" + failures := make(chan InitialPromptFailure, 1) + sm.SetInitialPromptFailureHandler(func(failure InitialPromptFailure) { + if failure.ExecutionID != "execution-initial-prompt" { + t.Errorf("execution ID = %q", failure.ExecutionID) + } + failures <- failure + }) + execution := &AgentExecution{ + ID: "execution-initial-prompt", + TaskID: "task-initial-prompt", + SessionID: "session-initial-prompt", + } + + sm.dispatchInitialPrompt( + context.Background(), + execution, + agentConfig, + "deliver this prompt", + nil, + func(string) error { return errors.New("mark ready must not run") }, + ) + + select { + case failure := <-failures: + if failure.Err == nil || !strings.Contains(failure.Err.Error(), wantErr) { + t.Fatalf("initial prompt error = %v, want %q", failure.Err, wantErr) + } + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for initial prompt failure callback") + } +} + type testAttachmentReader struct{} func (testAttachmentReader) OpenClaimed(context.Context, string, string, string) (io.ReadCloser, string, string, int64, error) { diff --git a/apps/backend/internal/backendapp/orchestrator.go b/apps/backend/internal/backendapp/orchestrator.go index 887715dbb9..e91950aed0 100644 --- a/apps/backend/internal/backendapp/orchestrator.go +++ b/apps/backend/internal/backendapp/orchestrator.go @@ -130,6 +130,7 @@ func provideOrchestrator( orchestratorSvc.SetGitHubCredentialBroker(gitCredentialBroker, githubCredentialBrokerEndpoint(cfg)) } orchestratorSvc.SetAttachmentReader(taskSvc.AttachmentService()) + orchestratorSvc.SetLaunchAttachmentClaimer(taskSvc) orchestratorSvc.SetTitleBranchRuntime(lifecycleMgr) if githubSvc != nil { orchestratorSvc.SetTaskGitCredentialPolicyResolver(githubExecutorCredentialPolicyAdapter{service: githubSvc}) diff --git a/apps/backend/internal/orchestrator/service.go b/apps/backend/internal/orchestrator/service.go index dfe1d86d6b..a3e34091eb 100644 --- a/apps/backend/internal/orchestrator/service.go +++ b/apps/backend/internal/orchestrator/service.go @@ -79,6 +79,12 @@ type AttachmentReader interface { OpenClaimed(ctx context.Context, id, taskID, sessionID string) (io.ReadCloser, string, string, int64, error) } +// LaunchAttachmentClaimer is the narrow task-service seam used to bind staged +// attachment descriptors to an authorized task/session before launch dispatch. +type LaunchAttachmentClaimer interface { + ClaimMessageAttachments(ctx context.Context, taskID, sessionID string, attachments []v1.MessageAttachment) error +} + // DefaultServiceConfig returns default configuration func DefaultServiceConfig() ServiceConfig { return ServiceConfig{ @@ -571,6 +577,10 @@ type Service struct { taskEvents TaskEventPublisher feederPulls FeederPullReconciler + // launchAttachmentClaimer binds staged descriptors before any launch intent + // can dispatch them to the runtime. Inline attachments need no claim. + launchAttachmentClaimer LaunchAttachmentClaimer + // sessionAccessCheck enforces per-user workspace scoping on the // session-keyed WS actions. Nil = unscoped. See SetSessionAccessChecker. sessionAccessCheck func(ctx context.Context, sessionID string) error @@ -1412,6 +1422,12 @@ func (s *Service) SetAttachmentReader(reader AttachmentReader) { s.executor.SetAttachmentReader(reader) } +// SetLaunchAttachmentClaimer wires staged-descriptor admission into the +// unified session launch boundary. +func (s *Service) SetLaunchAttachmentClaimer(claimer LaunchAttachmentClaimer) { + s.launchAttachmentClaimer = claimer +} + // SetOnPrimarySessionSet sets a callback on the executor for when the first session // of a task is marked primary. Used to publish a task.updated event so the frontend // receives the primary_session_id. diff --git a/apps/backend/internal/orchestrator/session_launch.go b/apps/backend/internal/orchestrator/session_launch.go index 592b615f08..3a4509e0d3 100644 --- a/apps/backend/internal/orchestrator/session_launch.go +++ b/apps/backend/internal/orchestrator/session_launch.go @@ -156,11 +156,11 @@ func IsBenignLaunchTeardownErr(err error) bool { func (s *Service) LaunchSession(ctx context.Context, req *LaunchSessionRequest) (*LaunchSessionResponse, error) { // Every intent funnels through here. SessionID is empty when creating, so // that case is carried by the task check alone. - if err := s.authorizeTask(ctx, req.TaskID); err != nil { + if err := s.authorizeTaskSessionPair(ctx, req.TaskID, req.SessionID); err != nil { return nil, err } - if err := s.authorizeSession(ctx, req.SessionID); err != nil { - return nil, err + if err := s.claimLaunchAttachments(ctx, req); err != nil { + return nil, fmt.Errorf("claim launch attachments: %w", err) } intent := ResolveIntent(req) req.Prompt = strings.TrimSpace(req.Prompt) @@ -183,6 +183,28 @@ func (s *Service) LaunchSession(ctx context.Context, req *LaunchSessionRequest) } } +func (s *Service) claimLaunchAttachments(ctx context.Context, req *LaunchSessionRequest) error { + hasDescriptor := false + for _, attachment := range req.Attachments { + if attachment.AttachmentID != "" { + hasDescriptor = true + break + } + } + if !hasDescriptor { + return nil + } + if s.launchAttachmentClaimer == nil { + return errors.New("launch attachment claimer is not configured") + } + return s.launchAttachmentClaimer.ClaimMessageAttachments( + ctx, + req.TaskID, + req.SessionID, + req.Attachments, + ) +} + // launchPrepare creates a session entry without launching the agent. // Passthrough profiles can't be "prepared" without a running PTY — the terminal // has nothing to attach to until the agent process exists. Upgrade those calls diff --git a/apps/backend/internal/orchestrator/session_launch_test.go b/apps/backend/internal/orchestrator/session_launch_test.go index 54f19c6777..a37e0941ee 100644 --- a/apps/backend/internal/orchestrator/session_launch_test.go +++ b/apps/backend/internal/orchestrator/session_launch_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strings" "testing" "time" @@ -15,6 +16,24 @@ import ( v1 "github.com/kandev/kandev/pkg/api/v1" ) +type rejectingLaunchAttachmentClaimer struct { + wantErr error + taskID string + sessionID string + attachments []v1.MessageAttachment +} + +func (c *rejectingLaunchAttachmentClaimer) ClaimMessageAttachments( + _ context.Context, + taskID, sessionID string, + attachments []v1.MessageAttachment, +) error { + c.taskID = taskID + c.sessionID = sessionID + c.attachments = attachments + return c.wantErr +} + func TestExecutionToLaunchResponseReportsAgentProfile(t *testing.T) { response := executionToLaunchResponse("task-1", &executor.TaskExecution{ SessionID: "session-1", @@ -155,6 +174,51 @@ func TestResolveIntent(t *testing.T) { } } +func TestLaunchSession_RejectsAttachmentClaimBeforeStart(t *testing.T) { + wantErr := errors.New("attachment is not available") + claimer := &rejectingLaunchAttachmentClaimer{wantErr: wantErr} + service := &Service{} + service.SetLaunchAttachmentClaimer(claimer) + + attachment := v1.MessageAttachment{AttachmentID: "attachment-1", Name: "notes.txt"} + _, err := service.LaunchSession(context.Background(), &LaunchSessionRequest{ + TaskID: "task-1", + SessionID: "session-1", + Intent: IntentStartCreated, + Prompt: "read the attachment", + Attachments: []v1.MessageAttachment{attachment}, + }) + if !errors.Is(err, wantErr) { + t.Fatalf("LaunchSession error = %v, want %v", err, wantErr) + } + if claimer.taskID != "task-1" || claimer.sessionID != "session-1" { + t.Fatalf("claim scope = (%q, %q), want (task-1, session-1)", claimer.taskID, claimer.sessionID) + } + if len(claimer.attachments) != 1 || claimer.attachments[0].AttachmentID != attachment.AttachmentID { + t.Fatalf("claimed attachments = %+v", claimer.attachments) + } +} + +func TestLaunchSession_RejectsMismatchedTaskSessionBeforeAttachmentClaim(t *testing.T) { + claimer := &rejectingLaunchAttachmentClaimer{wantErr: errors.New("claimer must not run")} + service := &Service{repo: &pairStubRepo{sessionTaskID: "task-other"}} + service.SetLaunchAttachmentClaimer(claimer) + + _, err := service.LaunchSession(context.Background(), &LaunchSessionRequest{ + TaskID: "task-1", + SessionID: "session-1", + Intent: IntentStartCreated, + Prompt: "read the attachment", + Attachments: []v1.MessageAttachment{{AttachmentID: "attachment-1"}}, + }) + if err == nil || !strings.Contains(err.Error(), "does not belong to task") { + t.Fatalf("LaunchSession error = %v, want task/session mismatch", err) + } + if claimer.taskID != "" || len(claimer.attachments) != 0 { + t.Fatalf("attachment claimer ran for mismatched pair: %+v", claimer) + } +} + func TestNormalizeRecoverSessionError(t *testing.T) { t.Run("maps profile not found errors to actionable profile guidance", func(t *testing.T) { in := errors.New("failed to resolve agent profile: profile not found: sql: no rows in result set") diff --git a/apps/backend/internal/task/repository/sqlite/attachment.go b/apps/backend/internal/task/repository/sqlite/attachment.go index c61afe4c0e..6746b29c0a 100644 --- a/apps/backend/internal/task/repository/sqlite/attachment.go +++ b/apps/backend/internal/task/repository/sqlite/attachment.go @@ -169,7 +169,8 @@ func addAttachmentToClaim(selection *attachmentClaimSelection, attachment *model selection.claimIDs = append(selection.claimIDs, id) return nil case models.AttachmentStateClaimed: - if attachment.TaskID != taskID || attachment.SessionID != sessionID { + if attachment.TaskID != taskID || + (attachment.SessionID != "" && attachment.SessionID != sessionID) { return models.ErrAttachmentClaimConflict } return nil diff --git a/apps/backend/internal/task/repository/sqlite/attachment_test.go b/apps/backend/internal/task/repository/sqlite/attachment_test.go index 5a5d671cbe..f3408ff8f6 100644 --- a/apps/backend/internal/task/repository/sqlite/attachment_test.go +++ b/apps/backend/internal/task/repository/sqlite/attachment_test.go @@ -37,6 +37,57 @@ func TestClaimMessageAttachments_IsIdempotentForSameTaskSession(t *testing.T) { } } +func TestClaimMessageAttachments_AllowsTaskScopedClaimForSameTaskSession(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "workspace-attachments") + now := time.Now().UTC() + attachment := &models.TaskMessageAttachment{ + ID: "attachment-task-scoped", OwnerID: "owner-1", WorkspaceID: "workspace-attachments", + Name: "notes.txt", MimeType: "text/plain", Kind: "resource", DeliveryMode: "path", + SizeBytes: 16, StorageKey: "attachment-task-scoped", State: models.AttachmentStateStaged, + ExpiresAt: now.Add(time.Hour), CreatedAt: now, + } + if err := repo.CreateMessageAttachment(ctx, attachment); err != nil { + t.Fatal(err) + } + if err := repo.ClaimMessageAttachments(ctx, []string{attachment.ID}, "owner-1", "workspace-attachments", "task-1", ""); err != nil { + t.Fatalf("task-scoped claim: %v", err) + } + if err := repo.ClaimMessageAttachments(ctx, []string{attachment.ID}, "owner-1", "workspace-attachments", "task-1", "session-1"); err != nil { + t.Fatalf("session claim after task-scoped claim: %v", err) + } + got, err := repo.GetMessageAttachment(ctx, attachment.ID) + if err != nil { + t.Fatal(err) + } + if got.State != models.AttachmentStateClaimed || got.TaskID != "task-1" || got.SessionID != "" { + t.Fatalf("claimed attachment = %+v", got) + } +} + +func TestClaimMessageAttachments_RejectsDifferentSessionAfterSessionClaim(t *testing.T) { + repo := newRepoForSessionTests(t) + ctx := context.Background() + seedWorkspace(t, repo, "workspace-attachments") + now := time.Now().UTC() + attachment := &models.TaskMessageAttachment{ + ID: "attachment-session-scoped", OwnerID: "owner-1", WorkspaceID: "workspace-attachments", + Name: "notes.txt", MimeType: "text/plain", Kind: "resource", DeliveryMode: "path", + SizeBytes: 16, StorageKey: "attachment-session-scoped", State: models.AttachmentStateStaged, + ExpiresAt: now.Add(time.Hour), CreatedAt: now, + } + if err := repo.CreateMessageAttachment(ctx, attachment); err != nil { + t.Fatal(err) + } + if err := repo.ClaimMessageAttachments(ctx, []string{attachment.ID}, "owner-1", "workspace-attachments", "task-1", "session-1"); err != nil { + t.Fatalf("initial claim: %v", err) + } + if err := repo.ClaimMessageAttachments(ctx, []string{attachment.ID}, "owner-1", "workspace-attachments", "task-1", "session-2"); err == nil { + t.Fatal("expected a different session claim to fail") + } +} + func TestClaimMessageAttachments_AllowsSeparateSubmissionsToReachTheLimit(t *testing.T) { repo := newRepoForSessionTests(t) ctx := context.Background() diff --git a/apps/web/e2e/tests/session/new-session-dialog.spec.ts b/apps/web/e2e/tests/session/new-session-dialog.spec.ts index 3c56a7d32a..4aa5de85f0 100644 --- a/apps/web/e2e/tests/session/new-session-dialog.spec.ts +++ b/apps/web/e2e/tests/session/new-session-dialog.spec.ts @@ -1,3 +1,5 @@ +import fs from "node:fs"; +import path from "node:path"; import { test, expect } from "../../fixtures/test-base"; import { waitForSessionDone } from "../../helpers/session"; import { KanbanPage } from "../../pages/kanban-page"; @@ -257,6 +259,73 @@ test.describe("New session dialog", () => { expect(allSessions).toHaveLength(2); }); + test("starts a second session with a staged attachment", async ({ + testPage, + apiClient, + seedData, + }, testInfo) => { + test.setTimeout(120_000); + const prompt = "/e2e:simple-message"; + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + "New Session Attachment Task", + seedData.agentProfileId, + { + description: prompt, + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id"); + await waitForSessionDone(apiClient, task.id, task.session_id, "Waiting for first session"); + + await testPage.goto(`/t/${task.id}`); + const session = new SessionPage(testPage); + await session.waitForLoad(); + await session.openNewSessionDialog(); + + fs.mkdirSync(testInfo.outputDir, { recursive: true }); + const attachmentPath = path.join(testInfo.outputDir, "new-session-note.txt"); + fs.writeFileSync(attachmentPath, "new session attachment body"); + const dialog = session.newSessionDialog(); + await dialog.locator('input[type="file"]').setInputFiles(attachmentPath); + await expect(dialog.getByText("new-session-note.txt", { exact: true })).toBeVisible(); + await session.newSessionPromptInput().fill(prompt); + await session.newSessionStartButton().click(); + await expect(dialog).not.toBeVisible({ timeout: 10_000 }); + + let secondSessionId = ""; + await expect + .poll( + async () => { + const { sessions } = await apiClient.listTaskSessions(task.id); + const second = sessions.find((candidate) => candidate.id !== task.session_id); + secondSessionId = second?.id ?? ""; + return DONE_STATES.includes(second?.state ?? ""); + }, + { timeout: 30_000, message: "Waiting for attached New Agent session" }, + ) + .toBe(true); + + const { messages } = await apiClient.listSessionMessages(secondSessionId); + const userMessage = messages.find( + (message) => message.author_type === "user" && message.content.includes(prompt), + ); + const attachments = userMessage?.metadata?.attachments; + expect(Array.isArray(attachments) ? attachments[0] : undefined).toMatchObject({ + name: "new-session-note.txt", + }); + + await testPage.reload(); + await session.waitForLoad(); + await expect( + session.activeChat().getByText("new-session-note.txt", { exact: true }), + ).toBeVisible({ + timeout: 15_000, + }); + }); + test("selects a saved prompt without submitting, then launches explicitly", async ({ testPage, apiClient, diff --git a/docs/plans/session-launch-prompt-admission/plan.md b/docs/plans/session-launch-prompt-admission/plan.md new file mode 100644 index 0000000000..a5dec94129 --- /dev/null +++ b/docs/plans/session-launch-prompt-admission/plan.md @@ -0,0 +1,136 @@ +--- +created: 2026-09-01 +status: implemented +requirements: + - REQ-TASKS-PROMPT-ATTACHMENTS-001 + - REQ-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001 +system_design: + - ../../specs/tasks/system-design/prompt-attachments.md + - ../../specs/tasks/system-design/task-launch-failure-recovery.md +legacy_specs: [] +--- + +# Implementation Plan: Session Launch Prompt Admission + +## Overview + +The `session.launch` entry point did not claim staged attachments. The runtime +therefore rejected attachment materialization before the initial ACP prompt. + +The asynchronous dispatch path logged that error but did not publish a terminal +execution result. The session and its turn remained `RUNNING` for five hours. + +This plan first adds attachment admission to the shared launch boundary. It then +makes every genuine initial-prompt error use the existing agent-failure path. + +## Scope + +### In scope + +- Claim file-backed attachments before a launch intent starts runtime work. +- Preserve idempotency for task-scoped claims from task creation. +- Reject cross-task and cross-session attachment reuse. +- Settle the current execution, turn, and session after initial-prompt errors. +- Add backend regression coverage and one New Agent end-to-end scenario. + +### Out of scope + +- Changes to upload limits, file retention, or delivery modes. +- Changes to the New Agent layout or prompt composer. +- Automatic cancellation of quiet turns that already produced agent activity. +- Recovery of the original incident session or its discarded ACP thread. + +## Technical approach + +### Launch attachment admission + +Add a narrow attachment-claimer dependency to +`apps/backend/internal/orchestrator/service.go`. Wire the task service through +`apps/backend/internal/backendapp/orchestrator.go`. + +In `apps/backend/internal/orchestrator/session_launch.go`, claim file-backed +descriptors after task and session authorization. Complete the claim before the +selected launch intent can change task, session, turn, or executor state. + +Use the request session ID when it exists. A launch without a session ID uses a +task-scoped claim because the orchestrator has not created the session. + +Update `apps/backend/internal/task/repository/sqlite/attachment.go` so an +existing task-scoped claim is idempotent for a later session on the same task. +Keep an existing session-scoped claim unavailable to another session. + +### Initial prompt failure reconciliation + +Change the initial-prompt failure callback in +`apps/backend/internal/agent/runtime/lifecycle/session.go` to carry the dispatch +error. Wire the callback in `NewManager` from +`apps/backend/internal/agent/runtime/lifecycle/manager.go`. + +For a genuine dispatch error, use the existing terminal execution path. It +publishes `agent.failed` with current prompt evidence and releases execution +activity. + +Keep shutdown and transport teardown on the existing stopped-session path. Do +not create a user-visible failure for backend shutdown. + +The orchestrator uses its existing execution and prompt correlation before it +settles the turn and session. Duplicate process-exit errors remain idempotent. + +### User-visible regression coverage + +Extend `apps/web/e2e/tests/session/new-session-dialog.spec.ts`. The scenario +uploads a file in the New Agent dialog and starts the second session. + +The scenario waits for agent output and a settled session. It also checks the +persisted user-message attachment descriptor after a reload. + +## Tests + +| Acceptance criterion | Evidence | +| --- | --- | +| `AC-TASKS-PROMPT-ATTACHMENTS-001.2` | `starts a second session with a staged attachment` in `apps/web/e2e/tests/session/new-session-dialog.spec.ts` | +| `AC-TASKS-PROMPT-ATTACHMENTS-001.3` | `TestLaunchSession_RejectsAttachmentClaimBeforeStart` in `apps/backend/internal/orchestrator/session_launch_test.go` | +| `AC-TASKS-PROMPT-ATTACHMENTS-001.4` | `TestClaimMessageAttachments_AllowsTaskScopedClaimForSameTaskSession` in `apps/backend/internal/task/repository/sqlite/attachment_test.go` | +| `AC-TASKS-PROMPT-ATTACHMENTS-001.5` | `TestDispatchInitialPromptReportsDeliveryFailure` in `apps/backend/internal/agent/runtime/lifecycle/session_attachments_test.go` | +| `AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.9` | `TestInitialPromptFailureMarksExecutionFailedAndReleasesActivity` in `apps/backend/internal/agent/runtime/lifecycle/activity_test.go` | +| `AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.10` | `TestInitialPromptFailureCannotFailReplacementExecution` in `apps/backend/internal/agent/runtime/lifecycle/activity_test.go` | + +## E2E tests + +`apps/web/e2e/tests/session/new-session-dialog.spec.ts` will contain +`starts a second session with a staged attachment`. + +The test covers `AC-TASKS-PROMPT-ATTACHMENTS-001.2`. It must fail before the fix +because the second session receives no agent output and does not settle. + +`TestDispatchInitialPromptReportsDeliveryFailure` and +`TestInitialPromptFailureMarksExecutionFailedAndReleasesActivity` provide the +failure-path evidence for `AC-TASKS-PROMPT-ATTACHMENTS-001.5`. + +## Work orders + +- [x] [Task 01: Admit launch attachments](task-01-admit-launch-attachments.md) +- [x] [Task 02: Settle initial prompt failures](task-02-settle-initial-prompt-failures.md) + +## Verification results + +- The attachment repository claim tests passed, including task-scoped promotion + and rejection of a different session after a session-scoped claim. +- The orchestrator package passed all 2,317 tests. +- The task repository and backend composition packages passed all 1,597 tests. +- The complete backend test suite passed after removing the runtime-injected + `KANDEV_INTERNAL_CONFIG_FILE` variables from the test command. +- The focused lifecycle tests passed under the race detector. +- The New Agent attachment Playwright scenario failed before implementation + with a `RUNNING` session timeout, then passed after implementation and reload. + +## Risks + +- Task creation already claims attachments before it creates a session. The + launch claim must accept that exact task scope without broadening access. +- Initial prompt dispatch runs in a goroutine. A process exit or replacement + can race with the new terminal callback. +- Some launch callers are internal. The claimer must ignore inline descriptors + and preserve trusted calls that carry no staged attachment IDs. +- Playwright uses prebuilt backend and web artifacts. Stale artifacts can hide + or misreport the regression. diff --git a/docs/plans/session-launch-prompt-admission/task-01-admit-launch-attachments.md b/docs/plans/session-launch-prompt-admission/task-01-admit-launch-attachments.md new file mode 100644 index 0000000000..ff52f34453 --- /dev/null +++ b/docs/plans/session-launch-prompt-admission/task-01-admit-launch-attachments.md @@ -0,0 +1,91 @@ +--- +id: "01-admit-launch-attachments" +title: "Admit launch attachments" +status: complete +wave: 1 +depends_on: [] +plan: "plan.md" +requirements: + - REQ-TASKS-PROMPT-ATTACHMENTS-001 +acceptance_criteria: + - AC-TASKS-PROMPT-ATTACHMENTS-001.2 + - AC-TASKS-PROMPT-ATTACHMENTS-001.3 + - AC-TASKS-PROMPT-ATTACHMENTS-001.4 +system_design: + - ../../specs/tasks/system-design/prompt-attachments.md +--- + +# Task 01: Admit Launch Attachments + +## Summary + +Claim staged attachments at the shared `session.launch` boundary. Reject an +invalid claim before the launch intent starts runtime work. + +## In scope + +- Add and wire the orchestrator attachment-claimer dependency. +- Claim attachment IDs after authorization and before intent dispatch. +- Preserve same-task idempotency for earlier task-scoped claims. +- Keep cross-task and session-scoped isolation strict. +- Add backend tests and the New Agent attachment scenario. + +## Out of scope + +- Initial-prompt errors after a valid claim reaches the runtime. +- Upload, retention, and attachment presentation changes. + +## Acceptance + +- A valid staged attachment reaches the launch intent as a claimed descriptor. +- An invalid descriptor starts no agent and creates no prompt turn. +- A task-scoped claim remains reusable only for its task. + +## Verification + +```bash +cd apps/backend && go test ./internal/task/repository/sqlite -run 'TestClaimMessageAttachments_(AllowsTaskScopedClaimForSameTaskSession|IsIdempotentForSameTaskSession)' -count=1 +cd apps/backend && go test ./internal/orchestrator -run 'TestLaunchSession_RejectsAttachmentClaimBeforeStart' -count=1 +make -C apps/backend build +make -C apps/backend e2e-plugin-package +cd apps/web && pnpm run build:e2e +cd apps/web && pnpm e2e:run tests/session/new-session-dialog.spec.ts -- --grep 'staged attachment' +``` + +## Files likely touched + +- `apps/backend/internal/orchestrator/service.go` +- `apps/backend/internal/orchestrator/session_launch.go` +- `apps/backend/internal/orchestrator/session_launch_test.go` +- `apps/backend/internal/backendapp/orchestrator.go` +- `apps/backend/internal/task/repository/sqlite/attachment.go` +- `apps/backend/internal/task/repository/sqlite/attachment_test.go` +- `apps/web/e2e/tests/session/new-session-dialog.spec.ts` + +## Dependencies + +None. + +## Risks + +- An overbroad idempotency rule can let one session use a file from another session. +- A missing production dependency can make every file-backed launch fail closed. + +## Parallelism + +`sequential` + +## Inputs + +- Prompt attachment requirements and system design. +- ADR `2026-08-04-file-backed-prompt-attachments`. +- Existing direct-message and queue claim paths. +- Existing New Agent Playwright scenarios. + +## Results + +- Added the launch attachment claimer seam and production task-service wiring. +- Claimed file-backed descriptors after authorization and before intent dispatch. +- Allowed an existing task-scoped claim only for a later session on the same task. +- Kept session-scoped claims unavailable to a different session. +- Added focused repository and orchestrator tests plus the New Agent E2E scenario. diff --git a/docs/plans/session-launch-prompt-admission/task-02-settle-initial-prompt-failures.md b/docs/plans/session-launch-prompt-admission/task-02-settle-initial-prompt-failures.md new file mode 100644 index 0000000000..e6adf6a1e4 --- /dev/null +++ b/docs/plans/session-launch-prompt-admission/task-02-settle-initial-prompt-failures.md @@ -0,0 +1,89 @@ +--- +id: "02-settle-initial-prompt-failures" +title: "Settle initial prompt failures" +status: complete +wave: 2 +depends_on: + - "01-admit-launch-attachments" +plan: "plan.md" +requirements: + - REQ-TASKS-PROMPT-ATTACHMENTS-001 + - REQ-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001 +acceptance_criteria: + - AC-TASKS-PROMPT-ATTACHMENTS-001.5 + - AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.9 + - AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.10 +system_design: + - ../../specs/tasks/system-design/prompt-attachments.md + - ../../specs/tasks/system-design/task-launch-failure-recovery.md +--- + +# Task 02: Settle Initial Prompt Failures + +## Summary + +Route every genuine initial-prompt error through the terminal execution path. +Settle only the execution and prompt that own the error. + +## In scope + +- Carry the dispatch error through the initial-prompt callback. +- Wire the callback in the lifecycle manager constructor. +- Publish the existing correlated `agent.failed` result. +- Keep shutdown teardown non-failing. +- Add race-aware lifecycle and orchestrator tests. + +## Out of scope + +- Attachment claim admission. +- Provider-specific retry policy. +- Timeout changes for quiet turns. + +## Acceptance + +- A materialization or ACP submission error fails the current execution. +- The active turn and session receive a durable terminal result without paths + or provider secrets. +- A stale or duplicate error cannot fail replacement work. + +## Verification + +```bash +cd apps/backend && go test -race ./internal/agent/runtime/lifecycle -run 'Test(DispatchInitialPromptReportsDeliveryFailure|InitialPromptFailureMarksExecutionFailedAndReleasesActivity|InitialPromptFailureCannotFailReplacementExecution)' -count=1 +make -C apps/backend test +``` + +## Files likely touched + +- `apps/backend/internal/agent/runtime/lifecycle/session.go` +- `apps/backend/internal/agent/runtime/lifecycle/manager.go` +- `apps/backend/internal/agent/runtime/lifecycle/activity_test.go` +- `apps/backend/internal/agent/runtime/lifecycle/session_attachments_test.go` + +## Dependencies + +- Task 01 establishes the attachment admission boundary. + +## Risks + +- The callback can race with process exit and execution replacement. +- A shutdown transport error can create a false durable failure. +- The terminal event must retain current prompt evidence for stale-event checks. + +## Parallelism + +`sequential` + +## Inputs + +- Task launch failure requirements and system design. +- ADR `2026-08-18-never-started-agent-stall-terminal`. +- Existing `MarkCompleted` and `agent.failed` correlation behavior. + +## Results + +- Carried the asynchronous prompt delivery error through the session callback. +- Wired the lifecycle manager to settle that execution through `MarkCompleted`. +- Verified failure state, activity release, and exact execution identity. +- Verified a late callback for a removed execution does not affect its replacement. +- Reused the existing shutdown-aware and duplicate-terminal guards. diff --git a/docs/specs/tasks/README.md b/docs/specs/tasks/README.md index 09e011d37c..893c41054f 100644 --- a/docs/specs/tasks/README.md +++ b/docs/specs/tasks/README.md @@ -128,6 +128,7 @@ signals, and task-scoped scheduling contracts. - [Passthrough Queued Prompt Dispatch](system-design/passthrough-queued-prompt-dispatch.md) - [Saved Prompt Delivery](system-design/saved-prompt-delivery.md) - [Passthrough Initial Prompt Turn Boundary](system-design/passthrough-initial-prompt-turn-boundary.md) +- [Prompt attachments](system-design/prompt-attachments.md) - [Task Archive Confirmation](system-design/archive-confirmation.md) - [Task plan write consistency](system-design/plan-write-consistency.md) - [Task plan write lifecycle](system-design/plan-write-lifecycle.md) diff --git a/docs/specs/tasks/requirements/prompt-attachments.md b/docs/specs/tasks/requirements/prompt-attachments.md index babaaea87d..f587cb8a66 100644 --- a/docs/specs/tasks/requirements/prompt-attachments.md +++ b/docs/specs/tasks/requirements/prompt-attachments.md @@ -2,6 +2,7 @@ status: draft system: tasks created: 2026-08-04 +updated: 2026-09-01 owners: - Kandev team --- @@ -20,6 +21,18 @@ This document is the migrated task-system source for the capability. The source #### Acceptance criteria - **AC-TASKS-PROMPT-ATTACHMENTS-001.1:** When a consumer uses this capability, the system shall provide the observable behavior and exclusions documented below. +- **AC-TASKS-PROMPT-ATTACHMENTS-001.2:** When a session launch references + valid staged attachments, the system shall claim them for the authorized task + before it starts the agent or creates the prompt turn. +- **AC-TASKS-PROMPT-ATTACHMENTS-001.3:** When a session launch references an + expired, unauthorized, missing, or conflicting attachment, the system shall + reject the launch without starting the agent or creating a prompt turn. +- **AC-TASKS-PROMPT-ATTACHMENTS-001.4:** When a task-scoped attachment claim + reaches a later launch for the same task, the system shall accept that claim + without weakening cross-task or cross-session isolation. +- **AC-TASKS-PROMPT-ATTACHMENTS-001.5:** When attachment materialization fails + before the initial prompt reaches the agent, the system shall show a durable + launch error and shall not present the session as active work. ## Migrated source detail @@ -203,4 +216,4 @@ request paths never determine ownership. - Per-user or per-workspace configurable attachment limits. - External object storage or direct browser-to-executor uploads. - Changing the separate 10 MiB task-document attachment contract. -- Changing the queue's existing compact thumbnail dimensions or crop policy. \ No newline at end of file +- Changing the queue's existing compact thumbnail dimensions or crop policy. diff --git a/docs/specs/tasks/requirements/task-launch-failure-recovery.md b/docs/specs/tasks/requirements/task-launch-failure-recovery.md index 1e7da056de..8c48956e47 100644 --- a/docs/specs/tasks/requirements/task-launch-failure-recovery.md +++ b/docs/specs/tasks/requirements/task-launch-failure-recovery.md @@ -2,7 +2,7 @@ status: draft system: tasks created: 2026-08-19 -updated: 2026-08-24 +updated: 2026-09-01 owners: - cfl12 --- @@ -53,6 +53,13 @@ launching work against the wrong pull request or repository branch. source error record shall remain visible and update with the new typed cause, bounded details, and valid actions; a successful recovery shall clear it only after the required relaunch or task move succeeds. +- **AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.9:** When the initial prompt fails + before the agent produces an event, the system shall settle the active turn + and session with a durable error without waiting for stall detection. The + error shall exclude file paths and provider secrets. +- **AC-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001.10:** When an initial-prompt error + belongs to an old execution or prompt, the system shall not fail a replacement + execution or successor turn. ## Out of scope diff --git a/docs/specs/tasks/system-design/prompt-attachments.md b/docs/specs/tasks/system-design/prompt-attachments.md new file mode 100644 index 0000000000..2416a88ad7 --- /dev/null +++ b/docs/specs/tasks/system-design/prompt-attachments.md @@ -0,0 +1,118 @@ +--- +status: draft +system: tasks +requirements: + - REQ-TASKS-PROMPT-ATTACHMENTS-001 +created: 2026-09-01 +owners: + - Kandev team +--- + +# Prompt Attachments System Design + +## Purpose and boundaries + +The task system owns prompt-attachment staging, claims, delivery descriptors, +and retention. It also owns admission of those descriptors to a task session. + +The agent runtime materializes claimed files and sends them with the prompt. +It cannot claim staged files or infer task ownership. + +## Requirement mapping + +| Requirement | Design sections | +| --- | --- | +| `REQ-TASKS-PROMPT-ATTACHMENTS-001` | Claim admission, materialization and delivery, failure and recovery, security | + +## Components and responsibilities + +- The attachment service stores staged files and validates the authenticated + owner, workspace, task, and optional session. +- The task repository changes a complete attachment set from `staged` to + `claimed` in one transaction. +- The orchestrator admits `LaunchSessionRequest.Attachments` before any agent + start or prompt-turn creation. +- The lifecycle manager reads claimed files and streams them to agentctl before + it sends the prompt. +- The orchestrator converts a rejected initial prompt into the durable launch + failure state. + +## Claim admission + +`LaunchSessionRequest.Attachments` contains untrusted attachment identifiers. +The orchestrator authorizes the task and optional session before each claim. + +The orchestrator uses a narrow attachment-claimer interface. The task service +implements this interface and derives the owner and workspace from server +state. + +The launch entry point claims attachments before it calls an intent handler. +This order prevents invalid descriptors from starting an agent or prompt turn. + +A launch with an existing session uses that session as the claim scope. A new +session launch can use a task-scoped claim because no session identity exists. + +Task creation also creates task-scoped claims before it prepares a session. A +later launch for that task treats the existing claim as idempotent. + +A session-scoped claim remains bound to its session. Another session cannot use +that claim, even when both sessions belong to the same task. + +## Materialization and delivery + +The lifecycle manager accepts only claimed file descriptors. The attachment +reader checks the task and the optional session before it opens a file. + +The lifecycle manager streams each file to the active agentctl instance. It +then uses the returned safe name for native-prompt or workspace-path delivery. + +The lifecycle manager starts prompt generation before materialization. A +materialization error therefore uses the same terminal prompt-error path as an +ACP submission error. + +## Failure and recovery + +A claim error returns from `session.launch` before the intent changes runtime +state. The response does not disclose another owner, workspace, task, or path. + +A materialization or ACP submission error can occur after the launch response. +The lifecycle manager reports that error with the current execution identity. + +The existing agent-failure path settles the current turn and session with a +safe generic error. It also publishes the durable task and session state used +by the chat surface. + +A delayed error cannot settle a replacement execution or successor prompt. The +existing execution and prompt evidence checks reject stale terminal events. + +Shutdown cancellation remains a stopped execution. It does not become a user +visible launch error. + +## Persistence + +The attachment registry is the source of truth for claim state. Claim changes +are transactional and survive backend restarts. + +Claim admission does not write file bytes to a task message. The initial user +message stores bounded attachment descriptors after the launch succeeds. + +## Security + +Clients cannot provide an owner, workspace, storage key, or executor path. The +backend derives those values from authenticated and persisted state. + +Task-scoped idempotency applies only when the stored task matches. Session +scoping remains strict when the stored claim contains a session identity. + +## Observability + +Existing launch request diagnostics correlate claim errors with their task and +session. They do not include attachment bytes or storage paths. + +Initial prompt errors retain the execution identity. The terminal event and +durable state provide evidence that the prompt did not remain active. + +## Related decisions + +- [ADR-2026-08-04-file-backed-prompt-attachments](../../../decisions/2026-08-04-file-backed-prompt-attachments.md) +- [ADR-2026-08-18-never-started-agent-stall-terminal](../../../decisions/2026-08-18-never-started-agent-stall-terminal.md) diff --git a/docs/specs/tasks/system-design/task-launch-failure-recovery.md b/docs/specs/tasks/system-design/task-launch-failure-recovery.md index 565c626f0f..4d3210f2c0 100644 --- a/docs/specs/tasks/system-design/task-launch-failure-recovery.md +++ b/docs/specs/tasks/system-design/task-launch-failure-recovery.md @@ -4,6 +4,7 @@ system: tasks requirements: - REQ-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001 created: 2026-08-24 +updated: 2026-09-01 owners: - cfl12 --- @@ -21,7 +22,7 @@ task-owned projection. | Requirement | Design sections | | --- | --- | -| REQ-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001 | PR gate, error projection, recovery actions, responsive surface | +| REQ-TASKS-TASK-LAUNCH-FAILURE-RECOVERY-001 | PR gate, initial prompt admission, error projection, recovery actions, responsive surface | ## PR gate and launch paths @@ -71,6 +72,24 @@ preserves the source error record, keeps it visible, and updates its typed category, bounded details, and valid actions. A successful recovery clears the source error only after its write and relaunch or move succeed. +## Initial prompt admission + +The lifecycle manager owns initial prompt submission after an agent process +starts. Materialization and ACP submission errors occur inside that asynchronous +boundary. + +The lifecycle manager sends each error to the existing terminal execution path. +That path publishes `agent.failed` with the execution and prompt evidence. + +The orchestrator accepts the failure only for the current execution and prompt. +It completes the active turn and persists the safe session failure. + +The task moves to `FAILED` only while the same session still owns its runtime +state. A successor execution or prompt remains unchanged. + +Backend shutdown keeps its existing stopped-session behavior. A shutdown error +does not create a durable user-visible launch failure. + ## Branch resolution and persistence Local default detection remains a pure helper and returns empty when only a @@ -87,6 +106,10 @@ network, missing branch, and unresolved default remain distinct diagnostics. Ambiguous repository identity omits repository-scoped actions. Foreign session or repository IDs and stale stamps fail without mutation. +Initial-prompt errors use a safe generic durable message and the same +stale-event checks as other agent failures. Raw attachment paths and provider +details remain in backend diagnostics and do not reach the durable projection. + ## Responsive presentation The task Chat surface renders one persistent error card from the shared @@ -101,3 +124,7 @@ authorization and remain free of horizontal overflow. - Test error projection limits, stamps, persistence, and recovery authorization. - Test branch self-healing and mark-review-done terminal-step checks. - Cover desktop and mobile recovery actions with the existing task Chat tests. + +## Related decisions + +- [ADR-2026-08-18-never-started-agent-stall-terminal](../../../decisions/2026-08-18-never-started-agent-stall-terminal.md)