Merge pull request #9898 from SivanCola/fix/stop-unconditional-interrupt

fix(control,desktop): make Stop a session-level interrupt / 停止按钮改为会话级无条件中断

desktop/frontend/scripts/check-bundle-budget.mjs

@@ -286,7 +286,9 @@ for (const path of localeChunks) {
   // 61.027/61.881 KiB; retain bounded cross-platform headroom.
   // Recovery retry copy reaches the rounded 61.1 KiB boundary on Node/zlib
   // toolchains; keep the next one-decimal ceiling for cross-platform CI.
-  const budget = name.startsWith("zh-TW-") ? 62.0 * 1024 : 61.2 * 1024;
+  // The #9889/#9890 series adds recovery-wait, dialog-failure, and stall copy:
+  // zh-TW measures 63492 B (62.004 KiB) with the four PRs merged together.
+  const budget = name.startsWith("zh-TW-") ? 62.1 * 1024 : 61.2 * 1024;
   assertBudget(`${name} gzip`, gzipBytes(path), budget);
 }
 
@@ -393,6 +395,9 @@ const rawInitialBytes = [...initialJS, ...initialCSS, ...appShellCSS]
 // 2496.6 KiB; retain the smallest bounded ceiling.
 // The complete block renderer and input ownership gates measure 2371.7 KiB
 // on the settings + pure-kernel baseline. Keep the smallest bounded ceiling.
-const rawInitialBudgetKiB = 2_371.8;
+// The #9889/#9890 series (#9898, #9899, #9900, #9901) measures 2431847 B
+// (2374.851 KiB) merged together over a 2428639 B base; the four PRs share
+// this ceiling so they merge in any order. Re-measure once the series lands.
+const rawInitialBudgetKiB = 2_375.0;
 assertBudget("initial raw JavaScript and CSS", rawInitialBytes, rawInitialBudgetKiB * 1024);
 assertBudget("largest initial JavaScript chunk raw", largestInitialJSRaw, 1_000 * 1024);

desktop/frontend/src/__tests__/use-controller-cancel-reconcile.test.tsx

@@ -106,6 +106,8 @@ let cancelCalls = 0;
 let cancelInboxCalls = 0;
 let cancelInboxError: Error | null = null;
 let cancelDiscardedItemIDs: string[] = [];
+let interruptCalls = 0;
+let interruptError: Error | null = null;
 let effortCalls = 0;
 let checkpointHistoryCalls = 0;
 let historyLoads = 0;
@@ -194,6 +196,11 @@ window.go = {
         backendRunning = false;
         return { discardedItemIds: [...cancelDiscardedItemIDs] };
       },
+      InterruptTurnForTab: async () => {
+        interruptCalls += 1;
+        if (interruptError) throw interruptError;
+        backendRunning = false;
+      },
     } as Partial<AppBindings> as AppBindings,
   },
 };
@@ -342,6 +349,43 @@ eq(cancelInboxCalls, 2, "receipt-capable cancellation is called for durable guid
 ok(Boolean(inboxCancelNotice), "cancel failure formats the stable inbox code for the active locale");
 ok(inboxCancelNotice?.kind === "notice" && !inboxCancelNotice.text.includes("reasonix_error:"), "cancel failure never renders the stable transport code");
 
+// Stop is a session-level request: an exact-turn fence rejection (stale or
+// replaced turn id) must fall back to the unconditional cancel instead of
+// leaving the user with a "Cancel failed" notice and a running turn.
+const noticesBefore = controller?.state.items.filter((item) => item.kind === "notice").length ?? 0;
+const cancelCallsBefore = cancelCalls;
+backendRunning = true;
+interruptError = new Error('turn "turn-live" is not the active turn for tab "tab-a"');
+await act(async () => {
+  for (const handler of eventHandlers) handler({ kind: "turn_started", tabId: "tab-a", turnId: "turn-live" });
+  await flushPromises();
+});
+eq(controller?.state.activeTurnId, "turn-live", "turn_started with a turn id records the active turn");
+await act(async () => {
+  await controller?.cancel();
+  await flushPromises();
+});
+eq(interruptCalls, 1, "exact-turn stop is attempted first");
+eq(cancelCalls, cancelCallsBefore + 1, "fence rejection falls back to the unconditional CancelTab");
+eq(controller?.state.items.filter((item) => item.kind === "notice").length, noticesBefore, "fence rejection does not surface a Cancel failed notice");
+await waitFor("fallback cancel reconciliation", () => controller?.state.running === false);
+
+// An idle backend answers with a stable code; the UI reconciles quietly.
+backendRunning = false;
+interruptError = new Error("reasonix_error:turn_not_running");
+await act(async () => {
+  for (const handler of eventHandlers) handler({ kind: "turn_started", tabId: "tab-a", turnId: "turn-idle" });
+  await flushPromises();
+});
+await act(async () => {
+  await controller?.cancel();
+  await flushPromises();
+});
+eq(interruptCalls, 2, "idle stop still asks the backend once");
+eq(cancelCalls, cancelCallsBefore + 1, "idle stop does not retry through CancelTab");
+eq(controller?.state.items.filter((item) => item.kind === "notice").length, noticesBefore, "idle stop does not surface a Cancel failed notice");
+await waitFor("idle stop reconciliation", () => controller?.state.running === false);
+
 await act(async () => {
   root.unmount();
 });

desktop/frontend/src/lib/inboxCancel.ts

@@ -1,3 +1,5 @@
+import { isTurnNotRunning } from "./inboxError";
+
 export type InboxCancelReceipt = {
   discardedItemIds: string[];
   warning?: string;
@@ -15,6 +17,24 @@ type InboxCancelBridge = {
   InterruptTurnWithInboxItemsForTab?(tabId: string, turnId: string, itemIds: string[]): Promise<InboxCancelReceipt>;
 };
 
+// Stop is a session-level request: a turn-id fence rejection (stale or
+// replaced turn) still stops whatever is running now, and an idle backend is
+// not a failure. Only the unconditional path's own error reaches the caller.
+export async function requestSessionCancel(
+  app: InboxCancelBridge,
+  tabId: string,
+  itemIds: string[],
+  turnId?: string,
+): Promise<InboxCancelReceipt> {
+  try {
+    return await requestInboxCancel(app, tabId, itemIds, turnId);
+  } catch (error) {
+    if (isTurnNotRunning(error)) return { discardedItemIds: [] };
+    if (!turnId) throw error;
+    return requestInboxCancel(app, tabId, itemIds, undefined);
+  }
+}
+
 export async function requestInboxCancel(
   app: InboxCancelBridge,
   tabId: string,

desktop/frontend/src/lib/inboxError.ts

@@ -121,6 +121,11 @@ export function isInboxItemMissing(error: unknown): boolean {
   return raw === `${CODE_PREFIX}inbox_item_not_found` || raw === "inbox item not found";
 }
 
+// The idle-tab code is consumed by the Stop path, never displayed.
+export function isTurnNotRunning(error: unknown): boolean {
+  return errorText(error) === `${CODE_PREFIX}turn_not_running`;
+}
+
 export function formatInboxCancelError(error: unknown, locale: Locale): string {
   return ERROR_COPY[locale][CANCEL_FAILED_INDEX].replace("{error}", formatInboxError(error, locale));
 }

desktop/frontend/src/lib/useController.ts

@@ -12,7 +12,7 @@ import { formatInboxCancelError } from "./inboxError";
 import { settleForkConversationForTab } from "./forkWorktree";
 import type { MessageActionScope, MessageActionState } from "./messageActions";
 import { mergeRateBand, type AggregatedRateBand } from "./costRateBand";
-import { requestInboxCancel, type CancelOutcome } from "./inboxCancel";
+import { requestSessionCancel, type CancelOutcome } from "./inboxCancel";
 import { answerPromptForActiveTurn, normalizeTurnSubmit, resolveActiveTurnId, resolvePromptForTab } from "./inboxSubmit";
 import { findTabAfterSubmitFailure, reduceManagementConfirmation, reduceSubmitFailure } from "./turnSubmissionFailure";
 import { formatContextMaintenanceNotice, isNewMaintenanceOperation, rememberMaintenanceOperation } from "./contextMaintenanceTypes";
@@ -3921,14 +3921,14 @@ export function useController() {
       if (!turnId && exactAPIAvailable) {
         turnId = await resolveActiveTurnId(app, tabId);
       }
-      if (exactAPIAvailable && !turnId) throw new Error("active turn id is unavailable; refresh and try Stop again");
-      const result = await requestInboxCancel(app, tabId, inboxItemIDs, turnId);
-      scheduleCancelReconcile(tabId, 0);
+      const result = await requestSessionCancel(app, tabId, inboxItemIDs, turnId);
       if (result.warning) dispatchTo(tabId, { type: "local_notice", level: "warn", text: result.warning });
       return result;
     } catch (error) {
       dispatchTo(tabId, { type: "local_notice", level: "warn", text: formatInboxCancelError(error, getLocale()) });
       return { discardedItemIds: [] };
+    } finally {
+      scheduleCancelReconcile(tabId, 0);
     }
   }, [bumpCancelHydrateSeq, dispatchTo, scheduleCancelReconcile]);
 

desktop/turn_runtime_api.go

@@ -1,7 +1,9 @@
 package main
 
 import (
+	"errors"
 	"fmt"
+	"log/slog"
 	"strings"
 
 	"reasonix/internal/control"
@@ -37,6 +39,23 @@ func (a *App) validatePromptIdentity(tabID, turnID, runtimeEpoch string) (contro
 	return ctrl, nil
 }
 
+// stoppableCtrl resolves the controller a Stop request targets. Only an idle
+// tab is rejected; a stale turn id is logged and the active work still stops.
+func (a *App) stoppableCtrl(tabID, turnID string) (control.SessionAPI, error) {
+	tab, ctrl := a.tabAndCtrlByID(tabID)
+	if ctrl == nil {
+		return nil, a.workspaceNotReadyErr(tab)
+	}
+	status := ctrl.RuntimeStatus()
+	if !status.Running && !status.Cancellable {
+		return nil, errTurnNotRunning
+	}
+	if turnID = strings.TrimSpace(turnID); turnID != status.TurnID {
+		slog.Info("desktop: stop targeted a stale turn id; interrupting the active turn", "tab", tabID, "requested", turnID, "active", status.TurnID)
+	}
+	return ctrl, nil
+}
+
 // StartTurnForTab is the turn-id-aware replacement for SubmitToTab. Existing
 // Submit entry points remain compatibility wrappers during the protocol cutover.
 func (a *App) StartTurnForTab(tabID, input, submissionID string) (TurnStartView, error) {
@@ -71,37 +90,29 @@ func (a *App) StartTurnForTab(tabID, input, submissionID string) (TurnStartView,
 	return TurnStartView{TurnID: turnID, Status: event.TurnQueued, Disposition: control.SubmitTurnStarted, RuntimeEpoch: epoch, SubmissionID: submissionID}, nil
 }
 
-// InterruptTurnForTab cancels only the exact active turn. A stale Stop button
-// can no longer cancel a replacement turn admitted in the same tab.
+// errTurnNotRunning tells the frontend the tab is already idle so it can
+// reconcile its runtime view instead of reporting a failed Stop.
+var errTurnNotRunning = &inboxCodedError{code: "turn_not_running", cause: errors.New("no turn is running")}
+
+// InterruptTurnForTab stops the tab's active work. Stop is a session-level
+// request: a turn id from a stale button still interrupts whatever is running
+// now, because an unstoppable turn is worse than stopping its replacement.
 func (a *App) InterruptTurnForTab(tabID, turnID string) error {
-	turnID = strings.TrimSpace(turnID)
-	if turnID == "" {
-		return fmt.Errorf("turnId is required")
-	}
-	tab, ctrl := a.tabAndCtrlByID(tabID)
-	if ctrl == nil {
-		return a.workspaceNotReadyErr(tab)
-	}
-	status := ctrl.RuntimeStatus()
-	if status.TurnID != turnID || !status.Running {
-		return fmt.Errorf("turn %q is not the active turn for tab %q", turnID, tabID)
+	ctrl, err := a.stoppableCtrl(tabID, turnID)
+	if err != nil {
+		return err
 	}
 	ctrl.Cancel()
 	return nil
 }
 
-// InterruptTurnWithInboxItemsForTab is the receipt-capable exact-turn Stop
-// used by the Composer when it also discards queued follow-ups.
+// InterruptTurnWithInboxItemsForTab is the receipt-capable Stop used by the
+// Composer when it also discards queued follow-ups.
 func (a *App) InterruptTurnWithInboxItemsForTab(tabID, turnID string, itemIDs []string) (InboxCancelResultView, error) {
 	view := InboxCancelResultView{DiscardedItemIDs: []string{}}
-	turnID = strings.TrimSpace(turnID)
-	tab, ctrl := a.tabAndCtrlByID(tabID)
-	if ctrl == nil {
-		return view, a.workspaceNotReadyErr(tab)
-	}
-	status := ctrl.RuntimeStatus()
-	if turnID == "" || status.TurnID != turnID || !status.Running {
-		return view, fmt.Errorf("turn %q is not the active turn for tab %q", turnID, tabID)
+	ctrl, err := a.stoppableCtrl(tabID, turnID)
+	if err != nil {
+		return view, err
 	}
 	result, err := ctrl.CancelWithInboxItemsResult(itemIDs, "desktop")
 	if err != nil {

desktop/turn_runtime_api_test.go

@@ -56,9 +56,6 @@ func TestTurnRuntimeAPIRoutesStopAnswerAndReplayByExactTurn(t *testing.T) {
 		t.Fatal("turn runner did not start")
 	}
 
-	if err := app.InterruptTurnForTab(tab.ID, "turn_stale"); err == nil {
-		t.Fatal("stale turn id cancelled the active turn")
-	}
 	if err := app.AnswerPromptForTab(tab.ID, "turn_stale", "prompt-1", nil); err == nil {
 		t.Fatal("stale turn id answered an active turn prompt")
 	}
@@ -103,6 +100,55 @@ func TestTurnRuntimeAPIRoutesStopAnswerAndReplayByExactTurn(t *testing.T) {
 	}
 }
 
+// A Stop button rendered for an earlier turn must still stop the turn that is
+// running now; only an idle tab is reported back, with a stable code.
+func TestInterruptTurnForTabStopsActiveWorkDespiteStaleTurnID(t *testing.T) {
+	dir := t.TempDir()
+	runner := &exactTurnRunner{started: make(chan struct{})}
+	sink := &tabEventSink{tabID: "tab", ctx: context.Background()}
+	terminal := make(chan event.Event, 1)
+	sink.SetBotSink(event.FuncSink(func(e event.Event) {
+		if e.Kind == event.TurnDone {
+			terminal <- e
+		}
+	}))
+	ctrl := control.New(control.Options{
+		Runner: runner, Sink: sink, SessionDir: dir,
+		SessionPath: filepath.Join(dir, "session.jsonl"),
+	})
+	t.Cleanup(ctrl.Close)
+	tab := &WorkspaceTab{ID: "tab", Scope: "global", Ready: true, Ctrl: ctrl, sink: sink}
+	app := &App{tabs: map[string]*WorkspaceTab{tab.ID: tab}, activeTabID: tab.ID}
+	sink.app = app
+
+	if err := app.InterruptTurnForTab(tab.ID, "turn_none"); !errors.Is(err, errTurnNotRunning) {
+		t.Fatalf("idle stop = %v, want %v", err, errTurnNotRunning)
+	}
+	if err := app.InterruptTurnForTab(tab.ID, "turn_none"); err == nil || err.Error() != "reasonix_error:turn_not_running" {
+		t.Fatalf("idle stop wire error = %v, want stable reasonix_error code", err)
+	}
+
+	if _, err := app.StartTurnForTab(tab.ID, "hold this turn", "submission-1"); err != nil {
+		t.Fatalf("StartTurnForTab: %v", err)
+	}
+	select {
+	case <-runner.started:
+	case <-time.After(5 * time.Second):
+		t.Fatal("turn runner did not start")
+	}
+	if err := app.InterruptTurnForTab(tab.ID, "turn_stale"); err != nil {
+		t.Fatalf("stale-id stop = %v, want the active turn interrupted", err)
+	}
+	select {
+	case done := <-terminal:
+		if done.Status != event.TurnInterrupted {
+			t.Fatalf("terminal status = %q, want interrupted", done.Status)
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("turn did not reach terminal state after stale-id stop")
+	}
+}
+
 func TestStartTurnForTabReturnsManagementDispositionWithoutTurnID(t *testing.T) {
 	dir := t.TempDir()
 	sink := &tabEventSink{tabID: "tab", ctx: context.Background()}

internal/control/cancel.go

@@ -0,0 +1,51 @@
+package control
+
+import "reasonix/internal/event"
+
+// Cancel aborts the in-flight turn. A goroutine blocked awaiting approval
+// unblocks via the cancelled context.
+func (c *Controller) Cancel() {
+	c.promptResolveMu.Lock()
+	turnID, cancelled := c.cancelTurnLocked()
+	c.promptResolveMu.Unlock()
+	c.finishCancel(turnID, cancelled)
+}
+
+// cancelLocked is Cancel for callers that already hold promptResolveMu.
+func (c *Controller) cancelLocked() {
+	turnID, cancelled := c.cancelTurnLocked()
+	c.finishCancel(turnID, cancelled)
+}
+
+// cancelTurnLocked signals the turn before any observable work: the status
+// emit that follows is a synchronous event barrier, and a stalled event lane
+// must never keep the provider stream or a tool process alive after Stop.
+func (c *Controller) cancelTurnLocked() (string, bool) {
+	c.mu.Lock()
+	cancel := c.cancel
+	if cancel != nil {
+		c.canceling = true
+	}
+	c.mu.Unlock()
+	if cancel == nil {
+		return "", false
+	}
+	turnID := ""
+	if ledger := c.turnEventLedger(); ledger != nil {
+		turnID = ledger.ActiveTurnID()
+	}
+	cancel()
+	c.promptOwner.CancelAll()
+	c.approval.clearAll()
+	return turnID, true
+}
+
+func (c *Controller) finishCancel(turnID string, cancelled bool) {
+	if cancelled {
+		c.emitTurnStatus(event.TurnCancelling, turnID)
+		return
+	}
+	if c.goals.active() {
+		c.stopGoal(GoalStatusStopped)
+	}
+}

internal/control/cancel_ordering_test.go

@@ -0,0 +1,104 @@
+package control
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"reasonix/internal/event"
+)
+
+// Stop must reach the turn context before the cancelling status crosses the
+// synchronous event barrier; a stalled sink cannot be allowed to keep the
+// provider stream or a tool process alive.
+func TestCancelSignalsTurnBeforeStatusBarrier(t *testing.T) {
+	releaseStatus := make(chan struct{})
+	statusEntered := make(chan struct{}, 1)
+	c := New(Options{Sink: event.FuncSink(func(e event.Event) {
+		if e.Kind == event.TurnStatusChanged && e.Status == event.TurnCancelling {
+			statusEntered <- struct{}{}
+			<-releaseStatus
+		}
+	})})
+	t.Cleanup(c.Close)
+
+	turnCtxDone := make(chan struct{})
+	releaseTurn := make(chan struct{})
+	started := make(chan struct{})
+	c.runGuarded(func(ctx context.Context) error {
+		close(started)
+		<-ctx.Done()
+		close(turnCtxDone)
+		// Hold the turn open so TurnDone cannot race ahead of the cancelling
+		// status; the assertion is about ordering inside Cancel itself.
+		<-releaseTurn
+		return ctx.Err()
+	})
+	<-started
+	defer close(releaseTurn)
+
+	cancelReturned := make(chan struct{})
+	go func() {
+		c.Cancel()
+		close(cancelReturned)
+	}()
+	select {
+	case <-turnCtxDone:
+	case <-time.After(5 * time.Second):
+		close(releaseStatus)
+		t.Fatal("turn context was not cancelled before the status barrier")
+	}
+	select {
+	case <-statusEntered:
+	case <-time.After(5 * time.Second):
+		close(releaseStatus)
+		t.Fatal("cancel never emitted the cancelling status")
+	}
+	select {
+	case <-cancelReturned:
+		close(releaseStatus)
+		t.Fatal("Cancel returned before the status barrier drained")
+	default:
+	}
+	close(releaseStatus)
+	select {
+	case <-cancelReturned:
+	case <-time.After(5 * time.Second):
+		t.Fatal("Cancel did not return after the barrier was released")
+	}
+}
+
+// A cancelling status stamped for a turn that already terminated must not turn
+// the next admitted turn into a permanently "cancelling" one.
+func TestStaleCancellingStatusDoesNotStickToNextTurn(t *testing.T) {
+	dir := t.TempDir()
+	done := make(chan event.Event, 4)
+	c := New(Options{SessionDir: dir, SessionPath: dir + "/session.jsonl", Sink: event.FuncSink(func(e event.Event) {
+		if e.Kind == event.TurnDone {
+			done <- e
+		}
+	})})
+	t.Cleanup(c.Close)
+
+	c.runGuarded(func(context.Context) error { return nil })
+	first := waitTurnDoneEvent(t, done)
+	if first.TurnID == "" {
+		t.Fatal("first turn has no ledger id")
+	}
+
+	started := make(chan struct{})
+	c.runGuarded(func(ctx context.Context) error {
+		close(started)
+		<-ctx.Done()
+		return ctx.Err()
+	})
+	<-started
+	c.emitTurnStatus(event.TurnCancelling, first.TurnID)
+	if st := c.RuntimeStatus(); st.Status == event.TurnCancelling || st.CancelRequested {
+		t.Fatalf("stale cancelling status leaked into the next turn: %+v", st)
+	}
+	c.Cancel()
+	if second := waitTurnDoneEvent(t, done); second.Status != event.TurnInterrupted {
+		t.Fatalf("second turn terminal = %q, want interrupted", second.Status)
+	}
+}

internal/control/controller.go

@@ -2127,33 +2127,6 @@ func (c *Controller) RunSubagentProfile(ctx context.Context, name, task string,
 	return tool.GuardSubagentHostDecisionText(answer), nil
 }
 
-// Cancel aborts the in-flight turn. A goroutine blocked awaiting approval
-// unblocks via the cancelled context.
-func (c *Controller) Cancel() {
-	c.promptResolveMu.Lock()
-	defer c.promptResolveMu.Unlock()
-	c.cancelLocked()
-}
-
-func (c *Controller) cancelLocked() {
-	c.mu.Lock()
-	cancel := c.cancel
-	if cancel != nil {
-		c.canceling = true
-	}
-	c.mu.Unlock()
-	if cancel != nil {
-		c.emitTurnStatus(event.TurnCancelling)
-		c.promptOwner.CancelAll()
-		c.approval.clearAll()
-		cancel()
-		return
-	}
-	if c.goals.active() {
-		c.stopGoal(GoalStatusStopped)
-	}
-}
-
 // beginRotation claims the session-rotation gate. It fails if a turn is running
 // or another rotation is already in progress, so the caller holds exclusive
 // rights to swap the executor session from the check here through endRotation.

internal/control/turn_events.go

@@ -156,6 +156,9 @@ func (s *turnEventSink) persistAndPublish(e event.Event) error {
 		s.publishInner(e)
 		return nil
 	}
+	if staleTurnStatus(e, ledger) {
+		return nil
+	}
 	// Outside-turn notices are not lifecycle records and must pass through after
 	// bootstrap or a terminal event.
 	if ledger.ActiveTurnID() == "" {
@@ -403,11 +406,19 @@ func (c *Controller) failTurnEventLedger(err error) {
 	}
 }
 
-func (c *Controller) emitTurnStatus(status event.TurnStatus) {
+// staleTurnStatus reports a status stamped for a turn that has since reached
+// its terminal event; cancelling is sticky, so it must not reach the next turn.
+func staleTurnStatus(e event.Event, ledger *turnevent.Ledger) bool {
+	return e.Kind == event.TurnStatusChanged && e.TurnID != "" && e.TurnID != ledger.ActiveTurnID()
+}
+
+// emitTurnStatus stamps the transition with the turn that requested it so the
+// ledger can drop it if that turn already reached its terminal event.
+func (c *Controller) emitTurnStatus(status event.TurnStatus, turnID string) {
 	if c == nil || status == "" {
 		return
 	}
-	c.sink.Emit(event.Event{Kind: event.TurnStatusChanged, Status: status})
+	c.sink.Emit(event.Event{Kind: event.TurnStatusChanged, Status: status, TurnID: turnID})
 }
 
 // emitTurnEventChecked reaches the lifecycle sink below the inbox observer so