feat(daemon): keep model & approval-mode state consistent across clients sharing a session

packages/acp-bridge/src/bridgeClient.test.ts

@@ -17,12 +17,12 @@
  *      proxy is fully bypassed — no `fs.writeFile` syscall);
  *   2. when `fileSystem` is omitted, the inline proxy runs and
  *      reads / writes real disk (sanity check that the fallback
- *      path the 7-arg constructor's positional slot opt-outs to
+ *      path the 8-arg constructor's positional slot opt-outs to
  *      still works).
  *
- * Regression guard: the constructor takes 7 positional args; the
- * 7th (`fileSystem`) is optional and at the tail. A subtle re-
- * ordering (or dropping the arg from `bridge.ts:773` factory's
+ * Regression guard: the constructor takes 8 positional args; the
+ * 6th (`fileSystem`) is optional. A subtle re-ordering (or
+ * dropping the arg from `bridge.ts`'s factory
  * `new BridgeClient(..., opts.fileSystem)` call) would silently
  * bypass the adapter in production. Test #1 + #2 catch that
  * because the mock fileSystem would never be called.

packages/acp-bridge/src/bridgeClient.ts

@@ -119,6 +119,19 @@ const MAX_EARLY_EVENTS_PER_SESSION = 32;
 const MAX_SUGGESTION_LENGTH = 500;
 const EARLY_EVENT_TTL_MS = 60_000;
 
+// Known approval-mode ids accepted on the in-session `current_mode_update`
+// demux path. Mirrors the `modeMap` keys in `Session.setMode` (CLI); an id
+// outside this set is dropped before it fans out to SSE clients / the SDK
+// reducer. Keep the two in lockstep. Exported so the bridge's reconcile and
+// snapshot-seed paths apply the same enum backstop to agent-supplied mode ids.
+export const KNOWN_APPROVAL_MODES: ReadonlySet<string> = new Set([
+  'plan',
+  'default',
+  'auto-edit',
+  'auto',
+  'yolo',
+]);
+
 /**
  * Human-readable label for a `fs.Stats` object's kind, used in the
  * `readTextFile` "not a regular file" rejection message (BX8YO).
@@ -195,6 +208,8 @@ export interface BridgeClientSessionEntry {
    * in `bridge.ts`; surfaced here for the demux.
    */
   modelRoundtripInFlight?: boolean;
+  /** A2: mirrors `modelRoundtripInFlight` for approval-mode roundtrips. */
+  approvalModeRoundtripInFlight?: boolean;
 }
 
 /**
@@ -271,6 +286,28 @@ export class BridgeClient implements Client {
      * companion — preserves the inline proxy behavior.
      */
     private readonly fileSystem?: BridgeFileSystem,
+    /**
+     * §2.3 callback: centralised `model_switched` publish through the
+     * bridge factory's cache-updating helper. The BridgeClient calls
+     * this instead of inlining `entry.events.publish(...)` so the
+     * cache update + generation bump stays atomic in one place.
+     */
+    private readonly onModelPromoted?: (
+      entry: BridgeClientSessionEntry,
+      modelId: string,
+      originatorClientId: string | undefined,
+    ) => void,
+    /**
+     * §2.3 / A2 callback: centralised `approval_mode_changed` publish.
+     * Called by the A2 `current_mode_update` demux when the agent
+     * switches approval mode in-session (exit_plan_mode, ProceedAlways,
+     * /mode). `previous` is read from the bridge state cache.
+     */
+    private readonly onModePromoted?: (
+      entry: BridgeClientSessionEntry,
+      modeId: string,
+      originatorClientId: string | undefined,
+    ) => void,
   ) {}
 
   async requestPermission(
@@ -438,6 +475,10 @@ export class BridgeClient implements Client {
       this.handleInSessionModelUpdate(params);
       return;
     }
+    if (method === 'qwen/notify/session/mode-update') {
+      this.handleInSessionModeUpdate(params);
+      return;
+    }
     if (method === 'qwen/notify/session/prompt-suggestion') {
       const sessionId = params['sessionId'];
       const suggestion = params['suggestion'];
@@ -532,20 +573,151 @@ export class BridgeClient implements Client {
       );
       return;
     }
-    try {
+    if (this.onModelPromoted) {
+      this.onModelPromoted(
+        entry,
+        currentModelId,
+        entry.activePromptOriginatorClientId,
+      );
+    } else {
+      // `EventBus.publish` never throws (closed bus → undefined no-op); per
+      // its documented contract we don't wrap it.
       entry.events.publish({
         type: 'model_switched',
         data: { sessionId, modelId: currentModelId },
         ...(entry.activePromptOriginatorClientId
           ? { originatorClientId: entry.activePromptOriginatorClientId }
           : {}),
       });
+    }
+    writeStderrLine(
+      `[demux] session=${sessionId} type=current_model_update action=promoted model=${currentModelId}`,
+    );
+  }
+
+  /**
+   * A2: promote an in-session `current_mode_update` extNotification to
+   * `approval_mode_changed`. Uses the same suppression pattern as
+   * `handleInSessionModelUpdate` — suppressed while the bridge is driving
+   * its own approval-mode roundtrip (`entry.approvalModeRoundtripInFlight`)
+   * — but diverges with two additions the model handler lacks: enum
+   * validation against `KNOWN_APPROVAL_MODES`, and a legacy
+   * `session_update{current_mode_update}` dual-emit for IDE companion
+   * compat (transition — see §6 of the design doc), itself deduped via the
+   * `legacyFrameSent` flag.
+   */
+  private handleInSessionModeUpdate(params: Record<string, unknown>): void {
+    const sessionId = params['sessionId'];
+    const currentModeId = params['currentModeId'];
+    if (typeof sessionId !== 'string' || typeof currentModeId !== 'string') {
+      return;
+    }
+    // Validate against the known approval-mode enum before it fans out.
+    // `Session.setMode` guards the symmetric send path with the same set
+    // ("an unknown id would call setApprovalMode(undefined), leaving the
+    // permission system undefined"); this is the receive path the agent
+    // can reach without that validation, so an unknown id here would
+    // propagate through `approval_mode_changed` to every SSE client and
+    // land in the SDK reducer's `state.approvalMode`. Keep in lockstep
+    // with `Session.setMode`'s `modeMap` keys (includes `auto`).
+    if (!KNOWN_APPROVAL_MODES.has(currentModeId)) {
+      writeStderrLine(
+        `[demux] session=${sessionId} type=current_mode_update action=dropped reason=unknown_mode mode=${currentModeId}`,
+      );
+      return;
+    }
+    const entry = this.resolveEntry(sessionId);
+    if (!entry) {
+      writeStderrLine(
+        `[demux] session=${sessionId} type=current_mode_update action=dropped reason=no_entry`,
+      );
+      return;
+    }
+    if (entry.approvalModeRoundtripInFlight) {
+      writeStderrLine(
+        `[demux] session=${sessionId} type=current_mode_update action=suppressed reason=bridge_roundtrip_in_flight`,
+      );
+      return;
+    }
+    if (this.onModePromoted) {
+      this.onModePromoted(
+        entry,
+        currentModeId,
+        entry.activePromptOriginatorClientId,
+      );
+    } else {
+      // Fallback path (no `onModePromoted` injected — tests / non-bridge
+      // consumers; production always wires the bridge callback). Mirror
+      // the main path's full payload: the SDK's
+      // `isApprovalModeChangedData` requires `previous` (non-empty
+      // string) and `persisted` (boolean), so a `{ sessionId, next }`
+      // shape fails validation and `asKnownDaemonEvent` drops the event.
+      // `previous` is unavailable on this path (the cache lives on the
+      // bridge's `SessionEntry`, not the demux interface), so seed it
+      // with the protocol default.
+      //
+      // `EventBus.publish` never throws (a closed bus is a return-undefined
+      // no-op and subscriber-enqueue failures are caught internally), so
+      // per its documented contract we don't wrap it in try/catch.
+      entry.events.publish({
+        type: 'approval_mode_changed',
+        data: {
+          sessionId,
+          previous: 'default',
+          next: currentModeId,
+          persisted: false,
+        },
+        ...(entry.activePromptOriginatorClientId
+          ? { originatorClientId: entry.activePromptOriginatorClientId }
+          : {}),
+      });
+    }
+    // TODO(dual-emit-removal): also emit the legacy generic
+    // `session_update{current_mode_update}` for one release cycle so the
+    // VS Code IDE companion's existing `case 'current_mode_update'`
+    // handler keeps working. Remove this block (and its tracking issue)
+    // once the companion ships an `approval_mode_changed` handler.
+    //
+    // Skip it when the producer already sent the legacy frame itself: the
+    // `exit_plan_mode` path (`Session.sendCurrentModeUpdateNotification`)
+    // calls `sendUpdate` before this extNotification, which
+    // `BridgeClient.sessionUpdate` already fanned onto the bus as the same
+    // `session_update{current_mode_update}` frame. Dual-emitting here would
+    // deliver it twice. The `setMode` path omits the flag (it has no
+    // `sendUpdate`), so its dual-emit still fires.
+    //
+    // Use the canonical ACP-nested shape (`data.update.sessionUpdate`),
+    // matching what `BridgeClient.sessionUpdate` publishes for a real
+    // `current_mode_update` notification. A flat
+    // `{ sessionId, sessionUpdate, currentModeId }` would (a) not be
+    // recognised by the companion's standard `data.update.sessionUpdate`
+    // switch, and (b) collide structurally with the real `session_update`
+    // the agent already emits on the `exit_plan_mode` path — leaving two
+    // incompatible shapes on the bus for one change.
+    if (params['legacyFrameSent'] === true) {
       writeStderrLine(
-        `[demux] session=${sessionId} type=current_model_update action=promoted model=${currentModelId}`,
+        `[demux] session=${sessionId} type=current_mode_update action=promoted mode=${currentModeId} legacy_frame=skipped`,
       );
-    } catch {
-      /* bus closed */
+      return;
     }
+    // `EventBus.publish` never throws (closed bus → undefined no-op); per its
+    // documented contract we don't wrap it in try/catch.
+    entry.events.publish({
+      type: 'session_update',
+      data: {
+        sessionId,
+        update: {
+          sessionUpdate: 'current_mode_update',
+          currentModeId,
+        },
+      },
+      ...(entry.activePromptOriginatorClientId
+        ? { originatorClientId: entry.activePromptOriginatorClientId }
+        : {}),
+    });
+    writeStderrLine(
+      `[demux] session=${sessionId} type=current_mode_update action=promoted mode=${currentModeId}`,
+    );
   }
 
   /**

packages/acp-bridge/src/bridgeTypes.ts

@@ -239,7 +239,10 @@ export interface AcpSessionBridge {
    */
   subscribeEvents(
     sessionId: string,
-    opts?: SubscribeOptions,
+    opts?: SubscribeOptions & {
+      /** Yield a synthetic `session_snapshot` frame after replay completes. */
+      snapshot?: boolean;
+    },
   ): AsyncIterable<BridgeEvent>;
 
   /**

packages/cli/src/acp-integration/session/Session.test.ts

@@ -352,6 +352,68 @@ describe('Session', () => {
 
       expect(mockConfig.setApprovalMode).toHaveBeenCalledWith(expected);
     });
+
+    it('emits a current_mode_update extNotification after switching (A2)', async () => {
+      await session.setMode({
+        sessionId: 'test-session-id',
+        modeId: 'auto-edit',
+      });
+
+      expect(mockClient.extNotification).toHaveBeenCalledWith(
+        'qwen/notify/session/mode-update',
+        expect.objectContaining({
+          v: 1,
+          sessionId: 'test-session-id',
+          currentModeId: 'auto-edit',
+        }),
+      );
+    });
+
+    it('rejects an unknown modeId and does NOT touch approval mode (A2)', async () => {
+      await expect(
+        session.setMode({
+          sessionId: 'test-session-id',
+          // eslint-disable-next-line @typescript-eslint/no-explicit-any
+          modeId: 'totally-bogus' as any,
+        }),
+      ).rejects.toThrow(/Unknown approval mode/);
+
+      expect(mockConfig.setApprovalMode).not.toHaveBeenCalled();
+      expect(mockClient.extNotification).not.toHaveBeenCalledWith(
+        'qwen/notify/session/mode-update',
+        expect.anything(),
+      );
+    });
+  });
+
+  describe('sendCurrentModeUpdateNotification', () => {
+    // The exit_plan_mode / edit-ProceedAlways path publishes the legacy
+    // `session_update{current_mode_update}` frame itself (via sendUpdate),
+    // so its extNotification must carry `legacyFrameSent: true` to stop the
+    // bridge demux from emitting a second, duplicate legacy frame. Unlike
+    // `setMode` (which omits the flag), a regression dropping it here would
+    // double-publish to the IDE companion. (A2)
+    it('marks the extNotification legacyFrameSent so the demux skips its dual-emit', async () => {
+      await (
+        session as unknown as {
+          sendCurrentModeUpdateNotification: (
+            outcome: core.ToolConfirmationOutcome,
+          ) => Promise<void>;
+        }
+      ).sendCurrentModeUpdateNotification(
+        core.ToolConfirmationOutcome.ProceedAlways,
+      );
+
+      expect(mockClient.extNotification).toHaveBeenCalledWith(
+        'qwen/notify/session/mode-update',
+        expect.objectContaining({
+          v: 1,
+          sessionId: 'test-session-id',
+          currentModeId: 'auto-edit',
+          legacyFrameSent: true,
+        }),
+      );
+    });
   });
 
   describe('rewindToTurn', () => {

packages/cli/src/acp-integration/session/Session.ts

@@ -1812,8 +1812,34 @@ export class Session implements SessionContext {
       yolo: ApprovalMode.YOLO,
     };
 
+    // `modeId` arrives over the wire (ACP `session/set_mode`, or
+    // `setSessionConfigOption` casting an unknown `value` to string), so
+    // validate at this boundary. An unknown id would otherwise call
+    // `setApprovalMode(undefined)` — leaving the permission system in an
+    // undefined state — and the A2 broadcast below would fan the bogus id
+    // out to every attached SSE client.
     const approvalMode = modeMap[params.modeId as ApprovalModeValue];
+    if (approvalMode === undefined) {
+      throw RequestError.invalidParams(
+        undefined,
+        `Unknown approval mode: ${params.modeId}`,
+      );
+    }
     this.config.setApprovalMode(approvalMode);
+
+    // A2 (#4511): notify attached clients of an in-session mode switch.
+    // Mirrors the model-update extNotification in `setModel`.
+    void this.client
+      .extNotification('qwen/notify/session/mode-update', {
+        v: 1,
+        sessionId: this.sessionId,
+        currentModeId: params.modeId,
+      })
+      .catch((error) => {
+        // Advisory only; a failed notification must not fail the mode
+        // switch. Matches the model-update extNotification in `setModel`.
+        debugLogger.debug('mode-update extNotification failed', error);
+      });
   }
 
   /**
@@ -1870,8 +1896,9 @@ export class Session implements SessionContext {
         sessionId: this.sessionId,
         currentModelId: effectiveModelId,
       })
-      .catch(() => {
+      .catch((error) => {
         // Advisory only; a failed notification must not fail the model switch.
+        debugLogger.debug('model-update extNotification failed', error);
       });
 
     if (options.persistDefault ?? true) {
@@ -1927,6 +1954,29 @@ export class Session implements SessionContext {
     };
 
     await this.sendUpdate(update);
+
+    // A2 (#4511): promote the mode change to the bridge side-channel so
+    // it reaches `approval_mode_changed` on the SSE bus, matching the
+    // extNotification in `setMode`.
+    //
+    // Unlike `setMode`, this path already published the legacy
+    // `session_update{current_mode_update}` frame via `sendUpdate` above
+    // (BridgeClient.sessionUpdate fans it onto the bus). Tell the demux to
+    // skip its compat dual-emit so the IDE companion sees exactly one
+    // legacy frame for this change, not two. `setMode` omits the flag, so
+    // its dual-emit still fires (it has no `sendUpdate`).
+    void this.client
+      .extNotification('qwen/notify/session/mode-update', {
+        v: 1,
+        sessionId: this.sessionId,
+        currentModeId: newModeId,
+        legacyFrameSent: true,
+      })
+      .catch((error) => {
+        // Advisory only; a failed notification must not fail the mode
+        // change. Matches the model-update extNotification in `setModel`.
+        debugLogger.debug('mode-update extNotification failed', error);
+      });
   }
 
   /**