fix(acp): deliver follow-ups after foreground handoff (#1959)
This commit is contained in:
@@ -10,6 +10,7 @@ import (
|
||||
|
||||
agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl"
|
||||
"github.com/kandev/kandev/internal/agentctl/tracing"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
v1 "github.com/kandev/kandev/pkg/api/v1"
|
||||
)
|
||||
@@ -572,6 +573,53 @@ func (m *Manager) publishStreamDisconnectError(execution *AgentExecution, err er
|
||||
)
|
||||
}
|
||||
|
||||
// handlePromptHandoffEvent finalizes lifecycle's shared streaming-buffer and
|
||||
// completion-wait ownership at the same generation-bearing boundary where the
|
||||
// ACP adapter made one human successor eligible to inherit prompt ownership.
|
||||
// It does not mark the execution Ready: foreground/background activity and
|
||||
// coarse turn completion remain separate orchestrator/provider concerns.
|
||||
func (m *Manager) handlePromptHandoffEvent(
|
||||
execution *AgentExecution,
|
||||
event agentctl.AgentEvent,
|
||||
) {
|
||||
handoff, _ := event.Data[streams.AgentEventDataPromptHandoff].(bool)
|
||||
if !handoff || event.PromptGeneration == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
execution.promptLifecycleMu.Lock()
|
||||
defer execution.promptLifecycleMu.Unlock()
|
||||
|
||||
ownsGeneration := false
|
||||
err := m.executionStore.WithLock(execution.ID, func(current *AgentExecution) {
|
||||
ownsGeneration = current == execution &&
|
||||
current.promptGeneration == event.PromptGeneration
|
||||
})
|
||||
if err != nil || !ownsGeneration {
|
||||
m.logger.Debug("ignoring prompt handoff for superseded generation",
|
||||
zap.String("execution_id", execution.ID),
|
||||
zap.Uint64("event_prompt_generation", event.PromptGeneration))
|
||||
return
|
||||
}
|
||||
|
||||
m.flushMessageBuffer(execution)
|
||||
execution.messageMu.Lock()
|
||||
execution.clearProtocolMessageCorrelationLocked()
|
||||
execution.messageMu.Unlock()
|
||||
m.flushAssistantHistory(execution)
|
||||
|
||||
select {
|
||||
case execution.promptDoneCh <- PromptCompletionSignal{
|
||||
StopReason: streams.EventTypeForegroundIdle,
|
||||
PromptGeneration: event.PromptGeneration,
|
||||
}:
|
||||
default:
|
||||
m.logger.Warn("prompt handoff could not signal lifecycle waiter",
|
||||
zap.String("execution_id", execution.ID),
|
||||
zap.Uint64("prompt_generation", event.PromptGeneration))
|
||||
}
|
||||
}
|
||||
|
||||
// handleAgentEvent processes incoming agent events from the agent
|
||||
func (m *Manager) handleAgentEvent(execution *AgentExecution, event agentctl.AgentEvent) {
|
||||
if event.PromptGeneration == 0 || (event.Type != toolStatusComplete && event.Type != "error") {
|
||||
@@ -658,6 +706,9 @@ func (m *Manager) handleAgentEvent(execution *AgentExecution, event agentctl.Age
|
||||
}
|
||||
// No return — must flow through to PublishAgentStreamEvent so the orchestrator
|
||||
// can persist the model and re-publish to the dedicated session models subject.
|
||||
|
||||
case streams.EventTypeForegroundIdle:
|
||||
m.handlePromptHandoffEvent(execution, event)
|
||||
}
|
||||
|
||||
m.eventPublisher.PublishAgentStreamEvent(execution, event)
|
||||
|
||||
@@ -13,6 +13,7 @@ import (
|
||||
"go.uber.org/zap/zapcore"
|
||||
|
||||
agentctl "github.com/kandev/kandev/internal/agent/runtime/agentctl"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
"github.com/kandev/kandev/internal/common/logger"
|
||||
"github.com/kandev/kandev/internal/events"
|
||||
"github.com/kandev/kandev/internal/events/bus"
|
||||
@@ -1484,6 +1485,46 @@ func TestHandleAgentEvent_ForegroundIdlePreservesPromptGeneration(t *testing.T)
|
||||
if events[0].Data.PromptGeneration != 42 {
|
||||
t.Fatalf("prompt generation = %d, want 42", events[0].Data.PromptGeneration)
|
||||
}
|
||||
select {
|
||||
case signal := <-execution.promptDoneCh:
|
||||
t.Fatalf("unattested foreground idle released lifecycle waiter: %+v", signal)
|
||||
default:
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleAgentEvent_StalePromptHandoffCannotReleaseReplacement(t *testing.T) {
|
||||
mgr, _ := createTestManagerWithTracking()
|
||||
execution := createTestExecution("exec-stale-handoff", "task-1", "session-1")
|
||||
if err := mgr.executionStore.Add(execution); err != nil {
|
||||
t.Fatalf("add execution: %v", err)
|
||||
}
|
||||
if _, err := mgr.executionStore.BeginPrompt(execution.ID); err != nil {
|
||||
t.Fatalf("begin original prompt: %v", err)
|
||||
}
|
||||
if _, err := mgr.executionStore.BeginPrompt(execution.ID); err != nil {
|
||||
t.Fatalf("begin replacement prompt: %v", err)
|
||||
}
|
||||
execution.messageMu.Lock()
|
||||
execution.messageBuffer.WriteString("replacement output")
|
||||
execution.messageMu.Unlock()
|
||||
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeForegroundIdle,
|
||||
PromptGeneration: 1,
|
||||
Data: map[string]any{streams.AgentEventDataPromptHandoff: true},
|
||||
})
|
||||
|
||||
select {
|
||||
case signal := <-execution.promptDoneCh:
|
||||
t.Fatalf("stale handoff released replacement waiter: %+v", signal)
|
||||
default:
|
||||
}
|
||||
execution.messageMu.Lock()
|
||||
buffer := execution.messageBuffer.String()
|
||||
execution.messageMu.Unlock()
|
||||
if buffer != "replacement output" {
|
||||
t.Fatalf("stale handoff mutated replacement buffer: %q", buffer)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleAgentEvent_ErrorClaimCannotRaceReplacementPrompt(t *testing.T) {
|
||||
|
||||
@@ -1562,6 +1562,132 @@ func TestSendPrompt_SerializesCompletionWaitersPerExecution(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestSendPrompt_AuthoritativeHandoffReleasesNextGeneration(t *testing.T) {
|
||||
mock := newMockAgentServer(t)
|
||||
t.Cleanup(mock.Close)
|
||||
|
||||
firstPromptSeen := make(chan struct{})
|
||||
secondPromptSeen := make(chan struct{})
|
||||
var promptCount int
|
||||
mock.handler = func(msg ws.Message) *ws.Message {
|
||||
if msg.Action == "agent.prompt" {
|
||||
promptCount++
|
||||
switch promptCount {
|
||||
case 1:
|
||||
close(firstPromptSeen)
|
||||
case 2:
|
||||
close(secondPromptSeen)
|
||||
}
|
||||
}
|
||||
return mock.defaultHandler(msg)
|
||||
}
|
||||
|
||||
mgr, eventBus := createTestManagerWithTracking()
|
||||
client := createTestClient(t, mock.server.URL)
|
||||
t.Cleanup(client.Close)
|
||||
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
t.Cleanup(cancel)
|
||||
if err := client.StreamUpdates(ctx, func(agentctl.AgentEvent) {}, nil, nil); err != nil {
|
||||
t.Fatalf("connect stream: %v", err)
|
||||
}
|
||||
waitForWSConnected(t, mock)
|
||||
|
||||
execution := createTestExecution("exec-handoff", "task-handoff", "session-handoff")
|
||||
execution.agentctl = client
|
||||
if err := mgr.executionStore.Add(execution); err != nil {
|
||||
t.Fatalf("add execution: %v", err)
|
||||
}
|
||||
|
||||
firstResult := make(chan error, 1)
|
||||
go func() {
|
||||
_, err := mgr.sessionManager.SendPrompt(ctx, execution, "first", false, nil, false)
|
||||
firstResult <- err
|
||||
}()
|
||||
select {
|
||||
case <-firstPromptSeen:
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("first prompt did not reach agentctl")
|
||||
}
|
||||
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeMessageChunk,
|
||||
Text: "parent answer",
|
||||
PromptGeneration: 1,
|
||||
})
|
||||
|
||||
secondResult := make(chan error, 1)
|
||||
go func() {
|
||||
_, err := mgr.sessionManager.SendPrompt(ctx, execution, "second", false, nil, false)
|
||||
secondResult <- err
|
||||
}()
|
||||
select {
|
||||
case <-secondPromptSeen:
|
||||
t.Fatal("second prompt reached agentctl before foreground handoff")
|
||||
case <-time.After(50 * time.Millisecond):
|
||||
}
|
||||
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeForegroundIdle,
|
||||
PromptGeneration: 1,
|
||||
Data: map[string]any{streams.AgentEventDataPromptHandoff: true},
|
||||
})
|
||||
|
||||
select {
|
||||
case err := <-firstResult:
|
||||
if err != nil {
|
||||
t.Fatalf("first prompt handoff returned error: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("foreground handoff did not release the first lifecycle waiter")
|
||||
}
|
||||
select {
|
||||
case <-secondPromptSeen:
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("second prompt did not reach agentctl after foreground handoff")
|
||||
}
|
||||
|
||||
// The original bridge RPC can return only after the follow-up echo. Its late
|
||||
// result must not complete or reset generation 2.
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeComplete,
|
||||
PromptGeneration: 1,
|
||||
Data: map[string]any{"stop_reason": "end_turn"},
|
||||
})
|
||||
select {
|
||||
case err := <-secondResult:
|
||||
t.Fatalf("late generation-1 completion released generation 2: %v", err)
|
||||
case <-time.After(50 * time.Millisecond):
|
||||
}
|
||||
if !mgr.executionStore.OwnsPromptGeneration(execution.SessionID, execution.ID, 2) {
|
||||
t.Fatal("late original completion replaced generation-2 ownership")
|
||||
}
|
||||
|
||||
var streamed string
|
||||
for _, event := range eventBus.getStreamEvents() {
|
||||
if event.Data != nil && event.Data.Type == "message_streaming" {
|
||||
streamed += event.Data.Text
|
||||
}
|
||||
}
|
||||
if streamed != "parent answer" {
|
||||
t.Fatalf("streamed handoff boundary = %q, want parent answer", streamed)
|
||||
}
|
||||
|
||||
mgr.handleAgentEvent(execution, agentctl.AgentEvent{
|
||||
Type: streams.EventTypeComplete,
|
||||
PromptGeneration: 2,
|
||||
Data: map[string]any{"stop_reason": "end_turn"},
|
||||
})
|
||||
select {
|
||||
case err := <-secondResult:
|
||||
if err != nil {
|
||||
t.Fatalf("second prompt returned error: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("generation-2 completion did not release its waiter")
|
||||
}
|
||||
}
|
||||
|
||||
func TestWaitForPromptDone_TreatsPromptAbandonedAfterCancelAsCancelEscalated(t *testing.T) {
|
||||
log := newSessionTestLogger()
|
||||
sm := NewSessionManager(log, make(chan struct{}))
|
||||
|
||||
@@ -170,6 +170,12 @@ type Adapter struct {
|
||||
// Tool call tracking for result normalization
|
||||
// Maps toolCallId -> NormalizedPayload so we can update with results
|
||||
activeToolCalls map[string]*streams.NormalizedPayload
|
||||
// toolCallParents preserves nested tool lineage while a handed-off
|
||||
// predecessor continues streaming beside its human successor.
|
||||
toolCallParents map[string]string
|
||||
// handoffProtectedToolCalls identifies predecessor background work that a
|
||||
// successor's prompt-end sweep must not terminate.
|
||||
handoffProtectedToolCalls map[string]struct{}
|
||||
// codexSubagentCorrelations deduplicates the collaboration and activity
|
||||
// tool_call frames codex-acp emits for one logical child, keyed by session,
|
||||
// wire tool-call ID, and child session ID. Incomplete entries are retained
|
||||
@@ -202,8 +208,9 @@ type Adapter struct {
|
||||
|
||||
// OTel tracing: active prompt span context.
|
||||
// Notification spans become children of the prompt span for visual grouping.
|
||||
promptTraceCtx context.Context
|
||||
promptTraceMu sync.RWMutex
|
||||
promptTraceCtx context.Context
|
||||
promptTraceTurn *promptTurnState
|
||||
promptTraceMu sync.RWMutex
|
||||
|
||||
// Attachment management
|
||||
attachMgr *shared.AttachmentManager
|
||||
@@ -255,16 +262,12 @@ type Adapter struct {
|
||||
promptTurnMu sync.Mutex
|
||||
promptTurn *promptTurnState
|
||||
|
||||
// promptGate is a 1-slot semaphore that serializes session/prompt calls so
|
||||
// at most one is in flight against the bridge at a time. The ScheduleWakeup
|
||||
// path injects a synthetic prompt via fireWakeup; without this gate it can
|
||||
// race a user prompt, and the claude-agent-acp bridge then returns each
|
||||
// prompt's stop_reason against the wrong turn — shifting chat turns one
|
||||
// prompt behind. A queued synthetic prompt waits here and drains the wakeup
|
||||
// turn once the in-flight prompt finishes. It is a channel rather than a
|
||||
// sync.Mutex so the wait honours the caller's context (a wakeup whose
|
||||
// timeout/lifetime context is cancelled while queued aborts instead of
|
||||
// blocking on a stuck turn).
|
||||
// promptGate is a 1-slot ownership token for session/prompt calls. Calls are
|
||||
// serialized by default so ScheduleWakeup cannot race a user prompt and
|
||||
// misalign the bridge's prompt responses. A provider-attested human
|
||||
// foreground handoff may transfer the existing token directly to one waiting
|
||||
// human successor while the old RPC returns; synthetic wakeups never observe
|
||||
// that handoff and continue waiting for the owning RPC to finish.
|
||||
promptGate chan struct{}
|
||||
|
||||
// asyncTurnFinalizers synthesize a turn completion for ACP updates that
|
||||
@@ -288,7 +291,12 @@ type promptTurnState struct {
|
||||
endTurn context.CancelCauseFunc
|
||||
rpcDone chan struct{}
|
||||
abortCh chan struct{}
|
||||
handoffCh chan struct{}
|
||||
promptGeneration uint64
|
||||
allowHandoff bool
|
||||
handedOff bool
|
||||
gateOwned bool
|
||||
finishing bool
|
||||
}
|
||||
|
||||
type asyncTurnFinalizer struct {
|
||||
@@ -308,24 +316,26 @@ func NewAdapter(cfg *shared.Config, log *logger.Logger) *Adapter {
|
||||
l := log.WithFields(zap.String("adapter", "acp"), zap.String("agent_id", cfg.AgentID))
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
a := &Adapter{
|
||||
cfg: cfg,
|
||||
logger: l,
|
||||
agentID: cfg.AgentID,
|
||||
normalizer: NewNormalizer(cfg.AgentID),
|
||||
dialect: newACPDialect(cfg.AgentID),
|
||||
updatesCh: make(chan AgentEvent, 100),
|
||||
notifQueue: make(chan notifWork, notifQueueCapacity),
|
||||
activeToolCalls: make(map[string]*streams.NormalizedPayload),
|
||||
activeMonitors: make(map[string]map[string]string),
|
||||
pendingWakeups: make(map[string]*pendingWakeup),
|
||||
usageBySession: make(map[string]*usageTracker),
|
||||
contextSamples: make(map[string]contextWindowSample),
|
||||
attachMgr: shared.NewAttachmentManager(cfg.WorkDir, l.Zap()),
|
||||
promptGate: make(chan struct{}, 1),
|
||||
asyncTurnFinalizers: make(map[string]*asyncTurnFinalizer),
|
||||
asyncTurnEpochs: make(map[string]uint64),
|
||||
lifetimeCtx: ctx,
|
||||
lifetimeCancel: cancel,
|
||||
cfg: cfg,
|
||||
logger: l,
|
||||
agentID: cfg.AgentID,
|
||||
normalizer: NewNormalizer(cfg.AgentID),
|
||||
dialect: newACPDialect(cfg.AgentID),
|
||||
updatesCh: make(chan AgentEvent, 100),
|
||||
notifQueue: make(chan notifWork, notifQueueCapacity),
|
||||
activeToolCalls: make(map[string]*streams.NormalizedPayload),
|
||||
toolCallParents: make(map[string]string),
|
||||
handoffProtectedToolCalls: make(map[string]struct{}),
|
||||
activeMonitors: make(map[string]map[string]string),
|
||||
pendingWakeups: make(map[string]*pendingWakeup),
|
||||
usageBySession: make(map[string]*usageTracker),
|
||||
contextSamples: make(map[string]contextWindowSample),
|
||||
attachMgr: shared.NewAttachmentManager(cfg.WorkDir, l.Zap()),
|
||||
promptGate: make(chan struct{}, 1),
|
||||
asyncTurnFinalizers: make(map[string]*asyncTurnFinalizer),
|
||||
asyncTurnEpochs: make(map[string]uint64),
|
||||
lifetimeCtx: ctx,
|
||||
lifetimeCancel: cancel,
|
||||
}
|
||||
a.wakeup = newWakeupScheduler(l, a.fireWakeup)
|
||||
// Start the update worker before returning so any caller that connects
|
||||
@@ -543,6 +553,7 @@ func (a *Adapter) Close() error {
|
||||
a.workerWg.Wait()
|
||||
a.mu.Lock()
|
||||
a.clearCodexSubagentCorrelationsLocked("")
|
||||
a.clearPromptHandoffToolTrackingLocked()
|
||||
clear(a.usageBySession)
|
||||
a.mu.Unlock()
|
||||
|
||||
@@ -580,17 +591,22 @@ func (a *Adapter) getPromptTraceCtx() context.Context {
|
||||
}
|
||||
|
||||
// setPromptTraceCtx stores the prompt span context.
|
||||
func (a *Adapter) setPromptTraceCtx(ctx context.Context) {
|
||||
func (a *Adapter) setPromptTraceCtx(turn *promptTurnState, ctx context.Context) {
|
||||
a.promptTraceMu.Lock()
|
||||
defer a.promptTraceMu.Unlock()
|
||||
a.promptTraceCtx = ctx
|
||||
a.promptTraceTurn = turn
|
||||
}
|
||||
|
||||
// clearPromptTraceCtx clears the prompt span context.
|
||||
func (a *Adapter) clearPromptTraceCtx() {
|
||||
// clearPromptTraceCtx clears the prompt span context only when turn still owns
|
||||
// it. A handed-off RPC may return after its successor installed a new trace.
|
||||
func (a *Adapter) clearPromptTraceCtx(turn *promptTurnState) {
|
||||
a.promptTraceMu.Lock()
|
||||
defer a.promptTraceMu.Unlock()
|
||||
a.promptTraceCtx = nil
|
||||
if a.promptTraceTurn == turn {
|
||||
a.promptTraceCtx = nil
|
||||
a.promptTraceTurn = nil
|
||||
}
|
||||
}
|
||||
|
||||
// GetACPConnection returns the underlying ACP connection for advanced usage.
|
||||
|
||||
@@ -42,15 +42,16 @@ func (a *Adapter) sendPrompt(
|
||||
expectSession string,
|
||||
promptGeneration uint64,
|
||||
) error {
|
||||
// Acquire the prompt gate, honouring ctx so a queued wakeup whose context
|
||||
// is cancelled (timeout / adapter Close) aborts instead of blocking on a
|
||||
// stuck in-flight turn.
|
||||
select {
|
||||
case a.promptGate <- struct{}{}:
|
||||
defer func() { <-a.promptGate }()
|
||||
case <-ctx.Done():
|
||||
return ctx.Err()
|
||||
humanPrompt := expectSession == ""
|
||||
promptCtx, turn := newPromptTurnState(
|
||||
ctx,
|
||||
promptGeneration,
|
||||
humanPrompt && a.supportsPromptHandoff() && promptGeneration != 0,
|
||||
)
|
||||
if err := a.acquirePromptTurn(ctx, turn, humanPrompt); err != nil {
|
||||
return err
|
||||
}
|
||||
defer a.finishPromptTurn(turn)
|
||||
|
||||
a.mu.Lock()
|
||||
conn := a.acpConn
|
||||
@@ -93,15 +94,18 @@ func (a *Adapter) sendPrompt(
|
||||
contentBlocks := a.buildPromptContentBlocks(finalMessage, attachments)
|
||||
|
||||
// Start prompt span — notification spans become children via getPromptTraceCtx()
|
||||
traceCtx, promptSpan := shared.TraceProtocolRequest(ctx, shared.ProtocolACP, a.agentID, "prompt")
|
||||
traceCtx, promptSpan := shared.TraceProtocolRequest(
|
||||
promptCtx,
|
||||
shared.ProtocolACP,
|
||||
a.agentID,
|
||||
"prompt",
|
||||
)
|
||||
promptSpan.SetAttributes(
|
||||
attribute.String("session_id", sessionID),
|
||||
attribute.Int("prompt_length", len(finalMessage)),
|
||||
attribute.Int("image_count", len(attachments)),
|
||||
)
|
||||
promptCtx, turn := a.registerPromptTurn(traceCtx, promptGeneration)
|
||||
defer a.clearPromptTurn(turn)
|
||||
a.setPromptTraceCtx(promptCtx)
|
||||
a.setPromptTraceCtx(turn, traceCtx)
|
||||
|
||||
// Clear the loading flag before sending the prompt.
|
||||
// If we're resuming a session, history replay is complete by the time we send a new prompt.
|
||||
@@ -124,7 +128,7 @@ func (a *Adapter) sendPrompt(
|
||||
var err error
|
||||
go func() {
|
||||
defer close(turn.rpcDone)
|
||||
resp, err = conn.Prompt(promptCtx, acp.PromptRequest{
|
||||
resp, err = conn.Prompt(traceCtx, acp.PromptRequest{
|
||||
SessionId: acp.SessionId(sessionID),
|
||||
Prompt: contentBlocks,
|
||||
})
|
||||
@@ -133,12 +137,14 @@ func (a *Adapter) sendPrompt(
|
||||
if waitErr := a.waitForPromptRPCAfterUserCancel(turn); waitErr != nil {
|
||||
promptSpan.RecordError(waitErr)
|
||||
promptSpan.End()
|
||||
a.clearPromptTraceCtx()
|
||||
a.clearPromptTraceCtx(turn)
|
||||
return waitErr
|
||||
}
|
||||
|
||||
// Clear prompt context and end span regardless of outcome
|
||||
a.clearPromptTraceCtx()
|
||||
ownsCompletion := a.claimPromptTurnCompletion(turn)
|
||||
// Clear prompt context and end span regardless of outcome. A transferred
|
||||
// predecessor cannot clear the successor's trace.
|
||||
a.clearPromptTraceCtx(turn)
|
||||
stopReason := ""
|
||||
if err != nil {
|
||||
promptSpan.RecordError(err)
|
||||
@@ -148,8 +154,15 @@ func (a *Adapter) sendPrompt(
|
||||
}
|
||||
promptSpan.End()
|
||||
|
||||
if !ownsCompletion {
|
||||
a.logger.Debug("suppressing completion from handed-off prompt RPC",
|
||||
zap.String("session_id", sessionID),
|
||||
zap.Uint64("prompt_generation", promptGeneration),
|
||||
zap.Error(err))
|
||||
return nil
|
||||
}
|
||||
if err != nil {
|
||||
return normalizePromptErrorAfterCancel(promptCtx, err)
|
||||
return normalizePromptErrorAfterCancel(traceCtx, err)
|
||||
}
|
||||
|
||||
// Drain queued ACP notifications before running the post-prompt sweeps and
|
||||
@@ -169,7 +182,7 @@ func (a *Adapter) sendPrompt(
|
||||
|
||||
// Cancel any tool calls still in-flight (e.g. a denied permission leaves the
|
||||
// tool_call without a terminal status update from the agent).
|
||||
a.cancelActiveToolCalls(sessionID)
|
||||
a.cancelPromptEndToolCalls(sessionID)
|
||||
|
||||
// Mark any tracked Monitors as ended. They live longer than a typical tool
|
||||
// call (the script keeps running across model turns), so this sweep runs
|
||||
@@ -218,6 +231,10 @@ func (a *Adapter) sendPrompt(
|
||||
return nil
|
||||
}
|
||||
|
||||
func (a *Adapter) supportsPromptHandoff() bool {
|
||||
return a.agentID == claudeAgentID || a.agentID == mockAgentID
|
||||
}
|
||||
|
||||
func normalizePromptErrorAfterCancel(promptCtx context.Context, err error) error {
|
||||
if err == nil {
|
||||
return nil
|
||||
@@ -438,7 +455,24 @@ func (a *Adapter) Cancel(ctx context.Context) error {
|
||||
// when the subagent finishes seconds later. Leaving the entry in
|
||||
// activeToolCalls lets that authoritative terminal update land naturally.
|
||||
func (a *Adapter) cancelActiveToolCalls(sessionID string) {
|
||||
a.cancelActiveToolCallsPreservingHandoff(sessionID, false)
|
||||
}
|
||||
|
||||
// cancelPromptEndToolCalls preserves background work inherited from a
|
||||
// handed-off predecessor. Explicit session cancellation still uses
|
||||
// cancelActiveToolCalls so it can terminate the whole session.
|
||||
func (a *Adapter) cancelPromptEndToolCalls(sessionID string) {
|
||||
a.cancelActiveToolCallsPreservingHandoff(sessionID, true)
|
||||
}
|
||||
|
||||
func (a *Adapter) cancelActiveToolCallsPreservingHandoff(
|
||||
sessionID string,
|
||||
preserveHandoff bool,
|
||||
) {
|
||||
a.mu.Lock()
|
||||
if !preserveHandoff {
|
||||
a.clearPromptHandoffToolTrackingLocked()
|
||||
}
|
||||
monitorToolCallIDs := make(map[string]bool)
|
||||
for _, tcID := range a.activeMonitors[sessionID] {
|
||||
monitorToolCallIDs[tcID] = true
|
||||
@@ -447,12 +481,15 @@ func (a *Adapter) cancelActiveToolCalls(sessionID string) {
|
||||
preserved := make(map[string]*streams.NormalizedPayload)
|
||||
for tcID, payload := range a.activeToolCalls {
|
||||
switch {
|
||||
case preserveHandoff && a.isPromptHandoffToolProtectedLocked(tcID):
|
||||
preserved[tcID] = payload
|
||||
case monitorToolCallIDs[tcID]:
|
||||
preserved[tcID] = payload
|
||||
case payload != nil && payload.Kind() == streams.ToolKindSubagentTask:
|
||||
preserved[tcID] = payload
|
||||
default:
|
||||
toCancel[tcID] = payload
|
||||
a.forgetPromptHandoffToolLocked(tcID)
|
||||
}
|
||||
}
|
||||
a.activeToolCalls = preserved
|
||||
@@ -471,3 +508,60 @@ func (a *Adapter) cancelActiveToolCalls(sessionID string) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func (a *Adapter) protectActiveBackgroundWorkForHandoff(sessionID string) {
|
||||
a.mu.Lock()
|
||||
defer a.mu.Unlock()
|
||||
|
||||
for toolCallID, payload := range a.activeToolCalls {
|
||||
if payload != nil && payload.IsActiveBackgroundWork() {
|
||||
a.handoffProtectedToolCalls[toolCallID] = struct{}{}
|
||||
}
|
||||
}
|
||||
for _, toolCallID := range a.activeMonitors[sessionID] {
|
||||
a.handoffProtectedToolCalls[toolCallID] = struct{}{}
|
||||
}
|
||||
|
||||
// Include nested children that were already live at the handoff boundary.
|
||||
// Children arriving later inherit protection in trackToolCallLineage.
|
||||
for changed := true; changed; {
|
||||
changed = false
|
||||
for toolCallID, parentToolCallID := range a.toolCallParents {
|
||||
if _, parentProtected := a.handoffProtectedToolCalls[parentToolCallID]; !parentProtected {
|
||||
continue
|
||||
}
|
||||
if _, alreadyProtected := a.handoffProtectedToolCalls[toolCallID]; alreadyProtected {
|
||||
continue
|
||||
}
|
||||
a.handoffProtectedToolCalls[toolCallID] = struct{}{}
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (a *Adapter) trackToolCallLineage(toolCallID, parentToolCallID string) {
|
||||
if toolCallID == "" || parentToolCallID == "" {
|
||||
return
|
||||
}
|
||||
a.mu.Lock()
|
||||
defer a.mu.Unlock()
|
||||
a.toolCallParents[toolCallID] = parentToolCallID
|
||||
if _, protected := a.handoffProtectedToolCalls[parentToolCallID]; protected {
|
||||
a.handoffProtectedToolCalls[toolCallID] = struct{}{}
|
||||
}
|
||||
}
|
||||
|
||||
func (a *Adapter) isPromptHandoffToolProtectedLocked(toolCallID string) bool {
|
||||
_, protected := a.handoffProtectedToolCalls[toolCallID]
|
||||
return protected
|
||||
}
|
||||
|
||||
func (a *Adapter) forgetPromptHandoffToolLocked(toolCallID string) {
|
||||
delete(a.toolCallParents, toolCallID)
|
||||
delete(a.handoffProtectedToolCalls, toolCallID)
|
||||
}
|
||||
|
||||
func (a *Adapter) clearPromptHandoffToolTrackingLocked() {
|
||||
clear(a.toolCallParents)
|
||||
clear(a.handoffProtectedToolCalls)
|
||||
}
|
||||
|
||||
+140
-2
@@ -11,18 +11,156 @@ import (
|
||||
func (a *Adapter) registerPromptTurn(
|
||||
parent context.Context,
|
||||
promptGeneration uint64,
|
||||
) (context.Context, *promptTurnState) {
|
||||
promptCtx, turn := newPromptTurnState(parent, promptGeneration, false)
|
||||
a.promptTurnMu.Lock()
|
||||
a.promptTurn = turn
|
||||
a.promptTurnMu.Unlock()
|
||||
return promptCtx, turn
|
||||
}
|
||||
|
||||
func newPromptTurnState(
|
||||
parent context.Context,
|
||||
promptGeneration uint64,
|
||||
allowHandoff bool,
|
||||
) (context.Context, *promptTurnState) {
|
||||
promptCtx, endTurn := context.WithCancelCause(parent)
|
||||
turn := &promptTurnState{
|
||||
endTurn: endTurn,
|
||||
rpcDone: make(chan struct{}),
|
||||
abortCh: make(chan struct{}),
|
||||
handoffCh: make(chan struct{}),
|
||||
promptGeneration: promptGeneration,
|
||||
allowHandoff: allowHandoff,
|
||||
}
|
||||
return promptCtx, turn
|
||||
}
|
||||
|
||||
// acquirePromptTurn obtains prompt ownership. Human prompts may atomically
|
||||
// inherit the existing gate token after an authoritative provider handoff.
|
||||
// Synthetic prompts never listen to handoffCh, so ScheduleWakeup remains
|
||||
// serialized until the owning RPC really returns.
|
||||
func (a *Adapter) acquirePromptTurn(
|
||||
ctx context.Context,
|
||||
turn *promptTurnState,
|
||||
human bool,
|
||||
) error {
|
||||
for {
|
||||
handoffCh, transferred := a.tryTransferPromptTurn(turn, human)
|
||||
if transferred {
|
||||
return nil
|
||||
}
|
||||
|
||||
select {
|
||||
case a.promptGate <- struct{}{}:
|
||||
a.promptTurnMu.Lock()
|
||||
turn.gateOwned = true
|
||||
a.promptTurn = turn
|
||||
a.promptTurnMu.Unlock()
|
||||
return nil
|
||||
case <-handoffCh:
|
||||
// Re-check under promptTurnMu. Another human caller may have won the
|
||||
// transfer and installed a non-idle successor.
|
||||
case <-ctx.Done():
|
||||
return ctx.Err()
|
||||
case <-a.lifetimeCtx.Done():
|
||||
return context.Cause(a.lifetimeCtx)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func (a *Adapter) tryTransferPromptTurn(
|
||||
turn *promptTurnState,
|
||||
human bool,
|
||||
) (<-chan struct{}, bool) {
|
||||
a.promptTurnMu.Lock()
|
||||
defer a.promptTurnMu.Unlock()
|
||||
current := a.promptTurn
|
||||
if !human || current == nil || !current.allowHandoff || current.finishing {
|
||||
return nil, false
|
||||
}
|
||||
if !current.handedOff || !current.gateOwned {
|
||||
return current.handoffCh, false
|
||||
}
|
||||
current.gateOwned = false
|
||||
turn.gateOwned = true
|
||||
a.promptTurn = turn
|
||||
return nil, true
|
||||
}
|
||||
|
||||
// claimPromptTurnCompletion decides whether turn still owns prompt-end
|
||||
// finalization. A predecessor whose gate token was transferred must not sweep
|
||||
// tool state, consume usage, clear the successor trace, or emit completion.
|
||||
func (a *Adapter) claimPromptTurnCompletion(turn *promptTurnState) bool {
|
||||
a.promptTurnMu.Lock()
|
||||
defer a.promptTurnMu.Unlock()
|
||||
if a.promptTurn != turn || !turn.gateOwned {
|
||||
return false
|
||||
}
|
||||
turn.finishing = true
|
||||
return true
|
||||
}
|
||||
|
||||
// finishPromptTurn releases whichever turn still owns the physical gate token.
|
||||
// Transferred predecessors are harmless no-ops.
|
||||
func (a *Adapter) finishPromptTurn(turn *promptTurnState) {
|
||||
a.promptTurnMu.Lock()
|
||||
if a.promptTurn == turn {
|
||||
a.promptTurn = nil
|
||||
}
|
||||
releaseGate := turn.gateOwned
|
||||
turn.gateOwned = false
|
||||
a.promptTurnMu.Unlock()
|
||||
if releaseGate {
|
||||
<-a.promptGate
|
||||
}
|
||||
}
|
||||
|
||||
// invalidatePromptTurnOwnership detaches the prompt captured before a session
|
||||
// replacement began. A successor may inherit ownership while session/new or
|
||||
// session/load is in flight; the identity guard must not drain that successor's
|
||||
// gate. The old RPC may still return, but it no longer owns finalization.
|
||||
func (a *Adapter) invalidatePromptTurnOwnership(turn *promptTurnState) {
|
||||
if turn == nil {
|
||||
return
|
||||
}
|
||||
a.promptTurnMu.Lock()
|
||||
a.promptTurn = turn
|
||||
if a.promptTurn != turn {
|
||||
a.promptTurnMu.Unlock()
|
||||
return
|
||||
}
|
||||
a.promptTurn = nil
|
||||
releaseGate := turn.gateOwned
|
||||
turn.gateOwned = false
|
||||
a.promptTurnMu.Unlock()
|
||||
return promptCtx, turn
|
||||
if releaseGate {
|
||||
<-a.promptGate
|
||||
}
|
||||
}
|
||||
|
||||
// markPromptHandoff records the generation-matched provider boundary and wakes
|
||||
// waiting human callers. It deliberately leaves the physical gate token in
|
||||
// place so synthetic wakeups cannot consume the handoff.
|
||||
func (a *Adapter) markPromptHandoff(sessionID string, promptGeneration uint64) bool {
|
||||
if promptGeneration == 0 {
|
||||
return false
|
||||
}
|
||||
a.promptTurnMu.Lock()
|
||||
defer a.promptTurnMu.Unlock()
|
||||
turn := a.promptTurn
|
||||
if turn == nil ||
|
||||
turn.promptGeneration != promptGeneration ||
|
||||
!turn.allowHandoff ||
|
||||
!turn.gateOwned ||
|
||||
turn.finishing {
|
||||
return false
|
||||
}
|
||||
if !turn.handedOff {
|
||||
a.protectActiveBackgroundWorkForHandoff(sessionID)
|
||||
turn.handedOff = true
|
||||
close(turn.handoffCh)
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
func (a *Adapter) currentPromptGeneration() uint64 {
|
||||
|
||||
+75
@@ -188,3 +188,78 @@ func TestNormalizePromptErrorAfterCancel_MapsTimeoutCanceledRPCToAbandonedPrompt
|
||||
t.Fatalf("expected errPromptAbandonedAfterCancel, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTransferredPromptOwnsCancelAndReleasesGate(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
a.agentID = claudeAgentID
|
||||
t.Cleanup(func() { _ = a.Close() })
|
||||
|
||||
_, first := newPromptTurnState(context.Background(), 1, true)
|
||||
if err := a.acquirePromptTurn(context.Background(), first, true); err != nil {
|
||||
t.Fatalf("acquire first prompt: %v", err)
|
||||
}
|
||||
if !a.markPromptHandoff("session-1", 1) {
|
||||
t.Fatal("first prompt did not accept authoritative handoff")
|
||||
}
|
||||
|
||||
_, second := newPromptTurnState(context.Background(), 2, true)
|
||||
if err := a.acquirePromptTurn(context.Background(), second, true); err != nil {
|
||||
t.Fatalf("transfer prompt gate: %v", err)
|
||||
}
|
||||
a.finishPromptTurn(first)
|
||||
|
||||
cancelled := a.signalPromptTurnAbort()
|
||||
if cancelled != second {
|
||||
t.Fatalf("cancel targeted %p, want transferred successor %p", cancelled, second)
|
||||
}
|
||||
select {
|
||||
case <-second.abortCh:
|
||||
default:
|
||||
t.Fatal("transferred successor did not receive cancel signal")
|
||||
}
|
||||
select {
|
||||
case <-first.abortCh:
|
||||
t.Fatal("transferred predecessor received successor cancel signal")
|
||||
default:
|
||||
}
|
||||
|
||||
// The same cleanup runs on prompt failure, cancellation, or normal return.
|
||||
// Once the successor finishes, a synthetic prompt must be able to acquire
|
||||
// the physical token rather than leaking behind the transferred owner.
|
||||
a.finishPromptTurn(second)
|
||||
_, synthetic := newPromptTurnState(context.Background(), 0, false)
|
||||
acquireCtx, cancel := context.WithTimeout(context.Background(), time.Second)
|
||||
defer cancel()
|
||||
if err := a.acquirePromptTurn(acquireCtx, synthetic, false); err != nil {
|
||||
t.Fatalf("synthetic prompt remained blocked after successor cleanup: %v", err)
|
||||
}
|
||||
a.finishPromptTurn(synthetic)
|
||||
}
|
||||
|
||||
func TestQueuedPromptOwnershipWaiterUnblocksOnAdapterClose(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
|
||||
_, owner := newPromptTurnState(context.Background(), 1, false)
|
||||
if err := a.acquirePromptTurn(context.Background(), owner, true); err != nil {
|
||||
t.Fatalf("acquire owner: %v", err)
|
||||
}
|
||||
|
||||
waiterDone := make(chan error, 1)
|
||||
go func() {
|
||||
_, waiter := newPromptTurnState(context.Background(), 2, false)
|
||||
waiterDone <- a.acquirePromptTurn(context.Background(), waiter, true)
|
||||
}()
|
||||
|
||||
if err := a.Close(); err != nil {
|
||||
t.Fatalf("close adapter: %v", err)
|
||||
}
|
||||
select {
|
||||
case err := <-waiterDone:
|
||||
if !errors.Is(err, context.Canceled) {
|
||||
t.Fatalf("queued waiter error = %v, want context canceled", err)
|
||||
}
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("adapter close leaked queued prompt ownership waiter")
|
||||
}
|
||||
a.finishPromptTurn(owner)
|
||||
}
|
||||
|
||||
@@ -56,6 +56,64 @@ func TestCancelActiveToolCalls_PreservesSubagentTask(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestPromptEndSweepsPreserveHandoffBackgroundLineage(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
parent := streams.NewSubagentTask("Investigate", "keep working", "general-purpose")
|
||||
parent.SetBackgroundWorkIdentity(
|
||||
streams.BackgroundWorkKindSubagent,
|
||||
"subagent-1",
|
||||
false,
|
||||
false,
|
||||
)
|
||||
a.activeToolCalls["subagent-1"] = parent
|
||||
a.activeToolCalls["child-bash-1"] = streams.NewShellExec("sleep 30", "", "child work", 0, false)
|
||||
a.activeToolCalls["monitor-1"] = streams.NewGeneric("Monitor", map[string]any{})
|
||||
a.activeMonitors["session-1"] = map[string]string{"task-1": "monitor-1"}
|
||||
a.trackToolCallLineage("child-bash-1", "subagent-1")
|
||||
|
||||
a.protectActiveBackgroundWorkForHandoff("session-1")
|
||||
|
||||
// This tool belongs to the human successor and should still receive normal
|
||||
// prompt-end cleanup.
|
||||
a.activeToolCalls["successor-bash-1"] = streams.NewShellExec("false", "", "successor work", 0, false)
|
||||
a.cancelPromptEndToolCalls("session-1")
|
||||
a.sweepMonitorsOnPromptEnd("session-1")
|
||||
|
||||
a.mu.RLock()
|
||||
_, parentTracked := a.activeToolCalls["subagent-1"]
|
||||
_, childTracked := a.activeToolCalls["child-bash-1"]
|
||||
_, monitorTracked := a.activeToolCalls["monitor-1"]
|
||||
_, successorTracked := a.activeToolCalls["successor-bash-1"]
|
||||
monitorToolCallID, monitorActive := a.activeMonitors["session-1"]["task-1"]
|
||||
a.mu.RUnlock()
|
||||
if !parentTracked || !childTracked || !monitorTracked {
|
||||
t.Fatalf(
|
||||
"handoff background tracking = parent:%t child:%t monitor:%t, want all true",
|
||||
parentTracked,
|
||||
childTracked,
|
||||
monitorTracked,
|
||||
)
|
||||
}
|
||||
if successorTracked {
|
||||
t.Fatal("successor-owned tool escaped prompt-end cleanup")
|
||||
}
|
||||
if !monitorActive || monitorToolCallID != "monitor-1" {
|
||||
t.Fatalf("predecessor monitor tracking = (%q, %t), want (monitor-1, true)", monitorToolCallID, monitorActive)
|
||||
}
|
||||
|
||||
events := drainEvents(a)
|
||||
if len(events) != 1 ||
|
||||
events[0].ToolCallID != "successor-bash-1" ||
|
||||
events[0].ToolStatus != toolStatusCancelled {
|
||||
t.Fatalf("prompt-end events = %+v, want one cancelled successor tool", events)
|
||||
}
|
||||
|
||||
a.cancelActiveToolCalls("session-1")
|
||||
if len(a.toolCallParents) != 0 || len(a.handoffProtectedToolCalls) != 0 {
|
||||
t.Fatal("explicit session cancellation retained prompt handoff tool ownership")
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildPromptContentBlocks_PathModeAttachmentSavesWritableFile(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
t.Cleanup(func() { _ = a.Close() })
|
||||
|
||||
@@ -24,6 +24,7 @@ func (a *Adapter) NewSession(ctx context.Context, mcpServers []types.McpServer)
|
||||
if conn == nil {
|
||||
return "", fmt.Errorf("adapter not initialized")
|
||||
}
|
||||
priorPromptTurn := a.currentPromptTurn()
|
||||
|
||||
// A fresh session invalidates any pending wakeup keyed to the prior
|
||||
// session. Reset pendingWakeups and cancel the scheduler under one
|
||||
@@ -32,6 +33,7 @@ func (a *Adapter) NewSession(ctx context.Context, mcpServers []types.McpServer)
|
||||
a.mu.Lock()
|
||||
a.pendingWakeups = make(map[string]*pendingWakeup)
|
||||
a.clearCodexSubagentCorrelationsLocked("")
|
||||
a.clearPromptHandoffToolTrackingLocked()
|
||||
clear(a.usageBySession)
|
||||
a.wakeup.cancel()
|
||||
a.mu.Unlock()
|
||||
@@ -88,6 +90,7 @@ func (a *Adapter) NewSession(ctx context.Context, mcpServers []types.McpServer)
|
||||
a.availableModels = initialModels.AvailableModels
|
||||
}
|
||||
a.mu.Unlock()
|
||||
a.invalidatePromptTurnOwnership(priorPromptTurn)
|
||||
a.attachMgr.SetSessionID(sessionID)
|
||||
|
||||
span.SetAttributes(attribute.String("session_id", sessionID))
|
||||
@@ -296,6 +299,7 @@ func (a *Adapter) LoadSession(ctx context.Context, sessionID string, mcpServers
|
||||
zap.String("session_id", sessionID))
|
||||
return fmt.Errorf("agent does not support session loading (LoadSession capability is false)")
|
||||
}
|
||||
priorPromptTurn := a.currentPromptTurn()
|
||||
|
||||
// Loading a different session invalidates any pending wakeup or async turn
|
||||
// finalizer keyed to the prior session — same reset block as NewSession to
|
||||
@@ -304,6 +308,7 @@ func (a *Adapter) LoadSession(ctx context.Context, sessionID string, mcpServers
|
||||
a.mu.Lock()
|
||||
a.pendingWakeups = make(map[string]*pendingWakeup)
|
||||
a.clearCodexSubagentCorrelationsLocked("")
|
||||
a.clearPromptHandoffToolTrackingLocked()
|
||||
a.wakeup.cancel()
|
||||
a.mu.Unlock()
|
||||
a.cancelAllAsyncTurnCompletes()
|
||||
@@ -374,6 +379,7 @@ func (a *Adapter) LoadSession(ctx context.Context, sessionID string, mcpServers
|
||||
a.availableModels = initialModels.AvailableModels
|
||||
}
|
||||
a.mu.Unlock()
|
||||
a.invalidatePromptTurnOwnership(priorPromptTurn)
|
||||
a.attachMgr.SetSessionID(sessionID)
|
||||
|
||||
span.SetAttributes(attribute.String("session_id", sessionID))
|
||||
|
||||
@@ -4,6 +4,7 @@ import (
|
||||
"context"
|
||||
"io"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
acpsdk "github.com/coder/acp-go-sdk"
|
||||
acpclient "github.com/kandev/kandev/internal/agentctl/server/acp"
|
||||
@@ -13,6 +14,8 @@ import (
|
||||
type sessionRequestCaptureAgent struct {
|
||||
newRequest acpsdk.NewSessionRequest
|
||||
loadRequest acpsdk.LoadSessionRequest
|
||||
newStarted chan struct{}
|
||||
releaseNew chan struct{}
|
||||
}
|
||||
|
||||
var (
|
||||
@@ -50,6 +53,10 @@ func (*sessionRequestCaptureAgent) ListSessions(context.Context, acpsdk.ListSess
|
||||
|
||||
func (a *sessionRequestCaptureAgent) NewSession(_ context.Context, request acpsdk.NewSessionRequest) (acpsdk.NewSessionResponse, error) {
|
||||
a.newRequest = request
|
||||
if a.newStarted != nil {
|
||||
close(a.newStarted)
|
||||
<-a.releaseNew
|
||||
}
|
||||
return acpsdk.NewSessionResponse{SessionId: "session-1"}, nil
|
||||
}
|
||||
|
||||
@@ -105,6 +112,100 @@ func TestMCPSessionNewAndLoadUseHTTPWithSSEFallback(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestResetSessionInvalidatesPromptOwnership(t *testing.T) {
|
||||
adapter, _ := newSessionRequestCaptureAdapter(t, acpsdk.McpCapabilities{})
|
||||
|
||||
_, owner := newPromptTurnState(context.Background(), 1, false)
|
||||
if err := adapter.acquirePromptTurn(context.Background(), owner, true); err != nil {
|
||||
t.Fatalf("acquire original prompt: %v", err)
|
||||
}
|
||||
adapter.toolCallParents["child-1"] = "parent-1"
|
||||
adapter.handoffProtectedToolCalls["parent-1"] = struct{}{}
|
||||
|
||||
type acquireResult struct {
|
||||
turn *promptTurnState
|
||||
err error
|
||||
}
|
||||
waiterDone := make(chan acquireResult, 1)
|
||||
go func() {
|
||||
_, waiter := newPromptTurnState(context.Background(), 0, false)
|
||||
err := adapter.acquirePromptTurn(context.Background(), waiter, false)
|
||||
waiterDone <- acquireResult{turn: waiter, err: err}
|
||||
}()
|
||||
|
||||
if _, err := adapter.ResetSession(context.Background(), nil); err != nil {
|
||||
t.Fatalf("ResetSession: %v", err)
|
||||
}
|
||||
|
||||
var successor *promptTurnState
|
||||
select {
|
||||
case result := <-waiterDone:
|
||||
if result.err != nil {
|
||||
t.Fatalf("queued prompt did not acquire after reset: %v", result.err)
|
||||
}
|
||||
successor = result.turn
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("reset leaked the queued prompt ownership waiter")
|
||||
}
|
||||
defer adapter.finishPromptTurn(successor)
|
||||
|
||||
if adapter.claimPromptTurnCompletion(owner) {
|
||||
t.Fatal("stale pre-reset prompt retained response ownership")
|
||||
}
|
||||
adapter.finishPromptTurn(owner)
|
||||
if len(adapter.toolCallParents) != 0 || len(adapter.handoffProtectedToolCalls) != 0 {
|
||||
t.Fatal("reset retained stale prompt handoff tool ownership")
|
||||
}
|
||||
}
|
||||
|
||||
func TestResetSessionCannotInvalidateInheritedPromptOwnership(t *testing.T) {
|
||||
adapter, capture := newSessionRequestCaptureAdapter(t, acpsdk.McpCapabilities{})
|
||||
capture.newStarted = make(chan struct{})
|
||||
capture.releaseNew = make(chan struct{})
|
||||
|
||||
_, owner := newPromptTurnState(context.Background(), 1, true)
|
||||
if err := adapter.acquirePromptTurn(context.Background(), owner, true); err != nil {
|
||||
t.Fatalf("acquire original prompt: %v", err)
|
||||
}
|
||||
if !adapter.markPromptHandoff("session-1", 1) {
|
||||
t.Fatal("original prompt did not accept handoff")
|
||||
}
|
||||
|
||||
resetDone := make(chan error, 1)
|
||||
go func() {
|
||||
_, err := adapter.ResetSession(context.Background(), nil)
|
||||
resetDone <- err
|
||||
}()
|
||||
select {
|
||||
case <-capture.newStarted:
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("reset did not reach provider session creation")
|
||||
}
|
||||
|
||||
_, successor := newPromptTurnState(context.Background(), 2, true)
|
||||
if err := adapter.acquirePromptTurn(context.Background(), successor, true); err != nil {
|
||||
t.Fatalf("successor did not inherit prompt ownership: %v", err)
|
||||
}
|
||||
close(capture.releaseNew)
|
||||
select {
|
||||
case err := <-resetDone:
|
||||
if err != nil {
|
||||
t.Fatalf("ResetSession: %v", err)
|
||||
}
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("reset did not complete")
|
||||
}
|
||||
|
||||
if !adapter.claimPromptTurnCompletion(successor) {
|
||||
t.Fatal("reset invalidated the inherited successor prompt")
|
||||
}
|
||||
if adapter.claimPromptTurnCompletion(owner) {
|
||||
t.Fatal("transferred predecessor reclaimed completion ownership")
|
||||
}
|
||||
adapter.finishPromptTurn(owner)
|
||||
adapter.finishPromptTurn(successor)
|
||||
}
|
||||
|
||||
func newSessionRequestCaptureAdapter(t *testing.T, capabilities acpsdk.McpCapabilities) (*Adapter, *sessionRequestCaptureAgent) {
|
||||
t.Helper()
|
||||
clientToAgentReader, clientToAgentWriter := io.Pipe()
|
||||
|
||||
@@ -161,6 +161,7 @@ func (a *Adapter) convertToolCallUpdate(sessionID string, tc *acp.SessionUpdateT
|
||||
if codexParentToolCallID != "" {
|
||||
parentToolCallID = codexParentToolCallID
|
||||
}
|
||||
a.trackToolCallLineage(emittedToolCallID, parentToolCallID)
|
||||
|
||||
return &AgentEvent{
|
||||
Type: eventType,
|
||||
@@ -790,6 +791,7 @@ func (a *Adapter) convertToolCallResultUpdate(sessionID string, tcu *acp.Session
|
||||
|
||||
if isTerminal {
|
||||
delete(a.activeToolCalls, emittedToolCallID)
|
||||
a.forgetPromptHandoffToolLocked(emittedToolCallID)
|
||||
// Also drop tracked Monitor: this terminal update is the
|
||||
// agent-emitted close, so the prompt-end sweep must not re-emit a
|
||||
// "Monitor exited" event for this same toolCallID.
|
||||
|
||||
@@ -173,7 +173,22 @@ func (a *Adapter) handleACPUpdate(
|
||||
if !suppressed {
|
||||
event = a.convertNotification(n)
|
||||
if n.Update.UsageUpdate != nil {
|
||||
if lifecycleEvent := usageLifecycleEvent(sessionID, n.Update.UsageUpdate.Meta, promptGeneration); lifecycleEvent != nil {
|
||||
lifecycleEvent := usageLifecycleEvent(
|
||||
sessionID,
|
||||
n.Update.UsageUpdate.Meta,
|
||||
promptGeneration,
|
||||
)
|
||||
// Generation zero identifies synthetic/legacy turns such as
|
||||
// ScheduleWakeup. They cannot transfer human prompt ownership, but
|
||||
// their foreground-idle activity event must still close the wakeup
|
||||
// cycle as it did before handoff support.
|
||||
if lifecycleEvent != nil &&
|
||||
lifecycleEvent.Type == streams.EventTypeForegroundIdle &&
|
||||
promptGeneration != 0 &&
|
||||
!a.supportsPromptHandoff() {
|
||||
lifecycleEvent = nil
|
||||
}
|
||||
if lifecycleEvent != nil {
|
||||
leadingEvent, event = event, lifecycleEvent
|
||||
}
|
||||
}
|
||||
@@ -186,6 +201,13 @@ func (a *Adapter) handleACPUpdate(
|
||||
a.maybeScheduleAsyncTurnComplete(*leadingEvent)
|
||||
}
|
||||
if event != nil {
|
||||
if event.Type == streams.EventTypeForegroundIdle &&
|
||||
a.markPromptHandoff(sessionID, event.PromptGeneration) {
|
||||
if event.Data == nil {
|
||||
event.Data = make(map[string]any)
|
||||
}
|
||||
event.Data[streams.AgentEventDataPromptHandoff] = true
|
||||
}
|
||||
shared.LogNormalizedEvent(shared.ProtocolACP, a.agentID, sessionID, event)
|
||||
shared.TraceProtocolEvent(a.getPromptTraceCtx(), shared.ProtocolACP, a.agentID,
|
||||
event.Type, rawData, event)
|
||||
|
||||
+45
-1
@@ -11,6 +11,7 @@ import (
|
||||
|
||||
func TestEnqueueACPUpdateSnapshotsPromptGenerationBeforeWorkerConversion(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
a.agentID = claudeAgentID
|
||||
t.Cleanup(func() { _ = a.Close() })
|
||||
|
||||
var notification sdk.SessionNotification
|
||||
@@ -59,6 +60,7 @@ func TestEnqueueACPUpdateSnapshotsPromptGenerationBeforeWorkerConversion(t *test
|
||||
// shared.LogNormalizedEvent.
|
||||
func TestHandleACPUpdate_HumanOriginDeliversLeadingContextWindowThenForegroundIdle(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
a.agentID = claudeAgentID
|
||||
t.Cleanup(func() { _ = a.Close() })
|
||||
|
||||
var notification sdk.SessionNotification
|
||||
@@ -67,7 +69,7 @@ func TestHandleACPUpdate_HumanOriginDeliversLeadingContextWindowThenForegroundId
|
||||
t.Fatalf("decode notification: %v", err)
|
||||
}
|
||||
|
||||
a.handleACPUpdate(notification, 0)
|
||||
a.handleACPUpdate(notification, 42)
|
||||
|
||||
events := drainEvents(a)
|
||||
if len(events) != 2 {
|
||||
@@ -81,3 +83,45 @@ func TestHandleACPUpdate_HumanOriginDeliversLeadingContextWindowThenForegroundId
|
||||
t.Fatalf("second delivered event type = %q, want %q", events[1].Type, streams.EventTypeForegroundIdle)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandleACPUpdate_HumanOriginOnlySuppressesUnsupportedHumanTurnHandoff(t *testing.T) {
|
||||
var notification sdk.SessionNotification
|
||||
raw := []byte(`{"sessionId":"s1","update":{"sessionUpdate":"usage_update","size":1000000,"used":23638,"_meta":{"_claude/origin":{"kind":"human"}}}}`)
|
||||
if err := json.Unmarshal(raw, ¬ification); err != nil {
|
||||
t.Fatalf("decode notification: %v", err)
|
||||
}
|
||||
|
||||
for _, test := range []struct {
|
||||
name string
|
||||
agentID string
|
||||
promptGeneration uint64
|
||||
wantIdle bool
|
||||
}{
|
||||
{name: "unsupported adapter", agentID: "other-acp", promptGeneration: 42},
|
||||
{name: "synthetic generation zero", agentID: claudeAgentID, promptGeneration: 0, wantIdle: true},
|
||||
} {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
a := newTestAdapter()
|
||||
a.agentID = test.agentID
|
||||
t.Cleanup(func() { _ = a.Close() })
|
||||
|
||||
a.handleACPUpdate(notification, test.promptGeneration)
|
||||
|
||||
events := drainEvents(a)
|
||||
if !test.wantIdle && (len(events) != 1 || events[0].Type != streams.EventTypeContextWindow) {
|
||||
t.Fatalf("events = %+v, want context-window only", events)
|
||||
}
|
||||
if test.wantIdle {
|
||||
if len(events) != 2 || events[1].Type != streams.EventTypeForegroundIdle {
|
||||
t.Fatalf("events = %+v, want context-window then foreground-idle", events)
|
||||
}
|
||||
if events[1].PromptGeneration != 0 {
|
||||
t.Fatalf("synthetic idle generation = %d, want 0", events[1].PromptGeneration)
|
||||
}
|
||||
if handoff, _ := events[1].Data[streams.AgentEventDataPromptHandoff].(bool); handoff {
|
||||
t.Fatal("generation-zero idle incorrectly attested human prompt handoff")
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -206,6 +206,28 @@ func (a *Adapter) takeActiveMonitors(sessionID string) map[string]string {
|
||||
return monitors
|
||||
}
|
||||
|
||||
// takePromptEndMonitors removes only monitors owned by the completing prompt.
|
||||
// Monitors protected at a human handoff continue routing predecessor output
|
||||
// until their authoritative terminal update or an explicit session reset.
|
||||
func (a *Adapter) takePromptEndMonitors(sessionID string) map[string]string {
|
||||
a.mu.Lock()
|
||||
defer a.mu.Unlock()
|
||||
|
||||
active := a.activeMonitors[sessionID]
|
||||
taken := make(map[string]string)
|
||||
for taskID, toolCallID := range active {
|
||||
if a.isPromptHandoffToolProtectedLocked(toolCallID) {
|
||||
continue
|
||||
}
|
||||
taken[taskID] = toolCallID
|
||||
delete(active, taskID)
|
||||
}
|
||||
if len(active) == 0 {
|
||||
delete(a.activeMonitors, sessionID)
|
||||
}
|
||||
return taken
|
||||
}
|
||||
|
||||
// extractMonitorEvents parses every `<task-notification>` envelope out of an
|
||||
// agent_message_chunk text, replaces each match with empty string, and
|
||||
// returns the cleaned text plus the parsed events in order.
|
||||
@@ -529,11 +551,12 @@ func (a *Adapter) dropMonitorByToolCallID(sessionID, toolCallID string) {
|
||||
// parent prompt naturally completes — the Monitor process exits with the
|
||||
// agent turn, so the card should flip from "watching" to "ended".
|
||||
func (a *Adapter) sweepMonitorsOnPromptEnd(sessionID string) {
|
||||
for _, toolCallID := range a.takeActiveMonitors(sessionID) {
|
||||
for _, toolCallID := range a.takePromptEndMonitors(sessionID) {
|
||||
a.mu.Lock()
|
||||
payload := a.activeToolCalls[toolCallID]
|
||||
markMonitorEnded(payload, "exited")
|
||||
delete(a.activeToolCalls, toolCallID)
|
||||
a.forgetPromptHandoffToolLocked(toolCallID)
|
||||
a.mu.Unlock()
|
||||
a.sendUpdate(monitorTerminalEvent(sessionID, toolCallID, toolStatusComplete, "Monitor exited", payload))
|
||||
}
|
||||
@@ -550,6 +573,7 @@ func (a *Adapter) sweepMonitorsOnReplayEnd(sessionID string) {
|
||||
payload := a.activeToolCalls[toolCallID]
|
||||
markMonitorEnded(payload, "session_restart")
|
||||
delete(a.activeToolCalls, toolCallID)
|
||||
a.forgetPromptHandoffToolLocked(toolCallID)
|
||||
a.mu.Unlock()
|
||||
a.sendUpdate(monitorTerminalEvent(sessionID, toolCallID, toolStatusCancelled, "Monitor ended (session restart)", payload))
|
||||
}
|
||||
|
||||
@@ -0,0 +1,399 @@
|
||||
package acp
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"io"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
sdk "github.com/coder/acp-go-sdk"
|
||||
"github.com/kandev/kandev/internal/agentctl/types/streams"
|
||||
)
|
||||
|
||||
// handoffFakeAgent models claude-agent-acp's detached-child behavior: the
|
||||
// original session/prompt remains open after the human model cycle goes idle,
|
||||
// then returns only after the bridge receives the next human prompt echo.
|
||||
// The replacement prompt remains open while child updates continue streaming.
|
||||
type handoffFakeAgent struct {
|
||||
concurrencyFakeAgent
|
||||
entered chan handoffPromptCall
|
||||
replacementIn chan struct{}
|
||||
release chan struct{}
|
||||
releaseOnce sync.Once
|
||||
promptCount atomic.Int32
|
||||
}
|
||||
|
||||
type handoffPromptCall struct {
|
||||
number int
|
||||
text string
|
||||
}
|
||||
|
||||
func (f *handoffFakeAgent) releasePrompts() {
|
||||
f.releaseOnce.Do(func() { close(f.release) })
|
||||
}
|
||||
|
||||
func (f *handoffFakeAgent) Prompt(_ context.Context, request sdk.PromptRequest) (sdk.PromptResponse, error) {
|
||||
call := int(f.promptCount.Add(1))
|
||||
var text string
|
||||
if len(request.Prompt) > 0 && request.Prompt[0].Text != nil {
|
||||
text = request.Prompt[0].Text.Text
|
||||
}
|
||||
f.entered <- handoffPromptCall{number: call, text: text}
|
||||
switch call {
|
||||
case 1:
|
||||
select {
|
||||
case <-f.replacementIn:
|
||||
case <-f.release:
|
||||
}
|
||||
case 2:
|
||||
close(f.replacementIn)
|
||||
<-f.release
|
||||
}
|
||||
return sdk.PromptResponse{StopReason: sdk.StopReasonEndTurn}, nil
|
||||
}
|
||||
|
||||
func (*handoffFakeAgent) NewSession(
|
||||
context.Context,
|
||||
sdk.NewSessionRequest,
|
||||
) (sdk.NewSessionResponse, error) {
|
||||
return sdk.NewSessionResponse{SessionId: "session-handoff"}, nil
|
||||
}
|
||||
|
||||
func setupHandoffFakeAgent(
|
||||
t *testing.T,
|
||||
) (*Adapter, *handoffFakeAgent, *sdk.AgentSideConnection) {
|
||||
t.Helper()
|
||||
|
||||
a := newTestAdapter()
|
||||
a.agentID = claudeAgentID
|
||||
a.normalizer = NewNormalizer(claudeAgentID)
|
||||
clientToAgentR, clientToAgentW := io.Pipe()
|
||||
agentToClientR, agentToClientW := io.Pipe()
|
||||
|
||||
fake := &handoffFakeAgent{
|
||||
entered: make(chan handoffPromptCall, 4),
|
||||
replacementIn: make(chan struct{}),
|
||||
release: make(chan struct{}),
|
||||
}
|
||||
if err := a.Connect(clientToAgentW, agentToClientR); err != nil {
|
||||
t.Fatalf("connect adapter: %v", err)
|
||||
}
|
||||
conn := sdk.NewAgentSideConnection(fake, agentToClientW, clientToAgentR)
|
||||
t.Cleanup(func() {
|
||||
fake.releasePrompts()
|
||||
_ = a.Close()
|
||||
_ = clientToAgentW.Close()
|
||||
_ = agentToClientW.Close()
|
||||
})
|
||||
return a, fake, conn
|
||||
}
|
||||
|
||||
func sendCapturedUpdate(
|
||||
t *testing.T,
|
||||
conn *sdk.AgentSideConnection,
|
||||
raw string,
|
||||
) {
|
||||
t.Helper()
|
||||
var notification sdk.SessionNotification
|
||||
if err := json.Unmarshal([]byte(raw), ¬ification); err != nil {
|
||||
t.Fatalf("decode captured notification: %v", err)
|
||||
}
|
||||
if err := conn.SessionUpdate(context.Background(), notification); err != nil {
|
||||
t.Fatalf("send captured notification: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func waitForEventType(t *testing.T, a *Adapter, eventType string) AgentEvent {
|
||||
t.Helper()
|
||||
for {
|
||||
select {
|
||||
case event := <-a.updatesCh:
|
||||
if event.Type == eventType {
|
||||
return event
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatalf("timed out waiting for %q event", eventType)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestHumanPromptHandoffAfterAuthoritativeForegroundIdle(t *testing.T) {
|
||||
a, fake, conn := setupHandoffFakeAgent(t)
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
t.Cleanup(cancel)
|
||||
|
||||
if err := a.Initialize(ctx); err != nil {
|
||||
t.Fatalf("initialize: %v", err)
|
||||
}
|
||||
if _, err := a.NewSession(ctx, nil); err != nil {
|
||||
t.Fatalf("new session: %v", err)
|
||||
}
|
||||
|
||||
firstDone := make(chan error, 1)
|
||||
go func() {
|
||||
firstDone <- a.Prompt(ctx, "launch a detached child", nil, 41)
|
||||
}()
|
||||
if call := <-fake.entered; call.number != 1 {
|
||||
t.Fatalf("first provider prompt call = %d, want 1", call.number)
|
||||
}
|
||||
|
||||
sendCapturedUpdate(t, conn, `{
|
||||
"sessionId":"session-handoff",
|
||||
"update":{
|
||||
"sessionUpdate":"tool_call",
|
||||
"toolCallId":"subagent-1",
|
||||
"title":"Agent",
|
||||
"kind":"other",
|
||||
"status":"in_progress",
|
||||
"rawInput":{"description":"detached child","prompt":"keep working","subagent_type":"general-purpose"},
|
||||
"_meta":{"claudeCode":{"toolName":"Agent"}}
|
||||
}
|
||||
}`)
|
||||
subagent := waitForEventType(t, a, streams.EventTypeToolCall)
|
||||
if subagent.NormalizedPayload == nil || !subagent.NormalizedPayload.IsActiveBackgroundWork() {
|
||||
t.Fatal("captured detached child launch was not adapter-attested as live background work")
|
||||
}
|
||||
|
||||
sendCapturedUpdate(t, conn, `{
|
||||
"sessionId":"session-handoff",
|
||||
"update":{
|
||||
"sessionUpdate":"usage_update",
|
||||
"size":1000000,
|
||||
"used":23638,
|
||||
"_meta":{"_claude/origin":{"kind":"human"}}
|
||||
}
|
||||
}`)
|
||||
idle := waitForEventType(t, a, streams.EventTypeForegroundIdle)
|
||||
if idle.PromptGeneration != 41 {
|
||||
t.Fatalf("foreground-idle generation = %d, want 41", idle.PromptGeneration)
|
||||
}
|
||||
if handoff, _ := idle.Data[streams.AgentEventDataPromptHandoff].(bool); !handoff {
|
||||
t.Fatal("authoritative Claude foreground-idle did not attest prompt handoff")
|
||||
}
|
||||
|
||||
secondDone := make(chan error, 1)
|
||||
go func() {
|
||||
secondDone <- a.Prompt(ctx, "human follow-up", nil, 42)
|
||||
}()
|
||||
select {
|
||||
case call := <-fake.entered:
|
||||
if call.number != 2 || call.text != "human follow-up" {
|
||||
t.Fatalf("replacement provider prompt = %+v, want call 2 with human follow-up", call)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("human follow-up did not reach provider after authoritative foreground idle")
|
||||
}
|
||||
|
||||
select {
|
||||
case err := <-firstDone:
|
||||
if err != nil {
|
||||
t.Fatalf("original prompt returned error after handoff: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("original provider RPC did not release after receiving the follow-up echo")
|
||||
}
|
||||
|
||||
sendCapturedUpdate(t, conn, `{
|
||||
"sessionId":"session-handoff",
|
||||
"update":{
|
||||
"sessionUpdate":"tool_call",
|
||||
"toolCallId":"child-bash-1",
|
||||
"title":"Bash",
|
||||
"kind":"execute",
|
||||
"status":"in_progress",
|
||||
"rawInput":{"command":"printf child-stream"},
|
||||
"_meta":{"claudeCode":{"toolName":"Bash","parentToolUseId":"subagent-1"}}
|
||||
}
|
||||
}`)
|
||||
child := waitForEventType(t, a, streams.EventTypeToolCall)
|
||||
if child.ParentToolCallID != "subagent-1" {
|
||||
t.Fatalf("streaming child parent = %q, want subagent-1", child.ParentToolCallID)
|
||||
}
|
||||
|
||||
fake.releasePrompts()
|
||||
select {
|
||||
case err := <-secondDone:
|
||||
if err != nil {
|
||||
t.Fatalf("follow-up prompt returned error: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("follow-up prompt did not complete")
|
||||
}
|
||||
|
||||
complete := waitForEventType(t, a, streams.EventTypeComplete)
|
||||
if complete.PromptGeneration != 42 {
|
||||
t.Fatalf("delivered completion generation = %d, want follow-up generation 42", complete.PromptGeneration)
|
||||
}
|
||||
a.mu.RLock()
|
||||
_, parentStillTracked := a.activeToolCalls["subagent-1"]
|
||||
_, childStillTracked := a.activeToolCalls["child-bash-1"]
|
||||
a.mu.RUnlock()
|
||||
if !parentStillTracked || !childStillTracked {
|
||||
t.Fatalf(
|
||||
"follow-up completion dropped predecessor background lineage: parent=%t child=%t",
|
||||
parentStillTracked,
|
||||
childStillTracked,
|
||||
)
|
||||
}
|
||||
for _, event := range drainEvents(a) {
|
||||
if event.Type == streams.EventTypeComplete && event.PromptGeneration == 41 {
|
||||
t.Fatal("held original RPC emitted a stale completion after handoff")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestSyntheticWakeupCannotConsumeHumanPromptHandoff(t *testing.T) {
|
||||
a, fake, conn := setupHandoffFakeAgent(t)
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
t.Cleanup(cancel)
|
||||
|
||||
if err := a.Initialize(ctx); err != nil {
|
||||
t.Fatalf("initialize: %v", err)
|
||||
}
|
||||
sessionID, err := a.NewSession(ctx, nil)
|
||||
if err != nil {
|
||||
t.Fatalf("new session: %v", err)
|
||||
}
|
||||
|
||||
firstDone := make(chan error, 1)
|
||||
go func() {
|
||||
firstDone <- a.Prompt(ctx, "first human prompt", nil, 1)
|
||||
}()
|
||||
if call := <-fake.entered; call.number != 1 {
|
||||
t.Fatalf("first provider prompt call = %d, want 1", call.number)
|
||||
}
|
||||
|
||||
sendCapturedUpdate(t, conn, `{
|
||||
"sessionId":"session-handoff",
|
||||
"update":{
|
||||
"sessionUpdate":"usage_update",
|
||||
"size":1000000,
|
||||
"used":23638,
|
||||
"_meta":{"_claude/origin":{"kind":"human"}}
|
||||
}
|
||||
}`)
|
||||
_ = waitForEventType(t, a, streams.EventTypeForegroundIdle)
|
||||
|
||||
a.fireWakeup(sessionID, "synthetic wakeup")
|
||||
select {
|
||||
case call := <-fake.entered:
|
||||
t.Fatalf("synthetic prompt consumed human handoff: %+v", call)
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
}
|
||||
|
||||
secondDone := make(chan error, 1)
|
||||
go func() {
|
||||
secondDone <- a.Prompt(ctx, "second human prompt", nil, 2)
|
||||
}()
|
||||
select {
|
||||
case call := <-fake.entered:
|
||||
if call.number != 2 || call.text != "second human prompt" {
|
||||
t.Fatalf("handoff successor = %+v, want second human prompt", call)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("human successor did not inherit prompt handoff")
|
||||
}
|
||||
select {
|
||||
case err := <-firstDone:
|
||||
if err != nil {
|
||||
t.Fatalf("first prompt: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("first prompt did not return after human successor entered")
|
||||
}
|
||||
|
||||
fake.releasePrompts()
|
||||
select {
|
||||
case err := <-secondDone:
|
||||
if err != nil {
|
||||
t.Fatalf("second prompt: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("second prompt did not return")
|
||||
}
|
||||
select {
|
||||
case call := <-fake.entered:
|
||||
if call.number != 3 || call.text != "synthetic wakeup" {
|
||||
t.Fatalf("serialized wakeup = %+v, want call 3", call)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("synthetic wakeup did not run after the human successor completed")
|
||||
}
|
||||
}
|
||||
|
||||
func TestHumanFollowupWaitsWithoutForegroundHandoff(t *testing.T) {
|
||||
a, fake, _ := setupHandoffFakeAgent(t)
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
t.Cleanup(cancel)
|
||||
|
||||
if err := a.Initialize(ctx); err != nil {
|
||||
t.Fatalf("initialize: %v", err)
|
||||
}
|
||||
if _, err := a.NewSession(ctx, nil); err != nil {
|
||||
t.Fatalf("new session: %v", err)
|
||||
}
|
||||
|
||||
firstDone := make(chan error, 1)
|
||||
go func() {
|
||||
firstDone <- a.Prompt(ctx, "provider-owned foreground", nil, 1)
|
||||
}()
|
||||
<-fake.entered
|
||||
|
||||
secondDone := make(chan error, 1)
|
||||
go func() {
|
||||
secondDone <- a.Prompt(ctx, "queued follow-up", nil, 2)
|
||||
}()
|
||||
select {
|
||||
case call := <-fake.entered:
|
||||
t.Fatalf("follow-up reached provider without foreground handoff: %+v", call)
|
||||
case <-time.After(100 * time.Millisecond):
|
||||
}
|
||||
|
||||
fake.releasePrompts()
|
||||
select {
|
||||
case err := <-firstDone:
|
||||
if err != nil {
|
||||
t.Fatalf("first prompt: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("first prompt did not return")
|
||||
}
|
||||
select {
|
||||
case call := <-fake.entered:
|
||||
if call.number != 2 || call.text != "queued follow-up" {
|
||||
t.Fatalf("queued follow-up = %+v, want call 2", call)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("queued follow-up did not run after provider released ownership")
|
||||
}
|
||||
select {
|
||||
case err := <-secondDone:
|
||||
if err != nil {
|
||||
t.Fatalf("second prompt: %v", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("second prompt did not return")
|
||||
}
|
||||
}
|
||||
|
||||
func TestPromptHandoffCapabilityIsAdapterAttested(t *testing.T) {
|
||||
for _, test := range []struct {
|
||||
agentID string
|
||||
want bool
|
||||
}{
|
||||
{agentID: claudeAgentID, want: true},
|
||||
{agentID: mockAgentID, want: true},
|
||||
{agentID: codexAgentID, want: false},
|
||||
{agentID: "other-acp", want: false},
|
||||
} {
|
||||
a := newTestAdapter()
|
||||
a.agentID = test.agentID
|
||||
if got := a.supportsPromptHandoff(); got != test.want {
|
||||
t.Fatalf("agent %q handoff support = %t, want %t", test.agentID, got, test.want)
|
||||
}
|
||||
_ = a.Close()
|
||||
}
|
||||
}
|
||||
@@ -75,6 +75,11 @@ const (
|
||||
EventTypeAuthRequired = "auth_required"
|
||||
)
|
||||
|
||||
// AgentEventDataPromptHandoff marks a generation-bearing foreground-idle event
|
||||
// whose provider and transport have attested that the next human prompt may
|
||||
// take ownership before the held session/prompt RPC returns.
|
||||
const AgentEventDataPromptHandoff = "prompt_handoff"
|
||||
|
||||
// Session status constants for EventTypeSessionStatus events.
|
||||
const (
|
||||
SessionStatusResumed = "resumed"
|
||||
|
||||
@@ -48,6 +48,13 @@ to the foreground only:
|
||||
the ACP prompt RPC remains held open for spawned subagents; a later
|
||||
task-notification-origin cycle may temporarily take foreground precedence
|
||||
while it generates the completion summary.
|
||||
- A generation-bearing human-origin idle boundary may also attest transport
|
||||
handoff, but only for adapters whose provider bridge is fixture-tested to
|
||||
accept the next human prompt echo while the earlier RPC remains open. The
|
||||
adapter transfers its existing prompt-gate token directly to one human
|
||||
successor; it does not release the token into the general queue. Synthetic
|
||||
`ScheduleWakeup` prompts therefore remain serialized and cannot consume or
|
||||
misalign the human handoff.
|
||||
|
||||
- A `RUNNING` session accepts a new prompt when its foreground turn is idle and at least one recognized background task is outstanding; otherwise it keeps rejecting input exactly as before.
|
||||
- Recognition rests on **adapter attestation, not normalized presentation
|
||||
@@ -88,7 +95,21 @@ to the foreground only:
|
||||
current UI to one.
|
||||
- Admission is **check-and-claim, not check-then-act**. The gate is a pure read, so `PromptTask` follows a passing read with an atomic `claimForegroundTurn` before it drives the turn: the check and the flip back to foreground-generating happen under one lock. Without this, two prompts landing in the background-idle window together (a double-send, two tabs) would both pass the read — the window spans a session reload, `ensureSessionRunning`, and a possibly network-bound model switch — and both reach `executor.Prompt`, starting overlapping turns on one ACP session. Exactly one prompt wins; the losers are rejected with `ErrAgentPromptInProgress` just as they were before this ADR.
|
||||
- The claim is **held, not merely taken**, and is tracked independently of background-idle activity. It survives until agentctl accepts the prompt, so a background tool call landing while the prompt is in preflight or queued cannot reopen the gate underneath it. The lifecycle adapter reports that exact dispatch boundary before waiting for turn completion. Claim tokens bind the activity record and a monotonically increasing admission generation, so a delayed completion or release cannot mutate a newer claim for the same session. A failed pre-dispatch prompt reopens the gate only if no newer foreground output invalidated its captured foreground epoch. Every transition that changes the substate is broadcast, including a release and background work that becomes visible when a claim completes, so clients cannot remain stranded on the admission-time value.
|
||||
- Lifecycle prompt delivery is serialized per agent execution. Agentctl acknowledges transport dispatch before the adapter's prompt RPC completes, so adapter-level queuing alone does not protect lifecycle's shared completion channel and response buffers from concurrent callers. An execution-scoped mutex gives each prompt one waiter and one buffer set; dispatch-only sends leave a pending-completion barrier that the next send must consume before resetting those buffers.
|
||||
- Lifecycle prompt delivery is serialized per agent execution. Agentctl
|
||||
acknowledges transport dispatch before the adapter's prompt RPC completes,
|
||||
so adapter-level queuing alone does not protect lifecycle's shared completion
|
||||
channel and response buffers from concurrent callers. An execution-scoped
|
||||
mutex gives each prompt one waiter and one buffer set; dispatch-only sends
|
||||
leave a pending-completion barrier that the next send must consume before
|
||||
resetting those buffers. The sole exception is an adapter-attested,
|
||||
generation-matched human handoff: lifecycle flushes the earlier streaming
|
||||
boundary and releases its waiter before the successor can reset shared state.
|
||||
A delayed result from the earlier RPC is then stale by generation and is
|
||||
suppressed before prompt-end sweeps, usage consumption, trace clearing, or
|
||||
completion emission can affect the successor. The successor's own prompt-end
|
||||
sweeps also preserve typed background work that was live at handoff, including
|
||||
nested child-tool and Monitor lineage; explicit session cancellation or reset
|
||||
still owns full cleanup.
|
||||
- Tool activity marks the foreground as generating only from positive ownership
|
||||
evidence. The initial tool call records whether it is top-level foreground,
|
||||
recognized background work, or a child of background work; subsequent
|
||||
@@ -145,7 +166,14 @@ The distinction is surfaced to the operator as a fine-grained substate (`foregro
|
||||
|
||||
## Consequences
|
||||
|
||||
The operator is no longer falsely locked out while background work runs, and the UI truthfully shows "working in the background" as distinct from both "generating" and "done". Accepting input earlier does not make delivery earlier for a held-open subagent turn — that message still waits behind the open exchange, as the queue does today — but out-of-turn work (a Monitor burst, a backgrounded shell after the main turn yielded) is forwarded promptly with no false "already running" rejection.
|
||||
The operator is no longer falsely locked out while background work runs, and
|
||||
the UI truthfully shows "working in the background" as distinct from both
|
||||
"generating" and "done". For an attested held-open Claude subagent turn,
|
||||
acceptance now also means immediate provider delivery after the matching
|
||||
human-origin idle boundary; detached child output may continue concurrently.
|
||||
Without that authoritative, generation-matched handoff, the prompt remains
|
||||
queued and the UI retains the foreground-busy state. Out-of-turn work such as a
|
||||
Monitor burst or backgrounded shell keeps its existing behavior.
|
||||
|
||||
The signal and count are best-effort across a backend or agent-execution
|
||||
restart. Connected executions retain background liveness and exact subagent
|
||||
@@ -160,6 +188,10 @@ it, so completed session history does not retain live-memory bookkeeping.
|
||||
|
||||
- **Rely on upstream synthetic idle-turn completion alone.** Rejected: it structurally cannot arm while the foreground prompt exchange is open, and its debounce re-extends under event bursts, so the held-open and chained-burst windows remain. A falsifiable acceptance test drives a chatty Monitor and confirms a prompt sent during a burst is accepted only with the fine-grained gate.
|
||||
- **Drop the `RUNNING` gate / always accept input.** Rejected: it would let a new message race a genuinely-generating foreground turn, risking dropped or reordered messages and regressing non-steering agents.
|
||||
- **Remove ACP prompt serialization after any foreground-idle event.** Rejected:
|
||||
synthetic wakeups and unsupported providers could race the held prompt,
|
||||
stealing or shifting prompt responses. Handoff is generation-matched,
|
||||
adapter-attested, and transferable only to a human successor.
|
||||
- **Persist the foreground/background distinction to the database (like the coarse states).** Rejected: persistence without an agent-side reconciliation API becomes a second source of truth and can survive restart as a false live value after the workload has died. Connected-execution tracking is kept in memory and read at serialization boundaries; turn close no longer destroys it, while execution teardown does.
|
||||
- **Recognize background work by normalized payload shape.** Rejected: the shape
|
||||
is shared presentation data and proves neither provider capability nor
|
||||
|
||||
@@ -110,14 +110,23 @@
|
||||
"audiences": ["user"],
|
||||
"stability": "stable",
|
||||
"docs": ["sessions-and-review", "developer-tools", "automation-and-mcp"],
|
||||
"sources": ["apps/web/components/review/review-dialog.tsx", "apps/web/components/review/walkthrough-overlay.tsx"],
|
||||
"tests": ["apps/web/components/review/review-dialog.build-files.test.ts", "apps/web/components/review/walkthrough-overlay.test.tsx"],
|
||||
"sources": [
|
||||
"apps/backend/internal/mcp/handlers/review.go",
|
||||
"apps/web/components/review/review-dialog.tsx",
|
||||
"apps/web/components/review/walkthrough-overlay.tsx"
|
||||
],
|
||||
"tests": [
|
||||
"apps/backend/internal/mcp/handlers/review_test.go",
|
||||
"apps/web/components/review/review-dialog.build-files.test.ts",
|
||||
"apps/web/components/review/walkthrough-overlay.test.tsx"
|
||||
],
|
||||
"mcpTools": [
|
||||
"create_task_plan_kandev",
|
||||
"delete_task_plan_kandev",
|
||||
"delete_walkthrough_kandev",
|
||||
"get_task_plan_kandev",
|
||||
"get_walkthrough_kandev",
|
||||
"publish_review_findings_kandev",
|
||||
"show_walkthrough_kandev",
|
||||
"update_task_plan_kandev"
|
||||
]
|
||||
|
||||
@@ -133,6 +133,14 @@ A task created with **Create without starting agent** opens in a prepared workbe
|
||||
|
||||
If the selected profile is unhealthy or incompatible with the executor, fix that configuration before launch. Starting an agent is separate from moving the task through its workflow; entry actions and turn-complete transitions can move or restart work afterward.
|
||||
|
||||
When a session shows **Working in background**, its foreground turn has ended
|
||||
but recognized child work is still live. You can send a follow-up without
|
||||
waiting for that work to finish. Providers with an attested handoff, including
|
||||
Claude ACP, receive the follow-up immediately while the child may continue
|
||||
streaming. While the session still shows **Generating**, the provider retains
|
||||
foreground ownership and Kandev keeps another message queued until that
|
||||
ownership is released.
|
||||
|
||||
## Find and organize tasks
|
||||
|
||||
On desktop and tablet, the header switches between **Kanban**, **Pipeline**, and **List**. Kanban and Pipeline show the same workflow steps in different layouts. Kandev remembers the last selected view in that browser on the current device. Phones offer **Kanban** and **List** only; a saved desktop Pipeline preference is kept but shown as Kanban on the phone.
|
||||
|
||||
@@ -21,6 +21,11 @@ and incorrectly prevents prompt delivery.
|
||||
may appear as done.
|
||||
- A foreground-idle session with recognized background work accepts a new
|
||||
prompt. A session with foreground activity continues to reject it.
|
||||
- When a provider-attested, generation-bearing foreground-idle boundary also
|
||||
guarantees prompt handoff, the next human prompt reaches the provider
|
||||
immediately even if the prior prompt RPC remains open for detached work.
|
||||
Synthetic wakeups remain serialized behind the owning RPC. Providers that do
|
||||
not attest handoff keep the follow-up queued until they release ownership.
|
||||
- Only adapter-attested workloads relax prompt admission. An adapter opts in a
|
||||
workload only when Kandev has fixture-tested both its launch and accountable
|
||||
terminal lifecycle. Claude subagents, background shells, and Monitor watches,
|
||||
@@ -80,12 +85,22 @@ and incorrectly prevents prompt delivery.
|
||||
Prompt admission is atomic: only one prompt can claim foreground ownership for
|
||||
a session. Delayed release or completion from an earlier prompt cycle cannot
|
||||
mutate a later accepted claim. Prompt delivery is serialized per agent
|
||||
execution so each prompt owns its completion wait and response buffers.
|
||||
execution except for one generation-matched human ownership transfer at an
|
||||
adapter-attested provider handoff. The transfer finalizes the earlier lifecycle
|
||||
buffer/completion owner before the successor starts; synthetic wakeups cannot
|
||||
consume it, and a delayed response from the earlier RPC cannot finalize the
|
||||
successor. Prompt-end cleanup for that successor excludes predecessor background
|
||||
work and its nested tool/Monitor lineage, while explicit cancellation and session
|
||||
replacement still clear all ownership.
|
||||
|
||||
## Failure modes
|
||||
|
||||
- An unknown activity value for an in-flight `RUNNING` session is rendered as
|
||||
generating, not done.
|
||||
- A foreground-idle signal without a nonzero matching prompt generation and
|
||||
adapter-attested handoff capability does not relax transport serialization.
|
||||
The session remains visibly foreground-owned and the follow-up remains
|
||||
queued rather than being reported as delivered.
|
||||
- A normalized tool-card shape cannot attest to background capability. An
|
||||
unsupported adapter that emits a subagent-looking card remains foreground-busy
|
||||
until its launch, yield, and terminal lifecycle are explicitly supported.
|
||||
@@ -135,6 +150,20 @@ an empty activity record. Durable coarse state continues to survive as before.
|
||||
- **GIVEN** a foreground-idle session with a recognized background workload,
|
||||
**WHEN** the operator sends a prompt, **THEN** the prompt is accepted while
|
||||
the status remains background-running until foreground activity begins.
|
||||
- **GIVEN** an attested provider keeps a prompt RPC open for a detached child
|
||||
after emitting foreground-idle for the matching generation, **WHEN** the
|
||||
operator sends a follow-up, **THEN** that human prompt reaches the provider
|
||||
immediately and the detached child may continue streaming.
|
||||
- **GIVEN** a synthetic wakeup is waiting when an attested human handoff occurs,
|
||||
**WHEN** a human follow-up is submitted, **THEN** the human prompt inherits
|
||||
ownership and the wakeup remains serialized until the human turn completes.
|
||||
- **GIVEN** predecessor background tools or Monitors are still live after the
|
||||
follow-up completes, **WHEN** the successor runs prompt-end cleanup, **THEN**
|
||||
those predecessor-owned streams remain tracked until their authoritative
|
||||
terminal event, explicit cancellation, or session replacement.
|
||||
- **GIVEN** the provider has not authoritatively released the foreground,
|
||||
**WHEN** the operator submits a follow-up, **THEN** Kandev reports it as
|
||||
queued and does not send it concurrently.
|
||||
- **GIVEN** two adapter-attested subagents and one background shell are live,
|
||||
**WHEN** the session is serialized or publishes an activity update, **THEN**
|
||||
`active_subagent_count` is two while all three workloads still contribute to
|
||||
|
||||
Reference in New Issue
Block a user