diff --git a/apps/backend/cmd/mock-agent/main.go b/apps/backend/cmd/mock-agent/main.go index c60e6b4791..599b5674c7 100644 --- a/apps/backend/cmd/mock-agent/main.go +++ b/apps/backend/cmd/mock-agent/main.go @@ -51,6 +51,7 @@ type mockAgent struct { model string sessions map[acp.SessionId]bool promptCancels map[acp.SessionId]context.CancelFunc + promptCancelHolds map[acp.SessionId]chan struct{} sessionMCPServers map[acp.SessionId]map[string]mcpServerDef sessionConfig map[acp.SessionId][]acp.SessionConfigOption commandsEmitted map[acp.SessionId]bool @@ -87,6 +88,7 @@ func main() { model: model, sessions: make(map[acp.SessionId]bool), promptCancels: make(map[acp.SessionId]context.CancelFunc), + promptCancelHolds: make(map[acp.SessionId]chan struct{}), sessionMCPServers: make(map[acp.SessionId]map[string]mcpServerDef), sessionConfig: make(map[acp.SessionId][]acp.SessionConfigOption), commandsEmitted: make(map[acp.SessionId]bool), @@ -348,23 +350,37 @@ func (a *mockAgent) LoadSession(ctx context.Context, req acp.LoadSessionRequest) // Prompt processes a user message and streams responses via SessionUpdate. func (a *mockAgent) Prompt(ctx context.Context, req acp.PromptRequest) (acp.PromptResponse, error) { promptCtx, cancelPrompt := context.WithCancel(ctx) + prompt := extractPromptText(req.Prompt) + var cancelHold chan struct{} + if isCancelHoldPrompt(prompt) { + cancelHold = make(chan struct{}) + } a.mu.Lock() if a.promptCancels == nil { a.promptCancels = make(map[acp.SessionId]context.CancelFunc) } + if a.promptCancelHolds == nil { + a.promptCancelHolds = make(map[acp.SessionId]chan struct{}) + } a.promptCancels[req.SessionId] = cancelPrompt + if cancelHold != nil { + a.promptCancelHolds[req.SessionId] = cancelHold + } a.mu.Unlock() defer func() { cancelPrompt() a.mu.Lock() - if current := a.promptCancels[req.SessionId]; current != nil { - delete(a.promptCancels, req.SessionId) - } + delete(a.promptCancels, req.SessionId) + delete(a.promptCancelHolds, req.SessionId) a.mu.Unlock() }() a.emitAvailableCommandsOnce(promptCtx, req.SessionId) - prompt := extractPromptText(req.Prompt) + if cancelHold != nil { + <-cancelHold + time.Sleep(mockCancelHoldDuration()) + return acp.PromptResponse{StopReason: acp.StopReasonCancelled}, nil + } // The /overloaded scenario must surface a real prompt-time ACP *error* // (a JSON-RPC error response), which handlePrompt's emitter cannot do — // so intercept it here and return the error from Prompt directly. @@ -402,8 +418,17 @@ func (a *mockAgent) sessionModel(sessionID acp.SessionId) string { // Cancel handles session cancellation. func (a *mockAgent) Cancel(_ context.Context, req acp.CancelNotification) error { a.mu.Lock() + cancelHold := a.promptCancelHolds[req.SessionId] cancelPrompt := a.promptCancels[req.SessionId] a.mu.Unlock() + if cancelHold != nil { + select { + case <-cancelHold: + default: + close(cancelHold) + } + return nil + } if cancelPrompt != nil { cancelPrompt() } @@ -729,3 +754,18 @@ func parseMCPConfigFromArgs(args []string) string { } return "" } + +// isCancelHoldPrompt identifies the E2E-only fixture that holds an acknowledged +// cancellation long enough to exercise remount and hydration projections. +func isCancelHoldPrompt(prompt string) bool { + return strings.EqualFold(strings.TrimSpace(stripKandevSystem(prompt)), "/e2e:cancel-hold") +} + +func mockCancelHoldDuration() time.Duration { + if raw := strings.TrimSpace(os.Getenv("KANDEV_E2E_CANCEL_HOLD_DURATION")); raw != "" { + if duration, err := time.ParseDuration(raw); err == nil && duration >= 0 { + return duration + } + } + return 8 * time.Second +} diff --git a/apps/backend/cmd/mock-agent/mock_agent_test.go b/apps/backend/cmd/mock-agent/mock_agent_test.go index 0b0be9e8b6..17d547dd27 100644 --- a/apps/backend/cmd/mock-agent/mock_agent_test.go +++ b/apps/backend/cmd/mock-agent/mock_agent_test.go @@ -7,6 +7,7 @@ import ( "strings" "sync" "testing" + "testing/synctest" "time" acp "github.com/coder/acp-go-sdk" @@ -26,6 +27,74 @@ func (u *promptCancelUpdater) RequestPermission(context.Context, acp.RequestPerm return acp.RequestPermissionResponse{}, nil } +func TestMockAgentCancelHoldDefersPromptCompletion(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + const sessionID = acp.SessionId("cancel-hold-session") + t.Setenv("KANDEV_E2E_CANCEL_HOLD_DURATION", "30ms") + updater := newCapturingUpdater() + agent := &mockAgent{ + model: "mock-fast", + conn: updater, + sessions: map[acp.SessionId]bool{sessionID: true}, + promptCancels: make(map[acp.SessionId]context.CancelFunc), + promptCancelHolds: make(map[acp.SessionId]chan struct{}), + commandsEmitted: make(map[acp.SessionId]bool), + } + + result := make(chan struct { + response acp.PromptResponse + err error + }, 1) + go func() { + response, err := agent.Prompt(context.Background(), acp.PromptRequest{ + SessionId: sessionID, + Prompt: []acp.ContentBlock{acp.TextBlock("/e2e:cancel-hold")}, + }) + result <- struct { + response acp.PromptResponse + err error + }{response: response, err: err} + }() + + synctest.Wait() + select { + case <-updater.anySeen: + default: + t.Fatal("hold prompt did not start") + } + if err := agent.Cancel(context.Background(), acp.CancelNotification{SessionId: sessionID}); err != nil { + t.Fatalf("cancel prompt: %v", err) + } + synctest.Wait() + select { + case <-result: + t.Fatal("cancel-hold prompt completed before its configured hold elapsed") + default: + } + + time.Sleep(29 * time.Millisecond) + synctest.Wait() + select { + case <-result: + t.Fatal("cancel-hold prompt completed before its configured hold elapsed") + default: + } + + time.Sleep(time.Millisecond) + synctest.Wait() + outcome := <-result + if outcome.err != nil { + t.Fatalf("prompt returned error: %v", outcome.err) + } + if outcome.response.StopReason != acp.StopReasonCancelled { + t.Fatalf("stop reason = %q, want cancelled", outcome.response.StopReason) + } + if texts := updater.textMessages(); len(texts) != 0 { + t.Fatalf("cancel-hold emitted assistant text: %v", texts) + } + }) +} + func TestMockAgentCancelStopsPrompt(t *testing.T) { const sessionID = acp.SessionId("cancel-session") updater := &promptCancelUpdater{started: make(chan struct{})} diff --git a/apps/backend/internal/agentctl/server/adapter/adapter.go b/apps/backend/internal/agentctl/server/adapter/adapter.go index 9fd27f422d..98662221b2 100644 --- a/apps/backend/internal/agentctl/server/adapter/adapter.go +++ b/apps/backend/internal/agentctl/server/adapter/adapter.go @@ -321,6 +321,9 @@ type Config struct { // NotificationQueueCapacity is the server-resolved ACP inbound queue size. NotificationQueueCapacity int + // PromptCancelJoinTimeout is an optional per-adapter ACP cancellation join bound. + PromptCancelJoinTimeout time.Duration + // ProviderGatewayAuth authenticates the ACP agent against an // OpenAI-compatible gateway right after initialize. ProviderGatewayAuth *acpprovider.GatewayAuth @@ -355,6 +358,7 @@ func (c *Config) ToSharedConfig() *shared.Config { AssumeMcpHttp: c.AssumeMcpHttp, RequiresProcessKill: c.RequiresProcessKill, NotificationQueueCapacity: c.NotificationQueueCapacity, + PromptCancelJoinTimeout: c.PromptCancelJoinTimeout, ProviderGatewayAuth: c.ProviderGatewayAuth, } } diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter.go index 01ab1cab24..fd9cc677aa 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter.go @@ -315,6 +315,7 @@ type Adapter struct { // prompt response, so sendPrompt's normal complete emission never runs. asyncTurnMu sync.Mutex asyncTurnFinalizers map[string]*asyncTurnFinalizer + cancelJoinTimeout time.Duration asyncTurnEpochs map[string]uint64 // turnStartedAt records, per session, the time agentctl last dispatched @@ -422,8 +423,7 @@ type asyncTurnFinalizer struct { promptEpoch uint64 } -// promptCancelJoinTimeout bounds how long Cancel and sendPrompt wait for a stuck -// session/prompt RPC to end after a user cancel. Exposed as a var for tests. +// promptCancelJoinTimeout is the production default and preserves existing direct-test behavior. var promptCancelJoinTimeout = 3 * time.Second // NewAdapter creates a new ACP protocol adapter. @@ -451,6 +451,7 @@ func NewAdapter(cfg *shared.Config, log *logger.Logger) *Adapter { attachMgr: shared.NewAttachmentManager(cfg.WorkDir, l.Zap()), promptGate: make(chan struct{}, 1), asyncTurnFinalizers: make(map[string]*asyncTurnFinalizer), + cancelJoinTimeout: cfg.PromptCancelJoinTimeout, asyncTurnEpochs: make(map[string]uint64), turnStartedAt: make(map[string]time.Time), lifetimeCtx: ctx, diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go index 84610957d4..1f666d7844 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt.go @@ -590,7 +590,7 @@ func (a *Adapter) Cancel(ctx context.Context) error { } turn := a.signalPromptTurnAbort() - if err := waitForPromptRPCAfterCancel(turn); err != nil { + if err := a.waitForPromptRPCAfterCancel(turn); err != nil { span.RecordError(err) a.logger.Warn("session/cancel sent but in-flight prompt did not end", zap.String("session_id", sessionID), diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_cancel.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_cancel.go index 741a092864..83c55cdc94 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_cancel.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_cancel.go @@ -241,19 +241,26 @@ func (a *Adapter) signalPromptTurnAbort() *promptTurnState { return turn } -func waitForPromptRPCAfterCancel(turn *promptTurnState) error { +func (a *Adapter) promptCancelTimeout() time.Duration { + if a.cancelJoinTimeout > 0 { + return a.cancelJoinTimeout + } + return promptCancelJoinTimeout +} + +func (a *Adapter) waitForPromptRPCAfterCancel(turn *promptTurnState) error { if turn == nil { return nil } select { case <-turn.rpcDone: return nil - case <-time.After(promptCancelJoinTimeout): + case <-time.After(a.promptCancelTimeout()): if turn.endTurn != nil { turn.endTurn(ErrTurnCancelNotAcknowledged) } return fmt.Errorf("%w: in-flight session/prompt did not end within %s", - ErrTurnCancelNotAcknowledged, promptCancelJoinTimeout) + ErrTurnCancelNotAcknowledged, a.promptCancelTimeout()) } } @@ -280,19 +287,19 @@ func (a *Adapter) waitForPromptRPCAfterUserCancel(turn *promptTurnState, session select { case <-turn.rpcDone: return providerErr - case <-time.After(promptCancelJoinTimeout): + case <-time.After(a.promptCancelTimeout()): return providerErr } case <-turn.abortCh: select { case <-turn.rpcDone: return nil - case <-time.After(promptCancelJoinTimeout): + case <-time.After(a.promptCancelTimeout()): if turn.endTurn != nil { turn.endTurn(ErrTurnCancelNotAcknowledged) } a.logger.Warn("in-flight session/prompt did not end after cancel; releasing prompt gate", - zap.Duration("timeout", promptCancelJoinTimeout)) + zap.Duration("timeout", a.promptCancelTimeout())) return errPromptAbandonedAfterCancel } } diff --git a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_cancel_test.go b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_cancel_test.go index fb3ade2170..3eff7ce7ab 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_cancel_test.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/acp/adapter_prompt_cancel_test.go @@ -10,10 +10,12 @@ import ( ) func TestWaitForPromptRPCAfterCancel_Acknowledged(t *testing.T) { + a := newTestAdapter() + t.Cleanup(func() { _ = a.Close() }) turn := &promptTurnState{rpcDone: make(chan struct{})} close(turn.rpcDone) - if err := waitForPromptRPCAfterCancel(turn); err != nil { + if err := a.waitForPromptRPCAfterCancel(turn); err != nil { t.Fatalf("expected nil, got %v", err) } } @@ -23,8 +25,10 @@ func TestWaitForPromptRPCAfterCancel_TimesOut(t *testing.T) { promptCancelJoinTimeout = 20 * time.Millisecond t.Cleanup(func() { promptCancelJoinTimeout = prev }) + a := newTestAdapter() + t.Cleanup(func() { _ = a.Close() }) turn := &promptTurnState{rpcDone: make(chan struct{})} - err := waitForPromptRPCAfterCancel(turn) + err := a.waitForPromptRPCAfterCancel(turn) if !errors.Is(err, ErrTurnCancelNotAcknowledged) { t.Fatalf("expected ErrTurnCancelNotAcknowledged, got %v", err) } @@ -98,6 +102,75 @@ func TestWaitForPromptRPCAfterUserCancel_CompletesAfterAbort(t *testing.T) { }) } +func TestWaitForPromptRPCAfterUserCancelUsesAdapterConfiguration(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + a := newTestAdapter() + defer func() { _ = a.Close() }() + a.cancelJoinTimeout = 40 * time.Millisecond + turn := &promptTurnState{endTurn: func(error) {}, rpcDone: make(chan struct{}), abortCh: make(chan struct{})} + close(turn.abortCh) + + done := make(chan error, 1) + go func() { + done <- a.waitForPromptRPCAfterUserCancel(turn, "") + }() + synctest.Wait() + select { + case err := <-done: + t.Fatalf("wait returned before configured timeout: %v", err) + default: + } + + time.Sleep(39 * time.Millisecond) + synctest.Wait() + select { + case err := <-done: + t.Fatalf("wait returned before configured timeout: %v", err) + default: + } + + time.Sleep(time.Millisecond) + synctest.Wait() + if err := <-done; !errors.Is(err, errPromptAbandonedAfterCancel) { + t.Fatalf("expected errPromptAbandonedAfterCancel, got %v", err) + } + }) +} + +func TestPromptCancelTimeoutUsesAdapterConfiguration(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + a := newTestAdapter() + defer func() { _ = a.Close() }() + a.cancelJoinTimeout = 40 * time.Millisecond + turn := &promptTurnState{rpcDone: make(chan struct{})} + + done := make(chan error, 1) + go func() { + done <- a.waitForPromptRPCAfterCancel(turn) + }() + synctest.Wait() + select { + case err := <-done: + t.Fatalf("wait returned before configured timeout: %v", err) + default: + } + + time.Sleep(39 * time.Millisecond) + synctest.Wait() + select { + case err := <-done: + t.Fatalf("wait returned before configured timeout: %v", err) + default: + } + + time.Sleep(time.Millisecond) + synctest.Wait() + if err := <-done; !errors.Is(err, ErrTurnCancelNotAcknowledged) { + t.Fatalf("expected ErrTurnCancelNotAcknowledged, got %v", err) + } + }) +} + func TestRegisterPromptTurn_CancelCause(t *testing.T) { a := newTestAdapter() ctx, turn := a.registerPromptTurn(context.Background(), 0) @@ -170,7 +243,7 @@ func TestWaitForPromptRPCAfterCancel_CancelsPromptCtxOnTimeout(t *testing.T) { defer a.clearPromptTurn(turn) turn.rpcDone = make(chan struct{}) - if err := waitForPromptRPCAfterCancel(turn); !errors.Is(err, ErrTurnCancelNotAcknowledged) { + if err := a.waitForPromptRPCAfterCancel(turn); !errors.Is(err, ErrTurnCancelNotAcknowledged) { t.Fatalf("expected ErrTurnCancelNotAcknowledged, got %v", err) } if !errors.Is(context.Cause(ctx), ErrTurnCancelNotAcknowledged) { diff --git a/apps/backend/internal/agentctl/server/adapter/transport/shared/config.go b/apps/backend/internal/agentctl/server/adapter/transport/shared/config.go index f74b9112a7..a3f3d45e48 100644 --- a/apps/backend/internal/agentctl/server/adapter/transport/shared/config.go +++ b/apps/backend/internal/agentctl/server/adapter/transport/shared/config.go @@ -61,6 +61,9 @@ type Config struct { // capacity. Zero keeps compatibility for directly constructed test configs. NotificationQueueCapacity int + // PromptCancelJoinTimeout is an optional per-adapter ACP cancellation join bound. + PromptCancelJoinTimeout time.Duration + // ProviderGatewayAuth, when set, makes the ACP adapter authenticate the // agent against an OpenAI-compatible gateway right after initialize. ProviderGatewayAuth *acpprovider.GatewayAuth diff --git a/apps/backend/internal/agentctl/server/config/config.go b/apps/backend/internal/agentctl/server/config/config.go index 14475c4bdf..a91a331570 100644 --- a/apps/backend/internal/agentctl/server/config/config.go +++ b/apps/backend/internal/agentctl/server/config/config.go @@ -120,6 +120,9 @@ type Config struct { // queue capacity for every instance created by this server. NotificationQueueCapacity int + // PromptCancelJoinTimeout overrides ACP cancellation acknowledgement only for the E2E profile. + PromptCancelJoinTimeout time.Duration + // OTLPEndpoint is the resolved endpoint used by agentctl transport tracing. OTLPEndpoint string @@ -289,6 +292,9 @@ type InstanceConfig struct { // inherited from the server startup contract. NotificationQueueCapacity int + // PromptCancelJoinTimeout is inherited from the server startup configuration. + PromptCancelJoinTimeout time.Duration + // DetachedEventLimit bounds the per-instance retained-event count // (AC-EXECUTORS-SURVIVAL-001.6), inherited from the server startup // contract. It sizes the process manager's updates channel buffer. @@ -419,10 +425,27 @@ func StartupConfigFromEnv() (commonconfig.AgentctlStartupConfig, bool, error) { return startup, true, nil } +func directE2EPromptCancelJoinTimeout() time.Duration { + if !isProfileTruthy(os.Getenv("KANDEV_E2E_MOCK")) { + return 0 + } + return getEnvDuration("KANDEV_E2E_PROMPT_CANCEL_JOIN_TIMEOUT", 0) +} + +func isProfileTruthy(value string) bool { + switch strings.TrimSpace(value) { + case "true", "1", "yes", "on": + return true + default: + return false + } +} + func load(startup *commonconfig.AgentctlStartupConfig) *Config { idleTimeout := getEnvDuration("KANDEV_ACP_IDLE_TIMEOUT", time.Hour) idleReaperInterval := getEnvDuration("KANDEV_ACP_IDLE_REAPER_INTERVAL", time.Minute) notificationQueueCapacity := getEnvInt("KANDEV_ACP_NOTIF_QUEUE", 131072) + promptCancelJoinTimeout := directE2EPromptCancelJoinTimeout() otlpEndpoint := getEnv("OTEL_EXPORTER_OTLP_ENDPOINT", "") unownedPeriod := getEnvDuration("KANDEV_ACP_UNOWNED_PERIOD", defaultUnownedPeriod) detachedEventLimit := getEnvInt("KANDEV_ACP_DETACHED_EVENT_LIMIT", defaultDetachedEventLimit) @@ -433,6 +456,7 @@ func load(startup *commonconfig.AgentctlStartupConfig) *Config { idleReaperInterval = startup.IdleReaperInterval notificationQueueCapacity = startup.NotificationQueueCapacity otlpEndpoint = startup.OTLPEndpoint + promptCancelJoinTimeout = startup.PromptCancelJoinTimeout // Zero means the caller did not resolve these (an older backend, or // one built before agent survival existed) — keep the env/built-in // value already computed above rather than adopting zero. @@ -472,6 +496,7 @@ func load(startup *commonconfig.AgentctlStartupConfig) *Config { IdleTimeout: idleTimeout, IdleReaperInterval: idleReaperInterval, NotificationQueueCapacity: notificationQueueCapacity, + PromptCancelJoinTimeout: promptCancelJoinTimeout, OTLPEndpoint: otlpEndpoint, UnownedPeriod: unownedPeriod, DetachedEventLimit: detachedEventLimit, @@ -607,6 +632,7 @@ func (c *Config) NewInstanceConfig(port int, overrides *InstanceOverrides) *Inst LogFormat: c.LogFormat, ProcessBufferMaxBytes: c.Defaults.ProcessBufferMaxBytes, NotificationQueueCapacity: c.NotificationQueueCapacity, + PromptCancelJoinTimeout: c.PromptCancelJoinTimeout, DetachedEventLimit: c.DetachedEventLimit, VscodeCommand: c.VscodeCommand, McpMode: "task", diff --git a/apps/backend/internal/agentctl/server/config/config_test.go b/apps/backend/internal/agentctl/server/config/config_test.go index 3bc22a05a5..60dabd3bb1 100644 --- a/apps/backend/internal/agentctl/server/config/config_test.go +++ b/apps/backend/internal/agentctl/server/config/config_test.go @@ -27,6 +27,7 @@ func TestLoadWithStartupUsesExplicitManagedValues(t *testing.T) { IdleReaperInterval: 3 * time.Minute, NotificationQueueCapacity: 4096, OTLPEndpoint: "http://configured:4318", + PromptCancelJoinTimeout: 12 * time.Second, } cfg, err := LoadWithStartup(startup) if err != nil { @@ -41,6 +42,9 @@ func TestLoadWithStartupUsesExplicitManagedValues(t *testing.T) { if cfg.OTLPEndpoint != startup.OTLPEndpoint { t.Fatalf("managed OTLP endpoint = %q, want %q", cfg.OTLPEndpoint, startup.OTLPEndpoint) } + if cfg.PromptCancelJoinTimeout != startup.PromptCancelJoinTimeout { + t.Fatalf("managed prompt cancel join timeout = %s, want %s", cfg.PromptCancelJoinTimeout, startup.PromptCancelJoinTimeout) + } } // TestLoadWithStartupPropagatesAgentSurvivalEnabled pins that @@ -79,6 +83,14 @@ func TestLoadWithStartupPropagatesAgentSurvivalEnabled(t *testing.T) { // TestLoadWithoutStartupLeavesAgentSurvivalDisabled pins that a legacy/direct // (unmanaged) launch -- Load(), no startup contract -- never engages the // capability, matching AC-EXECUTORS-SURVIVAL-005.2's "defaults disabled". +func TestLoadWithoutStartupAcceptsTruthyE2ESelector(t *testing.T) { + t.Setenv("KANDEV_E2E_MOCK", "1") + t.Setenv("KANDEV_E2E_PROMPT_CANCEL_JOIN_TIMEOUT", "12s") + if got := Load().PromptCancelJoinTimeout; got != 12*time.Second { + t.Fatalf("prompt cancel join timeout = %s, want 12s", got) + } +} + func TestLoadWithoutStartupLeavesAgentSurvivalDisabled(t *testing.T) { cfg := Load() if cfg.AgentSurvivalEnabled { diff --git a/apps/backend/internal/agentctl/server/process/manager.go b/apps/backend/internal/agentctl/server/process/manager.go index ca0d0f8c56..add9f89b9f 100644 --- a/apps/backend/internal/agentctl/server/process/manager.go +++ b/apps/backend/internal/agentctl/server/process/manager.go @@ -1456,6 +1456,7 @@ func (m *Manager) buildAdapterConfig() error { AssumeMcpHttp: m.cfg.AssumeMcpHttp, RequiresProcessKill: m.cfg.RequiresProcessKill, NotificationQueueCapacity: m.cfg.NotificationQueueCapacity, + PromptCancelJoinTimeout: m.cfg.PromptCancelJoinTimeout, ProviderGatewayAuth: m.cfg.ProviderGatewayAuth, } diff --git a/apps/backend/internal/agentctl/server/process/manager_test.go b/apps/backend/internal/agentctl/server/process/manager_test.go index 72ce50aa2e..1df6a7043c 100644 --- a/apps/backend/internal/agentctl/server/process/manager_test.go +++ b/apps/backend/internal/agentctl/server/process/manager_test.go @@ -488,6 +488,7 @@ func TestBuildAdapterConfigForwardsNotificationQueueCapacity(t *testing.T) { AgentEnv: []string{"PATH=/usr/bin"}, Protocol: agent.ProtocolACP, NotificationQueueCapacity: 4096, + PromptCancelJoinTimeout: 12 * time.Second, }, logger: newTestLogger(t), } @@ -500,6 +501,9 @@ func TestBuildAdapterConfigForwardsNotificationQueueCapacity(t *testing.T) { if got := m.adapterCfg.NotificationQueueCapacity; got != 4096 { t.Fatalf("adapter notification queue capacity = %d, want 4096", got) } + if got := m.adapterCfg.PromptCancelJoinTimeout; got != 12*time.Second { + t.Fatalf("adapter prompt cancel join timeout = %s, want 12s", got) + } } func TestStartOneShotPreservesConfiguredTempEnvironment(t *testing.T) { diff --git a/apps/backend/internal/common/config/agentctl.go b/apps/backend/internal/common/config/agentctl.go index 9a45a1c39a..b048f03646 100644 --- a/apps/backend/internal/common/config/agentctl.go +++ b/apps/backend/internal/common/config/agentctl.go @@ -3,6 +3,7 @@ package config import ( "encoding/json" "fmt" + "os" "strings" "time" ) @@ -35,6 +36,9 @@ type AgentctlStartupConfig struct { // "disabled for this launch" answer agentctl must not second-guess with // its own default. AgentSurvivalEnabled bool `json:"agentSurvivalEnabled"` + // PromptCancelJoinTimeout is the managed ACP cancellation acknowledgement bound. + // Zero preserves agentctl's built-in default for non-E2E launches. + PromptCancelJoinTimeout time.Duration `json:"promptCancelJoinTimeout"` } // ManagedAgentctlStartupConfig returns the agentctl settings resolved by the @@ -52,6 +56,29 @@ func (c *Config) ManagedAgentctlStartupConfig() AgentctlStartupConfig { UnownedPeriod: c.Agentctl.UnownedPeriod, DetachedEventLimit: c.Agentctl.DetachedEventLimit, AgentSurvivalEnabled: c.Features.AgentSurvival, + PromptCancelJoinTimeout: e2ePromptCancelJoinTimeout(), + } +} + +// e2ePromptCancelJoinTimeout resolves the profile-owned cancellation bound for +// managed agentctl processes. Keep selector truthiness aligned with profiles. +func e2ePromptCancelJoinTimeout() time.Duration { + if !isProfileTruthy(os.Getenv("KANDEV_E2E_MOCK")) { + return 0 + } + value, err := time.ParseDuration(strings.TrimSpace(os.Getenv("KANDEV_E2E_PROMPT_CANCEL_JOIN_TIMEOUT"))) + if err != nil || value <= 0 { + return 0 + } + return value +} + +func isProfileTruthy(value string) bool { + switch strings.TrimSpace(value) { + case "true", "1", "yes", "on": + return true + default: + return false } } @@ -75,6 +102,9 @@ func (c AgentctlStartupConfig) Validate() error { if c.UnownedPeriod < 0 { return fmt.Errorf("agentctl unowned period must be zero (unset) or positive") } + if c.PromptCancelJoinTimeout < 0 { + return fmt.Errorf("agentctl prompt cancel join timeout must be zero or greater") + } if c.DetachedEventLimit != 0 && (c.DetachedEventLimit < 1 || c.DetachedEventLimit > 10000) { return fmt.Errorf("agentctl detached event limit must be zero (unset) or between 1 and 10000") } diff --git a/apps/backend/internal/common/config/catalog_test.go b/apps/backend/internal/common/config/catalog_test.go index 6ce878b407..8af524a9c0 100644 --- a/apps/backend/internal/common/config/catalog_test.go +++ b/apps/backend/internal/common/config/catalog_test.go @@ -241,6 +241,7 @@ func TestAgentctlStartupConfigRoundTripsAndRejectsInvalidValues(t *testing.T) { UnownedPeriod: 10 * time.Minute, DetachedEventLimit: 100, AgentSurvivalEnabled: true, + PromptCancelJoinTimeout: 12 * time.Second, } raw, err := EncodeAgentctlStartupConfig(want) if err != nil { @@ -272,8 +273,15 @@ func TestAgentctlStartupConfigRoundTripsAndRejectsInvalidValues(t *testing.T) { unset := want unset.UnownedPeriod = 0 unset.DetachedEventLimit = 0 + unset.PromptCancelJoinTimeout = 0 if _, err := EncodeAgentctlStartupConfig(unset); err != nil { - t.Fatalf("EncodeAgentctlStartupConfig rejected zero-valued (unset) survival tunables: %v", err) + t.Fatalf("EncodeAgentctlStartupConfig rejected zero-valued optional values: %v", err) + } + + invalidCancelJoinTimeout := want + invalidCancelJoinTimeout.PromptCancelJoinTimeout = -time.Second + if _, err := EncodeAgentctlStartupConfig(invalidCancelJoinTimeout); err == nil { + t.Fatal("EncodeAgentctlStartupConfig accepted a negative prompt cancel join timeout") } invalidDetachedEventLimit := want @@ -303,3 +311,27 @@ func TestManagedAgentctlStartupConfigReflectsAgentSurvivalFlag(t *testing.T) { t.Fatal("AgentSurvivalEnabled = false, want true when the runtime flag is on") } } + +func TestManagedAgentctlStartupConfigResolvesTruthyE2ECancelJoinTimeout(t *testing.T) { + t.Setenv("KANDEV_E2E_MOCK", "yes") + t.Setenv("KANDEV_E2E_PROMPT_CANCEL_JOIN_TIMEOUT", "12s") + cfg := &Config{} + cfg.Agentctl.NotificationQueueCapacity = 4096 + cfg.Agentctl.IdleReaperInterval = time.Minute + + if got := cfg.ManagedAgentctlStartupConfig().PromptCancelJoinTimeout; got != 12*time.Second { + t.Fatalf("prompt cancel join timeout = %s, want 12s", got) + } +} + +func TestManagedAgentctlStartupConfigOmitsCancelJoinTimeoutOutsideE2E(t *testing.T) { + t.Setenv("KANDEV_E2E_MOCK", "false") + t.Setenv("KANDEV_E2E_PROMPT_CANCEL_JOIN_TIMEOUT", "12s") + cfg := &Config{} + cfg.Agentctl.NotificationQueueCapacity = 4096 + cfg.Agentctl.IdleReaperInterval = time.Minute + + if got := cfg.ManagedAgentctlStartupConfig().PromptCancelJoinTimeout; got != 0 { + t.Fatalf("prompt cancel join timeout = %s, want zero outside E2E", got) + } +} diff --git a/apps/backend/internal/profiles/profiles.yaml b/apps/backend/internal/profiles/profiles.yaml index 7f8e50e428..eaedf4da9a 100644 --- a/apps/backend/internal/profiles/profiles.yaml +++ b/apps/backend/internal/profiles/profiles.yaml @@ -220,6 +220,17 @@ ui: # don't belong in dev (would mask real interactions) and never in # prod (would leak test-only behaviour). e2eTuning: + # Holds a mock ACP cancellation through navigation and hydration without + # changing production cancellation behavior. + KANDEV_E2E_CANCEL_HOLD_DURATION: + prod: "" + dev: "" + e2e: "8s" + KANDEV_E2E_PROMPT_CANCEL_JOIN_TIMEOUT: + prod: "" + dev: "" + e2e: "3s" + # Auto-approve every permission request in agentctl so e2e specs # don't have to script the approval UI. Read by the agentctl child # process; the backend forwards its env to children, so setting it diff --git a/apps/web/e2e/tests/chat/cancel-progress-task-switch.spec.ts b/apps/web/e2e/tests/chat/cancel-progress-task-switch.spec.ts index f20027a582..dbbfc5fccf 100644 --- a/apps/web/e2e/tests/chat/cancel-progress-task-switch.spec.ts +++ b/apps/web/e2e/tests/chat/cancel-progress-task-switch.spec.ts @@ -3,6 +3,19 @@ import { waitForSessionDone, seedIdleSession } from "../../helpers/session"; import { waitForActiveSessionCancellationPending } from "../../helpers/session-store"; test.describe("Cancel progress across task switches", () => { + let releaseBackendEnv: (() => Promise) | undefined; + + test.beforeEach(async ({ backend }) => { + releaseBackendEnv = await backend.useEnv({ + KANDEV_E2E_PROMPT_CANCEL_JOIN_TIMEOUT: "12s", + }); + }); + + test.afterEach(async () => { + await releaseBackendEnv?.(); + releaseBackendEnv = undefined; + }); + test("keeps backend-owned cancel progress across task switches", async ({ testPage, apiClient, @@ -30,9 +43,8 @@ test.describe("Cancel progress across task switches", () => { ); const session = await seedIdleSession(testPage, apiClient, seedData, "Cancel progress A"); - // Reload hydration is covered independently by mobile-cancel-progress-reload.spec.ts. - // Keep this regression focused on the two component remounts caused by task navigation. - await session.sendMessage("/slow 30s"); + // /e2e:cancel-hold keeps an acknowledged backend cancellation pending through task switching. + await session.sendMessage("/e2e:cancel-hold"); const activeCancel = session.activeChat().getByTestId("cancel-agent-button"); await expect(activeCancel).toBeVisible({ timeout: 15_000 }); diff --git a/apps/web/e2e/tests/chat/mobile-cancel-progress-reload.spec.ts b/apps/web/e2e/tests/chat/mobile-cancel-progress-reload.spec.ts index 040a5d85ba..4a6dd07f58 100644 --- a/apps/web/e2e/tests/chat/mobile-cancel-progress-reload.spec.ts +++ b/apps/web/e2e/tests/chat/mobile-cancel-progress-reload.spec.ts @@ -3,6 +3,19 @@ import { seedIdleSession } from "../../helpers/session"; import { waitForActiveSessionCancellationPending } from "../../helpers/session-store"; test.describe("Mobile cancel progress across reloads", () => { + let releaseBackendEnv: (() => Promise) | undefined; + + test.beforeEach(async ({ backend }) => { + releaseBackendEnv = await backend.useEnv({ + KANDEV_E2E_PROMPT_CANCEL_JOIN_TIMEOUT: "12s", + }); + }); + + test.afterEach(async () => { + await releaseBackendEnv?.(); + releaseBackendEnv = undefined; + }); + test("keeps backend-owned cancel progress after a reload", async ({ testPage, apiClient, @@ -11,7 +24,8 @@ test.describe("Mobile cancel progress across reloads", () => { test.setTimeout(120_000); const session = await seedIdleSession(testPage, apiClient, seedData, "Mobile cancel progress"); - await session.sendMessageViaButton("/slow 8s"); + // /e2e:cancel-hold keeps an acknowledged backend cancellation pending through reload hydration. + await session.sendMessageViaButton("/e2e:cancel-hold"); const cancel = session.activeChat().getByTestId("cancel-agent-button"); await expect(cancel).toBeVisible({ timeout: 15_000 }); diff --git a/docs/plans/cancel-turn-progress/plan.md b/docs/plans/cancel-turn-progress/plan.md index dacc589d5e..3d6c440cf9 100644 --- a/docs/plans/cancel-turn-progress/plan.md +++ b/docs/plans/cancel-turn-progress/plan.md @@ -1,7 +1,11 @@ --- -spec: docs/specs/ui/requirements/cancel-turn-progress.md created: 2026-08-03 status: completed +requirements: + - REQ-UI-CANCEL-TURN-PROGRESS-001 +system_design: + - ../../specs/ui/system-design/cancel-turn-progress.md +legacy_specs: [] --- # Implementation Plan: Backend-owned cancel-turn progress @@ -142,21 +146,23 @@ layouts. Because reload persistence changes user-visible behavior on compact scr ## E2E Tests - **Task switch and reload:** revise - `apps/web/e2e/tests/chat/cancel-progress-task-switch.spec.ts`. Start the existing slow mock-agent - turn, send cancellation through to the backend, wait until the exposed session store reports + `apps/web/e2e/tests/chat/cancel-progress-task-switch.spec.ts`. Start the mock agent's + `/e2e:cancel-hold` prompt, send cancellation through to the backend, wait until the exposed session store reports `cancellation_pending=true`, switch away and back, reload the page, and assert the same disabled animated control until the backend settles. This replaces the current helper that holds the request before backend acceptance. - **Compact reload parity:** add `apps/web/e2e/tests/chat/mobile-cancel-progress-reload.spec.ts`, limited to `mobile-chrome`. Tap - cancel during the slow mock turn, wait for backend-owned pending state, reload, and assert the + cancel during the held mock turn, wait for backend-owned pending state, reload, and assert the shared control remains disabled and animated without desktop-only navigation assumptions. - **Test support:** add a cancellation-pending reader/waiter to `apps/web/e2e/helpers/session-store.ts`; remove `routeMainWebSocketWithHeldCancelRequest` from `apps/web/e2e/helpers/ws-drop.ts` once no test uses - it. The existing `/slow` mock scenario is cancellation-insensitive long enough to keep the - lifecycle operation deterministically pending; no production delay or new test-only endpoint is - required. + it. The mock-agent hold gives the acknowledged cancellation a single predictable boundary in every + mock run. Managed E2E startup carries the profile's three-second baseline through + `AgentctlStartupConfig` to ACP; the two cancel-progress regressions scope a 12-second override, + while normal startup retains ACP's three-second default. No browser request hold, test timeout + increase, or production lifecycle change is required. ## Verification Results diff --git a/docs/plans/cancel-turn-progress/task-06-cancel-reload-regression-e2e.md b/docs/plans/cancel-turn-progress/task-06-cancel-reload-regression-e2e.md index ef6f7b2c7e..0f8b056c32 100644 --- a/docs/plans/cancel-turn-progress/task-06-cancel-reload-regression-e2e.md +++ b/docs/plans/cancel-turn-progress/task-06-cancel-reload-regression-e2e.md @@ -5,14 +5,22 @@ status: completed wave: 4 depends_on: ["05-backend-owned-cancel-control"] plan: "plan.md" -spec: "../../specs/ui/requirements/cancel-turn-progress.md" +requirements: + - REQ-UI-CANCEL-TURN-PROGRESS-001 +acceptance_criteria: + - AC-UI-CANCEL-TURN-PROGRESS-001.2 + - AC-UI-CANCEL-TURN-PROGRESS-001.3 + - AC-UI-CANCEL-TURN-PROGRESS-001.7 + - AC-UI-CANCEL-TURN-PROGRESS-001.8 +system_design: + - ../../specs/ui/system-design/cancel-turn-progress.md --- # Task 06: Cancel reload regression ## Acceptance -- The desktop regression sends cancellation to the backend during an existing slow mock turn, +- The desktop regression sends cancellation to the backend during the E2E cancellation-hold mock turn, observes backend `cancellation_pending=true`, switches tasks, returns, reloads, and sees the same disabled animated control until cancellation settles. - A `mobile-chrome` regression taps cancel, waits for backend-owned pending state, reloads, and @@ -52,7 +60,7 @@ Sequential. This task validates the complete backend-to-hydration-to-control pat - Spec: task navigation, reload, session-isolation, failure, and compact-mobile scenarios. - Plan: `E2E Tests` and `Mobile design contract`. -- Existing patterns: the `/slow` mock-agent command, `SessionPage.clickTaskInSidebar`, +- Existing patterns: the `/e2e:cancel-hold` mock-agent command, `SessionPage.clickTaskInSidebar`, `page.reload()`, `waitForActiveSessionForegroundActivity`, and mobile-only spec naming. ## Output contract @@ -76,3 +84,12 @@ attempt used the desktop keyboard helper; it was corrected to submit through the final tests observe backend `cancellation_pending=true`, verify the disabled spinner after navigation/reload, and wait for the explicit `false` settle. The old held-frame helper was removed, and each run used only the isolated mock backend/workspace. + +The original `/slow 30s` fixture divided its delay across several emitter boundaries, so a real +cancellation could settle before navigation and hydration completed. The `/e2e:cancel-hold` fixture +has one cancellation boundary and emits no assistant output in every mock-agent run. The managed E2E +profile supplies a three-second `PromptCancelJoinTimeout` baseline through `AgentctlStartupConfig`, +the process adapter, and ACP; these two cancel-progress regressions scope a 12-second override, and +zero preserves ACP's three-second default for normal startup. Fresh managed Chromium repeats +passed five desktop task-switch executions and five mobile reload executions, with the pending +control observed before eventual idle settlement. diff --git a/docs/specs/ui/system-design/cancel-turn-progress.md b/docs/specs/ui/system-design/cancel-turn-progress.md new file mode 100644 index 0000000000..7c38411473 --- /dev/null +++ b/docs/specs/ui/system-design/cancel-turn-progress.md @@ -0,0 +1,79 @@ +--- +status: current +system: ui +requirements: + - REQ-UI-CANCEL-TURN-PROGRESS-001 +--- + +# Backend-owned Cancel-turn Progress System Design + +## Purpose and boundaries + +This design keeps the cancel control tied to the cancellation operation that the backend accepts, +instead of to the browser component that requested it. The UI system owns the shared control's +rendering and hydration behavior. The orchestrator and agent runtime retain ownership of execution +cancellation, session lifecycle, and prompt termination. + +## Requirement mapping + +| Requirement | Design sections | +| --- | --- | +| `REQ-UI-CANCEL-TURN-PROGRESS-001` | [Runtime projection and client contract](#runtime-projection-and-client-contract), [Control flow](#control-flow), [Responsive behavior](#responsive-behavior), [Verification design](#verification-design) | + +## Runtime projection and client contract + +The orchestrator exposes a per-session `cancellation_pending` value and monotonically increasing +`cancellation_revision`. Task-session boot and REST payloads, subscription snapshots, and +`session.cancellation_changed` notifications carry both values. Clients merge a notification only +when its revision is current, so a delayed hydration response cannot replace a newer transition. + +The shared chat cancel control combines this backend state with a short-lived local request flag. +The local flag supplies immediate feedback before acceptance; once the backend accepts, the runtime +projection is authoritative across remounts, route changes, and replacement pages. The projection +is not a durable session lifecycle state and is not persisted across backend restart. + +## Control flow + +1. A user activates the existing `agent.cancel` action for a running session. +2. The backend guards the per-session cancellation operation, publishes pending, and starts or joins + lifecycle cancellation. +3. The current client and any later task-session hydration render the disabled loading control from + the projection. +4. After lifecycle and session reconciliation settle, the backend publishes a false value with the + next revision; the shared control returns to its ordinary state. + +The mock agent recognizes `/e2e:cancel-hold` in every mock-agent run. Its prompt waits at one +predictable cancellation boundary and produces no assistant output, allowing browser regressions to +observe the backend-owned pending operation without intercepting a browser request. The E2E profile +sets a three-second `PromptCancelJoinTimeout` baseline through private `AgentctlStartupConfig`, the +process adapter, and ACP. The two cancel-progress regressions scope a 12-second override through +their backend fixture; a zero value leaves ACP's normal three-second join bound in effect. + +## Responsive behavior + +Desktop task navigation remounts the same shared cancel control after returning to a task. On mobile, +the compact composer hydrates the same session projection after a full reload. Both surfaces preserve +their existing composition, controls, and touch behavior; no viewport-specific cancellation state or +new user-facing copy exists. + +## Failure and recovery + +If a cancel request is rejected before backend acceptance, the local request flag clears and no +pending projection is published. A missed live event is repaired by the next boot, REST, or +subscription snapshot. On success, reconciliation, rejection, or a bounded cancellation timeout, +the backend releases pending so a still-running session is retryable. + +The E2E profile scopes the fixture timing and cancellation-join configuration to test runs. Production +and non-E2E managed launch paths retain their existing cancellation timing and ACP default join bound. + +## Verification design + +Mock-agent tests prove the hold does not emit assistant output. Configuration and process tests prove +the managed E2E timeout reaches ACP and that zero keeps the default. Desktop Chromium repeats prove +the disabled loading control remains visible through task navigation; mobile Chromium repeats prove +the same behavior through reload hydration. Both flows wait for the authoritative pending value and +eventual settlement rather than using a browser delay. + +## Related decisions + +- [ADR-2026-08-03: Keep Cancellation Progress Backend Owned](../../../decisions/2026-08-03-backend-owned-cancellation-progress.md)