Skip to content
Merged
40 changes: 39 additions & 1 deletion packages/acp-bridge/src/bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4165,6 +4165,22 @@ describe('createAcpSessionBridge', () => {
expect(setModelCalls).toHaveLength(1);
expect(setModelCalls[0]?.sessionId).toBe(session.sessionId);
expect(setModelCalls[0]?.modelId).toBe('qwen3-coder');
const abort = new AbortController();
const iter = bridge.subscribeEvents(session.sessionId, {
signal: abort.signal,
lastEventId: 0,
});
const it = iter[Symbol.asyncIterator]();
const switched = await it.next();
expect(switched.value?.type).toBe('model_switched');
const settingsChanged = await it.next();
expect(settingsChanged.value?.type).toBe('settings_changed');
expect(settingsChanged.value?.originatorClientId).toBe(session.clientId);
expect(settingsChanged.value?.data).toEqual({
key: 'model.name',
value: 'qwen3-coder',
});
abort.abort();
await bridge.shutdown();
});

Expand Down Expand Up @@ -5246,6 +5262,12 @@ describe('createAcpSessionBridge', () => {
sessionId: session.sessionId,
modelId: 'qwen3-coder',
});
const settingsChanged = await it.next();
expect(settingsChanged.value?.type).toBe('settings_changed');
expect(settingsChanged.value?.data).toEqual({
key: 'model.name',
value: 'qwen3-coder',
});
abort.abort();
await bridge.shutdown();
});
Expand All @@ -5268,6 +5290,9 @@ describe('createAcpSessionBridge', () => {
const next = await it.next();
expect(next.value?.type).toBe('model_switched');
expect(next.value?.originatorClientId).toBe(session.clientId);
const settingsChanged = await it.next();
expect(settingsChanged.value?.type).toBe('settings_changed');
expect(settingsChanged.value?.originatorClientId).toBe(session.clientId);
abort.abort();
await bridge.shutdown();
});
Expand Down Expand Up @@ -9312,18 +9337,31 @@ describe('createHttpAcpBridge — side-channel state layer (#4511)', () => {
undefined,
);

const seen: Array<{ type: string; modelId?: string }> = [];
const seen: Array<{ type: string; modelId?: string; value?: string }> =
[];
for await (const e of iter) {
seen.push({
type: e.type,
modelId: (e.data as { modelId?: string })?.modelId,
value: (e.data as { value?: string })?.value,
});
if (seen.filter((s) => s.type === 'model_switched').length === 2) break;
}
const switches = seen.filter((s) => s.type === 'model_switched');
// First the requested change, then the corrective one from reconcile.
expect(switches[0]?.modelId).toBe('qwen-max');
expect(switches[1]?.modelId).toBe('qwen-turbo');
const requestedSwitchIndex = seen.findIndex(
(s) => s.type === 'model_switched' && s.modelId === 'qwen-max',
);
const settingsChangedIndex = seen.findIndex(
(s) => s.type === 'settings_changed' && s.value === 'qwen-max',
);
const correctiveSwitchIndex = seen.findIndex(
(s) => s.type === 'model_switched' && s.modelId === 'qwen-turbo',
);
expect(settingsChangedIndex).toBeGreaterThan(requestedSwitchIndex);
expect(settingsChangedIndex).toBeLessThan(correctiveSwitchIndex);
abort.abort();
await bridge.shutdown();
});
Expand Down
18 changes: 16 additions & 2 deletions packages/acp-bridge/src/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1614,6 +1614,14 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
transportClosed,
]);
publishModelSwitched(entry, modelId, originatorClientId);
broadcastWorkspaceEvent({

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] settings_changed is broadcast here and in setSessionModel (line 3955), but two other publishModelSwitched call sites were not updated:

  1. Line 1220 (agent-initiated): onModelPromoted callback — invoked when the ACP child sends current_model_update (agent changes its own model mid-conversation). Only publishModelSwitched is called, no settings_changed.
  2. Line 1999 (reconciliation): After session reattach, if the actual model diverges from cached currentModelId, the bridge corrects via publishModelSwitched — but no settings_changed.

Clients using settings_changed to refresh settings/model-name display will show stale data when the agent autonomously switches models or the bridge corrects on reconnect. The existing reconciliation test (around line 9337) only asserts settings_changed for the client-requested switch, not the corrective one.

— qwen3.7-max via Qwen Code /review

type: 'settings_changed',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] This broadcastWorkspaceEvent({ type: 'settings_changed', ... }) block is duplicated verbatim at two call sites (here and in setSessionModel), always immediately after publishModelSwitched. Consider folding the broadcast into publishModelSwitched itself so the two events are always emitted as a pair. This also avoids the current silent asymmetry at the reconcile path (which calls publishModelSwitched without settings_changed).

— qwen3.7-max via Qwen Code /review

data: {
key: 'model.name',
value: modelId,
},
...(originatorClientId ? { originatorClientId } : {}),
});
succeeded = true;
} catch (err) {
// Surface the failure to ALL attached clients, not just the
Expand Down Expand Up @@ -3944,6 +3952,14 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
// the agent's authoritative canonical id and re-publishes if it
// differs.
publishModelSwitched(entry, req.modelId, originatorClientId);
broadcastWorkspaceEvent({
type: 'settings_changed',
data: {
key: 'model.name',
value: req.modelId,
},
...(originatorClientId ? { originatorClientId } : {}),
});
succeeded = true;
return result;
} finally {
Expand Down Expand Up @@ -3983,8 +3999,6 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
});
throw err;
}
// model_switched is published inside the work callback above (while the
// suppress flag is still set), mirroring applyModelServiceId.
return response;
},

Expand Down
1 change: 1 addition & 0 deletions packages/acp-bridge/src/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,7 @@ export interface ServeWorkspaceProvidersStatus {
v: typeof STATUS_SCHEMA_VERSION;
workspaceCwd: string;
initialized: boolean;
acpChannelLive?: boolean;
current?: ServeWorkspaceProviderCurrent;
providers: ServeWorkspaceProviderStatus[];
errors?: ServeStatusCell[];
Expand Down
22 changes: 14 additions & 8 deletions packages/cli/src/acp-integration/acpAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,14 +460,19 @@ vi.mock('./session/Session.js', () => ({
availableSkills: [],
}),
}));
vi.mock('../utils/acpModelUtils.js', () => ({
formatAcpModelId: vi.fn(
(modelId: string, authType: string) => `${modelId}(${authType})`,
),
parseAcpBaseModelId: vi.fn((modelId: string) =>
modelId.replace(/\([^)]+\)$/, ''),
),
}));
vi.mock('../utils/acpModelUtils.js', async (importOriginal) => {
const actual =
await importOriginal<typeof import('../utils/acpModelUtils.js')>();
return {
...actual,
formatAcpModelId: vi.fn(
(modelId: string, authType: string) => `${modelId}(${authType})`,
),
parseAcpBaseModelId: vi.fn((modelId: string) =>
modelId.replace(/\([^)]+\)$/, ''),
),
};
});
vi.mock('../utils/languageUtils.js', () => ({
updateOutputLanguageFile: vi.fn(),
writeOutputLanguageAndRegisterPath: vi.fn(
Expand Down Expand Up @@ -1067,6 +1072,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => {
beforeEach(() => {
vi.clearAllMocks();
mockConnectionState.reset();
mockRunExitCleanup.mockResolvedValue(undefined);
mockExtensionManagerState.extensions = [];
mockExtensionManagerState.refreshCache.mockResolvedValue(undefined);
lastSessionMock = undefined;
Expand Down
20 changes: 1 addition & 19 deletions packages/cli/src/acp-integration/acpAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ import { HistoryReplayer } from './session/HistoryReplayer.js';
import {
formatAcpModelId,
parseAcpBaseModelId,
sanitizeProviderBaseUrl,
} from '../utils/acpModelUtils.js';
import {
updateOutputLanguageFile,
Expand Down Expand Up @@ -243,25 +244,6 @@ function hasFailedDisplayStatus(
(display as { status?: unknown }).status === 'failed'
);
}

function sanitizeProviderBaseUrl(baseUrl: string): string {
const scheme = baseUrl.match(/^[A-Za-z][A-Za-z\d+.-]*:\/\//);
if (!scheme) {
return baseUrl;
}

const authorityStart = scheme[0].length;
const rest = baseUrl.slice(authorityStart);
const authorityEnd = rest.search(/[/?#]/);
const authority = authorityEnd === -1 ? rest : rest.slice(0, authorityEnd);
const at = authority.lastIndexOf('@');
if (at === -1) {
return baseUrl;
}

return `${baseUrl.slice(0, authorityStart)}${authority.slice(at + 1)}${rest.slice(authority.length)}`;
}

/**
* Env-var candidates per auth method, used by `buildAuthPreflightCell` for
* a side-effect-free presence check. Mirrors `AUTH_ENV_MAPPINGS` from
Expand Down
4 changes: 4 additions & 0 deletions packages/cli/src/serve/run-qwen-serve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import type {
} from '@qwen-code/qwen-code-core';
import { createBridgeFileSystemAdapter } from './bridge-file-system-adapter.js';
import { createDaemonStatusProvider } from './daemon-status-provider.js';
import { createWorkspaceProvidersStatusProvider } from './workspace-providers-status.js';
import { isLoopbackBind } from './loopback-binds.js';
import { resolveWebShellDir } from './web-shell-static.js';
import { parseAllowOriginPatterns } from './auth.js';
Expand Down Expand Up @@ -907,6 +908,8 @@ export async function runQwenServe(
// service so both answer env/preflight cells from the same daemon-local
// implementation.
const statusProvider = createDaemonStatusProvider();
const workspaceProvidersStatusProvider =
createWorkspaceProvidersStatusProvider();

const bridge =
deps.bridge ??
Expand Down Expand Up @@ -989,6 +992,7 @@ export async function runQwenServe(
contextFilename: contextFilenameForInit ?? 'QWEN.md',
// Daemon-host status provider for env + preflight cells.
statusProvider,
workspaceProvidersStatusProvider,
// Channel liveness check — proxied through the bridge's live-channel
// probe (not session count: a channel can be live with zero attached
// sessions during the cold-spawn window).
Expand Down
113 changes: 66 additions & 47 deletions packages/cli/src/serve/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ import type {
import { CAPABILITIES_SCHEMA_VERSION, type ServeOptions } from './types.js';
import type { DaemonLogger } from './daemon-logger.js';
import { FsError, type WorkspaceFileSystemFactory } from './fs/index.js';
import { resetHomeEnvBootstrapForTesting } from '../config/settings.js';

const baseOpts: ServeOptions = {
hostname: '127.0.0.1',
Expand All @@ -120,6 +121,14 @@ function fakeDaemonLog(): DaemonLogger {
};
}

function restoreEnv(key: string, value: string | undefined): void {
if (value === undefined) {
delete process.env[key];
} else {
process.env[key] = value;
}
}

// Workspace fixtures must round-trip through `path.resolve` so the
// expected values match the canonicalized form the route produces on
// every platform. On Windows `path.resolve('/work/bound')` returns
Expand Down Expand Up @@ -2197,7 +2206,16 @@ describe('createServeApp', () => {
expect(res.body.servers[2].disabledReason).toBe('budget');
});

it('returns workspace skills and providers status from the bridge', async () => {
it('returns workspace skills from the bridge and providers from daemon-local settings', async () => {
const tempHome = await fsp.mkdtemp(
path.join(os.tmpdir(), 'qwen-serve-providers-'),
);
const previousQwenHome = process.env['QWEN_HOME'];
const previousRuntimeDir = process.env['QWEN_RUNTIME_DIR'];
const previousSystemSettings =
process.env['QWEN_CODE_SYSTEM_SETTINGS_PATH'];
const previousSystemDefaults =
process.env['QWEN_CODE_SYSTEM_DEFAULTS_PATH'];
const skills: ServeWorkspaceSkillsStatus = {
v: 1,
workspaceCwd: WS_BOUND,
Expand All @@ -2213,54 +2231,55 @@ describe('createServeApp', () => {
},
],
};
const providers: ServeWorkspaceProvidersStatus = {
v: 1,
workspaceCwd: WS_BOUND,
initialized: true,
current: { authType: 'qwen', modelId: 'qwen3(qwen)' },
providers: [
{
kind: 'model_provider',
status: 'ok',
authType: 'qwen',
current: true,
models: [
{
modelId: 'qwen3(qwen)',
baseModelId: 'qwen3',
name: 'Qwen 3',
description: null,
contextLimit: 4096,
isCurrent: true,
isRuntime: false,
},
],
},
],
};
const bridge = fakeBridge({
workspaceSkillsImpl: async () => skills,
workspaceProvidersImpl: async () => providers,
});
const app = createServeApp(
{ ...baseOpts, workspace: WS_BOUND },
undefined,
{ bridge },
);
try {
process.env['QWEN_HOME'] = path.join(tempHome, 'home');
process.env['QWEN_RUNTIME_DIR'] = path.join(tempHome, 'runtime');
process.env['QWEN_CODE_SYSTEM_SETTINGS_PATH'] = path.join(
tempHome,
'system-settings.json',
);
process.env['QWEN_CODE_SYSTEM_DEFAULTS_PATH'] = path.join(
tempHome,
'system-defaults.json',
);
resetHomeEnvBootstrapForTesting();

const skillsRes = await request(app)
.get('/workspace/skills')
.set('Host', `127.0.0.1:${baseOpts.port}`);
const providersRes = await request(app)
.get('/workspace/providers')
.set('Host', `127.0.0.1:${baseOpts.port}`);
const bridge = fakeBridge({
workspaceSkillsImpl: async () => skills,
});
const app = createServeApp(
{ ...baseOpts, workspace: WS_BOUND },
undefined,
{ bridge },
);

const skillsRes = await request(app)
.get('/workspace/skills')
.set('Host', `127.0.0.1:${baseOpts.port}`);
const providersRes = await request(app)
.get('/workspace/providers')
.set('Host', `127.0.0.1:${baseOpts.port}`);

expect(skillsRes.status).toBe(200);
expect(skillsRes.body).toEqual(skills);
expect(providersRes.status).toBe(200);
expect(providersRes.body).toEqual(providers);
expect(bridge.workspaceSkillsCalls).toBe(1);
expect(bridge.workspaceProvidersCalls).toBe(1);
expect(skillsRes.status).toBe(200);
expect(skillsRes.body).toEqual(skills);
expect(providersRes.status).toBe(200);
expect(providersRes.body).toMatchObject({
v: 1,
workspaceCwd: WS_BOUND,
initialized: true,
acpChannelLive: false,
});
expect(providersRes.body.providers.length).toBeGreaterThan(0);
expect(bridge.workspaceSkillsCalls).toBe(1);
expect(bridge.workspaceProvidersCalls).toBe(0);
} finally {
restoreEnv('QWEN_HOME', previousQwenHome);
restoreEnv('QWEN_RUNTIME_DIR', previousRuntimeDir);
restoreEnv('QWEN_CODE_SYSTEM_SETTINGS_PATH', previousSystemSettings);
restoreEnv('QWEN_CODE_SYSTEM_DEFAULTS_PATH', previousSystemDefaults);
resetHomeEnvBootstrapForTesting();
await fsp.rm(tempHome, { recursive: true, force: true });
}
});

it('returns workspace tools status from the bridge', async () => {
Expand Down
3 changes: 3 additions & 0 deletions packages/cli/src/serve/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import { mapDomainErrorToErrorKind } from '@qwen-code/acp-bridge';
import { QwenOAuthDeviceFlowProvider } from './auth/qwen-device-flow-provider.js';
import { createBridgeFileSystemAdapter } from './bridge-file-system-adapter.js';
import { createDaemonStatusProvider } from './daemon-status-provider.js';
import { createWorkspaceProvidersStatusProvider } from './workspace-providers-status.js';
import { isServeDebugMode } from './debug-mode.js';
import { SUPPORTED_LANGUAGES } from '../i18n/index.js';
import { loadSettings } from '../config/settings.js';
Expand Down Expand Up @@ -1207,6 +1208,8 @@ export function createServeApp(
boundWorkspace,
contextFilename: deps.contextFilename ?? 'QWEN.md',
statusProvider: createDaemonStatusProvider(),
workspaceProvidersStatusProvider:
createWorkspaceProvidersStatusProvider(),
isChannelLive: () => bridge.isChannelLive(),
persistDisabledTools:
deps.persistDisabledTools ??
Expand Down
Loading
Loading