Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions packages/cli/src/serve/acpHttp/dispatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1771,8 +1771,10 @@ export class AcpDispatcher {
) {
closedIds.push(sid);
} else {
const safeSessionId = logSafe(sid.slice(0, 8));
Comment thread
doudouOUC marked this conversation as resolved.
const safeMessage = logSafe(msg);
writeStderrLine(
`qwen serve: /acp sessions/delete closeSession(${sid.slice(0, 8)}) failed: ${msg}`,
`qwen serve: /acp sessions/delete closeSession(${safeSessionId}) failed: ${safeMessage}`,
);
closeErrors.push({ sessionId: sid, error: msg });
}
Expand All @@ -1782,8 +1784,10 @@ export class AcpDispatcher {
const svc = new SessionService(this.boundWorkspace);
const removeResult = await svc.removeSessions(closedIds);
for (const e of removeResult.errors) {
const safeSessionId = logSafe(e.sessionId.slice(0, 8));
Comment thread
doudouOUC marked this conversation as resolved.
const safeMessage = logSafe(errMsg(e.error));
writeStderrLine(
`qwen serve: /acp sessions/delete removeSessions(${e.sessionId.slice(0, 8)}) failed: ${e.error.message}`,
`qwen serve: /acp sessions/delete removeSessions(${safeSessionId}) failed: ${safeMessage}`,
);
}
this.replyConn(conn, id, {
Expand All @@ -1793,7 +1797,7 @@ export class AcpDispatcher {
...closeErrors,
...removeResult.errors.map((e) => ({
sessionId: e.sessionId,
error: e.error.message,
error: errMsg(e.error),
})),
],
} as unknown);
Expand Down
10 changes: 7 additions & 3 deletions packages/cli/src/serve/acpHttp/jsonRpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,14 +120,18 @@ export function isResponse(m: unknown): m is JsonRpcResponse {
);
}

const LOG_SAFE_RE = new RegExp(
String.raw`[\x00-\x1f\x7f-\x9f\u200b-\u200f\u2028-\u202e\u2066-\u2069\ufeff]`,
'g',
);

/**
* Strip C0 control chars + DEL from values interpolated into operator-facing
* Strip terminal control chars from values interpolated into operator-facing
* stderr logs, so a client-controlled `sessionId`/`method`/error string can't
* forge or split log lines (log injection). Shared by the transport modules.
*/
export function logSafe(s: string): string {
// eslint-disable-next-line no-control-regex
return s.replace(/[\u0000-\u001f\u007f\u0080-\u009f]/g, ' ');
return s.replace(LOG_SAFE_RE, ' ');
}

export function success(id: JsonRpcId, result: unknown): JsonRpcSuccess {
Expand Down
89 changes: 89 additions & 0 deletions packages/cli/src/serve/acpHttp/transport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
SessionShellClientRequiredError,
SessionShellDisabledError,
} from '@qwen-code/acp-bridge/bridgeErrors';
import { SessionService } from '@qwen-code/qwen-code-core';
import type { DaemonWorkspaceService } from '../workspace-service/types.js';
import { mountAcpHttp } from './index.js';

Expand Down Expand Up @@ -90,6 +91,7 @@ class FakeBridge {
lastSetModel: unknown;
lastSpawnScope: string | undefined;
closeShouldThrow = false;
closeError: Error | undefined;
killed: string[] = [];
cancelled: string[] = [];
/** When set, spawnOrAttach/loadSession await it (to simulate a slow bridge). */
Expand Down Expand Up @@ -213,6 +215,7 @@ class FakeBridge {
async closeSession(sessionId: string) {
this.closedSessions.push(sessionId);
if (this.closeGate) await this.closeGate;
if (this.closeError) throw this.closeError;
if (this.closeShouldThrow) throw new Error('bridge close failed');
}
async detachClient(sessionId: string, clientId?: string) {
Expand Down Expand Up @@ -2204,6 +2207,92 @@ describe('ACP Streamable HTTP transport (over the wire)', () => {
const frames = await takeFrames(await streamRes, 1);
expect(frames[0]).toMatchObject({ error: { code: -32602 } });
});

it('_qwen/sessions/delete sanitizes stderr close errors', async () => {
const lineSep = '\u2028';
const bidiOverride = '\u202e';
bridge.closeError = new Error(
`close\nFAILED\r\x1b[31m${lineSep}${bidiOverride}`,
);
const connId = await initialize();
const streamRes = openStream(connId);
await new Promise((r) => setTimeout(r, 30));
await post(connId, {
jsonrpc: '2.0',
id: 67,
method: '_qwen/sessions/delete',
params: { sessionIds: [`sess${lineSep}FAKE\r\x1b[31m`] },
});
const frames = await takeFrames(await streamRes, 1);
expect(frames[0]).toMatchObject({
result: { removed: [], notFound: [] },
});
const deleteLog = stdioMocks.writeStderrLine.mock.calls
.map(([line]) => line)
.find((line) => line.includes('sessions/delete'));
expect(deleteLog).toContain(
'closeSession(sess FAK) failed: close FAILED [31m',
);
expect(deleteLog).not.toContain('\n');
expect(deleteLog).not.toContain('\r');
expect(deleteLog).not.toContain('\x1b');
expect(deleteLog).not.toContain(lineSep);
expect(deleteLog).not.toContain(bidiOverride);
});

it('_qwen/sessions/delete sanitizes stderr remove errors', async () => {
const lineSep = '\u2028';
const bidiOverride = '\u202e';
const sessionId = `sess${lineSep}FAKE\r\x1b[31m`;
const removeError = `remove\nFAILED\r\x1b[31m${lineSep}${bidiOverride}`;
const removeSessionsSpy = vi
.spyOn(SessionService.prototype, 'removeSessions')
.mockResolvedValueOnce({
removed: [],
notFound: [],
errors: [
{
sessionId,
error: removeError as unknown as Error,
},
],
});

try {
const connId = await initialize();
const streamRes = openStream(connId);
await new Promise((r) => setTimeout(r, 30));
await post(connId, {
jsonrpc: '2.0',
id: 68,
method: '_qwen/sessions/delete',
params: { sessionIds: [sessionId] },
});
const frames = await takeFrames(await streamRes, 1);
expect(frames[0]).toMatchObject({
result: {
removed: [],
notFound: [],
errors: [{ sessionId, error: removeError }],
},
});
expect(removeSessionsSpy).toHaveBeenCalledWith([sessionId]);

const deleteLog = stdioMocks.writeStderrLine.mock.calls
.map(([line]) => line)
.find((line) => line.includes('sessions/delete'));
expect(deleteLog).toContain(
'removeSessions(sess FAK) failed: remove FAILED [31m',
);
expect(deleteLog).not.toContain('\n');
expect(deleteLog).not.toContain('\r');
expect(deleteLog).not.toContain('\x1b');
expect(deleteLog).not.toContain(lineSep);
expect(deleteLog).not.toContain(bidiOverride);
} finally {
removeSessionsSpy.mockRestore();
}
});
});

describe('auth methods', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/tools/mcp-pool-key.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,8 @@ function sortedEntries(
* distinct entry.
*
* Hashed fields (transport-defining):
* transport, command, args, cwd, env, url, httpUrl, headers,
* timeout, oauth
* transport, command, args, cwd, env, url, httpUrl, tcp, headers,
* timeout, oauth, authProviderType, targetAudience, targetServiceAccount
*
* Excluded fields (per-session filter / metadata; do NOT change the
* underlying transport):
Expand Down
11 changes: 9 additions & 2 deletions packages/sdk-typescript/src/daemon/DaemonClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1502,14 +1502,21 @@ export class DaemonClient {
* `timeoutMs` when their threat model needs a tighter cap, or `0`
* to disable the timeout entirely.
*
* `entryIndex` targets one pooled entry by index. Use `'*'` to
* restart all entries for a pooled server.
*
* Pre-flight `caps.features.workspace_mcp_restart` before calling.
*/
async restartMcpServer(
serverName: string,
opts?: { clientId?: string; timeoutMs?: number },
opts?: { clientId?: string; entryIndex?: number | '*'; timeoutMs?: number },
Comment thread
doudouOUC marked this conversation as resolved.
): Promise<DaemonMcpRestartResult> {
const query =
opts?.entryIndex === undefined
? ''
: `?entryIndex=${encodeURIComponent(String(opts.entryIndex))}`;
return await this.fetchWithTimeout(
`${this.baseUrl}/workspace/mcp/${encodeURIComponent(serverName)}/restart`,
`${this.baseUrl}/workspace/mcp/${encodeURIComponent(serverName)}/restart${query}`,
{
method: 'POST',
headers: this.headers(
Expand Down
9 changes: 9 additions & 0 deletions packages/sdk-typescript/src/daemon/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,15 @@ export type DaemonMcpRestartResult =
restarted: false;
skipped: true;
reason: 'in_flight' | 'disabled' | 'budget_would_exceed';
}
| {
serverName: string;
entries: Array<{
entryIndex: number;
restarted: boolean;
durationMs?: number;
reason?: string;
}>;
};

export type DaemonMcpManageAction =
Expand Down
39 changes: 39 additions & 0 deletions packages/sdk-typescript/test/unit/DaemonClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1701,6 +1701,45 @@ describe('DaemonClient', () => {
expect(calls[0]?.headers['x-qwen-client-id']).toBe('client-1');
});

it('forwards entryIndex and returns pool entry results', async () => {
const { fetch, calls } = recordingFetch(() =>
jsonResponse(200, {
serverName: 'docs',
entries: [
{ entryIndex: 3, restarted: true, durationMs: 42 },
{ entryIndex: 4, restarted: false, reason: 'in_flight' },
],
}),
);
const client = new DaemonClient({ baseUrl: 'http://daemon', fetch });
const result = await client.restartMcpServer('docs', { entryIndex: 3 });
expect(calls[0]?.url).toBe(
'http://daemon/workspace/mcp/docs/restart?entryIndex=3',
);
expect('entries' in result).toBe(true);
if (!('entries' in result)) throw new Error('expected entry results');
expect(result.entries).toEqual([
{ entryIndex: 3, restarted: true, durationMs: 42 },
{ entryIndex: 4, restarted: false, reason: 'in_flight' },
]);
});

it('forwards wildcard entryIndex unchanged', async () => {
const { fetch, calls } = recordingFetch(() =>
jsonResponse(200, {
serverName: 'docs',
entries: [],
}),
);
const client = new DaemonClient({ baseUrl: 'http://daemon', fetch });

await client.restartMcpServer('docs', { entryIndex: '*' });

expect(calls[0]?.url).toBe(
'http://daemon/workspace/mcp/docs/restart?entryIndex=*',
);
});

it('throws on 404 when the daemon reports an unknown server', async () => {
const { fetch } = recordingFetch(() =>
jsonResponse(404, { error: 'no such server' }),
Expand Down
20 changes: 19 additions & 1 deletion packages/web-shell/client/components/dialogs/McpDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,25 @@ export function McpDialog({ onClose }: McpDialogProps) {
setMessage(null);
restartServer(serverName)
.then((result) => {
if (result.restarted) {
if ('entries' in result) {
Comment thread
doudouOUC marked this conversation as resolved.
const restartedCount = result.entries.filter(
(entry) => entry.restarted,
).length;
const failedReasons = result.entries
.filter((entry) => !entry.restarted)
.map(
(entry) => `#${entry.entryIndex}: ${entry.reason ?? 'unknown'}`,
)
.join(', ');
setMessage(
t('mcp.restartEntries', {
name: result.serverName,
restarted: restartedCount,
total: result.entries.length,
failedReasons,
}),
);
} else if (result.restarted) {
setMessage(
t('mcp.restarted', {
name: result.serverName,
Expand Down
4 changes: 4 additions & 0 deletions packages/web-shell/client/i18n.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,8 @@ const EN: Messages = {
'Note: First startup may take longer. Tool availability will update automatically.',
'mcp.restartSkipped': (v) => `Skipped ${v?.name ?? ''}: ${v?.reason ?? ''}`,
'mcp.restarted': (v) => `Restarted ${v?.name ?? ''} in ${v?.duration ?? 0}ms`,
'mcp.restartEntries': (v) =>
`Restarted ${v?.restarted ?? 0}/${v?.total ?? 0} ${v?.name ?? ''} entries${v?.failedReasons ? ` (failed: ${v.failedReasons})` : ''}`,
'mcp.source': 'Source',
'mcp.source.extension': 'Extension',
'mcp.source.project': 'Workspace Settings',
Expand Down Expand Up @@ -1308,6 +1310,8 @@ const ZH: Messages = {
'mcp.startingNote': '注意:首次启动可能需要更长时间。工具可用性会自动更新。',
'mcp.restartSkipped': (v) => `已跳过 ${v?.name ?? ''}:${v?.reason ?? ''}`,
'mcp.restarted': (v) => `已重启 ${v?.name ?? ''},耗时 ${v?.duration ?? 0}ms`,
'mcp.restartEntries': (v) =>
`已重启 ${v?.name ?? ''} 的 ${v?.restarted ?? 0}/${v?.total ?? 0} 个条目${v?.failedReasons ? `(失败:${v.failedReasons})` : ''}`,
'mcp.source': '来源',
'mcp.source.extension': '扩展',
'mcp.source.project': '工作区设置',
Expand Down
Loading