fix: settle session launches with attachments (#3258)
This commit is contained in:
@@ -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) {
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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})
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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")
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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.
|
||||
@@ -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.
|
||||
@@ -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.
|
||||
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
- Changing the queue's existing compact thumbnail dimensions or crop policy.
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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)
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user