fix: retract output from abandoned provider retries (#3728)
Co-authored-by: Kandev Agent <agent@kandev.dev>
This commit is contained in:
@@ -5,6 +5,7 @@ import (
|
||||
"fmt"
|
||||
|
||||
acp "github.com/coder/acp-go-sdk"
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
// sessionUpdater abstracts the ACP connection methods used by the emitter.
|
||||
@@ -50,6 +51,17 @@ func (e *emitter) text(msg string) {
|
||||
})
|
||||
}
|
||||
|
||||
func (e *emitter) textWithID(msg string) {
|
||||
messageID := acp.MessageId(uuid.NewString())
|
||||
_ = e.conn.SessionUpdate(e.ctx, acp.SessionNotification{
|
||||
SessionId: e.sid,
|
||||
Update: acp.SessionUpdate{AgentMessageChunk: &acp.SessionUpdateAgentMessageChunk{
|
||||
Content: acp.TextBlock(msg),
|
||||
MessageId: &messageID,
|
||||
}},
|
||||
})
|
||||
}
|
||||
|
||||
// thought sends an agent thinking/reasoning update.
|
||||
func (e *emitter) thought(msg string) {
|
||||
_ = e.conn.SessionUpdate(e.ctx, acp.SessionNotification{
|
||||
@@ -58,6 +70,28 @@ func (e *emitter) thought(msg string) {
|
||||
})
|
||||
}
|
||||
|
||||
func (e *emitter) thoughtWithID(msg string) {
|
||||
messageID := acp.MessageId(uuid.NewString())
|
||||
_ = e.conn.SessionUpdate(e.ctx, acp.SessionNotification{
|
||||
SessionId: e.sid,
|
||||
Update: acp.SessionUpdate{AgentThoughtChunk: &acp.SessionUpdateAgentThoughtChunk{
|
||||
Content: acp.TextBlock(msg),
|
||||
MessageId: &messageID,
|
||||
}},
|
||||
})
|
||||
}
|
||||
|
||||
func (e *emitter) responseAttemptReset() {
|
||||
_ = e.conn.SessionUpdate(e.ctx, acp.SessionNotification{
|
||||
SessionId: e.sid,
|
||||
Update: acp.SessionUpdate{SessionInfoUpdate: &acp.SessionSessionInfoUpdate{
|
||||
Meta: map[string]any{
|
||||
"kandevMock": map[string]any{"responseAttemptReset": true},
|
||||
},
|
||||
}},
|
||||
})
|
||||
}
|
||||
|
||||
// startTool announces a new tool call.
|
||||
func (e *emitter) startTool(id acp.ToolCallId, title string, kind acp.ToolKind, input any, locs ...acp.ToolCallLocation) {
|
||||
opts := []acp.ToolCallStartOpt{
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"testing"
|
||||
|
||||
acp "github.com/coder/acp-go-sdk"
|
||||
"github.com/google/uuid"
|
||||
)
|
||||
|
||||
func TestResponseRetryScenarioEmitsAbandonedResetReplacementSequence(t *testing.T) {
|
||||
emitter, updates := newTestEmitter()
|
||||
|
||||
emitPredefinedScenario(emitter, "response-retry")
|
||||
|
||||
got := updates.getUpdates()
|
||||
if len(got) != 4 {
|
||||
t.Fatalf("update count = %d, want abandoned thought, abandoned answer, reset, replacement: %+v", len(got), got)
|
||||
}
|
||||
thought := got[0].notification.Update.AgentThoughtChunk
|
||||
abandoned := got[1].notification.Update.AgentMessageChunk
|
||||
reset := got[2].notification.Update.SessionInfoUpdate
|
||||
replacement := got[3].notification.Update.AgentMessageChunk
|
||||
if thought == nil || getThoughtContent(got[0]) != "Abandoned response attempt reasoning." {
|
||||
t.Fatalf("first update = %+v, want abandoned reasoning", got[0])
|
||||
}
|
||||
if abandoned == nil || getTextContent(got[1]) != "Abandoned response attempt answer." {
|
||||
t.Fatalf("second update = %+v, want abandoned answer", got[1])
|
||||
}
|
||||
if reset == nil {
|
||||
t.Fatalf("third update = %+v, want response-attempt reset", got[2])
|
||||
}
|
||||
mockMeta, _ := reset.Meta["kandevMock"].(map[string]any)
|
||||
if resetValue, _ := mockMeta["responseAttemptReset"].(bool); !resetValue {
|
||||
t.Fatalf("reset metadata = %+v, want controlled mock marker", reset.Meta)
|
||||
}
|
||||
if replacement == nil || getTextContent(got[3]) != "Replacement response after provider retry." {
|
||||
t.Fatalf("fourth update = %+v, want replacement answer", got[3])
|
||||
}
|
||||
|
||||
ids := []*acp.MessageId{thought.MessageId, abandoned.MessageId, replacement.MessageId}
|
||||
seen := make(map[string]struct{}, len(ids))
|
||||
for _, id := range ids {
|
||||
if id == nil {
|
||||
t.Fatal("response-retry content update omitted its protocol message ID")
|
||||
}
|
||||
messageID := string(*id)
|
||||
if _, err := uuid.Parse(messageID); err != nil {
|
||||
t.Fatalf("protocol message ID %q is not a UUID: %v", *id, err)
|
||||
}
|
||||
if _, exists := seen[messageID]; exists {
|
||||
t.Fatalf("protocol message ID %q was reused across attempts or content types", *id)
|
||||
}
|
||||
seen[messageID] = struct{}{}
|
||||
}
|
||||
}
|
||||
@@ -54,6 +54,7 @@ var scenarioRegistry = map[string]func(e *emitter){
|
||||
"steer-fold-setup": scenarioSteerFoldSetup,
|
||||
"steer-defer-setup": scenarioSteerDeferSetup,
|
||||
"saved-prompt-delivery": scenarioSavedPromptDelivery,
|
||||
"response-retry": scenarioResponseRetry,
|
||||
"goal-active": scenarioGoalActive,
|
||||
"goal-complete": scenarioGoalComplete,
|
||||
"goal-clear": scenarioGoalClear,
|
||||
@@ -138,6 +139,13 @@ func scenarioSimpleMessage(e *emitter) {
|
||||
e.text("This is a simple mock response for e2e testing.")
|
||||
}
|
||||
|
||||
func scenarioResponseRetry(e *emitter) {
|
||||
e.thoughtWithID("Abandoned response attempt reasoning.")
|
||||
e.textWithID("Abandoned response attempt answer.")
|
||||
e.responseAttemptReset()
|
||||
e.textWithID("Replacement response after provider retry.")
|
||||
}
|
||||
|
||||
// scenarioReadAndEdit: read -> edit -> text with fixed delays, using real files.
|
||||
func scenarioReadAndEdit(e *emitter) {
|
||||
f := randomFile()
|
||||
|
||||
@@ -195,6 +195,9 @@ type AgentStreamEventData struct {
|
||||
IsAppend bool `json:"is_append,omitempty"`
|
||||
// MessageType distinguishes between "message" and "thinking" content types
|
||||
MessageType string `json:"message_type,omitempty"`
|
||||
// RetractedMessageIDs lists abandoned assistant and thinking records in
|
||||
// allocation order for a response-attempt reset.
|
||||
RetractedMessageIDs []string `json:"retracted_message_ids,omitempty"`
|
||||
|
||||
// AvailableCommands contains the slash commands available from the agent.
|
||||
// Populated when Type is "available_commands".
|
||||
|
||||
@@ -310,6 +310,7 @@ func buildAgentStreamEventData(event agentctl.AgentEvent) *AgentStreamEventData
|
||||
ProviderError: event.ProviderError,
|
||||
SessionStatus: event.SessionStatus,
|
||||
PromptGeneration: event.PromptGeneration,
|
||||
RetractedMessageIDs: append([]string(nil), event.RetractedMessageIDs...),
|
||||
TurnID: event.TurnID,
|
||||
Data: event.Data,
|
||||
Normalized: event.NormalizedPayload,
|
||||
|
||||
@@ -153,6 +153,24 @@ func (s *ExecutionStore) OwnsPromptGeneration(sessionID, executionID string, gen
|
||||
return exists && generation != 0 && execution.promptGeneration == generation
|
||||
}
|
||||
|
||||
func (s *ExecutionStore) ownsActivePromptGeneration(
|
||||
sessionID, executionID string,
|
||||
generation uint64,
|
||||
) bool {
|
||||
s.mu.RLock()
|
||||
defer s.mu.RUnlock()
|
||||
|
||||
currentExecutionID, exists := s.bySession[sessionID]
|
||||
if !exists || currentExecutionID != executionID {
|
||||
return false
|
||||
}
|
||||
execution, exists := s.executions[currentExecutionID]
|
||||
return exists && generation != 0 &&
|
||||
execution.promptGeneration == generation &&
|
||||
execution.dispatchedPromptGeneration == generation &&
|
||||
execution.promptCompletionGeneration != generation
|
||||
}
|
||||
|
||||
// OwnsPromptActivity reports whether the execution still owns the prompt and
|
||||
// its activity has not changed since the watchdog captured its snapshot.
|
||||
func (s *ExecutionStore) OwnsPromptActivity(
|
||||
|
||||
@@ -451,6 +451,7 @@ func (m *Manager) handleCompleteEventLeased(execution *AgentExecution, event *ag
|
||||
flushedText := m.flushMessageBuffer(execution, event.PromptGeneration, event.AttemptID)
|
||||
execution.messageMu.Lock()
|
||||
execution.clearProtocolMessageCorrelationLocked()
|
||||
execution.commitResponseAttemptLocked()
|
||||
execution.messageMu.Unlock()
|
||||
if flushedText != "" {
|
||||
event.Text = flushedText
|
||||
@@ -491,6 +492,9 @@ func (m *Manager) handleToolCallEvent(execution *AgentExecution, event agentctl.
|
||||
// flushMessageBuffer publishes any remaining buffered content through
|
||||
// the streaming path itself and always returns "".
|
||||
m.flushMessageBuffer(execution, promptGeneration, event.AttemptID)
|
||||
execution.messageMu.Lock()
|
||||
execution.commitResponseAttemptLocked()
|
||||
execution.messageMu.Unlock()
|
||||
}
|
||||
m.flushAssistantHistory(execution)
|
||||
if m.historyManager != nil && execution.historyEnabled && execution.SessionID != "" {
|
||||
@@ -833,6 +837,7 @@ func (m *Manager) handlePromptHandoffEvent(
|
||||
m.flushMessageBuffer(execution, event.PromptGeneration, event.AttemptID)
|
||||
execution.messageMu.Lock()
|
||||
execution.clearProtocolMessageCorrelationLocked()
|
||||
execution.commitResponseAttemptLocked()
|
||||
execution.messageMu.Unlock()
|
||||
m.flushAssistantHistory(execution)
|
||||
|
||||
@@ -953,6 +958,8 @@ func (m *Manager) handleAgentEventWithoutPublication(execution *AgentExecution,
|
||||
case "reasoning":
|
||||
m.handleReasoningEvent(execution, *event)
|
||||
return true
|
||||
case streams.EventTypeResponseAttemptReset:
|
||||
return !m.handleResponseAttemptReset(execution, event)
|
||||
case toolStatusError:
|
||||
m.handleErrorEvent(execution, *event)
|
||||
return true
|
||||
@@ -1025,6 +1032,30 @@ func (m *Manager) handleAgentEventState(execution *AgentExecution, event agentct
|
||||
return event
|
||||
}
|
||||
|
||||
func (m *Manager) handleResponseAttemptReset(
|
||||
execution *AgentExecution,
|
||||
event *agentctl.AgentEvent,
|
||||
) bool {
|
||||
if event.PromptGeneration == 0 {
|
||||
return false
|
||||
}
|
||||
execution.promptLifecycleMu.Lock()
|
||||
defer execution.promptLifecycleMu.Unlock()
|
||||
|
||||
if !m.executionStore.ownsActivePromptGeneration(
|
||||
execution.SessionID,
|
||||
execution.ID,
|
||||
event.PromptGeneration,
|
||||
) {
|
||||
return false
|
||||
}
|
||||
m.flushStreamCoalescer(execution)
|
||||
execution.messageMu.Lock()
|
||||
event.RetractedMessageIDs = execution.detachResponseAttemptLocked()
|
||||
execution.messageMu.Unlock()
|
||||
return true
|
||||
}
|
||||
|
||||
func (m *Manager) handleAgentEventWithStartupGeneration(
|
||||
execution *AgentExecution,
|
||||
event agentctl.AgentEvent,
|
||||
|
||||
@@ -0,0 +1,312 @@
|
||||
package lifecycle
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"reflect"
|
||||
"testing"
|
||||
|
||||
agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
)
|
||||
|
||||
// @covers AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.27
|
||||
func TestHandleResponseAttemptResetRetractsCurrentAttemptRecords(t *testing.T) {
|
||||
mgr, eventBus := createTestManagerWithTracking()
|
||||
execution := createTestExecution("exec-1", "task-1", "session-1")
|
||||
if err := mgr.executionStore.Add(execution); err != nil {
|
||||
t.Fatalf("add execution: %v", err)
|
||||
}
|
||||
generation, err := mgr.executionStore.BeginPrompt(execution.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("begin prompt: %v", err)
|
||||
}
|
||||
mgr.executionStore.MarkPromptDispatched(execution.ID, generation)
|
||||
t.Cleanup(func() { mgr.closeStreamCoalescer(execution) })
|
||||
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeMessageChunk,
|
||||
Text: "abandoned answer",
|
||||
ProtocolMessageID: "assistant-attempt-1",
|
||||
})
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeReasoning,
|
||||
ReasoningText: "abandoned reasoning",
|
||||
ProtocolMessageID: "thinking-attempt-1",
|
||||
})
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: "response_attempt_reset",
|
||||
PromptGeneration: generation,
|
||||
})
|
||||
|
||||
streamed := eventBus.getStreamEvents()
|
||||
if len(streamed) != 3 {
|
||||
t.Fatalf("streamed events = %+v, want assistant, thinking, then reset", streamed)
|
||||
}
|
||||
if streamed[0].Data.Type != "message_streaming" || streamed[1].Data.Type != thinkingStreamingEventType {
|
||||
t.Fatalf("events before reset = %+v, want assistant then thinking creation", streamed[:2])
|
||||
}
|
||||
wantIDs := []string{streamed[0].Data.MessageID, streamed[1].Data.MessageID}
|
||||
resets := streamEventsOfType(eventBus, "response_attempt_reset")
|
||||
if len(resets) != 1 {
|
||||
t.Fatalf("reset events = %+v, want one", resets)
|
||||
}
|
||||
encoded, err := json.Marshal(resets[0].Data)
|
||||
if err != nil {
|
||||
t.Fatalf("marshal reset data: %v", err)
|
||||
}
|
||||
var wire map[string]any
|
||||
if err := json.Unmarshal(encoded, &wire); err != nil {
|
||||
t.Fatalf("decode reset data: %v", err)
|
||||
}
|
||||
rawIDs, _ := wire["retracted_message_ids"].([]any)
|
||||
gotIDs := make([]string, 0, len(rawIDs))
|
||||
for _, rawID := range rawIDs {
|
||||
if id, ok := rawID.(string); ok {
|
||||
gotIDs = append(gotIDs, id)
|
||||
}
|
||||
}
|
||||
if !reflect.DeepEqual(gotIDs, wantIDs) {
|
||||
t.Fatalf("retracted IDs = %v, want allocation order %v", gotIDs, wantIDs)
|
||||
}
|
||||
|
||||
execution.messageMu.Lock()
|
||||
messageMap := execution.protocolMessageIDs
|
||||
thinkingMap := execution.protocolThinkingIDs
|
||||
history := execution.assistantHistoryBuffer.String()
|
||||
execution.messageMu.Unlock()
|
||||
if messageMap != nil || thinkingMap != nil || history != "" {
|
||||
t.Fatalf("abandoned stream state remains: messages=%v thinking=%v history=%q", messageMap, thinkingMap, history)
|
||||
}
|
||||
if evidence := execution.promptAttemptEvidenceSnapshot(); !evidence.OutputObserved || !evidence.EffectObserved {
|
||||
t.Fatalf("reset relaxed prompt replay evidence: %+v", evidence)
|
||||
}
|
||||
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: "response_attempt_reset",
|
||||
PromptGeneration: generation,
|
||||
})
|
||||
resets = streamEventsOfType(eventBus, "response_attempt_reset")
|
||||
if len(resets) != 2 || len(resets[1].Data.RetractedMessageIDs) != 0 {
|
||||
t.Fatalf("repeated reset = %+v, want one empty no-op boundary", resets)
|
||||
}
|
||||
}
|
||||
|
||||
// @covers AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.28
|
||||
func TestResponseAttemptResetRequiresDispatchedPrompt(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
completed bool
|
||||
}{
|
||||
{name: "admitted but not dispatched"},
|
||||
{name: "already completed", completed: true},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
mgr, eventBus := createTestManagerWithTracking()
|
||||
execution := createTestExecution("exec-1", "task-1", "session-1")
|
||||
if err := mgr.executionStore.Add(execution); err != nil {
|
||||
t.Fatalf("add execution: %v", err)
|
||||
}
|
||||
generation, err := mgr.executionStore.BeginPrompt(execution.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("begin prompt: %v", err)
|
||||
}
|
||||
if test.completed {
|
||||
mgr.executionStore.MarkPromptDispatched(execution.ID, generation)
|
||||
if err := mgr.executionStore.WithLock(execution.ID, func(current *AgentExecution) {
|
||||
current.promptCompletionGeneration = generation
|
||||
}); err != nil {
|
||||
t.Fatalf("complete prompt: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: "response_attempt_reset",
|
||||
PromptGeneration: generation,
|
||||
})
|
||||
|
||||
if resets := streamEventsOfType(eventBus, "response_attempt_reset"); len(resets) != 0 {
|
||||
t.Fatalf("inactive prompt published reset: %+v", resets)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestResponseAttemptRecordsIncludeLegacyStreams(t *testing.T) {
|
||||
mgr, eventBus := createTestManagerWithTracking()
|
||||
execution := createTestExecution("exec-1", "task-1", "session-1")
|
||||
if err := mgr.executionStore.Add(execution); err != nil {
|
||||
t.Fatalf("add execution: %v", err)
|
||||
}
|
||||
generation, err := mgr.executionStore.BeginPrompt(execution.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("begin prompt: %v", err)
|
||||
}
|
||||
mgr.executionStore.MarkPromptDispatched(execution.ID, generation)
|
||||
t.Cleanup(func() { mgr.closeStreamCoalescer(execution) })
|
||||
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeMessageChunk,
|
||||
Text: "legacy answer\n",
|
||||
})
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeReasoning,
|
||||
ReasoningText: "legacy reasoning\n",
|
||||
})
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeResponseAttemptReset,
|
||||
PromptGeneration: generation,
|
||||
})
|
||||
|
||||
events := eventBus.getStreamEvents()
|
||||
if len(events) != 3 {
|
||||
t.Fatalf("events = %+v, want legacy assistant, thinking, then reset", events)
|
||||
}
|
||||
wantIDs := []string{events[0].Data.MessageID, events[1].Data.MessageID}
|
||||
if got := events[2].Data.RetractedMessageIDs; !reflect.DeepEqual(got, wantIDs) {
|
||||
t.Fatalf("legacy retracted IDs = %v, want %v", got, wantIDs)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResponseAttemptResetPreservesCommittedProtocolRecord(t *testing.T) {
|
||||
mgr, eventBus := createTestManagerWithTracking()
|
||||
execution := createTestExecution("exec-1", "task-1", "session-1")
|
||||
if err := mgr.executionStore.Add(execution); err != nil {
|
||||
t.Fatalf("add execution: %v", err)
|
||||
}
|
||||
generation, err := mgr.executionStore.BeginPrompt(execution.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("begin prompt: %v", err)
|
||||
}
|
||||
mgr.executionStore.MarkPromptDispatched(execution.ID, generation)
|
||||
t.Cleanup(func() { mgr.closeStreamCoalescer(execution) })
|
||||
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeMessageChunk,
|
||||
Text: "committed answer",
|
||||
ProtocolMessageID: "shared-protocol-id",
|
||||
})
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeToolCall,
|
||||
ToolCallID: "tool-1",
|
||||
ToolName: "read_file",
|
||||
})
|
||||
committedEvents := streamEventsOfType(eventBus, "message_streaming")
|
||||
if len(committedEvents) != 1 {
|
||||
t.Fatalf("committed message events = %+v, want one", committedEvents)
|
||||
}
|
||||
committedID := committedEvents[0].Data.MessageID
|
||||
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeMessageChunk,
|
||||
Text: " abandoned append",
|
||||
ProtocolMessageID: "shared-protocol-id",
|
||||
})
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeMessageChunk,
|
||||
Text: "abandoned new record",
|
||||
ProtocolMessageID: "new-protocol-id",
|
||||
})
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeResponseAttemptReset,
|
||||
PromptGeneration: generation,
|
||||
})
|
||||
|
||||
resets := streamEventsOfType(eventBus, streams.EventTypeResponseAttemptReset)
|
||||
if len(resets) != 1 || len(resets[0].Data.RetractedMessageIDs) != 1 {
|
||||
t.Fatalf("reset = %+v, want only the newly allocated record", resets)
|
||||
}
|
||||
if resets[0].Data.RetractedMessageIDs[0] == committedID {
|
||||
t.Fatal("protocol ID reuse made a committed record deletion-eligible")
|
||||
}
|
||||
execution.messageMu.Lock()
|
||||
retainedID := execution.protocolMessageIDs["shared-protocol-id"]
|
||||
_, newRetained := execution.protocolMessageIDs["new-protocol-id"]
|
||||
execution.messageMu.Unlock()
|
||||
if retainedID != committedID || newRetained {
|
||||
t.Fatalf(
|
||||
"protocol correlations after reset: shared=%q new_present=%t, want shared=%q new_present=false",
|
||||
retainedID,
|
||||
newRetained,
|
||||
committedID,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
func TestResponseAttemptResetRejectsStaleGenerationAndExecution(t *testing.T) {
|
||||
t.Run("stale generation", func(t *testing.T) {
|
||||
mgr, eventBus := createTestManagerWithTracking()
|
||||
execution := createTestExecution("exec-1", "task-1", "session-1")
|
||||
if err := mgr.executionStore.Add(execution); err != nil {
|
||||
t.Fatalf("add execution: %v", err)
|
||||
}
|
||||
first, err := mgr.executionStore.BeginPrompt(execution.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("begin first prompt: %v", err)
|
||||
}
|
||||
second, err := mgr.executionStore.BeginPrompt(execution.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("begin second prompt: %v", err)
|
||||
}
|
||||
mgr.executionStore.MarkPromptDispatched(execution.ID, second)
|
||||
t.Cleanup(func() { mgr.closeStreamCoalescer(execution) })
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeMessageChunk,
|
||||
Text: "replacement",
|
||||
ProtocolMessageID: "replacement-id",
|
||||
})
|
||||
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeResponseAttemptReset,
|
||||
PromptGeneration: first,
|
||||
})
|
||||
|
||||
if resets := streamEventsOfType(eventBus, streams.EventTypeResponseAttemptReset); len(resets) != 0 {
|
||||
t.Fatalf("stale generation published reset: %+v", resets)
|
||||
}
|
||||
execution.messageMu.Lock()
|
||||
retained := execution.protocolMessageIDs["replacement-id"]
|
||||
execution.messageMu.Unlock()
|
||||
if retained == "" {
|
||||
t.Fatal("stale generation detached replacement record")
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("stale execution", func(t *testing.T) {
|
||||
mgr, eventBus := createTestManagerWithTracking()
|
||||
stale := createTestExecution("exec-old", "task-1", "session-1")
|
||||
if err := mgr.executionStore.Add(stale); err != nil {
|
||||
t.Fatalf("add stale execution: %v", err)
|
||||
}
|
||||
generation, err := mgr.executionStore.BeginPrompt(stale.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("begin prompt: %v", err)
|
||||
}
|
||||
mgr.executionStore.MarkPromptDispatched(stale.ID, generation)
|
||||
t.Cleanup(func() { mgr.closeStreamCoalescer(stale) })
|
||||
mgr.handleAgentEvent(stale, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeMessageChunk,
|
||||
Text: "old execution output",
|
||||
ProtocolMessageID: "old-id",
|
||||
})
|
||||
|
||||
mgr.executionStore.Remove(stale.ID)
|
||||
replacement := createTestExecution("exec-new", "task-1", "session-1")
|
||||
if err := mgr.executionStore.Add(replacement); err != nil {
|
||||
t.Fatalf("add replacement execution: %v", err)
|
||||
}
|
||||
mgr.handleAgentEvent(stale, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeResponseAttemptReset,
|
||||
PromptGeneration: generation,
|
||||
})
|
||||
|
||||
if resets := streamEventsOfType(eventBus, streams.EventTypeResponseAttemptReset); len(resets) != 0 {
|
||||
t.Fatalf("stale execution published reset: %+v", resets)
|
||||
}
|
||||
stale.messageMu.Lock()
|
||||
retained := stale.protocolMessageIDs["old-id"]
|
||||
stale.messageMu.Unlock()
|
||||
if retained == "" {
|
||||
t.Fatal("stale execution detached its stream state")
|
||||
}
|
||||
})
|
||||
}
|
||||
@@ -16,6 +16,45 @@ func (e *AgentExecution) clearProtocolMessageCorrelationLocked() {
|
||||
e.protocolThinkingIDs = nil
|
||||
}
|
||||
|
||||
func (e *AgentExecution) trackResponseAttemptMessageLocked(messageID string) {
|
||||
e.responseAttemptMessageIDs = append(e.responseAttemptMessageIDs, messageID)
|
||||
}
|
||||
|
||||
func (e *AgentExecution) commitResponseAttemptLocked() {
|
||||
e.responseAttemptMessageIDs = nil
|
||||
}
|
||||
|
||||
func (e *AgentExecution) detachResponseAttemptLocked() []string {
|
||||
messageIDs := append([]string(nil), e.responseAttemptMessageIDs...)
|
||||
owned := make(map[string]struct{}, len(messageIDs))
|
||||
for _, messageID := range messageIDs {
|
||||
owned[messageID] = struct{}{}
|
||||
}
|
||||
for protocolID, messageID := range e.protocolMessageIDs {
|
||||
if _, ok := owned[messageID]; ok {
|
||||
delete(e.protocolMessageIDs, protocolID)
|
||||
}
|
||||
}
|
||||
for protocolID, messageID := range e.protocolThinkingIDs {
|
||||
if _, ok := owned[messageID]; ok {
|
||||
delete(e.protocolThinkingIDs, protocolID)
|
||||
}
|
||||
}
|
||||
if len(e.protocolMessageIDs) == 0 {
|
||||
e.protocolMessageIDs = nil
|
||||
}
|
||||
if len(e.protocolThinkingIDs) == 0 {
|
||||
e.protocolThinkingIDs = nil
|
||||
}
|
||||
e.messageBuffer.Reset()
|
||||
e.thinkingBuffer.Reset()
|
||||
e.assistantHistoryBuffer.Reset()
|
||||
e.currentMessageID = ""
|
||||
e.currentThinkingID = ""
|
||||
e.commitResponseAttemptLocked()
|
||||
return messageIDs
|
||||
}
|
||||
|
||||
func (m *Manager) streamCoalescer(execution *AgentExecution) *streamCoalescer {
|
||||
execution.streamMu.Lock()
|
||||
defer execution.streamMu.Unlock()
|
||||
@@ -96,6 +135,7 @@ func (e *AgentExecution) resetStreamingStateLocked() {
|
||||
e.currentMessageID = ""
|
||||
e.currentThinkingID = ""
|
||||
e.clearProtocolMessageCorrelationLocked()
|
||||
e.commitResponseAttemptLocked()
|
||||
}
|
||||
|
||||
func protocolRecordID(ids *map[string]string, protocolMessageID string) (string, bool) {
|
||||
@@ -120,6 +160,9 @@ func (m *Manager) publishProtocolMessage(
|
||||
) {
|
||||
execution.messageMu.Lock()
|
||||
messageID, isAppend := protocolRecordID(&execution.protocolMessageIDs, protocolMessageID)
|
||||
if !isAppend {
|
||||
execution.trackResponseAttemptMessageLocked(messageID)
|
||||
}
|
||||
execution.messageMu.Unlock()
|
||||
|
||||
m.publishStreamingContent(execution, "message_streaming", messageID, content, isAppend, diagnostic, promptGeneration, attemptID)
|
||||
@@ -265,6 +308,9 @@ func (m *Manager) publishProtocolThinking(
|
||||
) {
|
||||
execution.messageMu.Lock()
|
||||
messageID, isAppend := protocolRecordID(&execution.protocolThinkingIDs, protocolMessageID)
|
||||
if !isAppend {
|
||||
execution.trackResponseAttemptMessageLocked(messageID)
|
||||
}
|
||||
execution.messageMu.Unlock()
|
||||
|
||||
m.enqueueStreamingContent(execution, thinkingStreamingEventType, messageID, content, isAppend, false, promptGeneration, attemptID)
|
||||
@@ -334,6 +380,7 @@ func (m *Manager) publishStreamingMessage(execution *AgentExecution, content str
|
||||
if !isAppend {
|
||||
messageID = uuid.New().String()
|
||||
execution.currentMessageID = messageID
|
||||
execution.trackResponseAttemptMessageLocked(messageID)
|
||||
}
|
||||
execution.messageMu.Unlock()
|
||||
|
||||
@@ -441,6 +488,7 @@ func (m *Manager) publishStreamingThinking(execution *AgentExecution, content st
|
||||
if !isAppend {
|
||||
thinkingID = uuid.New().String()
|
||||
execution.currentThinkingID = thinkingID
|
||||
execution.trackResponseAttemptMessageLocked(thinkingID)
|
||||
}
|
||||
execution.messageMu.Unlock()
|
||||
|
||||
|
||||
@@ -225,6 +225,9 @@ type AgentExecution struct {
|
||||
// reusing the same source ID cannot merge visible and reasoning content.
|
||||
protocolMessageIDs map[string]string
|
||||
protocolThinkingIDs map[string]string
|
||||
// responseAttemptMessageIDs preserves allocation order for assistant and
|
||||
// thinking records created since the latest committed response boundary.
|
||||
responseAttemptMessageIDs []string
|
||||
// assistantHistoryBuffer accumulates assistant chunks in wire order for
|
||||
// history-context injection. Tool and completion boundaries persist it as
|
||||
// one segment before recording the boundary event.
|
||||
|
||||
@@ -177,6 +177,13 @@ func (a *Adapter) handleACPUpdate(
|
||||
var event, leadingEvent *AgentEvent
|
||||
if !suppressed {
|
||||
event = a.convertNotification(n)
|
||||
if a.observesResponseAttemptReset(promptGeneration, event) {
|
||||
leadingEvent = &AgentEvent{
|
||||
Type: streams.EventTypeResponseAttemptReset,
|
||||
SessionID: sessionID,
|
||||
PromptGeneration: promptGeneration,
|
||||
}
|
||||
}
|
||||
if event != nil && (a.observeCodexProviderEvidence(promptGeneration, event) ||
|
||||
a.observeCursorRetriableEvidence(promptGeneration, event)) {
|
||||
// Suppress provider control/evidence chunks. The adapter emits one
|
||||
@@ -240,6 +247,15 @@ func (a *Adapter) handleACPUpdate(
|
||||
}
|
||||
}
|
||||
|
||||
func (a *Adapter) observesResponseAttemptReset(promptGeneration uint64, event *AgentEvent) bool {
|
||||
if event == nil || event.Type != streams.EventTypeSessionInfo || promptGeneration == 0 ||
|
||||
!a.dialect.resetsResponseAttempt(event.SessionMeta) {
|
||||
return false
|
||||
}
|
||||
turn := a.currentPromptTurn()
|
||||
return turn != nil && turn.promptGeneration == promptGeneration
|
||||
}
|
||||
|
||||
func (a *Adapter) observeCursorRetriableEvidence(promptGeneration uint64, event *AgentEvent) bool {
|
||||
if a.agentID != acpcompat.CursorAgentID || event == nil || promptGeneration == 0 {
|
||||
return false
|
||||
|
||||
@@ -27,6 +27,7 @@ type acpDialect struct {
|
||||
subagentFrame func(map[string]any, string, any) (subagentFrame, bool)
|
||||
mcpToolCall func(map[string]any, any) (mcpToolCallFrame, bool)
|
||||
mcpToolResult func(any) (any, bool)
|
||||
responseAttemptReset func(map[string]any) bool
|
||||
}
|
||||
|
||||
type mcpToolCallFrame struct {
|
||||
@@ -58,6 +59,8 @@ func newACPDialect(agentID string) acpDialect {
|
||||
return newGrokACPDialect()
|
||||
case codexAgentID:
|
||||
return newCodexACPDialect()
|
||||
case mockAgentID:
|
||||
return newMockACPDialect()
|
||||
}
|
||||
return acpDialect{}
|
||||
}
|
||||
@@ -148,3 +151,7 @@ func (d acpDialect) promptUsage(
|
||||
}
|
||||
return d.normalizePromptUsage(usage, meta)
|
||||
}
|
||||
|
||||
func (d acpDialect) resetsResponseAttempt(meta map[string]any) bool {
|
||||
return d.responseAttemptReset != nil && d.responseAttemptReset(meta)
|
||||
}
|
||||
|
||||
@@ -33,9 +33,31 @@ func newCodexACPDialect() acpDialect {
|
||||
normalizePromptUsage: normalizeCodexPromptUsage,
|
||||
mcpToolCall: parseCodexMCPToolCall,
|
||||
mcpToolResult: normalizeCodexMCPToolResult,
|
||||
responseAttemptReset: codexResponseAttemptResetMeta,
|
||||
}
|
||||
}
|
||||
|
||||
func codexResponseAttemptResetMeta(meta map[string]any) bool {
|
||||
codex, ok := nestedMap(meta, "codex")
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
errorMeta, ok := nestedMap(codex, "error")
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
willRetry, ok := errorMeta["willRetry"].(bool)
|
||||
if !ok || !willRetry {
|
||||
return false
|
||||
}
|
||||
errorInfo, ok := nestedMap(errorMeta, "codexErrorInfo")
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
_, ok = nestedMap(errorInfo, "responseStreamDisconnected")
|
||||
return ok
|
||||
}
|
||||
|
||||
// normalizeCodexPromptUsage marks codex-acp's typed usage frame as
|
||||
// estimated. codex-acp 1.4.0 does emit a typed usage block on the prompt
|
||||
// response, but its three response-construction sites (normal end_turn,
|
||||
|
||||
+136
@@ -0,0 +1,136 @@
|
||||
package acp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
acpsdk "github.com/coder/acp-go-sdk"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
)
|
||||
|
||||
func codexResponseAttemptResetNotification(t *testing.T) acpsdk.SessionNotification {
|
||||
t.Helper()
|
||||
var notification acpsdk.SessionNotification
|
||||
raw := []byte(`{"sessionId":"session-1","update":{"sessionUpdate":"session_info_update","_meta":{"codex":{"error":{"willRetry":true,"codexErrorInfo":{"responseStreamDisconnected":{}}}}}}}`)
|
||||
if err := json.Unmarshal(raw, ¬ification); err != nil {
|
||||
t.Fatalf("decode notification: %v", err)
|
||||
}
|
||||
return notification
|
||||
}
|
||||
|
||||
func TestCodexResponseAttemptResetRequiresExactStructuredMetadata(t *testing.T) {
|
||||
valid := map[string]any{
|
||||
"codex": map[string]any{
|
||||
"error": map[string]any{
|
||||
"willRetry": true,
|
||||
"codexErrorInfo": map[string]any{
|
||||
"responseStreamDisconnected": map[string]any{},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
if !codexResponseAttemptResetMeta(valid) {
|
||||
t.Fatal("exact Codex response-disconnect retry metadata was not recognized")
|
||||
}
|
||||
|
||||
tests := map[string]map[string]any{
|
||||
"false retry flag": {
|
||||
"codex": map[string]any{"error": map[string]any{
|
||||
"willRetry": false,
|
||||
"codexErrorInfo": map[string]any{
|
||||
"responseStreamDisconnected": map[string]any{},
|
||||
},
|
||||
}},
|
||||
},
|
||||
"string retry flag": {
|
||||
"codex": map[string]any{"error": map[string]any{
|
||||
"willRetry": "true",
|
||||
"codexErrorInfo": map[string]any{
|
||||
"responseStreamDisconnected": map[string]any{},
|
||||
},
|
||||
}},
|
||||
},
|
||||
"missing disconnect object": {
|
||||
"codex": map[string]any{"error": map[string]any{
|
||||
"willRetry": true,
|
||||
"codexErrorInfo": map[string]any{},
|
||||
}},
|
||||
},
|
||||
"string disconnect value": {
|
||||
"codex": map[string]any{"error": map[string]any{
|
||||
"willRetry": true,
|
||||
"codexErrorInfo": map[string]any{
|
||||
"responseStreamDisconnected": "connection reset",
|
||||
},
|
||||
}},
|
||||
},
|
||||
}
|
||||
for name, meta := range tests {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
if codexResponseAttemptResetMeta(meta) {
|
||||
t.Fatal("malformed metadata was recognized as a response-attempt reset")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestObserveResponseAttemptResetRequiresCurrentPromptGeneration(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
a.dialect = newCodexACPDialect()
|
||||
t.Cleanup(func() { _ = a.Close() })
|
||||
_, turn := a.registerPromptTurn(context.Background(), 7)
|
||||
t.Cleanup(func() { a.clearPromptTurn(turn) })
|
||||
|
||||
event := a.convertNotification(codexResponseAttemptResetNotification(t))
|
||||
if a.observesResponseAttemptReset(0, event) {
|
||||
t.Fatal("generation-zero metadata produced a response-attempt reset")
|
||||
}
|
||||
if a.observesResponseAttemptReset(6, event) {
|
||||
t.Fatal("stale metadata produced a response-attempt reset")
|
||||
}
|
||||
if !a.observesResponseAttemptReset(7, event) {
|
||||
t.Fatal("current prompt metadata did not produce a response-attempt reset")
|
||||
}
|
||||
if (acpDialect{}).resetsResponseAttempt(event.SessionMeta) {
|
||||
t.Fatal("unsupported dialect recognized Codex retry metadata")
|
||||
}
|
||||
}
|
||||
|
||||
// @covers AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.26
|
||||
func TestHandleACPUpdateCodexResponseAttemptReset(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
a.agentID = codexAgentID
|
||||
a.dialect = newCodexACPDialect()
|
||||
t.Cleanup(func() { _ = a.Close() })
|
||||
|
||||
_, turn := a.registerPromptTurn(context.Background(), 7)
|
||||
t.Cleanup(func() { a.clearPromptTurn(turn) })
|
||||
|
||||
a.handleACPUpdate(codexResponseAttemptResetNotification(t), 7)
|
||||
a.handleACPUpdate(makeNotification("session-1", acpsdk.SessionUpdate{
|
||||
AgentMessageChunk: &acpsdk.SessionUpdateAgentMessageChunk{
|
||||
Content: acpsdk.TextBlock("replacement"),
|
||||
},
|
||||
}), 7)
|
||||
|
||||
events := drainEvents(a)
|
||||
if len(events) != 3 {
|
||||
t.Fatalf("events = %+v, want reset boundary, session info, then replacement", events)
|
||||
}
|
||||
if events[0].Type != streams.EventTypeResponseAttemptReset {
|
||||
t.Fatalf("first event type = %q, want %q", events[0].Type, streams.EventTypeResponseAttemptReset)
|
||||
}
|
||||
if events[0].PromptGeneration != 7 {
|
||||
t.Fatalf("reset generation = %d, want 7", events[0].PromptGeneration)
|
||||
}
|
||||
if events[0].SessionMeta != nil || events[0].Data != nil {
|
||||
t.Fatalf("reset exposed provider metadata: %+v", events[0])
|
||||
}
|
||||
if events[1].Type != streams.EventTypeSessionInfo {
|
||||
t.Fatalf("second event type = %q, want %q", events[1].Type, streams.EventTypeSessionInfo)
|
||||
}
|
||||
if events[2].Type != streams.EventTypeMessageChunk || events[2].Text != "replacement" {
|
||||
t.Fatalf("third event = %+v, want replacement message chunk", events[2])
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,14 @@
|
||||
package acp
|
||||
|
||||
func newMockACPDialect() acpDialect {
|
||||
return acpDialect{responseAttemptReset: mockResponseAttemptResetMeta}
|
||||
}
|
||||
|
||||
func mockResponseAttemptResetMeta(meta map[string]any) bool {
|
||||
mock, ok := nestedMap(meta, "kandevMock")
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
reset, ok := mock["responseAttemptReset"].(bool)
|
||||
return ok && reset
|
||||
}
|
||||
@@ -0,0 +1,49 @@
|
||||
package acp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
acpsdk "github.com/coder/acp-go-sdk"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
)
|
||||
|
||||
func TestHandleACPUpdateMockResponseRetry(t *testing.T) {
|
||||
var notification acpsdk.SessionNotification
|
||||
raw := []byte(`{"sessionId":"session-1","update":{"sessionUpdate":"session_info_update","_meta":{"kandevMock":{"responseAttemptReset":true}}}}`)
|
||||
if err := json.Unmarshal(raw, ¬ification); err != nil {
|
||||
t.Fatalf("decode notification: %v", err)
|
||||
}
|
||||
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
agentID string
|
||||
wantReset bool
|
||||
}{
|
||||
{name: "controlled mock", agentID: mockAgentID, wantReset: true},
|
||||
{name: "production dialect", agentID: codexAgentID},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
a.agentID = test.agentID
|
||||
a.dialect = newACPDialect(test.agentID)
|
||||
t.Cleanup(func() { _ = a.Close() })
|
||||
_, turn := a.registerPromptTurn(context.Background(), 7)
|
||||
t.Cleanup(func() { a.clearPromptTurn(turn) })
|
||||
|
||||
a.handleACPUpdate(notification, 7)
|
||||
|
||||
events := drainEvents(a)
|
||||
if test.wantReset {
|
||||
if len(events) != 2 || events[0].Type != streams.EventTypeResponseAttemptReset {
|
||||
t.Fatalf("events = %+v, want reset then session info", events)
|
||||
}
|
||||
return
|
||||
}
|
||||
if len(events) != 1 || events[0].Type != streams.EventTypeSessionInfo {
|
||||
t.Fatalf("production dialect events = %+v, want session info only", events)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -77,6 +77,10 @@ const (
|
||||
// EventTypeSessionInfo indicates ACP session metadata such as title changed.
|
||||
EventTypeSessionInfo = "session_info"
|
||||
|
||||
// EventTypeResponseAttemptReset indicates that the provider abandoned its
|
||||
// current response attempt and will continue the active prompt with a new one.
|
||||
EventTypeResponseAttemptReset = "response_attempt_reset"
|
||||
|
||||
// EventTypeAuthRequired indicates the agent rejected session/new with an
|
||||
// authentication-required error. The event carries the available auth
|
||||
// methods (from ACP initialize); the client picks one and replays the
|
||||
@@ -168,6 +172,10 @@ type AgentEvent struct {
|
||||
// It is distinct from the Kandev message record ID used by downstream streaming.
|
||||
ProtocolMessageID string `json:"protocol_message_id,omitempty"`
|
||||
|
||||
// RetractedMessageIDs identifies Kandev transcript records owned by an
|
||||
// abandoned response attempt. It is populated only on response-attempt resets.
|
||||
RetractedMessageIDs []string `json:"retracted_message_ids,omitempty"`
|
||||
|
||||
// --- Reasoning fields (for "reasoning" type) ---
|
||||
|
||||
// ReasoningText contains full reasoning/chain-of-thought content.
|
||||
|
||||
@@ -166,6 +166,7 @@ func provideOrchestrator(
|
||||
msgCreator := &messageCreatorAdapter{svc: taskSvc, logger: log}
|
||||
orchestratorSvc.SetMessageCreator(msgCreator)
|
||||
orchestratorSvc.SetTransientRetryMessageService(taskSvc)
|
||||
orchestratorSvc.SetStreamingMessageRetractionService(taskSvc)
|
||||
orchestratorSvc.SetSubagentContextRecorder(&subagentContextAdapter{svc: taskSvc})
|
||||
|
||||
orchestratorSvc.SetTurnService(newTurnServiceAdapter(taskSvc))
|
||||
|
||||
@@ -0,0 +1,29 @@
|
||||
package backendapp
|
||||
|
||||
import (
|
||||
"go/ast"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestProvideOrchestratorWiresStreamingMessageRetraction(t *testing.T) {
|
||||
provideFn := findFuncDecl(t, "orchestrator.go", "provideOrchestrator")
|
||||
wired := false
|
||||
ast.Inspect(provideFn, func(node ast.Node) bool {
|
||||
call, ok := node.(*ast.CallExpr)
|
||||
if !ok || len(call.Args) != 1 {
|
||||
return true
|
||||
}
|
||||
selector, ok := call.Fun.(*ast.SelectorExpr)
|
||||
if !ok || selector.Sel.Name != "SetStreamingMessageRetractionService" {
|
||||
return true
|
||||
}
|
||||
receiver, receiverOK := selector.X.(*ast.Ident)
|
||||
argument, argumentOK := call.Args[0].(*ast.Ident)
|
||||
wired = receiverOK && receiver.Name == "orchestratorSvc" &&
|
||||
argumentOK && argument.Name == "taskSvc"
|
||||
return true
|
||||
})
|
||||
if !wired {
|
||||
t.Fatal("provideOrchestrator does not wire taskSvc as the streaming message retraction service")
|
||||
}
|
||||
}
|
||||
@@ -87,9 +87,7 @@ func RegisterTaskNotifications(ctx context.Context, eventBus bus.EventBus, hub *
|
||||
b.subscribe(eventBus, events.TaskSessionCancellationChanged, ws.ActionSessionCancellationChanged)
|
||||
b.subscribe(eventBus, events.SessionPendingActionChanged, ws.ActionSessionPendingActionChanged)
|
||||
b.subscribe(eventBus, events.TaskStatusSummaryUpdated, ws.ActionTaskStatusSummaryUpdated)
|
||||
b.subscribe(eventBus, events.MessageAdded, ws.ActionSessionMessageAdded)
|
||||
b.subscribe(eventBus, events.MessageUpdated, ws.ActionSessionMessageUpdated)
|
||||
b.subscribe(eventBus, events.MessageDeleted, ws.ActionSessionMessageDeleted)
|
||||
b.subscribeOrderedMessageEvents(eventBus)
|
||||
b.subscribe(eventBus, events.AgentctlStarting, ws.ActionSessionAgentctlStarting)
|
||||
b.subscribe(eventBus, events.AgentctlReady, ws.ActionSessionAgentctlReady)
|
||||
b.subscribe(eventBus, events.AgentctlError, ws.ActionSessionAgentctlError)
|
||||
@@ -141,7 +139,7 @@ func (b *TaskEventBroadcaster) subscribe(eventBus bus.EventBus, subject, action
|
||||
// state notifications remain ordered for clients when the event bus is remote.
|
||||
func (b *TaskEventBroadcaster) subscribeLifecycleStateEvents(eventBus bus.EventBus) {
|
||||
b.subscribeWithResolver(eventBus, ">", func(event *bus.Event) string {
|
||||
switch event.Type {
|
||||
switch event.EffectiveSubject() {
|
||||
case events.TaskStateChanged:
|
||||
return ws.ActionTaskStateChanged
|
||||
case events.TaskSessionStateChanged:
|
||||
@@ -152,6 +150,24 @@ func (b *TaskEventBroadcaster) subscribeLifecycleStateEvents(eventBus bus.EventB
|
||||
})
|
||||
}
|
||||
|
||||
// subscribeOrderedMessageEvents carries every transcript mutation through one
|
||||
// NATS callback queue. Separate subject subscriptions can run concurrently and
|
||||
// invert a deletion and its replacement before journal or client fan-out.
|
||||
func (b *TaskEventBroadcaster) subscribeOrderedMessageEvents(eventBus bus.EventBus) {
|
||||
b.subscribeWithResolver(eventBus, ">", func(event *bus.Event) string {
|
||||
switch event.EffectiveSubject() {
|
||||
case events.MessageAdded:
|
||||
return ws.ActionSessionMessageAdded
|
||||
case events.MessageUpdated:
|
||||
return ws.ActionSessionMessageUpdated
|
||||
case events.MessageDeleted:
|
||||
return ws.ActionSessionMessageDeleted
|
||||
default:
|
||||
return ""
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
func (b *TaskEventBroadcaster) subscribeWithResolver(
|
||||
eventBus bus.EventBus,
|
||||
subject string,
|
||||
|
||||
@@ -31,12 +31,78 @@ type recordingSubscription struct{}
|
||||
func (recordingSubscription) Unsubscribe() error { return nil }
|
||||
func (recordingSubscription) IsValid() bool { return true }
|
||||
|
||||
type queuedTransportSubscription struct {
|
||||
subject string
|
||||
handler bus.EventHandler
|
||||
events []*bus.Event
|
||||
valid bool
|
||||
}
|
||||
|
||||
func (s *queuedTransportSubscription) Unsubscribe() error {
|
||||
s.valid = false
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *queuedTransportSubscription) IsValid() bool { return s.valid }
|
||||
|
||||
// queuedTransportEventBus models NATS' independent per-subscription queues.
|
||||
// Tests can schedule one subject's subscription ahead of another while each
|
||||
// subscription still preserves FIFO delivery internally.
|
||||
type queuedTransportEventBus struct {
|
||||
*bus.MemoryEventBus
|
||||
subscriptions []*queuedTransportSubscription
|
||||
}
|
||||
|
||||
func (b *queuedTransportEventBus) Subscribe(
|
||||
subject string,
|
||||
handler bus.EventHandler,
|
||||
) (bus.Subscription, error) {
|
||||
subscription := &queuedTransportSubscription{subject: subject, handler: handler, valid: true}
|
||||
b.subscriptions = append(b.subscriptions, subscription)
|
||||
return subscription, nil
|
||||
}
|
||||
|
||||
func (b *queuedTransportEventBus) Publish(_ context.Context, subject string, event *bus.Event) error {
|
||||
event.Subject = subject
|
||||
for _, subscription := range b.subscriptions {
|
||||
if subscription.valid && (subscription.subject == subject || subscription.subject == ">") {
|
||||
subscription.events = append(subscription.events, event)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (b *queuedTransportEventBus) deliverSubjectFirst(ctx context.Context, subject string) error {
|
||||
for _, subscription := range b.subscriptions {
|
||||
if subscription.subject != subject || len(subscription.events) == 0 {
|
||||
continue
|
||||
}
|
||||
event := subscription.events[0]
|
||||
subscription.events = subscription.events[1:]
|
||||
return subscription.handler(ctx, event)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (b *queuedTransportEventBus) deliverAll(ctx context.Context) error {
|
||||
for _, subscription := range b.subscriptions {
|
||||
for len(subscription.events) > 0 {
|
||||
event := subscription.events[0]
|
||||
subscription.events = subscription.events[1:]
|
||||
if err := subscription.handler(ctx, event); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (b *subscriptionRecordingEventBus) Subscribe(subject string, _ bus.EventHandler) (bus.Subscription, error) {
|
||||
b.subjects = append(b.subjects, subject)
|
||||
return recordingSubscription{}, nil
|
||||
}
|
||||
|
||||
func TestTaskEventBroadcaster_UsesOrderedWildcardForLifecycleStates(t *testing.T) {
|
||||
func TestTaskEventBroadcaster_UsesOrderedWildcardsForRelatedTaskEvents(t *testing.T) {
|
||||
log := testLogger()
|
||||
eventBus := &subscriptionRecordingEventBus{MemoryEventBus: bus.NewMemoryEventBus(log)}
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
@@ -45,18 +111,23 @@ func TestTaskEventBroadcaster_UsesOrderedWildcardForLifecycleStates(t *testing.T
|
||||
hub := NewHub(nil, log)
|
||||
b := RegisterTaskNotifications(ctx, eventBus, hub, log)
|
||||
|
||||
seenWildcard := false
|
||||
wildcardCount := 0
|
||||
orderedSubjects := map[string]bool{
|
||||
events.TaskStateChanged: true,
|
||||
events.TaskSessionStateChanged: true,
|
||||
events.MessageAdded: true,
|
||||
events.MessageUpdated: true,
|
||||
events.MessageDeleted: true,
|
||||
}
|
||||
for _, subject := range eventBus.subjects {
|
||||
if subject == ">" {
|
||||
seenWildcard = true
|
||||
wildcardCount++
|
||||
}
|
||||
if subject == events.TaskStateChanged || subject == events.TaskSessionStateChanged {
|
||||
t.Fatalf("lifecycle state subject %q must use the ordered wildcard subscription", subject)
|
||||
if orderedSubjects[subject] {
|
||||
t.Fatalf("ordered task event subject %q must use the shared wildcard subscription", subject)
|
||||
}
|
||||
}
|
||||
if !seenWildcard {
|
||||
t.Fatal("lifecycle state notifications must share a NATS-style wildcard subscription")
|
||||
}
|
||||
require.Equal(t, 2, wildcardCount, "each ordered event group must use one NATS-style subscription")
|
||||
|
||||
_ = b
|
||||
}
|
||||
@@ -88,6 +159,49 @@ func TestTaskEventBroadcaster_OrdersLifecycleStateNotifications(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestTaskEventBroadcaster_OrdersTranscriptMutationsAcrossTransportSubjects(t *testing.T) {
|
||||
log := testLogger()
|
||||
eventBus := &queuedTransportEventBus{MemoryEventBus: bus.NewMemoryEventBus(log)}
|
||||
hub := newTestHub(t)
|
||||
client := newTestClient("ordered-transcript")
|
||||
registerTestClient(hub, client)
|
||||
hub.SubscribeToSession(client, "session-1")
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
_ = RegisterTaskNotifications(ctx, eventBus, hub, log)
|
||||
|
||||
deleted := bus.NewEvent(events.MessageDeleted, "test", map[string]interface{}{
|
||||
"session_id": "session-1", "message_id": "abandoned-message",
|
||||
})
|
||||
added := bus.NewEvent(events.MessageAdded, "test", map[string]interface{}{
|
||||
"session_id": "session-1", "message_id": "replacement-message",
|
||||
})
|
||||
require.NoError(t, eventBus.Publish(ctx, events.MessageDeleted, deleted))
|
||||
require.NoError(t, eventBus.Publish(ctx, events.MessageAdded, added))
|
||||
|
||||
// A NATS callback on an independent message.added subscription can run
|
||||
// while message.deleted is delayed. A shared subscription must keep the
|
||||
// replacement queued behind the deletion even under that schedule.
|
||||
require.NoError(t, eventBus.deliverSubjectFirst(ctx, events.MessageAdded))
|
||||
require.NoError(t, eventBus.deliverAll(ctx))
|
||||
|
||||
actions := make([]string, 0, 2)
|
||||
for range 2 {
|
||||
select {
|
||||
case data := <-client.send:
|
||||
var message ws.Message
|
||||
require.NoError(t, json.Unmarshal(data, &message))
|
||||
actions = append(actions, message.Action)
|
||||
default:
|
||||
t.Fatal("expected two transcript notifications")
|
||||
}
|
||||
}
|
||||
require.Equal(t, []string{
|
||||
ws.ActionSessionMessageDeleted,
|
||||
ws.ActionSessionMessageAdded,
|
||||
}, actions)
|
||||
}
|
||||
|
||||
// TestTaskEventBroadcaster_NoDuplicateSubscriptions verifies that
|
||||
// RegisterTaskNotifications creates one subscription per routed subject (with
|
||||
// lifecycle state events intentionally sharing one ordered wildcard).
|
||||
@@ -112,7 +226,7 @@ func TestTaskEventBroadcaster_NoDuplicateSubscriptions(t *testing.T) {
|
||||
//
|
||||
// Update this number when adding or removing event subscriptions in
|
||||
// RegisterTaskNotifications — it is intentionally exact.
|
||||
const wantSubscriptions = 77
|
||||
const wantSubscriptions = 75
|
||||
if got := len(b.subscriptions); got != wantSubscriptions {
|
||||
t.Errorf("RegisterTaskNotifications created %d subscriptions, want %d — "+
|
||||
"did an event get subscribed twice?", got, wantSubscriptions)
|
||||
|
||||
@@ -167,6 +167,12 @@ func (s *Service) handleAgentStreamEvent(ctx context.Context, payload *lifecycle
|
||||
case "thinking_streaming":
|
||||
s.handleThinkingStreamingEvent(ctx, payload)
|
||||
|
||||
case streams.EventTypeResponseAttemptReset:
|
||||
if !s.responseAttemptResetOwnsCurrentPrompt(payload) {
|
||||
return
|
||||
}
|
||||
s.handleResponseAttemptReset(ctx, payload)
|
||||
|
||||
case agentEventToolCall:
|
||||
s.saveAgentTextIfPresent(ctx, payload)
|
||||
s.handleToolCallEvent(ctx, payload)
|
||||
@@ -254,6 +260,51 @@ func (s *Service) handleAgentStreamEvent(ctx context.Context, payload *lifecycle
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Service) responseAttemptResetOwnsCurrentPrompt(
|
||||
payload *lifecycle.AgentStreamEventPayload,
|
||||
) bool {
|
||||
if payload == nil || payload.Data == nil || payload.Data.PromptGeneration == 0 {
|
||||
return false
|
||||
}
|
||||
generationOwner, ok := s.agentManager.(interface {
|
||||
OwnsPromptGeneration(sessionID, executionID string, generation uint64) bool
|
||||
})
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
executionID := payload.ExecutionID
|
||||
if executionID == "" {
|
||||
executionID = payload.AgentID
|
||||
}
|
||||
return generationOwner.OwnsPromptGeneration(
|
||||
payload.SessionID,
|
||||
executionID,
|
||||
payload.Data.PromptGeneration,
|
||||
)
|
||||
}
|
||||
|
||||
func (s *Service) handleResponseAttemptReset(
|
||||
ctx context.Context,
|
||||
payload *lifecycle.AgentStreamEventPayload,
|
||||
) {
|
||||
if s.streamingRetractions == nil {
|
||||
return
|
||||
}
|
||||
for _, messageID := range payload.Data.RetractedMessageIDs {
|
||||
if messageID == "" {
|
||||
continue
|
||||
}
|
||||
if err := s.streamingRetractions.DeleteMessage(ctx, messageID); err != nil {
|
||||
s.logger.Warn("failed to retract abandoned response message",
|
||||
zap.String("task_id", payload.TaskID),
|
||||
zap.String("session_id", payload.SessionID),
|
||||
zap.String("execution_id", payload.ExecutionID),
|
||||
zap.String("message_id", messageID),
|
||||
zap.Error(err))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (s *Service) backgroundCompletionActivityValue(ctx context.Context, sessionID string) interface{} {
|
||||
session, err := s.repo.GetTaskSession(ctx, sessionID)
|
||||
if err == nil && session != nil && session.State != models.TaskSessionStateRunning {
|
||||
|
||||
@@ -0,0 +1,183 @@
|
||||
package orchestrator
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"reflect"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/kandev/kandev/internal/agent/runtime/lifecycle"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
"github.com/kandev/kandev/internal/task/models"
|
||||
)
|
||||
|
||||
type responseAttemptMessageServiceStub struct {
|
||||
deleted []string
|
||||
fail map[string]error
|
||||
}
|
||||
|
||||
func (s *responseAttemptMessageServiceStub) DeleteMessage(_ context.Context, id string) error {
|
||||
s.deleted = append(s.deleted, id)
|
||||
return s.fail[id]
|
||||
}
|
||||
|
||||
func responseAttemptResetPayload(generation uint64, messageIDs ...string) *lifecycle.AgentStreamEventPayload {
|
||||
return &lifecycle.AgentStreamEventPayload{
|
||||
TaskID: "task-1",
|
||||
SessionID: "session-1",
|
||||
ExecutionID: "exec-1",
|
||||
Data: &lifecycle.AgentStreamEventData{
|
||||
Type: streams.EventTypeResponseAttemptReset,
|
||||
PromptGeneration: generation,
|
||||
RetractedMessageIDs: messageIDs,
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
// @covers AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.27
|
||||
func TestHandleResponseAttemptResetDeletesEveryRecord(t *testing.T) {
|
||||
messageService := &responseAttemptMessageServiceStub{}
|
||||
agentManager := &mockAgentManager{currentPromptExecutionID: "exec-1"}
|
||||
agentManager.currentPromptGeneration.Store(7)
|
||||
svc := &Service{logger: testLogger(), agentManager: agentManager}
|
||||
svc.SetStreamingMessageRetractionService(messageService)
|
||||
|
||||
svc.handleAgentStreamEvent(context.Background(), responseAttemptResetPayload(
|
||||
7, "assistant-1", "thinking-1",
|
||||
))
|
||||
|
||||
if want := []string{"assistant-1", "thinking-1"}; !reflect.DeepEqual(messageService.deleted, want) {
|
||||
t.Fatalf("deleted IDs = %v, want %v", messageService.deleted, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleResponseAttemptResetContinuesAfterDeleteFailure(t *testing.T) {
|
||||
deleteErr := errors.New("delete failed")
|
||||
messageService := &responseAttemptMessageServiceStub{
|
||||
fail: map[string]error{"assistant-1": deleteErr},
|
||||
}
|
||||
agentManager := &mockAgentManager{currentPromptExecutionID: "exec-1"}
|
||||
agentManager.currentPromptGeneration.Store(7)
|
||||
log, observed := observingTestLogger(t)
|
||||
svc := &Service{logger: log, agentManager: agentManager}
|
||||
svc.SetStreamingMessageRetractionService(messageService)
|
||||
|
||||
svc.handleAgentStreamEvent(context.Background(), responseAttemptResetPayload(
|
||||
7, "assistant-1", "thinking-1",
|
||||
))
|
||||
|
||||
if want := []string{"assistant-1", "thinking-1"}; !reflect.DeepEqual(messageService.deleted, want) {
|
||||
t.Fatalf("delete attempts = %v, want %v", messageService.deleted, want)
|
||||
}
|
||||
if got := observed.FilterMessage("failed to retract abandoned response message").Len(); got != 1 {
|
||||
t.Fatalf("delete failure warnings = %d, want 1", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleResponseAttemptResetRejectsInactiveOwnership(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
generation uint64
|
||||
current uint64
|
||||
completed bool
|
||||
}{
|
||||
{name: "zero generation", generation: 0, current: 7},
|
||||
{name: "stale generation", generation: 6, current: 7},
|
||||
{name: "completed execution", generation: 7, current: 7, completed: true},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
messageService := &responseAttemptMessageServiceStub{}
|
||||
agentManager := &mockAgentManager{currentPromptExecutionID: "exec-1"}
|
||||
agentManager.currentPromptGeneration.Store(test.current)
|
||||
svc := &Service{logger: testLogger(), agentManager: agentManager}
|
||||
svc.SetStreamingMessageRetractionService(messageService)
|
||||
if test.completed {
|
||||
svc.markExecutionCompleted("session-1", "exec-1")
|
||||
}
|
||||
|
||||
svc.handleAgentStreamEvent(
|
||||
context.Background(),
|
||||
responseAttemptResetPayload(test.generation, "assistant-1"),
|
||||
)
|
||||
|
||||
if len(messageService.deleted) != 0 {
|
||||
t.Fatalf("inactive reset deleted messages: %v", messageService.deleted)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
svc := &Service{logger: testLogger()}
|
||||
messageService := &responseAttemptMessageServiceStub{}
|
||||
svc.SetStreamingMessageRetractionService(messageService)
|
||||
svc.handleAgentStreamEvent(context.Background(), responseAttemptResetPayload(7, "assistant-1"))
|
||||
if len(messageService.deleted) != 0 {
|
||||
t.Fatalf("reset without a generation owner deleted messages: %v", messageService.deleted)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleResponseAttemptResetDeletesThroughTaskService(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
repo := setupTestRepo(t)
|
||||
seedSession(t, repo, "task-1", "session-1", "step-1")
|
||||
now := time.Now().UTC()
|
||||
if err := repo.CreateTurn(ctx, &models.Turn{
|
||||
ID: "turn-1", TaskSessionID: "session-1", TaskID: "task-1",
|
||||
StartedAt: now, CreatedAt: now, UpdatedAt: now,
|
||||
}); err != nil {
|
||||
t.Fatalf("create turn: %v", err)
|
||||
}
|
||||
for _, message := range []*models.Message{
|
||||
{
|
||||
ID: "assistant-1",
|
||||
TaskSessionID: "session-1",
|
||||
TaskID: "task-1",
|
||||
TurnID: "turn-1",
|
||||
AuthorType: models.MessageAuthorAgent,
|
||||
Content: "abandoned answer",
|
||||
Type: models.MessageTypeMessage,
|
||||
},
|
||||
{
|
||||
ID: "thinking-1",
|
||||
TaskSessionID: "session-1",
|
||||
TaskID: "task-1",
|
||||
TurnID: "turn-1",
|
||||
AuthorType: models.MessageAuthorAgent,
|
||||
Content: "abandoned reasoning",
|
||||
Type: models.MessageTypeThinking,
|
||||
},
|
||||
} {
|
||||
if err := repo.CreateMessage(ctx, message); err != nil {
|
||||
t.Fatalf("create message %s: %v", message.ID, err)
|
||||
}
|
||||
}
|
||||
|
||||
eventBus := &recordingEventBus{}
|
||||
taskServices := newTaskServiceStateRepository(repo, eventBus)
|
||||
agentManager := &mockAgentManager{currentPromptExecutionID: "exec-1"}
|
||||
agentManager.currentPromptGeneration.Store(7)
|
||||
svc := &Service{logger: testLogger(), agentManager: agentManager}
|
||||
svc.SetStreamingMessageRetractionService(taskServices.service)
|
||||
|
||||
svc.handleAgentStreamEvent(ctx, responseAttemptResetPayload(
|
||||
7, "assistant-1", "thinking-1",
|
||||
))
|
||||
|
||||
messages, err := repo.ListMessages(ctx, "session-1")
|
||||
if err != nil {
|
||||
t.Fatalf("list messages: %v", err)
|
||||
}
|
||||
if len(messages) != 0 {
|
||||
t.Fatalf("persisted messages after reset = %+v, want none", messages)
|
||||
}
|
||||
deletedEvents := 0
|
||||
for _, published := range eventBus.events {
|
||||
if published.subject == events.MessageDeleted {
|
||||
deletedEvents++
|
||||
}
|
||||
}
|
||||
if deletedEvents != 2 {
|
||||
t.Fatalf("message deletion events = %d, want 2", deletedEvents)
|
||||
}
|
||||
}
|
||||
@@ -152,6 +152,13 @@ type TransientRetryMessageService interface {
|
||||
DeleteMessage(ctx context.Context, id string) error
|
||||
}
|
||||
|
||||
// StreamingMessageRetractionService removes transcript records abandoned by a
|
||||
// provider response retry. The task service owns durable deletion and client
|
||||
// notification publication.
|
||||
type StreamingMessageRetractionService interface {
|
||||
DeleteMessage(ctx context.Context, id string) error
|
||||
}
|
||||
|
||||
// transientRetryNoticeState serializes retry-notice storage with the in-memory
|
||||
// retry lifecycle for one session. refs and fenceTimer are owned by
|
||||
// Service.transientRetryNoticeStatesMu. The bounded retirement fence keeps a
|
||||
@@ -696,6 +703,7 @@ type Service struct {
|
||||
// transientRetryMessages owns durable cleanup of persisted retry notices.
|
||||
// It is optional for focused tests and pre-composition callers.
|
||||
transientRetryMessages TransientRetryMessageService
|
||||
streamingRetractions StreamingMessageRetractionService
|
||||
// transientRetryNoticeStates serializes notice writes and cleanup by
|
||||
// session. Entries are reference counted while callers use the mutex,
|
||||
// retained while a prompt/retry is active, and retained for a bounded fence
|
||||
@@ -1897,6 +1905,12 @@ func (s *Service) SetTransientRetryMessageService(service TransientRetryMessageS
|
||||
s.transientRetryMessages = service
|
||||
}
|
||||
|
||||
// SetStreamingMessageRetractionService wires durable cleanup for provider
|
||||
// response attempts that were abandoned before the prompt completed.
|
||||
func (s *Service) SetStreamingMessageRetractionService(service StreamingMessageRetractionService) {
|
||||
s.streamingRetractions = service
|
||||
}
|
||||
|
||||
// SetSubagentContextRecorder wires the optional subagent-context writer.
|
||||
// If not set, subagent tool-call frames are recognized and rendered exactly
|
||||
// as before; only the durable relational record is skipped.
|
||||
|
||||
@@ -0,0 +1,120 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { watchWs } from "../../helpers/causal-waits";
|
||||
import { seedIdleSession } from "../../helpers/session";
|
||||
|
||||
const ABANDONED_ANSWER = "Abandoned response attempt answer.";
|
||||
const ABANDONED_REASONING = "Abandoned response attempt reasoning.";
|
||||
const REPLACEMENT_ANSWER = "Replacement response after provider retry.";
|
||||
|
||||
function payloadMessageId(payload: Record<string, unknown>): string | undefined {
|
||||
return typeof payload.message_id === "string" ? payload.message_id : undefined;
|
||||
}
|
||||
|
||||
function payloadMessageType(payload: Record<string, unknown>): string | undefined {
|
||||
if (typeof payload.message_type === "string") return payload.message_type;
|
||||
return typeof payload.type === "string" ? payload.type : undefined;
|
||||
}
|
||||
|
||||
test.describe("mobile: provider response retry", () => {
|
||||
test("retracts abandoned output live and after reload", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(90_000);
|
||||
const ws = watchWs(testPage);
|
||||
const session = await seedIdleSession(testPage, apiClient, seedData, "Mobile Provider Retry");
|
||||
const taskId = new URL(testPage.url()).pathname.split("/").filter(Boolean).at(-1);
|
||||
if (!taskId) throw new Error("task route did not contain a task ID");
|
||||
const { sessions } = await apiClient.listTaskSessions(taskId);
|
||||
const sessionId = sessions[0]?.id;
|
||||
if (!sessionId) throw new Error("seeded task did not contain a session");
|
||||
|
||||
let abandonedReasoningId: string | undefined;
|
||||
let abandonedAnswerId: string | undefined;
|
||||
const retractedMessageIds = new Set<string>();
|
||||
const abandonedReasoningAdded = ws.waitForEvent("session.message.added", {
|
||||
where: (payload) => {
|
||||
if (payload.session_id !== sessionId || payloadMessageType(payload) !== "thinking") {
|
||||
return false;
|
||||
}
|
||||
abandonedReasoningId = payloadMessageId(payload);
|
||||
return abandonedReasoningId !== undefined;
|
||||
},
|
||||
});
|
||||
const abandonedAnswerAdded = ws.waitForEvent("session.message.added", {
|
||||
where: (payload) => {
|
||||
if (payload.session_id !== sessionId || payload.content !== ABANDONED_ANSWER) return false;
|
||||
abandonedAnswerId = payloadMessageId(payload);
|
||||
return abandonedAnswerId !== undefined;
|
||||
},
|
||||
});
|
||||
const abandonedReasoningDeleted = ws.waitForEvent("session.message.deleted", {
|
||||
where: (payload) => {
|
||||
const messageId = payloadMessageId(payload);
|
||||
if (messageId === undefined || messageId !== abandonedReasoningId) return false;
|
||||
retractedMessageIds.add(messageId);
|
||||
return true;
|
||||
},
|
||||
});
|
||||
const abandonedAnswerDeleted = ws.waitForEvent("session.message.deleted", {
|
||||
where: (payload) => {
|
||||
const messageId = payloadMessageId(payload);
|
||||
if (messageId === undefined || messageId !== abandonedAnswerId) return false;
|
||||
retractedMessageIds.add(messageId);
|
||||
return true;
|
||||
},
|
||||
});
|
||||
const replacementAdded = ws.waitForEvent("session.message.added", {
|
||||
where: (payload) =>
|
||||
payload.session_id === sessionId &&
|
||||
payload.content === REPLACEMENT_ANSWER &&
|
||||
retractedMessageIds.size === 2,
|
||||
});
|
||||
|
||||
await session.sendMessageViaButton("/e2e:response-retry");
|
||||
|
||||
await Promise.all([
|
||||
abandonedReasoningAdded,
|
||||
abandonedAnswerAdded,
|
||||
abandonedReasoningDeleted,
|
||||
abandonedAnswerDeleted,
|
||||
replacementAdded,
|
||||
]);
|
||||
|
||||
await expect(session.activeChat().getByText(REPLACEMENT_ANSWER, { exact: true })).toBeVisible();
|
||||
await expect(session.activeChat().getByText(ABANDONED_ANSWER, { exact: true })).toHaveCount(0);
|
||||
await expect(session.activeChat().getByText(ABANDONED_REASONING, { exact: true })).toHaveCount(
|
||||
0,
|
||||
);
|
||||
await session.waitForChatIdle({ timeout: 30_000 });
|
||||
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const { messages } = await apiClient.listSessionMessages(sessionId);
|
||||
const text = (message: (typeof messages)[number]) =>
|
||||
String(message.metadata?.thinking ?? message.content ?? "");
|
||||
return {
|
||||
abandonedAnswer: messages.some((message) => text(message).includes(ABANDONED_ANSWER)),
|
||||
abandonedReasoning: messages.some((message) =>
|
||||
text(message).includes(ABANDONED_REASONING),
|
||||
),
|
||||
replacementCount: messages.filter((message) =>
|
||||
text(message).includes(REPLACEMENT_ANSWER),
|
||||
).length,
|
||||
};
|
||||
},
|
||||
{ timeout: 30_000, message: "waiting for the durable mobile replacement-only transcript" },
|
||||
)
|
||||
.toEqual({ abandonedAnswer: false, abandonedReasoning: false, replacementCount: 1 });
|
||||
|
||||
await testPage.reload();
|
||||
await session.waitForLoad();
|
||||
await expect(session.activeChat().getByText(REPLACEMENT_ANSWER, { exact: true })).toBeVisible();
|
||||
await expect(session.activeChat().getByText(ABANDONED_ANSWER, { exact: true })).toHaveCount(0);
|
||||
await expect(session.activeChat().getByText(ABANDONED_REASONING, { exact: true })).toHaveCount(
|
||||
0,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,120 @@
|
||||
import { test, expect } from "../../fixtures/test-base";
|
||||
import { watchWs } from "../../helpers/causal-waits";
|
||||
import { seedIdleSession } from "../../helpers/session";
|
||||
|
||||
const ABANDONED_ANSWER = "Abandoned response attempt answer.";
|
||||
const ABANDONED_REASONING = "Abandoned response attempt reasoning.";
|
||||
const REPLACEMENT_ANSWER = "Replacement response after provider retry.";
|
||||
|
||||
function payloadMessageId(payload: Record<string, unknown>): string | undefined {
|
||||
return typeof payload.message_id === "string" ? payload.message_id : undefined;
|
||||
}
|
||||
|
||||
function payloadMessageType(payload: Record<string, unknown>): string | undefined {
|
||||
if (typeof payload.message_type === "string") return payload.message_type;
|
||||
return typeof payload.type === "string" ? payload.type : undefined;
|
||||
}
|
||||
|
||||
test.describe("provider response retry", () => {
|
||||
test("retracts abandoned output live and after reload", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
test.setTimeout(90_000);
|
||||
const ws = watchWs(testPage);
|
||||
const session = await seedIdleSession(testPage, apiClient, seedData, "Provider Response Retry");
|
||||
const taskId = new URL(testPage.url()).pathname.split("/").filter(Boolean).at(-1);
|
||||
if (!taskId) throw new Error("task route did not contain a task ID");
|
||||
const { sessions } = await apiClient.listTaskSessions(taskId);
|
||||
const sessionId = sessions[0]?.id;
|
||||
if (!sessionId) throw new Error("seeded task did not contain a session");
|
||||
|
||||
let abandonedReasoningId: string | undefined;
|
||||
let abandonedAnswerId: string | undefined;
|
||||
const retractedMessageIds = new Set<string>();
|
||||
const abandonedReasoningAdded = ws.waitForEvent("session.message.added", {
|
||||
where: (payload) => {
|
||||
if (payload.session_id !== sessionId || payloadMessageType(payload) !== "thinking") {
|
||||
return false;
|
||||
}
|
||||
abandonedReasoningId = payloadMessageId(payload);
|
||||
return abandonedReasoningId !== undefined;
|
||||
},
|
||||
});
|
||||
const abandonedAnswerAdded = ws.waitForEvent("session.message.added", {
|
||||
where: (payload) => {
|
||||
if (payload.session_id !== sessionId || payload.content !== ABANDONED_ANSWER) return false;
|
||||
abandonedAnswerId = payloadMessageId(payload);
|
||||
return abandonedAnswerId !== undefined;
|
||||
},
|
||||
});
|
||||
const abandonedReasoningDeleted = ws.waitForEvent("session.message.deleted", {
|
||||
where: (payload) => {
|
||||
const messageId = payloadMessageId(payload);
|
||||
if (messageId === undefined || messageId !== abandonedReasoningId) return false;
|
||||
retractedMessageIds.add(messageId);
|
||||
return true;
|
||||
},
|
||||
});
|
||||
const abandonedAnswerDeleted = ws.waitForEvent("session.message.deleted", {
|
||||
where: (payload) => {
|
||||
const messageId = payloadMessageId(payload);
|
||||
if (messageId === undefined || messageId !== abandonedAnswerId) return false;
|
||||
retractedMessageIds.add(messageId);
|
||||
return true;
|
||||
},
|
||||
});
|
||||
const replacementAdded = ws.waitForEvent("session.message.added", {
|
||||
where: (payload) =>
|
||||
payload.session_id === sessionId &&
|
||||
payload.content === REPLACEMENT_ANSWER &&
|
||||
retractedMessageIds.size === 2,
|
||||
});
|
||||
|
||||
await session.sendMessage("/e2e:response-retry");
|
||||
|
||||
await Promise.all([
|
||||
abandonedReasoningAdded,
|
||||
abandonedAnswerAdded,
|
||||
abandonedReasoningDeleted,
|
||||
abandonedAnswerDeleted,
|
||||
replacementAdded,
|
||||
]);
|
||||
|
||||
await expect(session.activeChat().getByText(REPLACEMENT_ANSWER, { exact: true })).toBeVisible();
|
||||
await expect(session.activeChat().getByText(ABANDONED_ANSWER, { exact: true })).toHaveCount(0);
|
||||
await expect(session.activeChat().getByText(ABANDONED_REASONING, { exact: true })).toHaveCount(
|
||||
0,
|
||||
);
|
||||
await session.waitForChatIdle({ timeout: 30_000 });
|
||||
|
||||
await expect
|
||||
.poll(
|
||||
async () => {
|
||||
const { messages } = await apiClient.listSessionMessages(sessionId);
|
||||
const text = (message: (typeof messages)[number]) =>
|
||||
String(message.metadata?.thinking ?? message.content ?? "");
|
||||
return {
|
||||
abandonedAnswer: messages.some((message) => text(message).includes(ABANDONED_ANSWER)),
|
||||
abandonedReasoning: messages.some((message) =>
|
||||
text(message).includes(ABANDONED_REASONING),
|
||||
),
|
||||
replacementCount: messages.filter((message) =>
|
||||
text(message).includes(REPLACEMENT_ANSWER),
|
||||
).length,
|
||||
};
|
||||
},
|
||||
{ timeout: 30_000, message: "waiting for the durable replacement-only transcript" },
|
||||
)
|
||||
.toEqual({ abandonedAnswer: false, abandonedReasoning: false, replacementCount: 1 });
|
||||
|
||||
await testPage.reload();
|
||||
await session.waitForLoad();
|
||||
await expect(session.activeChat().getByText(REPLACEMENT_ANSWER, { exact: true })).toBeVisible();
|
||||
await expect(session.activeChat().getByText(ABANDONED_ANSWER, { exact: true })).toHaveCount(0);
|
||||
await expect(session.activeChat().getByText(ABANDONED_REASONING, { exact: true })).toHaveCount(
|
||||
0,
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,91 @@
|
||||
# ADR-2026-09-09-retract-abandoned-provider-output: Retract Output From Abandoned Provider Attempts
|
||||
|
||||
**Status:** accepted
|
||||
**Date:** 2026-09-09
|
||||
**Area:** backend, frontend, protocol
|
||||
|
||||
## Context
|
||||
|
||||
Some providers recover from a response-stream failure inside the active
|
||||
`session/prompt` call. Codex, for example, emits a structured reconnect update
|
||||
after partial assistant output and then streams a replacement under a new ACP
|
||||
message ID. Kandev currently maps each protocol message ID to a separate
|
||||
durable transcript row. Both rows therefore survive even though the provider
|
||||
abandoned the first response.
|
||||
|
||||
The web client is behaving correctly when it renders both rows. Consecutive
|
||||
assistant messages are valid, so presentation code cannot infer that similar
|
||||
text is a duplicate. The ACP adapter understands provider metadata, lifecycle
|
||||
owns protocol-to-Kandev message correlation, and the task service owns durable
|
||||
message deletion and its live WebSocket projection.
|
||||
|
||||
## Decision
|
||||
|
||||
Kandev represents a provider-owned response retry as a provider-neutral,
|
||||
prompt-generation-scoped `response_attempt_reset` boundary.
|
||||
|
||||
The active ACP dialect recognizes exact structured retry evidence. It does not
|
||||
classify arbitrary prose, and downstream lifecycle, orchestration, and UI code
|
||||
do not inspect provider names or raw error text. A retry boundary is separate
|
||||
from a terminal provider error and does not authorize Kandev to replay a
|
||||
prompt.
|
||||
|
||||
Lifecycle records the Kandev IDs of assistant and thinking rows first created
|
||||
during the current response attempt. A top-level effect boundary commits those
|
||||
rows and starts a new eligibility window. Before lifecycle publishes a reset,
|
||||
it flushes pending streaming chunks, detaches the eligible IDs, clears their
|
||||
protocol correlations, and discards the uncommitted assistant-history segment.
|
||||
A row that existed before the current eligibility window is never deleted or
|
||||
truncated, even if the provider later appends to the same protocol message ID.
|
||||
|
||||
Orchestration applies the reset under the existing per-session stream guard and
|
||||
deletes each eligible row through the task service. Task-service deletion is
|
||||
the durable mutation and publishes the existing `session.message.deleted`
|
||||
event, so connected desktop and mobile clients remove the same rows that a
|
||||
reload, search, or another viewer no longer reads.
|
||||
|
||||
The WebSocket gateway consumes all task-service message mutations through one
|
||||
ordered NATS wildcard subscription. This carries producer order into both the
|
||||
per-session journal and legacy live fan-out even though the underlying event
|
||||
subjects differ, so a replacement callback cannot overtake a delayed deletion
|
||||
callback.
|
||||
|
||||
A reset does not clear prompt-level output or effect evidence. That evidence
|
||||
continues to fail a later Kandev-owned automatic replay closed. A repeated
|
||||
reset without newly eligible output is idempotent. A deletion failure is logged
|
||||
and does not stop the provider's replacement attempt.
|
||||
|
||||
## Consequences
|
||||
|
||||
Provider-owned retry output is removed consistently from storage and live chat
|
||||
without hiding legitimate multi-message turns. Other providers can add an
|
||||
adapter dialect recognizer while reusing the same lifecycle and task-service
|
||||
path.
|
||||
|
||||
Lifecycle must retain a small ordered set of attempt-local record IDs and flush
|
||||
its coalescer before the reset crosses the event bus. The current design does
|
||||
not roll back an appended suffix on a row committed before the response
|
||||
attempt; preserving committed content is safer than guessing a truncation
|
||||
point. Providers that reuse one protocol message ID across an effect boundary
|
||||
need a future protocol-level segment identity before such suffixes can be
|
||||
retracted safely.
|
||||
|
||||
No message schema or frontend rendering change is required. Storage failures
|
||||
can still leave an abandoned row, but the failure is observable and does not
|
||||
interrupt the live provider turn.
|
||||
|
||||
## Alternatives Considered
|
||||
|
||||
- **Deduplicate similar consecutive messages in React.** Rejected because
|
||||
legitimate assistant messages can be similar, and hidden rows would remain
|
||||
in storage, search, reloads, and other viewers.
|
||||
- **Overwrite the previous message whenever a new protocol ID appears.**
|
||||
Rejected because a new ID normally means a legitimate new message.
|
||||
- **Mark abandoned rows as hidden.** Rejected because it introduces a new
|
||||
persistence state and leaves stale content available to non-rendering
|
||||
consumers when durable deletion already has a live projection.
|
||||
- **Buffer all assistant output until the prompt completes.** Rejected because
|
||||
it removes live streaming and makes long-running turns appear stalled.
|
||||
- **Delete messages directly from provider-specific adapter code.** Rejected
|
||||
because adapters do not own Kandev record IDs, persistence, or client event
|
||||
delivery.
|
||||
@@ -0,0 +1,156 @@
|
||||
---
|
||||
created: 2026-09-09
|
||||
status: done
|
||||
requirements:
|
||||
- REQ-PLATFORM-PROVIDER-ERROR-RECOVERY-001
|
||||
system_design:
|
||||
- ../../specs/platform/system-design/provider-error-recovery.md
|
||||
- ../../specs/platform/system-design/provider-response-attempt-recovery.md
|
||||
legacy_specs: []
|
||||
---
|
||||
|
||||
# Implementation Plan: Provider Retry Output Retraction
|
||||
|
||||
## Overview
|
||||
|
||||
Translate Codex's structured internal-retry metadata into a provider-neutral
|
||||
response-attempt reset, map that boundary to the provisional Kandev streaming
|
||||
records owned by lifecycle, and retire those records through the task service.
|
||||
The order follows the runtime flow so each work order can prove its contract
|
||||
before the next layer consumes it. A controlled mock-agent scenario then proves
|
||||
the durable and live desktop and mobile result end to end.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- Exact, prompt-generation-scoped detection of Codex response-stream retry
|
||||
metadata.
|
||||
- Provider-neutral reset delivery through agentctl and lifecycle.
|
||||
- Attempt-local assistant and thinking record ownership and history cleanup.
|
||||
- Durable task-message deletion and existing WebSocket removal projection.
|
||||
- Desktop and mobile proof that the abandoned response disappears live and
|
||||
stays absent after reload.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- Similarity-based message deduplication.
|
||||
- A new Kandev retry loop, provider fallback, or change to replay-safety policy.
|
||||
- Truncating a message record created before the latest top-level effect
|
||||
boundary.
|
||||
- Message schema changes, new chat components, layout changes, or localized
|
||||
copy.
|
||||
- Repairing transcript rows left by historical incidents.
|
||||
|
||||
## Technical approach
|
||||
|
||||
### ACP retry-boundary projection
|
||||
|
||||
- Add `EventTypeResponseAttemptReset` in
|
||||
`apps/backend/internal/agentctl/types/streams/agent.go`.
|
||||
- Extend `acpDialect` in
|
||||
`apps/backend/internal/agentctl/server/adapter/transport/acp/dialect.go` with
|
||||
an optional structured reset recognizer.
|
||||
- Match `codex.error.willRetry=true` plus the nested
|
||||
`responseStreamDisconnected` object in `dialect_codex.go`. Keep raw
|
||||
diagnostic text out of the normalized event.
|
||||
- Emit the generation-bearing reset from the ordered notification worker in
|
||||
`adapter_updates.go` before later provider output while preserving ordinary
|
||||
session-info fields.
|
||||
|
||||
### Lifecycle attempt records
|
||||
|
||||
- Add an ordered response-attempt record list to `AgentExecution` in
|
||||
`types.go`. Register every newly allocated assistant or thinking message ID
|
||||
in `manager_streaming.go`.
|
||||
- Commit the list at a top-level tool boundary while retaining existing
|
||||
protocol correlation. Clear it with the existing prompt and execution reset
|
||||
paths.
|
||||
- Add `handleResponseAttemptReset` in `manager_events.go`. Fence it to the
|
||||
current prompt generation, flush the coalescer, detach eligible IDs, clear
|
||||
pending buffers and fallback history, and publish
|
||||
`RetractedMessageIDs` through `event_types.go` and `events.go`.
|
||||
|
||||
### Durable transcript cleanup
|
||||
|
||||
- Add a narrow `StreamingMessageRetractionService` to
|
||||
`internal/orchestrator/service.go` and wire the task service in
|
||||
`internal/backendapp/orchestrator.go`.
|
||||
- Handle `response_attempt_reset` in
|
||||
`event_handlers_streaming.go` under the existing per-session stream guard.
|
||||
Delete each detached ID through `task/service.Service.DeleteMessage`, continue
|
||||
after individual failures, and keep the provider turn running.
|
||||
- Reuse the existing `session.message.deleted` gateway and frontend handler.
|
||||
No frontend production file changes are expected.
|
||||
|
||||
### Controlled end-to-end fixture
|
||||
|
||||
- Add mock-agent emitters and a `/e2e:response-retry` scenario that stream
|
||||
explicit-ID assistant and thinking rows, emit a test-only structured reset,
|
||||
and then stream a replacement under new IDs.
|
||||
- Add a mock-only ACP dialect recognizer. The mock marker remains unavailable
|
||||
to production agent dialects.
|
||||
- Exercise the scenario in desktop and mobile task chat, including live removal,
|
||||
durable API state, and reload.
|
||||
|
||||
## Tests
|
||||
|
||||
- `AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.26`: new
|
||||
`dialect_codex_retry_test.go` cases cover the exact captured metadata,
|
||||
malformed and false flags, adapter scope, prompt-generation scope, and event
|
||||
ordering.
|
||||
- `AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.27`: new lifecycle and orchestrator
|
||||
tests cover ordered assistant/thinking retraction, real task-service deletion,
|
||||
and `session.message.deleted` publication.
|
||||
- `AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.28`: lifecycle tests cover an empty
|
||||
second reset, committed rows, protocol-ID reuse, stale generations, pending
|
||||
buffers, fallback history, and unchanged prompt replay evidence.
|
||||
- Existing `apps/web/lib/ws/handlers/messages.test.ts` remains the focused proof
|
||||
that deletion flushes queued updates before removing a row.
|
||||
|
||||
## E2E tests
|
||||
|
||||
- `AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.27`: add
|
||||
`apps/web/e2e/tests/session/provider-response-retry.spec.ts` to the `chromium`
|
||||
project. It asserts live removal, the durable message list, and absence after
|
||||
reload.
|
||||
- `AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.27`: add
|
||||
`apps/web/e2e/tests/session/mobile-provider-response-retry.spec.ts` to the
|
||||
`mobile-chrome` project and assert the same task-chat outcome.
|
||||
- The change reuses the current task-chat composition and shared message store.
|
||||
The nearest recovery exemplars are `transient-retry.spec.ts` and
|
||||
`mobile-transient-retry.spec.ts`; no navigation, scroll, touch, safe-area, or
|
||||
viewport behavior changes.
|
||||
|
||||
## Work orders
|
||||
|
||||
- [x] [Task 01: Project Response-Attempt Resets](task-01-project-response-attempt-resets.md)
|
||||
- [x] [Task 02: Map Provisional Stream Records](task-02-map-provisional-stream-records.md)
|
||||
- [x] [Task 03: Apply Transcript Retractions](task-03-apply-transcript-retractions.md)
|
||||
- [x] [Task 04: Prove Chat Retry Recovery](task-04-prove-chat-retry-recovery.md)
|
||||
|
||||
## Verification results
|
||||
|
||||
All task-defined checks passed:
|
||||
|
||||
- Codex and controlled-mock ACP reset projection under the race detector.
|
||||
- Lifecycle attempt ownership, commit boundaries, stale-event fencing, and
|
||||
retraction publication under the race detector.
|
||||
- Orchestrator durable deletion, continued cleanup after individual failures,
|
||||
existing deletion-event publication, and production composition under the
|
||||
race detector.
|
||||
- Mock-agent retry sequence and the frontend message-deletion semantic barrier.
|
||||
- Chromium desktop and mobile Chrome end-to-end flows, including transient-row
|
||||
visibility, live removal, durable API readback, and reload.
|
||||
- Full backend lint and web TypeScript type checking.
|
||||
|
||||
## Risks
|
||||
|
||||
- A buffered streaming chunk can recreate an abandoned row after deletion if
|
||||
lifecycle publishes the reset before flushing the coalescer.
|
||||
- A stale reset can delete successor output unless execution and prompt
|
||||
generation are both fenced.
|
||||
- Deleting a record that predates a tool boundary can remove valid committed
|
||||
content when a provider reuses a protocol message ID.
|
||||
- An individual task-store failure can leave one abandoned row even though the
|
||||
replacement turn continues; structured logs must preserve that diagnosis.
|
||||
+98
@@ -0,0 +1,98 @@
|
||||
---
|
||||
id: "01-project-response-attempt-resets"
|
||||
title: "Project response-attempt resets"
|
||||
status: done
|
||||
wave: 1
|
||||
depends_on: []
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-PLATFORM-PROVIDER-ERROR-RECOVERY-001
|
||||
acceptance_criteria:
|
||||
- AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.26
|
||||
system_design:
|
||||
- ../../specs/platform/system-design/provider-error-recovery.md
|
||||
- ../../specs/platform/system-design/provider-response-attempt-recovery.md
|
||||
---
|
||||
|
||||
# Task 01: Project Response-Attempt Resets
|
||||
|
||||
## Summary
|
||||
|
||||
Recognize Codex's exact structured response-stream retry metadata on the active
|
||||
prompt and emit one provider-neutral reset boundary. Preserve ordinary session
|
||||
information and FIFO ordering without exposing provider diagnostics downstream.
|
||||
|
||||
## In scope
|
||||
|
||||
- Provider-neutral stream event type.
|
||||
- Optional ACP dialect reset recognizer.
|
||||
- Exact Codex structured metadata matcher.
|
||||
- Active prompt-generation correlation.
|
||||
- Ordered reset projection and ordinary session-info preservation.
|
||||
- Positive, malformed, stale, zero-generation, and unsupported-dialect tests.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Lifecycle record mapping or task-message deletion.
|
||||
- Terminal error classification and Kandev-owned prompt retry.
|
||||
- Raw error-string matching.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- The captured `willRetry=true` and `responseStreamDisconnected` shape emits
|
||||
exactly one generation-bearing reset before replacement chunks.
|
||||
- A false flag, missing or malformed nested object, unsupported dialect, and
|
||||
zero or stale prompt generation emit no reset and remove no ordinary event.
|
||||
- The normalized reset carries no raw provider diagnostic or provider-specific
|
||||
recovery policy.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test -race ./internal/agentctl/server/adapter/transport/acp -run 'Test(CodexResponseAttemptReset|ObserveResponseAttemptReset|HandleACPUpdate.*ResponseAttemptReset)')
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/agentctl/types/streams/agent.go`
|
||||
- `apps/backend/internal/agentctl/server/adapter/transport/acp/dialect.go`
|
||||
- `apps/backend/internal/agentctl/server/adapter/transport/acp/dialect_codex.go`
|
||||
- `apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_updates.go`
|
||||
- `apps/backend/internal/agentctl/server/adapter/transport/acp/dialect_codex_retry_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
None.
|
||||
|
||||
## Risks
|
||||
|
||||
- Matching a broad error shape would let unrelated session metadata erase
|
||||
valid output.
|
||||
- Emitting outside the notification worker would lose ordering with partial and
|
||||
replacement chunks.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- Provider Error Recovery criterion `.26`.
|
||||
- Adapter evidence projection in the response-attempt recovery design.
|
||||
- Existing Codex capacity and Cursor retry evidence observers.
|
||||
- Sanitized ACP metadata captured from the reported task.
|
||||
|
||||
## Results
|
||||
|
||||
Implemented a provider-neutral `response_attempt_reset` stream event, an
|
||||
optional ACP dialect recognizer, and an exact Codex matcher for the structured
|
||||
`willRetry=true` plus `responseStreamDisconnected` metadata. The ordered ACP
|
||||
worker emits the generation-bearing reset before the preserved session-info
|
||||
event and later replacement chunks. Unsupported dialects, malformed metadata,
|
||||
and zero or stale prompt generations remain inert.
|
||||
|
||||
Verified with:
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test -race ./internal/agentctl/server/adapter/transport/acp -run 'Test(CodexResponseAttemptReset|ObserveResponseAttemptReset|HandleACPUpdate.*ResponseAttemptReset)' -count=1)
|
||||
```
|
||||
+104
@@ -0,0 +1,104 @@
|
||||
---
|
||||
id: "02-map-provisional-stream-records"
|
||||
title: "Map provisional stream records"
|
||||
status: done
|
||||
wave: 2
|
||||
depends_on:
|
||||
- "01-project-response-attempt-resets"
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-PLATFORM-PROVIDER-ERROR-RECOVERY-001
|
||||
acceptance_criteria:
|
||||
- AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.27
|
||||
- AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.28
|
||||
system_design:
|
||||
- ../../specs/platform/system-design/provider-response-attempt-recovery.md
|
||||
---
|
||||
|
||||
# Task 02: Map Provisional Stream Records
|
||||
|
||||
## Summary
|
||||
|
||||
Make lifecycle own the Kandev assistant and thinking IDs created during the
|
||||
current provider response attempt. Convert a fenced reset into an ordered list
|
||||
of retractable IDs while preserving committed records and conservative replay
|
||||
evidence.
|
||||
|
||||
## In scope
|
||||
|
||||
- Attempt-local assistant and thinking record tracking.
|
||||
- Top-level tool commit boundary.
|
||||
- Current execution and prompt-generation fencing.
|
||||
- Coalescer flush before reset publication.
|
||||
- Protocol correlation, legacy buffer, and pending fallback-history cleanup.
|
||||
- Provider-neutral `RetractedMessageIDs` stream payload.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Durable deletion or frontend delivery.
|
||||
- Truncation of a record created before the current attempt.
|
||||
- Relaxing prompt-level output or effect evidence.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- A current reset detaches every assistant and thinking row first allocated in
|
||||
the active attempt, publishes their IDs once in allocation order, and clears
|
||||
pending abandoned history.
|
||||
- Empty, repeated, zero-generation, stale-execution, and stale-generation
|
||||
resets cannot retract successor or earlier-turn records.
|
||||
- A top-level tool boundary commits earlier rows; reuse of their protocol IDs
|
||||
cannot make those rows deletion-eligible again or relax replay safety.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test -race ./internal/agent/runtime/lifecycle -run 'Test(HandleResponseAttemptReset|ResponseAttemptRecords|ResponseAttemptReset)')
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/agent/runtime/lifecycle/types.go`
|
||||
- `apps/backend/internal/agent/runtime/lifecycle/event_types.go`
|
||||
- `apps/backend/internal/agent/runtime/lifecycle/events.go`
|
||||
- `apps/backend/internal/agent/runtime/lifecycle/manager_streaming.go`
|
||||
- `apps/backend/internal/agent/runtime/lifecycle/manager_events.go`
|
||||
- `apps/backend/internal/agent/runtime/lifecycle/manager_response_attempt_reset_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
- Task 01 supplies the normalized reset event and prompt generation.
|
||||
|
||||
## Risks
|
||||
|
||||
- Clearing the full protocol map would break valid message continuation across
|
||||
tool calls.
|
||||
- Publishing before the stream coalescer drains can reorder deletion before
|
||||
creation.
|
||||
- Resetting prompt-attempt evidence can accidentally authorize an unsafe prompt
|
||||
replay.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- Provider Error Recovery criteria `.27` and `.28`.
|
||||
- Lifecycle record ownership and commit-boundary sections in the design.
|
||||
- Existing protocol message correlation and stream coalescer tests.
|
||||
|
||||
## Results
|
||||
|
||||
Lifecycle now records each newly allocated assistant and thinking message ID in
|
||||
wire allocation order. A generation-fenced reset flushes the stream coalescer,
|
||||
detaches only records created since the latest committed boundary, selectively
|
||||
removes their protocol correlations, and clears legacy buffers plus pending
|
||||
assistant fallback history. Top-level tool calls commit earlier records, and
|
||||
prompt-level replay evidence remains conservative after a reset.
|
||||
|
||||
Verified current, empty/repeated, inactive, stale-generation, stale-execution,
|
||||
legacy-stream, protocol-reuse, and top-level-tool boundary behavior with:
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test -race ./internal/agent/runtime/lifecycle -run 'Test(HandleResponseAttemptReset|ResponseAttemptRecords|ResponseAttemptReset)' -count=1)
|
||||
```
|
||||
@@ -0,0 +1,109 @@
|
||||
---
|
||||
id: "03-apply-transcript-retractions"
|
||||
title: "Apply transcript retractions"
|
||||
status: done
|
||||
wave: 3
|
||||
depends_on:
|
||||
- "02-map-provisional-stream-records"
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-PLATFORM-PROVIDER-ERROR-RECOVERY-001
|
||||
acceptance_criteria:
|
||||
- AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.27
|
||||
- AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.28
|
||||
system_design:
|
||||
- ../../specs/platform/system-design/provider-response-attempt-recovery.md
|
||||
---
|
||||
|
||||
# Task 03: Apply Transcript Retractions
|
||||
|
||||
## Summary
|
||||
|
||||
Consume lifecycle's provider-neutral retraction list under the orchestrator's
|
||||
session stream guard. Delete each abandoned row through the task service so
|
||||
storage and every connected client receive the same result.
|
||||
|
||||
## In scope
|
||||
|
||||
- Narrow task-message retraction service and backend composition.
|
||||
- Reset handling after existing execution, generation, and cancellation gates.
|
||||
- Durable assistant and thinking message deletion.
|
||||
- Existing `session.message.deleted` publication.
|
||||
- Per-record failure continuation and structured logging.
|
||||
- Task-service-backed orchestration tests.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Adapter metadata matching or lifecycle ID ownership.
|
||||
- New task-message schema or frontend event type.
|
||||
- Retrying a failed database mutation indefinitely.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- A current reset deletes every supplied message ID through the task service
|
||||
and publishes one existing deletion notification per successful row.
|
||||
- Empty input is a no-op; stale and completed execution events cannot delete
|
||||
messages.
|
||||
- One failed deletion is logged without stopping later deletions or the live
|
||||
provider turn, and missing production wiring is caught by composition tests.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test -race -tags fts5 ./internal/orchestrator -run 'TestHandleResponseAttemptReset')
|
||||
(cd apps/backend && go test -race ./internal/backendapp -run 'Test.*StreamingMessageRetraction')
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/internal/orchestrator/service.go`
|
||||
- `apps/backend/internal/orchestrator/event_handlers_streaming.go`
|
||||
- `apps/backend/internal/orchestrator/event_handlers_streaming_retraction_test.go`
|
||||
- `apps/backend/internal/backendapp/orchestrator.go`
|
||||
- `apps/backend/internal/backendapp/orchestrator_streaming_retraction_test.go`
|
||||
|
||||
## Dependencies
|
||||
|
||||
- Task 02 supplies fenced Kandev message IDs in stream order.
|
||||
|
||||
## Risks
|
||||
|
||||
- Bypassing the task service would remove durable data without notifying live
|
||||
clients.
|
||||
- A handler outside the per-session stream guard can race replacement output or
|
||||
cancellation.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- Provider Error Recovery criteria `.27` and `.28`.
|
||||
- Durable and live cleanup section in the response-attempt recovery design.
|
||||
- Existing transient-retry notice cleanup and task-service deletion patterns.
|
||||
|
||||
## Results
|
||||
|
||||
The orchestrator now consumes `response_attempt_reset` under its existing
|
||||
per-session stream guard, requires the current nonzero prompt generation, and
|
||||
rejects terminal executions before mutation. A dedicated retraction seam is
|
||||
wired to the task service in production. Each successful deletion therefore
|
||||
removes the durable row and publishes the existing `MessageDeleted` event; an
|
||||
individual failure is logged and later IDs are still attempted.
|
||||
|
||||
The gateway now routes `MessageAdded`, `MessageUpdated`, and `MessageDeleted`
|
||||
through the same ordered wildcard subscription. A NATS-like transport test
|
||||
delays deletion delivery and schedules the replacement subject first, proving
|
||||
that the replacement remains queued behind the deletion before journal or
|
||||
client fan-out.
|
||||
|
||||
Verified handler behavior, durable SQLite deletion, deletion-event publication,
|
||||
inactive ownership, completion fencing, failure continuation, and production
|
||||
composition with:
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test -race -tags fts5 ./internal/orchestrator -run 'TestHandleResponseAttemptReset' -count=1)
|
||||
(cd apps/backend && go test -race ./internal/backendapp -run 'Test.*StreamingMessageRetraction' -count=1)
|
||||
(cd apps/backend && go test ./internal/gateway/websocket -run 'TestTaskEventBroadcaster_OrdersTranscriptMutationsAcrossTransportSubjects' -count=1)
|
||||
```
|
||||
@@ -0,0 +1,112 @@
|
||||
---
|
||||
id: "04-prove-chat-retry-recovery"
|
||||
title: "Prove chat retry recovery"
|
||||
status: done
|
||||
wave: 4
|
||||
depends_on:
|
||||
- "03-apply-transcript-retractions"
|
||||
plan: "plan.md"
|
||||
requirements:
|
||||
- REQ-PLATFORM-PROVIDER-ERROR-RECOVERY-001
|
||||
acceptance_criteria:
|
||||
- AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.26
|
||||
- AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.27
|
||||
system_design:
|
||||
- ../../specs/platform/system-design/provider-response-attempt-recovery.md
|
||||
---
|
||||
|
||||
# Task 04: Prove Chat Retry Recovery
|
||||
|
||||
## Summary
|
||||
|
||||
Add a controlled mock-agent response-retry sequence and drive it through a real
|
||||
task session. Prove on desktop and mobile that abandoned assistant and thinking
|
||||
rows disappear live, remain absent from the durable transcript, and do not
|
||||
return after reload.
|
||||
|
||||
## In scope
|
||||
|
||||
- Mock-agent explicit-ID response and thinking emitters.
|
||||
- Test-only structured response-attempt reset marker and mock ACP dialect.
|
||||
- Deterministic `/e2e:response-retry` scenario.
|
||||
- Desktop and mobile task-chat Playwright coverage.
|
||||
- Existing frontend deletion-barrier unit test.
|
||||
|
||||
## Out of scope
|
||||
|
||||
- Production provider shortcuts for the mock marker.
|
||||
- New task-chat markup, responsive composition, copy, or controls.
|
||||
- Real provider credentials or network calls in tests.
|
||||
|
||||
## Acceptance
|
||||
|
||||
- The mock scenario exercises the same normalized reset event and durable
|
||||
deletion path as the Codex recognizer without making its marker valid for a
|
||||
production dialect.
|
||||
- Desktop and mobile show only the replacement response after the live reset,
|
||||
and API readback plus reload contain no abandoned assistant or thinking row.
|
||||
- The existing message-deletion barrier still prevents a queued update from
|
||||
resurrecting a removed row.
|
||||
|
||||
## Verification
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test -race ./cmd/mock-agent ./internal/agentctl/server/adapter/transport/acp -run 'Test.*ResponseRetry')
|
||||
(cd apps && pnpm install --frozen-lockfile)
|
||||
(cd apps/web && pnpm exec vitest run lib/ws/handlers/messages.test.ts)
|
||||
(cd apps/web && pnpm e2e:run --project chromium tests/session/provider-response-retry.spec.ts -- --retries=0)
|
||||
(cd apps/web && pnpm e2e:run --project mobile-chrome tests/session/mobile-provider-response-retry.spec.ts -- --retries=0)
|
||||
```
|
||||
|
||||
## Files likely touched
|
||||
|
||||
- `apps/backend/cmd/mock-agent/emitter.go`
|
||||
- `apps/backend/cmd/mock-agent/scenarios.go`
|
||||
- `apps/backend/cmd/mock-agent/response_retry_test.go`
|
||||
- `apps/backend/internal/agentctl/server/adapter/transport/acp/dialect.go`
|
||||
- `apps/backend/internal/agentctl/server/adapter/transport/acp/dialect_mock.go`
|
||||
- `apps/backend/internal/agentctl/server/adapter/transport/acp/dialect_mock_test.go`
|
||||
- `apps/web/e2e/tests/session/provider-response-retry.spec.ts`
|
||||
- `apps/web/e2e/tests/session/mobile-provider-response-retry.spec.ts`
|
||||
|
||||
## Dependencies
|
||||
|
||||
- Task 03 completes the production deletion path exercised by the fixture.
|
||||
|
||||
## Risks
|
||||
|
||||
- A mock-only marker accepted by another dialect would create a production
|
||||
transcript-deletion input.
|
||||
- E2E assertions that inspect only the DOM can miss a row that returns on
|
||||
reload; durable API readback and reload are both required.
|
||||
|
||||
## Parallelism
|
||||
|
||||
`sequential`
|
||||
|
||||
## Inputs
|
||||
|
||||
- Provider Error Recovery criteria `.26` and `.27`.
|
||||
- Test strategy in the response-attempt recovery design.
|
||||
- Existing desktop and mobile transient-retry chat scenarios.
|
||||
- Existing frontend message-deletion semantic-barrier test.
|
||||
|
||||
## Results
|
||||
|
||||
Added explicit-ID mock assistant and thinking emitters, a mock-only structured
|
||||
reset marker, and a deterministic `/e2e:response-retry` scenario. The mock ACP
|
||||
dialect translates that marker through the same provider-neutral reset path as
|
||||
Codex while production dialects ignore it. Desktop and mobile Playwright tests
|
||||
arm causal WebSocket observers before the retry, correlate the provisional row
|
||||
IDs with their deletion events, require both deletions before replacement, and
|
||||
verify the replacement-only durable transcript and the same result after
|
||||
reload. The mock no longer pauses to create a transient DOM assertion window.
|
||||
|
||||
Verified with:
|
||||
|
||||
```bash
|
||||
(cd apps/backend && go test -race ./cmd/mock-agent ./internal/agentctl/server/adapter/transport/acp -run 'Test.*ResponseRetry' -count=1)
|
||||
(cd apps/web && pnpm exec vitest run lib/ws/handlers/messages.test.ts)
|
||||
(cd apps/web && pnpm e2e:run --project chromium tests/session/provider-response-retry.spec.ts -- --retries=0)
|
||||
(cd apps/web && pnpm e2e:run --project mobile-chrome tests/session/mobile-provider-response-retry.spec.ts -- --retries=0)
|
||||
```
|
||||
@@ -116,6 +116,7 @@ retains ownership of its values, validation, authority, and persistence.
|
||||
- [Session MCP Attachment Observability](system-design/mcp-session-observability.md)
|
||||
- [Required Persisted Store Parity](system-design/postgres-domain-store-parity.md)
|
||||
- [Provider Error Recovery](system-design/provider-error-recovery.md)
|
||||
- [Provider Response-Attempt Recovery](system-design/provider-response-attempt-recovery.md)
|
||||
- [Workspace Git Status](system-design/workspace-git-status.md)
|
||||
- [Go dev launcher and startup version](system-design/go-dev-launcher.md)
|
||||
- [Startup database selection continuity](system-design/startup-database-selection-continuity.md)
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
status: draft
|
||||
system: platform
|
||||
created: 2026-08-08
|
||||
updated: 2026-09-15
|
||||
updated: 2026-09-16
|
||||
owners:
|
||||
- Kandev
|
||||
---
|
||||
@@ -12,6 +12,18 @@ owners:
|
||||
|
||||
Agent CLIs and providers report equivalent failures through different ACP frames, HTTP metadata, process exits, and diagnostic strings. Capacity, network, subscription, and quota failures need different recovery behavior, but orchestration code must not branch on provider names or raw prose.
|
||||
|
||||
A provider can also abandon a partially streamed response and retry it inside
|
||||
the same prompt. That retry must replace the abandoned attempt without leaving
|
||||
duplicate transcript rows or stale recovery context.
|
||||
|
||||
## Terminology
|
||||
|
||||
- **Provider response attempt:** One provider generation attempt inside a
|
||||
Kandev prompt. A provider can retry this attempt without ending the prompt.
|
||||
- **Response-attempt retry boundary:** Structured, current-prompt evidence that
|
||||
the provider abandoned its active response attempt and will generate a
|
||||
replacement.
|
||||
|
||||
## Requirements
|
||||
|
||||
### REQ-PLATFORM-PROVIDER-ERROR-RECOVERY-001: Provider Error Recovery
|
||||
@@ -47,6 +59,9 @@ Agent CLIs and providers report equivalent failures through different ACP frames
|
||||
- **AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.24:** When a terminal ACP prompt error carries its transient signature only in `RequestError.Data`, Kandev shall present the failure as terminal and expose manual recovery on every occurrence, including repeated failures in the same session, and shall neither show a transient retry notice nor start an automatic retry. This criterion applies to the terminal failure whether or not a diagnostic preceded it, and it does not outrank acceptance criteria `.16` and `.23`: when a diagnostic did precede, those criteria reach the same manual-recovery outcome, because a terminal message that classifies to nothing can match no recorded diagnostic code. The existing session end-to-end specification that drives the mock agent's `/transport-lost` command shall be the surface that observes this outcome: its automatic-retry assertions shall be retargeted to manual recovery, and its retry-cancellation sub-test shall be retired rather than rewritten, because no retry loop exists on this path to cancel. The mock command's envelope shall keep the SDK's own peer-disconnect shape, `Message` `Internal error` with the signature only in `Data`, so the specification keeps exercising a shape that occurs in production. Repository documentation describing that command, or the recovery timeout it drives, shall not state that this path enters the automatic retry ladder. Losing end-to-end coverage of the automatic retry ladder for the transport-loss cause is accepted in this version; the catalogue rule keeps its unit coverage and its remaining surfaces.
|
||||
|
||||
- **AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.25:** During one interactive transient retry loop, desktop and mobile chat shall show at most one current retry banner. Each subsequent attempt shall update that banner's reason, provider, attempt count, deadline, and Cancel action. Reloading the session shall retain the latest persisted attempt. Existing duplicate retry notices shall consolidate on the next successful retry-notice write. Unrelated messages and other sessions shall remain unchanged. Storage failures shall remain non-fatal and shall not trigger creation of an additional notice when an existing notice cannot be read or updated.
|
||||
- **AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.26:** When a supported adapter receives exact structured evidence that the active provider abandoned the current response attempt and will retry it internally, Kandev shall accept a response-attempt retry boundary within that prompt generation. One prompt generation may contain multiple sequential boundaries; each boundary applies only to assistant and thinking records created after the preceding boundary or commit boundary. Malformed evidence, a false retry flag, an unsupported adapter, a zero or stale prompt generation, and terminal provider errors shall not create that boundary or remove transcript content.
|
||||
- **AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.27:** After Kandev accepts a response-attempt retry boundary, it shall durably remove assistant and thinking messages created by the abandoned attempt. Connected desktop and mobile chats shall remove those messages before later replacement output, and a reload or another viewer shall see only the replacement response.
|
||||
- **AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.28:** Response-attempt cleanup shall preserve messages, tool activity, permission state, and turns committed before the active provider attempt. A repeated retry boundary with no new output shall be a no-op, and abandoned assistant text that has not crossed a history boundary shall not enter later fallback resume context. The boundary shall not erase prompt-level output or effect evidence used to decide whether a separate Kandev-owned replay is safe.
|
||||
|
||||
## Out of scope
|
||||
|
||||
@@ -66,4 +81,6 @@ The migrated technical source is split into
|
||||
[part 2](../system-design/provider-error-recovery-02.md) (replay fixture matrix
|
||||
and harness) and
|
||||
[part 3](../system-design/provider-error-recovery-03.md) (marker propagation,
|
||||
diagnostic text correlation, allowlisted prompt-error metadata).
|
||||
diagnostic text correlation, allowlisted prompt-error metadata), and
|
||||
[part 4](../system-design/provider-response-attempt-recovery.md)
|
||||
(provider-owned response-attempt replacement and transcript retraction).
|
||||
|
||||
@@ -0,0 +1,176 @@
|
||||
---
|
||||
status: current
|
||||
system: platform
|
||||
requirements:
|
||||
- REQ-PLATFORM-PROVIDER-ERROR-RECOVERY-001
|
||||
created: 2026-09-09
|
||||
updated: 2026-09-16
|
||||
owners:
|
||||
- Kandev
|
||||
---
|
||||
|
||||
# Provider Response-Attempt Recovery System Design
|
||||
|
||||
## Purpose and boundaries
|
||||
|
||||
The platform system owns the recovery boundary between provider-specific ACP
|
||||
retry evidence, lifecycle streaming identity, durable task messages, and live
|
||||
session delivery. This design covers a provider retry that remains inside one
|
||||
active prompt. Terminal provider errors and Kandev-owned prompt replay remain
|
||||
in [Provider Error Recovery](provider-error-recovery.md).
|
||||
|
||||
The current production trigger is Codex's structured response-stream
|
||||
disconnect metadata. The downstream reset contract is provider neutral.
|
||||
|
||||
## Requirement mapping
|
||||
|
||||
| Acceptance criterion | Design section |
|
||||
| --- | --- |
|
||||
| `AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.26` | [Adapter evidence projection](#adapter-evidence-projection) |
|
||||
| `AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.27` | [Lifecycle record ownership](#lifecycle-record-ownership), [Durable and live cleanup](#durable-and-live-cleanup) |
|
||||
| `AC-PLATFORM-PROVIDER-ERROR-RECOVERY-001.28` | [Commit boundaries and replay safety](#commit-boundaries-and-replay-safety), [Failure behavior](#failure-behavior) |
|
||||
|
||||
## Adapter evidence projection
|
||||
|
||||
`internal/agentctl/server/adapter/transport/acp` translates implementation
|
||||
metadata before it crosses the agentctl stream boundary. `acpDialect` exposes
|
||||
an optional pure response-attempt reset recognizer. The Codex dialect accepts a
|
||||
`session_info_update` only when all of these fields are present with the stated
|
||||
types:
|
||||
|
||||
- `codex.error.willRetry` is the boolean `true`;
|
||||
- `codex.error.codexErrorInfo.responseStreamDisconnected` is an object; and
|
||||
- the notification belongs to the active, non-zero prompt generation.
|
||||
|
||||
The recognizer does not inspect `additionalDetails` or another error string.
|
||||
`willRetry: false`, a missing nested disconnect object, malformed maps, an
|
||||
unsupported dialect, and zero or stale prompt generations remain ordinary
|
||||
session information.
|
||||
|
||||
The FIFO ACP notification worker emits
|
||||
`streams.EventTypeResponseAttemptReset` before it can deliver later replacement
|
||||
chunks. The event contains the session and prompt generation, but no raw
|
||||
provider diagnostic or provider-specific data. Any ordinary title, timestamp,
|
||||
or session metadata in the same ACP update keeps its existing session-info
|
||||
projection.
|
||||
|
||||
The controlled `mock-agent` dialect has a test-only structured marker for the
|
||||
same normalized event. Production recognizers never accept that marker.
|
||||
|
||||
## Lifecycle record ownership
|
||||
|
||||
`lifecycle.AgentExecution` owns both streaming IDs and the current response
|
||||
attempt's retraction eligibility. Whenever lifecycle allocates a new Kandev
|
||||
assistant or thinking message ID, it appends that ID to an ordered attempt-local
|
||||
record list. Appends to a record created before the current attempt do not make
|
||||
that record eligible for deletion.
|
||||
|
||||
`Manager.handleResponseAttemptReset` first verifies that the event's
|
||||
execution and prompt generation still own the active prompt. It then performs
|
||||
one ordered reset:
|
||||
|
||||
1. Flush the stream coalescer so every earlier create or append precedes the
|
||||
reset on the session stream subject.
|
||||
2. Under `messageMu`, detach the eligible Kandev record IDs, remove only their
|
||||
protocol-ID correlations, reset legacy current IDs and pending message and
|
||||
thinking buffers, and discard the uncommitted assistant-history segment.
|
||||
3. Publish one provider-neutral `response_attempt_reset` stream event carrying
|
||||
the detached IDs in allocation order.
|
||||
|
||||
An empty record list still completes the local reset but does not require a
|
||||
message mutation. A second reset can retract only records allocated after the
|
||||
first one.
|
||||
|
||||
## Commit boundaries and replay safety
|
||||
|
||||
A top-level tool call is the effect boundary that commits visible response
|
||||
records. Lifecycle flushes their streaming content and assistant history before
|
||||
clearing the attempt-local eligibility list. Protocol-to-Kandev correlation can
|
||||
continue across the tool call for providers that reuse an ACP message ID, but
|
||||
that existing Kandev row cannot become deletion-eligible again. Subagent
|
||||
nesting does not let a provider reset remove the parent tool row or another
|
||||
turn's records.
|
||||
|
||||
Prompt start, completion, foreground handoff, cancellation replacement, and
|
||||
execution teardown clear attempt-local tracking with the existing streaming
|
||||
state. A reset never reaches backward into a completed prompt generation.
|
||||
|
||||
The orchestrator's prompt-attempt evidence remains conservative. Output or
|
||||
effects observed before the provider-owned reset remain observed for the whole
|
||||
Kandev prompt. Removing a transcript projection therefore cannot turn a later
|
||||
terminal failure into permission for an automatic Kandev replay.
|
||||
|
||||
Fallback session history uses the existing assistant-history buffer. Resetting
|
||||
the pending segment prevents abandoned text that has not crossed a history
|
||||
boundary from being injected into a later non-native resume. Previously
|
||||
committed history is not rewritten.
|
||||
|
||||
## Durable and live cleanup
|
||||
|
||||
`lifecycle.AgentStreamEventData` carries `RetractedMessageIDs` only for a
|
||||
`response_attempt_reset` event. The orchestrator handles it under the existing
|
||||
per-session cancellation and stream guard, after the completed-execution and
|
||||
generation fences used for other agent stream events.
|
||||
|
||||
A narrow `StreamingMessageRetractionService` exposes task-service
|
||||
`DeleteMessage` to the handler. The backend application wires the task service
|
||||
as the implementation. Each successful deletion removes the row and publishes
|
||||
`session.message.deleted`. The existing frontend message handler treats that
|
||||
notification as a semantic barrier: it flushes queued updates and removes the
|
||||
message from the shared session store. Desktop and mobile task chat use that
|
||||
same store and renderer, so no responsive layout or new user-facing copy is
|
||||
needed.
|
||||
|
||||
The lifecycle session stream serializes provisional creation, reset handling,
|
||||
deletion, and replacement persistence. The WebSocket gateway consumes
|
||||
`message.added`, `message.updated`, and `message.deleted` through one ordered
|
||||
NATS wildcard subscription, so independently scheduled subject callbacks cannot
|
||||
overtake one another before the per-session journal and live fan-out. A reload
|
||||
or a second viewer reads the durable transcript after deletion and therefore
|
||||
cannot restore the abandoned row.
|
||||
|
||||
## Failure behavior
|
||||
|
||||
- A stale or zero-generation reset is ignored before it can detach IDs.
|
||||
- A reset with no eligible output is an idempotent no-op at the task-service
|
||||
boundary.
|
||||
- Failure to delete one row is logged with task, session, execution, and
|
||||
message identity. Cleanup continues for the remaining IDs and the provider
|
||||
replacement turn remains live.
|
||||
- A protocol message that predates the latest commit boundary is preserved even
|
||||
if later chunks reuse its ID. The system does not guess a truncation offset.
|
||||
- Missing retraction-service wiring leaves the stream running; production
|
||||
composition tests require the task service to be wired.
|
||||
|
||||
## Observability
|
||||
|
||||
ACP normalized logs retain the provider-neutral reset event with agent type,
|
||||
session, and prompt generation without copying provider diagnostics. Existing
|
||||
task-service logs identify successful durable deletions, and orchestration logs
|
||||
failed deletions with task, session, execution, and message identity. A
|
||||
diagnostic trace can therefore distinguish an intended replacement from two
|
||||
legitimate messages.
|
||||
|
||||
## Test strategy
|
||||
|
||||
- ACP adapter tests use the captured Codex metadata shape and cover malformed,
|
||||
false, unsupported, zero-generation, and stale-generation negatives plus FIFO
|
||||
ordering before replacement output.
|
||||
- Lifecycle tests cover assistant and thinking IDs, pending buffers, repeated
|
||||
resets, committed rows, protocol-ID reuse, generation fencing, and fallback
|
||||
history.
|
||||
- Orchestrator tests use the real task service and event bus to prove durable
|
||||
deletion, `session.message.deleted` publication, continued cleanup after an
|
||||
individual failure, and safe behavior without wiring. A gateway transport
|
||||
test schedules the replacement subscription ahead of delayed deletion and
|
||||
proves that the shared subscription retains deletion-first delivery.
|
||||
- Desktop and mobile Playwright scenarios arm causal WebSocket observations
|
||||
before the controlled mock dialect streams an abandoned response. They
|
||||
correlate both created message IDs with their deletion events, require both
|
||||
deletions before replacement, then reload and assert that only the
|
||||
replacement remains.
|
||||
|
||||
## Related decisions
|
||||
|
||||
- [Retract Output From Abandoned Provider Attempts](../../../decisions/2026-09-09-retract-abandoned-provider-output.md)
|
||||
- [Separate Agent Error Evidence From Recovery Policy](../../../decisions/2026-08-08-provider-neutral-agent-error-recovery.md)
|
||||
Reference in New Issue
Block a user