diff --git a/packages/cli/src/serve/multi-workspace-sessions.test.ts b/packages/cli/src/serve/multi-workspace-sessions.test.ts index ea9758d350e..8bee3d3c192 100644 --- a/packages/cli/src/serve/multi-workspace-sessions.test.ts +++ b/packages/cli/src/serve/multi-workspace-sessions.test.ts @@ -69,6 +69,17 @@ interface FakeBridge extends AcpSessionBridge { }>; readonly listCalls: string[]; readonly summaryCalls: string[]; + readonly setModelCalls: Array<{ + sessionId: string; + req: { modelId?: unknown; sessionId?: unknown }; + context?: BridgeClientRequestContext; + }>; + readonly setApprovalModeCalls: Array<{ + sessionId: string; + mode: string; + opts: { persist?: boolean }; + context?: BridgeClientRequestContext; + }>; } function makeSummary( @@ -150,6 +161,8 @@ function makeBridge( const restoreCalls: FakeBridge['restoreCalls'] = []; const listCalls: string[] = []; const summaryCalls: string[] = []; + const setModelCalls: FakeBridge['setModelCalls'] = []; + const setApprovalModeCalls: FakeBridge['setApprovalModeCalls'] = []; const bridge = { permissionPolicy: 'first-responder' as const, spawnCalls, @@ -165,6 +178,8 @@ function makeBridge( restoreCalls, listCalls, summaryCalls, + setModelCalls, + setApprovalModeCalls, get sessionCount() { return live.size; }, @@ -260,6 +275,33 @@ function makeBridge( promptCalls.push({ sessionId, ...(context ? { context } : {}) }); return Promise.resolve({ stopReason: 'end_turn' }); }, + async setSessionModel( + sessionId: string, + req: { modelId?: unknown; sessionId?: unknown }, + context?: BridgeClientRequestContext, + ) { + setModelCalls.push({ sessionId, req, ...(context ? { context } : {}) }); + return { sessionId, modelId: req.modelId, _meta: { applied: true } }; + }, + async setSessionApprovalMode( + sessionId: string, + mode: string, + opts: { persist?: boolean }, + context?: BridgeClientRequestContext, + ) { + setApprovalModeCalls.push({ + sessionId, + mode, + opts, + ...(context ? { context } : {}), + }); + return { + sessionId, + mode, + previous: 'default', + persisted: opts?.persist === true, + }; + }, async cancelSession(sessionId: string) { cancelCalls.push(sessionId); }, @@ -484,6 +526,24 @@ describe('multi-workspace session dispatch', () => { expect(res.body.workspaceCwd).toBe(SECONDARY_CWD); }); + it('applies a creation-time approvalMode on the owning non-primary runtime', async () => { + const { app, primaryBridge, secondaryBridge } = makeHarness(); + const res = await request(app) + .post('/session') + .set('Host', host()) + .send({ cwd: SECONDARY_CWD, approvalMode: 'yolo' }); + + expect(res.status).toBe(200); + expect(primaryBridge.spawnCalls).toEqual([]); + expect(secondaryBridge.spawnCalls).toHaveLength(1); + // The approval mode rides along with creation on the non-primary runtime, + // so no follow-up primary-only approval-mode round-trip is required. + expect(secondaryBridge.spawnCalls[0]).toMatchObject({ + workspaceCwd: SECONDARY_CWD, + approvalMode: 'yolo', + }); + }); + it('rejects unknown and untrusted workspace session creation before touching a bridge', async () => { const unknown = makeHarness(); const unknownRes = await request(unknown.app) @@ -778,6 +838,75 @@ describe('multi-workspace session dispatch', () => { expect(secondaryBridge.restoreCalls).toEqual([]); }); + it('routes POST /session/:id/model to the owning non-primary workspace bridge', async () => { + const { app, primaryBridge, secondaryBridge } = makeHarness(); + + const res = await request(app) + .post('/session/secondary-session/model') + .set('Host', host()) + .set('X-Qwen-Client-Id', 'client-1') + .send({ modelId: 'qwen3-coder' }); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ _meta: { applied: true } }); + expect(secondaryBridge.setModelCalls).toHaveLength(1); + expect(secondaryBridge.setModelCalls[0]?.sessionId).toBe( + 'secondary-session', + ); + expect(secondaryBridge.setModelCalls[0]?.req.modelId).toBe('qwen3-coder'); + expect(secondaryBridge.setModelCalls[0]?.context).toEqual({ + clientId: 'client-1', + }); + // Owner-scoped: the mutation must land on the secondary bridge only, never + // the primary one. + expect(primaryBridge.setModelCalls).toEqual([]); + }); + + it('routes POST /session/:id/approval-mode to the owning non-primary workspace bridge', async () => { + const { app, primaryBridge, secondaryBridge } = makeHarness(); + + const res = await request(app) + .post('/session/secondary-session/approval-mode') + .set('Host', host()) + .send({ mode: 'yolo', persist: true }); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + sessionId: 'secondary-session', + mode: 'yolo', + persisted: true, + }); + expect(secondaryBridge.setApprovalModeCalls).toHaveLength(1); + expect(secondaryBridge.setApprovalModeCalls[0]).toMatchObject({ + sessionId: 'secondary-session', + mode: 'yolo', + opts: { persist: true }, + }); + expect(primaryBridge.setApprovalModeCalls).toEqual([]); + }); + + it('still rejects model/approval-mode mutations on an untrusted non-primary session', async () => { + // Opening these routes to non-primary owners must not bypass the trust + // gate: an untrusted workspace runtime is refused before the bridge runs. + const { app, secondaryBridge } = makeHarness({ secondaryTrusted: false }); + + const modelRes = await request(app) + .post('/session/secondary-session/model') + .set('Host', host()) + .send({ modelId: 'qwen3-coder' }); + expect(modelRes.status).toBe(403); + expect(modelRes.body.code).toBe('untrusted_workspace'); + expect(secondaryBridge.setModelCalls).toEqual([]); + + const approvalRes = await request(app) + .post('/session/secondary-session/approval-mode') + .set('Host', host()) + .send({ mode: 'yolo' }); + expect(approvalRes.status).toBe(403); + expect(approvalRes.body.code).toBe('untrusted_workspace'); + expect(secondaryBridge.setApprovalModeCalls).toEqual([]); + }); + it('lists active persisted and live non-primary workspace sessions by workspace id', async () => { await withRuntimeDir(async () => { const storedOnlyId = '550e8400-e29b-41d4-a716-446655440101'; diff --git a/packages/cli/src/serve/routes/session.ts b/packages/cli/src/serve/routes/session.ts index 96e885f6319..8236913b670 100644 --- a/packages/cli/src/serve/routes/session.ts +++ b/packages/cli/src/serve/routes/session.ts @@ -2444,9 +2444,9 @@ export function registerSessionRoutes( app.post( '/session/:id/model', mutate(), - withMutableSession( + withOwnerMutableSession( 'POST /session/:id/model', - async (req, res, sessionId) => { + async (req, res, sessionId, runtime) => { const body = safeBody(req); const modelId = body['modelId']; if (typeof modelId !== 'string' || !modelId) { @@ -2457,7 +2457,7 @@ export function registerSessionRoutes( } const clientId = parseClientIdHeader(req, res); if (clientId === null) return; - const response = await bridge.setSessionModel( + const response = await runtime.bridge.setSessionModel( sessionId, { ...(body as object), @@ -2777,9 +2777,9 @@ export function registerSessionRoutes( app.post( '/session/:id/approval-mode', mutate(), - withMutableSession( + withOwnerMutableSession( 'POST /session/:id/approval-mode', - async (req, res, sessionId) => { + async (req, res, sessionId, runtime) => { // Validates `mode` against `APPROVAL_MODES` and an optional // `persist: boolean` flag. const body = safeBody(req); @@ -2805,7 +2805,7 @@ export function registerSessionRoutes( } const clientId = parseClientIdHeader(req, res); if (clientId === null) return; - const response = await bridge.setSessionApprovalMode( + const response = await runtime.bridge.setSessionApprovalMode( sessionId, mode as ApprovalMode, { persist: persist === true }, diff --git a/packages/web-shell/client/utils/sessionPreparation.test.ts b/packages/web-shell/client/utils/sessionPreparation.test.ts index df71f34d533..385dc81e65e 100644 --- a/packages/web-shell/client/utils/sessionPreparation.test.ts +++ b/packages/web-shell/client/utils/sessionPreparation.test.ts @@ -4,7 +4,6 @@ import { createAndAttachSessionForPrompt } from './sessionPreparation'; type CreateSessionArgs = Parameters[0]; const sessionResult = { sessionId: 'session-1' }; const modelResult = { model: 'qwen3' }; -const approvalModeResult = { mode: 'yolo' }; function createActions( overrides: Partial = {}, @@ -15,18 +14,13 @@ function createActions( closeSession: vi.fn(async () => {}), clearSession: vi.fn(async () => {}), setModel: vi.fn(async () => modelResult), - setApprovalMode: vi.fn(async () => approvalModeResult), ...overrides, }; } describe('createAndAttachSessionForPrompt', () => { - it('attaches the session before a model switch failure can abort setup', async () => { + it('folds the approval mode into createSession and applies the model after attach', async () => { const order: string[] = []; - const error = new Error('model failed'); - const warn = vi.fn(); - const waitForModel = createDeferred(); - const approvalStarted = createDeferred(); const actions = createActions({ createSession: vi.fn(async () => { order.push('create'); @@ -37,40 +31,59 @@ describe('createAndAttachSessionForPrompt', () => { }), setModel: vi.fn(async () => { order.push('model'); - await waitForModel.promise; - throw error; - }), - setApprovalMode: vi.fn(async () => { - order.push('approval'); - approvalStarted.resolve(); - return approvalModeResult; + return modelResult; }), }); - const result = createAndAttachSessionForPrompt({ + await createAndAttachSessionForPrompt({ sessionActions: actions, modelId: 'qwen3', modeId: 'yolo', - warn, + workspaceCwd: '/ws/secondary', }); - await approvalStarted.promise; - waitForModel.resolve(); - await result; - expect(order.slice(0, 2)).toEqual(['create', 'attach']); - expect(order.slice(2).sort()).toEqual(['approval', 'model']); - expect(warn).toHaveBeenCalledWith( - '[WebShell] failed to set model for new session:', - error, - ); + // Approval mode rides along with creation — no follow-up round-trip. + expect(actions.createSession).toHaveBeenCalledWith({ + workspaceCwd: '/ws/secondary', + approvalMode: 'yolo', + }); + // Model is still a post-create call, sequenced after attach. + expect(order).toEqual(['create', 'attach', 'model']); + expect(actions.setModel).toHaveBeenCalledWith('qwen3'); + }); + + it('omits approvalMode when the mode is not a recognized daemon approval mode', async () => { + const actions = createActions(); + + await createAndAttachSessionForPrompt({ + sessionActions: actions, + modeId: 'not-a-mode', + }); + + expect(actions.createSession).toHaveBeenCalledWith({ + workspaceCwd: undefined, + }); + }); + + it('creates the session without a model call when no model is selected', async () => { + const actions = createActions(); + + await createAndAttachSessionForPrompt({ + sessionActions: actions, + modeId: 'plan', + }); + + expect(actions.createSession).toHaveBeenCalledWith({ + workspaceCwd: undefined, + approvalMode: 'plan', + }); + expect(actions.setModel).not.toHaveBeenCalled(); }); - it('keeps the attached session when approval mode setup fails', async () => { + it('warns but resolves when the post-create model switch fails', async () => { const order: string[] = []; - const error = new Error('mode failed'); + const error = new Error('model failed'); const warn = vi.fn(); - const waitForModel = createDeferred(); - const approvalStarted = createDeferred(); const actions = createActions({ createSession: vi.fn(async () => { order.push('create'); @@ -81,34 +94,57 @@ describe('createAndAttachSessionForPrompt', () => { }), setModel: vi.fn(async () => { order.push('model'); - await waitForModel.promise; - return modelResult; - }), - setApprovalMode: vi.fn(async () => { - order.push('approval'); - approvalStarted.resolve(); throw error; }), }); - const result = createAndAttachSessionForPrompt({ - sessionActions: actions, - modelId: 'qwen3', - modeId: 'yolo', - warn, - }); - await approvalStarted.promise; - waitForModel.resolve(); - await result; + await expect( + createAndAttachSessionForPrompt({ + sessionActions: actions, + modelId: 'qwen3', + modeId: 'yolo', + warn, + }), + ).resolves.toBeUndefined(); - expect(order.slice(0, 2)).toEqual(['create', 'attach']); - expect(order.slice(2).sort()).toEqual(['approval', 'model']); + expect(order).toEqual(['create', 'attach', 'model']); expect(warn).toHaveBeenCalledWith( - '[WebShell] failed to set approval mode for new session:', + '[WebShell] failed to set model for new session:', error, ); }); + it('propagates a create failure (fail-closed approval mode) without attaching or setting the model', async () => { + // The daemon tears the session down and rejects `POST /session` when the + // requested approval mode can't be applied at spawn. That rejection must + // abort the whole flow — no session, no model call — rather than leaving a + // half-created session in the wrong mode. + const error = new Error('approval_mode_initialization_failed'); + const actions = createActions({ + createSession: vi.fn(async () => { + throw error; + }), + }); + + await expect( + createAndAttachSessionForPrompt({ + sessionActions: actions, + modelId: 'qwen3', + modeId: 'yolo', + }), + ).rejects.toThrow(error); + + expect(actions.createSession).toHaveBeenCalledWith({ + workspaceCwd: undefined, + approvalMode: 'yolo', + }); + expect(actions.attachSession).not.toHaveBeenCalled(); + expect(actions.setModel).not.toHaveBeenCalled(); + // Nothing was created client-side, so there is nothing to close/clear. + expect(actions.closeSession).not.toHaveBeenCalled(); + expect(actions.clearSession).not.toHaveBeenCalled(); + }); + it('closes and clears the created session when attach fails', async () => { const order: string[] = []; const error = new Error('attach failed'); @@ -138,7 +174,6 @@ describe('createAndAttachSessionForPrompt', () => { expect(actions.clearSession).toHaveBeenCalledOnce(); expect(order).toEqual(['close', 'clear']); expect(actions.setModel).not.toHaveBeenCalled(); - expect(actions.setApprovalMode).not.toHaveBeenCalled(); expect(warn).toHaveBeenCalledWith( '[WebShell] failed to attach new session:', error, @@ -184,14 +219,3 @@ describe('createAndAttachSessionForPrompt', () => { }); }); }); - -function createDeferred(): { - promise: Promise; - resolve: (value?: T | PromiseLike) => void; -} { - let resolve!: (value?: T | PromiseLike) => void; - const promise = new Promise((res) => { - resolve = (value) => res(value as T | PromiseLike); - }); - return { promise, resolve }; -} diff --git a/packages/web-shell/client/utils/sessionPreparation.ts b/packages/web-shell/client/utils/sessionPreparation.ts index 596009bfff6..fa9603cb6b4 100644 --- a/packages/web-shell/client/utils/sessionPreparation.ts +++ b/packages/web-shell/client/utils/sessionPreparation.ts @@ -4,12 +4,14 @@ import { } from '@qwen-code/webui/daemon-react-sdk'; type PromptSessionActions = { - createSession: (options?: { workspaceCwd?: string }) => Promise; + createSession: (options?: { + workspaceCwd?: string; + approvalMode?: DaemonApprovalMode; + }) => Promise; attachSession: () => Promise; closeSession: () => Promise; clearSession: () => Promise; setModel: (modelId: string) => Promise; - setApprovalMode: (mode: DaemonApprovalMode) => Promise; }; export function isDaemonApprovalMode(mode: string): mode is DaemonApprovalMode { @@ -29,7 +31,18 @@ export async function createAndAttachSessionForPrompt({ workspaceCwd?: string; warn?: (message?: unknown, ...optionalParams: unknown[]) => void; }): Promise { - await sessionActions.createSession({ workspaceCwd }); + // Seed the approval mode in the create request itself so the daemon applies + // it atomically at spawn (`POST /session` → `spawnOrAttach({ approvalMode })`), + // saving a follow-up round-trip. Approval mode is fail-closed at spawn: if the + // requested mode can't be applied the session is not created (this call + // rejects), rather than silently running in a different mode than requested. + // The model, by contrast, stays a best-effort follow-up below. + const approvalMode = + modeId && isDaemonApprovalMode(modeId) ? modeId : undefined; + await sessionActions.createSession({ + workspaceCwd, + ...(approvalMode ? { approvalMode } : {}), + }); try { await sessionActions.attachSession(); } catch (error) { @@ -42,19 +55,13 @@ export async function createAndAttachSessionForPrompt({ }); throw error; } - await Promise.all([ - modelId - ? sessionActions.setModel(modelId).catch((error: unknown) => { - warn('[WebShell] failed to set model for new session:', error); - }) - : Promise.resolve(), - modeId && isDaemonApprovalMode(modeId) - ? sessionActions.setApprovalMode(modeId).catch((error: unknown) => { - warn( - '[WebShell] failed to set approval mode for new session:', - error, - ); - }) - : Promise.resolve(), - ]); + // The model still needs a post-create call: `POST /session` only accepts a + // `modelServiceId`, whereas the composer selects a plain `modelId`. The + // `POST /session/:id/model` route now resolves the owning workspace runtime, + // so this succeeds for non-primary workspaces too. + if (modelId) { + await sessionActions.setModel(modelId).catch((error: unknown) => { + warn('[WebShell] failed to set model for new session:', error); + }); + } } diff --git a/packages/webui/src/daemon/session/DaemonSessionProvider.tsx b/packages/webui/src/daemon/session/DaemonSessionProvider.tsx index da28ad91525..103b1af7f01 100644 --- a/packages/webui/src/daemon/session/DaemonSessionProvider.tsx +++ b/packages/webui/src/daemon/session/DaemonSessionProvider.tsx @@ -24,6 +24,7 @@ import { extractServerTimestamp, matchTurnEvent, normalizeDaemonEvent, + type CreateSessionRequest, type DaemonEvent, type DaemonTranscriptBlock, type DaemonTranscriptState, @@ -1681,7 +1682,10 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { workspaceCwd: activeWorkspaceCwdRef.current ?? sessionRef.current?.workspaceCwd, }), - createDetachedSession: (workspaceCwd?: string) => { + createDetachedSession: ( + workspaceCwd?: string, + overrides?: Pick, + ) => { const client = workspaceClientRef.current ?? new DaemonClient({ @@ -1695,6 +1699,9 @@ export function DaemonSessionProvider(props: DaemonSessionProviderProps) { workspaceCwd ?? activeWorkspaceCwdRef.current ?? sessionRef.current?.workspaceCwd, + ...(overrides?.approvalMode !== undefined + ? { approvalMode: overrides.approvalMode } + : {}), }; const requestClientId = clientId ? clientIdRef.current diff --git a/packages/webui/src/daemon/session/actions.test.ts b/packages/webui/src/daemon/session/actions.test.ts index c80e7c01ff2..2b241f09c29 100644 --- a/packages/webui/src/daemon/session/actions.test.ts +++ b/packages/webui/src/daemon/session/actions.test.ts @@ -164,7 +164,7 @@ describe('createDaemonSessionActions', () => { await actions.createSession({ workspaceCwd: '/ws/secondary' }); - expect(createDetachedSession).toHaveBeenCalledWith('/ws/secondary'); + expect(createDetachedSession).toHaveBeenCalledWith('/ws/secondary', {}); }); it('omits the workspaceCwd override on the detached branch by default', async () => { @@ -177,7 +177,22 @@ describe('createDaemonSessionActions', () => { await actions.createSession(); - expect(createDetachedSession).toHaveBeenCalledWith(undefined); + expect(createDetachedSession).toHaveBeenCalledWith(undefined, {}); + }); + + it('forwards options.approvalMode to the detached create branch', async () => { + const nextSession = createMockSession('session-b'); + const createDetachedSession = vi.fn(async () => nextSession); + const { actions } = createActionsHarness({ + connection: { status: 'connected' }, + createDetachedSession, + }); + + await actions.createSession({ approvalMode: 'yolo' }); + + expect(createDetachedSession).toHaveBeenCalledWith(undefined, { + approvalMode: 'yolo', + }); }); it('merges options.workspaceCwd into the active session request', async () => { @@ -196,6 +211,22 @@ describe('createDaemonSessionActions', () => { ); }); + it('folds options.approvalMode into the active session request', async () => { + const existingSession = createMockSession('session-a'); + const nextSession = createMockSession('session-b'); + existingSession.client.createOrAttachSession.mockResolvedValue(nextSession); + const { actions } = createActionsHarness({ + connection: { status: 'connected', sessionId: 'session-a' }, + session: existingSession, + }); + + await actions.createSession({ approvalMode: 'yolo' }); + + expect(existingSession.client.createOrAttachSession).toHaveBeenCalledWith( + expect.objectContaining({ approvalMode: 'yolo' }), + ); + }); + it('does not restore a detached session after the session was cleared', async () => { const nextSession = createMockSession('session-b'); const deferred = createDeferred(); diff --git a/packages/webui/src/daemon/session/actions.ts b/packages/webui/src/daemon/session/actions.ts index 9935e23dd0c..0a1962016b2 100644 --- a/packages/webui/src/daemon/session/actions.ts +++ b/packages/webui/src/daemon/session/actions.ts @@ -6,6 +6,7 @@ import type { Dispatch, SetStateAction } from 'react'; import type { + DaemonApprovalMode, DaemonSessionContextStatus, DaemonSessionClient, DaemonSessionBtwResult, @@ -59,6 +60,7 @@ export interface CreateDaemonSessionActionsArgs { getCreateSessionRequest: () => CreateSessionRequest; createDetachedSession: ( workspaceCwd?: string, + overrides?: Pick, ) => Promise; getConnection: () => DaemonConnectionState; hasSessionActivePrompt: () => boolean; @@ -580,9 +582,22 @@ export function createDaemonSessionActions({ return startSessionSwitch(sessionId, 'resume'); }, - async createSession(options?: { workspaceCwd?: string }) { + async createSession(options?: { + workspaceCwd?: string; + approvalMode?: DaemonApprovalMode; + }) { try { manualSessionClearRef.current = false; + // Fold the initial approval mode into the create request so the daemon + // applies it atomically at spawn (`POST /session` → + // `spawnOrAttach({ approvalMode })`), avoiding a follow-up + // `setApprovalMode` round-trip. Approval mode is fail-closed at spawn: + // an application failure aborts creation (this call rejects) rather than + // leaving the session in a different mode than the caller requested. + const approvalOverride = + options?.approvalMode !== undefined + ? { approvalMode: options.approvalMode } + : {}; const session = sessionRef.current; const activeSession = session && getConnection().sessionId === session.sessionId @@ -595,6 +610,7 @@ export function createDaemonSessionActions({ ...(options?.workspaceCwd !== undefined ? { workspaceCwd: options.workspaceCwd } : {}), + ...approvalOverride, }), 'Create session timed out', ); @@ -603,7 +619,7 @@ export function createDaemonSessionActions({ } const nextSession = await withActionTimeout( - createDetachedSession(options?.workspaceCwd), + createDetachedSession(options?.workspaceCwd, approvalOverride), 'Create session timed out', ); if (manualSessionClearRef.current) { diff --git a/packages/webui/src/daemon/session/types.ts b/packages/webui/src/daemon/session/types.ts index 4dbd0c94246..2726e3e8c96 100644 --- a/packages/webui/src/daemon/session/types.ts +++ b/packages/webui/src/daemon/session/types.ts @@ -333,8 +333,15 @@ export interface DaemonSessionActions { * `options.workspaceCwd` targets a specific registered workspace runtime for * this call only (multi-workspace daemons). Omit it to keep the provider's * active workspace / primary fallback. + * + * `options.approvalMode` seeds the session's approval mode in the create + * request itself, so the daemon applies it atomically at spawn instead of + * requiring a follow-up `setApprovalMode` call. */ - createSession(options?: { workspaceCwd?: string }): Promise; + createSession(options?: { + workspaceCwd?: string; + approvalMode?: DaemonApprovalMode; + }): Promise; attachSession(): Promise; clearSession(): Promise; newSession(): Promise;