From de9d618f0c66d89398496bc804e2c1e29a0ecdf9 Mon Sep 17 00:00:00 2001
From: kingwl <kingwenlu@gmail.com>
Date: Fri, 10 Jul 2026 16:28:11 +0800
Subject: [PATCH] fix(mode): a failed flush re-parks the pending intent instead
 of dropping it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Review hardening (the finding's ordering premise did not hold — see the
PR thread — but its failure-path kernel did): onBoundary cleared the
pending intent BEFORE appending the mode/set, so a backend rejecting
that one write lost the switch forever while the picker kept showing it
optimistically. The intent is now cleared only after the append lands;
a failed flush stays parked and the next healthy boundary converges the
log with the picker. The containment test extends to pin the re-park
and the retry.

The bridge's re-notify keeps deriving from the logged event's value —
now documented in place: the service holds ONE coalesced pending slot
(every flush reads the latest selection, so a stale flush cannot
exist), and for any other writer the logged value is the truth the
picker should track, in log order.
---
 packages/mode/mode/src/index.ts       | 11 +++++++++--
 packages/mode/mode/tests/mode.spec.ts |  8 ++++++++
 packages/ui/acp/src/index.ts          |  5 +++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/packages/mode/mode/src/index.ts b/packages/mode/mode/src/index.ts
index 917a1529fc..8f83e23811 100644
--- a/packages/mode/mode/src/index.ts
+++ b/packages/mode/mode/src/index.ts
@@ -398,10 +398,17 @@ export class ModesService extends Service {
     if (turnStart) this.noticeDroppedDefinition(session)
     const pending = this.pendingIntents.get(session)
     if (pending === undefined) return
-    this.pendingIntents.delete(session)
     const target = pending.mode
-    if (target === foldMode(session.events)) return
+    if (target === foldMode(session.events)) {
+      this.pendingIntents.delete(session)
+      return
+    }
     session.append('mode/set', { mode: target })
+    // Clear the intent only AFTER the append landed: if a backend rejects the
+    // write, the intent stays parked and the next boundary retries — the UI's
+    // optimistic picker state and the log re-converge instead of diverging
+    // forever on a swallowed one-shot.
+    this.pendingIntents.delete(session)
     if (!pending.narrate) return
     const told = modeAtLastHeader(session.events)
     if (told === undefined || told === target) return
diff --git a/packages/mode/mode/tests/mode.spec.ts b/packages/mode/mode/tests/mode.spec.ts
index 1aca78d44d..7ca37354b5 100644
--- a/packages/mode/mode/tests/mode.spec.ts
+++ b/packages/mode/mode/tests/mode.spec.ts
@@ -278,6 +278,14 @@ describe('the boundary flush', () => {
     agent.session.append = (() => { throw new Error('backend gone') })
     ctx.emit('session/event', agent.session, event as SessionEvent)
     expect(warn).toHaveBeenCalledOnce()
+    // The failed flush re-parks the intent (cleared only after a landed
+    // append), so the next healthy boundary converges the log with the
+    // picker's optimistic state instead of dropping the switch forever.
+    expect(ctx.modes.get(agent)).toEqual({ current: DEFAULT_MODE, pending: PLAN_MODE })
+    agent.session.append = original
+    boundary(ctx, agent.session, 'step/end')
+    expect(foldMode(agent.session.events)).toBe(PLAN_MODE)
+    expect(ctx.modes.get(agent).pending).toBeUndefined()
   })
 })
 
diff --git a/packages/ui/acp/src/index.ts b/packages/ui/acp/src/index.ts
index 2d9d48854c..128427c84c 100644
--- a/packages/ui/acp/src/index.ts
+++ b/packages/ui/acp/src/index.ts
@@ -511,6 +511,11 @@ export function apply(ctx: Context, config: AcpConfig): void {
       enabled: rec.terminalEnabled,
       cwd: session.header.cwd,
     }, { includeUserMessages: false })
+    // Re-notify from the EVENT's value, not from modes.get(): the service
+    // holds one coalesced pending slot (every flush reads the latest
+    // selection, so a flush can never be stale against the picker), and for
+    // any other writer — the exit tool, a test, a foreign plugin — the logged
+    // value IS the truth the picker should track, in log order.
     if (event.type === 'mode/set' && event.data.mode !== rec.lastModeId) {
       rec.lastModeId = event.data.mode
       notify({ sessionId: rec.sessionId, update: { sessionUpdate: 'current_mode_update', currentModeId: event.data.mode } })