Skip to content

Commit 9f243f4

Browse files
doudouOUCdoudouOUC
committed
feat(serve): add session approval-mode mutation route (#4175 Wave 4 PR 17)
Adds POST /session/:id/approval-mode — the first strict-gated session mutation surface introduced in Wave 4 alongside PR 16 / PR 21. Remote clients can switch a live session's approval mode (plan / default / auto-edit / yolo) without touching the user's host CLI. Routing: - Route handler validates `mode` against the closed `APPROVAL_MODES` enum and an optional `persist: boolean` flag (400 on either) - Bridge `setSessionApprovalMode` forwards through the new `qwen/control/session/approval_mode` ACP extMethod (introduced in a new `SERVE_CONTROL_EXT_METHODS` namespace) so the change lands inside the ACP child's per-session `Config` - `persist: true` writes `tools.approvalMode` to workspace settings via a new `BridgeOptions.persistApprovalMode` callback wired in `runQwenServe`. Default is ephemeral so a remote caller does not pollute the user's host settings unless asked Trust gate translation: - ACP child catches `TrustGateError` from `Config.setApprovalMode` and re-raises as a JSON-RPC error with `data.errorKind: 'trust_gate'` - Bridge detects the structured payload and re-instantiates the typed `TrustGateError` (since the class name does not survive the wire) - `sendBridgeError` translates to HTTP 403 with the closed PR-13 `errorKind: 'auth_env_error'` taxonomy SDK additions: - `DaemonClient.setSessionApprovalMode(sessionId, mode, opts?, clientId?)` mirrors the route shape and forwards `X-Qwen-Client-Id` - New `DaemonApprovalMode` literal union and `DAEMON_APPROVAL_MODES` const tuple; `DaemonApprovalModeResult` for the route response - New `approval_mode_changed` typed event on `DaemonControlEvent`, reducer integration on `DaemonSessionViewState` (`approvalMode` / `approvalModeChangedCount` / `lastApprovalModeChange`) - Drift detector `approvalMode.test.ts` walks core's `ApprovalMode` enum and fails CI if `APPROVAL_MODES` or `DAEMON_APPROVAL_MODES` drift in either direction New capability tag `session_approval_mode_control` (always-on, since v1). 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
1 parent c48439e commit 9f243f4

16 files changed

Lines changed: 777 additions & 5 deletions

File tree

‎integration-tests/cli/qwen-serve-routes.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,13 +208,23 @@ describe('qwen serve — capabilities envelope', () => {
208208
'workspace_mcp',
209209
'workspace_skills',
210210
'workspace_providers',
211+
// PR 16 (#4249) tags — pre-existing drift on this E2E test;
212+
// PR 16 updated the unit test in `server.test.ts` but not this
213+
// integration mirror. Folded in here as part of PR 17 to keep
214+
// the E2E test green now that origin/main advertises both.
215+
'workspace_memory',
216+
'workspace_agents',
211217
'workspace_env',
212218
'workspace_preflight',
213219
'session_context',
214220
'session_supported_commands',
215221
'session_close',
216222
'session_metadata',
217223
'mcp_guardrails',
224+
// PR 19 (#4269) tag — same drift situation as PR 16's tags above.
225+
'workspace_file_read',
226+
// #4175 Wave 4 PR 17.
227+
'session_approval_mode_control',
218228
]);
219229
});
220230
});

‎packages/cli/src/acp-integration/acpAgent.ts‎

Lines changed: 53 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,14 @@ import {
2121
getMCPServerStatus,
2222
MCPDiscoveryState,
2323
MCPServerStatus,
24-
type Config,
25-
type ConversationRecord,
26-
type DeviceAuthorizationData,
2724
SessionEndReason,
2825
} from '@qwen-code/qwen-code-core';
26+
import type {
27+
ApprovalMode,
28+
Config,
29+
ConversationRecord,
30+
DeviceAuthorizationData,
31+
} from '@qwen-code/qwen-code-core';
2932
import {
3033
AgentSideConnection,
3134
RequestError,
@@ -84,6 +87,7 @@ import { runExitCleanup } from '../utils/cleanup.js';
8487
import {
8588
ACP_PREFLIGHT_KINDS,
8689
STATUS_SCHEMA_VERSION,
90+
SERVE_CONTROL_EXT_METHODS,
8791
SERVE_STATUS_EXT_METHODS,
8892
mapDomainErrorToErrorKind,
8993
type AcpPreflightKind,
@@ -1433,6 +1437,52 @@ class QwenAgent implements Agent {
14331437
sessionId,
14341438
)) as unknown as Record<string, unknown>;
14351439
}
1440+
case SERVE_CONTROL_EXT_METHODS.sessionApprovalMode: {
1441+
// #4175 Wave 4 PR 17: remote callers change a live session's
1442+
// approval mode via this ACP extMethod. `Config.setApprovalMode`
1443+
// throws `TrustGateError` for privileged modes in an untrusted
1444+
// folder; we let it propagate — the bridge's mapping helper
1445+
// converts the name to `errorKind: 'auth_env_error'` on the
1446+
// wire so the SDK consumer gets a structured failure.
1447+
const sessionId = params['sessionId'];
1448+
const mode = params['mode'];
1449+
if (typeof sessionId !== 'string' || sessionId.length === 0) {
1450+
throw RequestError.invalidParams(
1451+
undefined,
1452+
'Invalid or missing sessionId',
1453+
);
1454+
}
1455+
if (
1456+
typeof mode !== 'string' ||
1457+
!APPROVAL_MODES.includes(mode as ApprovalMode)
1458+
) {
1459+
throw RequestError.invalidParams(
1460+
undefined,
1461+
`Invalid approval mode; allowed: ${APPROVAL_MODES.join(', ')}`,
1462+
);
1463+
}
1464+
const session = this.sessionOrThrow(sessionId);
1465+
const config = session.getConfig();
1466+
const previous = config.getApprovalMode();
1467+
try {
1468+
config.setApprovalMode(mode as ApprovalMode);
1469+
} catch (err) {
1470+
// `TrustGateError` is the core's structured rejection for
1471+
// untrusted-folder + privileged-mode. We re-raise it as a
1472+
// JSON-RPC error whose `data.errorKind` is the literal the
1473+
// bridge looks for to reconstruct a typed `TrustGateError` on
1474+
// the daemon side (JSON-RPC strips the class name across the
1475+
// wire). Other errors propagate unchanged.
1476+
if (err instanceof Error && err.name === 'TrustGateError') {
1477+
throw new RequestError(-32003, err.message, {
1478+
errorKind: 'trust_gate',
1479+
});
1480+
}
1481+
throw err;
1482+
}
1483+
const current = config.getApprovalMode();
1484+
return { previous, current };
1485+
}
14361486
case 'deleteSession': {
14371487
const sessionId = params['sessionId'] as string;
14381488
if (!sessionId || !SESSION_ID_RE.test(sessionId)) {

‎packages/cli/src/acp-integration/approvalMode.test.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/**
2+
* @license
3+
* Copyright 2025 Qwen Team
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
/**
8+
* Drift detector for the approval-mode triple-source contract:
9+
* 1. core's `ApprovalMode` enum (the source of truth — drives
10+
* `Config.setApprovalMode` and the trust-gate check)
11+
* 2. core's `APPROVAL_MODES` const array (consumed by daemon route +
12+
* ACP extMethod for body validation)
13+
* 3. SDK's `DAEMON_APPROVAL_MODES` literal tuple (mirrored for SDK
14+
* consumers; shape of `DaemonApprovalMode` union)
15+
*
16+
* If any of the three drifts (e.g., a future fifth mode added to the
17+
* enum but not the SDK list), this test fires before runtime and
18+
* before the protocol docs go out of sync.
19+
*
20+
* #4175 Wave 4 PR 17.
21+
*/
22+
import { describe, expect, it } from 'vitest';
23+
import { APPROVAL_MODES, ApprovalMode } from '@qwen-code/qwen-code-core';
24+
import { DAEMON_APPROVAL_MODES } from '@qwen-code/sdk';
25+
26+
describe('approval-mode triple-source drift detection', () => {
27+
it('APPROVAL_MODES contains every ApprovalMode enum value', () => {
28+
const enumValues = Object.values(ApprovalMode);
29+
for (const value of enumValues) {
30+
expect(APPROVAL_MODES).toContain(value);
31+
}
32+
expect(APPROVAL_MODES.length).toBe(enumValues.length);
33+
});
34+
35+
it('DAEMON_APPROVAL_MODES (SDK) mirrors core APPROVAL_MODES exactly', () => {
36+
// Order matters — the SDK test snapshots the advertised sequence so
37+
// diagnostic UIs that render modes in registration order stay
38+
// stable across SDK / daemon versions.
39+
expect([...DAEMON_APPROVAL_MODES]).toEqual([...APPROVAL_MODES]);
40+
});
41+
42+
it('DAEMON_APPROVAL_MODES contains every ApprovalMode enum value', () => {
43+
// Belt-and-suspenders: even if APPROVAL_MODES drifts away from the
44+
// enum (caught above), this assertion keeps the SDK / enum invariant
45+
// intact independently.
46+
const enumValues = new Set<string>(Object.values(ApprovalMode));
47+
for (const value of DAEMON_APPROVAL_MODES) {
48+
expect(enumValues.has(value)).toBe(true);
49+
}
50+
expect(DAEMON_APPROVAL_MODES.length).toBe(enumValues.size);
51+
});
52+
});

‎packages/cli/src/serve/capabilities.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,13 @@ export const SERVE_CAPABILITY_REGISTRY = {
108108
// `POST /file/edit`) ship under a separate `workspace_file_write`
109109
// tag in PR 20.
110110
workspace_file_read: { since: 'v1' },
111+
// #4175 Wave 4 PR 17. Daemon hosts the session-level approval-mode
112+
// control route `POST /session/:id/approval-mode` (gated by the
113+
// mutation gate, strict). The route accepts `{mode, persist?}` —
114+
// `persist:true` also writes `tools.approvalMode` to workspace
115+
// settings via the daemon's `loadedSettings` handle. SDK helper:
116+
// `DaemonClient.setSessionApprovalMode`.
117+
session_approval_mode_control: { since: 'v1' },
111118
// Issue #4175 PR 15. Daemon was booted with `--require-auth` (or
112119
// `requireAuth: true`), so even loopback callers must carry a bearer
113120
// token. Advertised CONDITIONALLY — only when the flag is on — so

‎packages/cli/src/serve/httpAcpBridge.ts‎

Lines changed: 143 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import {
2424
} from './eventBus.js';
2525
import {
2626
BridgeTimeoutError,
27+
SERVE_CONTROL_EXT_METHODS,
2728
SERVE_STATUS_EXT_METHODS,
2829
STATUS_SCHEMA_VERSION,
2930
createIdleAcpPreflightCells,
@@ -43,7 +44,12 @@ import {
4344
type ServeWorkspaceSkillsStatus,
4445
} from './status.js';
4546
import { buildEnvStatusFromProcess } from './envSnapshot.js';
46-
import { canUseRipgrep } from '@qwen-code/qwen-code-core';
47+
import type {
48+
ApprovalMode} from '@qwen-code/qwen-code-core';
49+
import {
50+
TrustGateError,
51+
canUseRipgrep,
52+
} from '@qwen-code/qwen-code-core';
4753
import { getGitVersion, getNpmVersion } from '../utils/systemInfo.js';
4854
import type {
4955
CancelNotification,
@@ -427,6 +433,35 @@ export interface HttpAcpBridge {
427433
context?: BridgeClientRequestContext,
428434
): Promise<SetSessionModelResponse>;
429435

436+
/**
437+
* Change the approval mode of a live session and broadcast an
438+
* `approval_mode_changed` event. Forwards through the
439+
* `qwen/control/session/approval_mode` ACP extMethod so the change
440+
* lands inside the ACP child's own `Config` instance.
441+
*
442+
* `opts.persist === true` also writes `tools.approvalMode` to the
443+
* workspace settings file so a future ACP child or a future daemon
444+
* restart picks up the new default. Default is ephemeral
445+
* (`persist: false`) — the remote caller does not pollute the
446+
* user's on-disk settings unless they ask for it.
447+
*
448+
* Throws `SessionNotFoundError` for unknown sessions. The ACP-side
449+
* trust-folder rejection surfaces as a `TrustGateError` from core
450+
* which the route maps via `mapDomainErrorToErrorKind` to
451+
* `auth_env_error`.
452+
*/
453+
setSessionApprovalMode(
454+
sessionId: string,
455+
mode: ApprovalMode,
456+
opts: { persist: boolean },
457+
context?: BridgeClientRequestContext,
458+
): Promise<{
459+
sessionId: string;
460+
mode: ApprovalMode;
461+
previous: ApprovalMode;
462+
persisted: boolean;
463+
}>;
464+
430465
/**
431466
* Kill the agent process for the session and remove it from the maps.
432467
* Used by the HTTP route layer to reap orphans created when a client
@@ -798,6 +833,21 @@ export interface BridgeOptions {
798833
* typically ignore it; the production factory merges it).
799834
*/
800835
childEnvOverrides?: Readonly<Record<string, string | undefined>>;
836+
/**
837+
* #4175 Wave 4 PR 17 — optional callback for persisting `tools.
838+
* approvalMode` to the workspace settings file. Invoked by
839+
* `setSessionApprovalMode` ONLY when the route caller passes
840+
* `{persist: true}`. The default `runQwenServe` wires this to
841+
* `loadSettings(boundWorkspace).setValue(SettingScope.Workspace,
842+
* 'tools.approvalMode', mode)`. Bridge tests and embedded callers
843+
* may omit it; when omitted, `setSessionApprovalMode` still applies
844+
* the in-process change and returns `persisted: false` regardless
845+
* of the request flag.
846+
*/
847+
persistApprovalMode?: (
848+
boundWorkspace: string,
849+
mode: ApprovalMode,
850+
) => Promise<void>;
801851
}
802852

803853
/**
@@ -1588,6 +1638,7 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge {
15881638
);
15891639
}
15901640
const boundWorkspace = opts.boundWorkspace;
1641+
const persistApprovalMode = opts.persistApprovalMode;
15911642

15921643
// #3803 §02 single-workspace model: the bridge hosts AT MOST one
15931644
// ATTACH-AVAILABLE channel and one default attach-target entry.
@@ -3487,6 +3538,97 @@ export function createHttpAcpBridge(opts: BridgeOptions): HttpAcpBridge {
34873538
return response;
34883539
},
34893540

3541+
async setSessionApprovalMode(sessionId, mode, opts, context) {
3542+
// #4175 Wave 4 PR 17. Forwards through `qwen/control/session/
3543+
// approval_mode` so the change lands inside the ACP child's own
3544+
// `Config` (per-session `setApprovalMode`). The bridge layer adds
3545+
// two things on top: trusted `originatorClientId` resolution and
3546+
// an opt-in persist hook that writes `tools.approvalMode` to the
3547+
// workspace settings file. Persist is OFF by default — see the
3548+
// interface doc for the reasoning.
3549+
const entry = byId.get(sessionId);
3550+
if (!entry) throw new SessionNotFoundError(sessionId);
3551+
const info = channelInfoForEntry(entry);
3552+
if (!info || info.isDying) throw new SessionNotFoundError(sessionId);
3553+
const originatorClientId = resolveTrustedClientId(
3554+
entry,
3555+
context?.clientId,
3556+
);
3557+
let response: { previous: ApprovalMode; current: ApprovalMode };
3558+
try {
3559+
response = (await Promise.race([
3560+
withTimeout(
3561+
entry.connection.extMethod(
3562+
SERVE_CONTROL_EXT_METHODS.sessionApprovalMode,
3563+
{ sessionId, mode },
3564+
),
3565+
initTimeoutMs,
3566+
SERVE_CONTROL_EXT_METHODS.sessionApprovalMode,
3567+
),
3568+
getTransportClosedReject(entry),
3569+
])) as { previous: ApprovalMode; current: ApprovalMode };
3570+
} catch (err) {
3571+
// The ACP child rethrows `TrustGateError` as a JSON-RPC error
3572+
// whose `data.errorKind` is the literal `'trust_gate'`. On the
3573+
// wire it arrives as a plain `{code, message, data}` object —
3574+
// re-instantiate the typed class here so the HTTP route layer
3575+
// recognizes it via `instanceof` / `err.name` and maps the
3576+
// failure to HTTP 403 with the `auth_env_error` errorKind.
3577+
const data = (err as { data?: unknown })?.data;
3578+
if (
3579+
data &&
3580+
typeof data === 'object' &&
3581+
'errorKind' in data &&
3582+
(data as { errorKind?: unknown }).errorKind === 'trust_gate'
3583+
) {
3584+
const message =
3585+
(err as { message?: unknown })?.message instanceof String ||
3586+
typeof (err as { message?: unknown })?.message === 'string'
3587+
? String((err as { message: string }).message)
3588+
: 'Trust-gate rejection from ACP child';
3589+
throw new TrustGateError(message);
3590+
}
3591+
throw err;
3592+
}
3593+
let persisted = false;
3594+
if (opts.persist) {
3595+
try {
3596+
await persistApprovalMode?.(boundWorkspace, mode);
3597+
persisted = persistApprovalMode !== undefined;
3598+
} catch (err) {
3599+
// Persist failure is non-fatal — the in-process change already
3600+
// took effect inside the ACP child. Log to stderr so operators
3601+
// notice but don't fail the route (the SDK consumer would have
3602+
// no good recovery path; the runtime change is real).
3603+
writeStderrLine(
3604+
`setSessionApprovalMode: persist failed: ${
3605+
err instanceof Error ? err.message : String(err)
3606+
}`,
3607+
);
3608+
}
3609+
}
3610+
try {
3611+
entry.events.publish({
3612+
type: 'approval_mode_changed',
3613+
data: {
3614+
sessionId: entry.sessionId,
3615+
previous: response.previous,
3616+
next: response.current,
3617+
persisted,
3618+
},
3619+
...(originatorClientId ? { originatorClientId } : {}),
3620+
});
3621+
} catch {
3622+
/* bus closed */
3623+
}
3624+
return {
3625+
sessionId: entry.sessionId,
3626+
mode: response.current,
3627+
previous: response.previous,
3628+
persisted,
3629+
};
3630+
},
3631+
34903632
async killSession(sessionId, opts) {
34913633
const entry = byId.get(sessionId);
34923634
if (!entry) return;

‎packages/cli/src/serve/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export {
3737
export {
3838
ACP_PREFLIGHT_KINDS,
3939
BridgeTimeoutError,
40+
SERVE_CONTROL_EXT_METHODS,
4041
SERVE_ERROR_KINDS,
4142
SERVE_STATUS_EXT_METHODS,
4243
STATUS_SCHEMA_VERSION,

‎packages/cli/src/serve/runQwenServe.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { type Server } from 'node:http';
99
import * as path from 'node:path';
1010
import { writeStderrLine, writeStdoutLine } from '../utils/stdioHelpers.js';
1111
import type { BridgeEvent } from './eventBus.js';
12+
import { loadSettings, SettingScope } from '../config/settings.js';
1213
import {
1314
canonicalizeWorkspace,
1415
createHttpAcpBridge,
@@ -277,6 +278,16 @@ export async function runQwenServe(
277278
: {}),
278279
boundWorkspace,
279280
childEnvOverrides,
281+
// #4175 Wave 4 PR 17: `POST /session/:id/approval-mode` accepts
282+
// an opt-in `persist: true` flag. We re-load settings on each
283+
// persist call rather than caching a `LoadedSettings` handle —
284+
// another writer (CLI, another daemon, an editor) could have
285+
// touched the file between calls, so the freshest state wins
286+
// over a stale in-memory cache.
287+
persistApprovalMode: async (workspace, mode) => {
288+
const fresh = loadSettings(workspace);
289+
fresh.setValue(SettingScope.Workspace, 'tools.approvalMode', mode);
290+
},
280291
});
281292
let actualPort = opts.port;
282293
// Pass the already-canonical `boundWorkspace` into `createServeApp`

0 commit comments

Comments
 (0)