Skip to content

fix(cli): use target dir for ACP core settings #9897

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
AaronZ345 wants to merge 3 commits into from
Closed

fix(cli): use target dir for ACP core settings #9897

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
158885a
fix(cli): use target dir for ACP core settings
Aug 24, 2026
38ccd72
test(cli): add ACP config target dir mock
Aug 24, 2026
bc6c0a4
fix(cli): resolve ACP core settings from session target
Aug 24, 2026
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
126 changes: 126 additions & 0 deletions packages/cli/src/acp-integration/acpAgent.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,6 +986,7 @@ import {
GoalConflictError,
GoalInvalidTransitionError,
sessionIdContext,
ExtensionManager,
} from '@qwen-code/qwen-code-core';
import { ndJsonStream } from '@qwen-code/acp-bridge/ndJsonStream';
import {
Expand Down Expand Up @@ -2125,6 +2126,7 @@ describe('QwenAgent MCP SSE/HTTP support', () => {
getWorkspaceContext: vi.fn().mockReturnValue({}),
getDebugMode: vi.fn().mockReturnValue(false),
getToolRegistry: vi.fn().mockReturnValue(undefined),
getTargetDir: vi.fn().mockReturnValue(process.cwd()),
} as unknown as Config;
vi.mocked(loadSettings).mockReturnValue(makeSessionSettings());

Expand Down Expand Up @@ -11014,6 +11016,9 @@ describe('QwenAgent MCP SSE/HTTP support', () => {

// Shared boot helper for the qwen/settings/* handler tests below.
async function bootCoreSettingsAgent(settings: LoadedSettings) {
if (typeof mockConfig.getTargetDir !== 'function') {
mockConfig.getTargetDir = vi.fn().mockReturnValue(process.cwd());
}
vi.mocked(loadSettings).mockReturnValue(settings);
const agentPromise = runAcpAgent(mockConfig, settings, mockArgv);
await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined());
Expand Down Expand Up @@ -11086,6 +11091,127 @@ describe('QwenAgent MCP SSE/HTTP support', () => {
await agentPromise;
});

it('qwen/settings/getCore falls back to config target dir when cwd is omitted', async () => {
const targetDir = '/worktree/.qwen';
mockConfig.getTargetDir = vi.fn().mockReturnValue(targetDir);
const settings = makeCoreSettings();
const { agent, agentPromise } = await bootCoreSettingsAgent(settings);

await agent.extMethod('qwen/settings/getCore', {});

@qwen-code-ci-bot qwen-code-ci-bot Aug 24, 2026

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] The requestedCwd || half of this fallback is pinned by zero tests: all 11 getCore/setCoreValue call sites in the suite omit cwd, and loadSettings is mocked to return the same object regardless of argument. The mutation requestedCwd || this.config.getTargetDir() → this.config.getTargetDir() survives the entire settings suite (probe-verified: Tests 25 passed | 440 skipped). Since the desktop client always sends an explicit cwd, that regression would silently redirect desktop settings calls to the config target dir while the suite stays green. Add a variant per method in which the explicit cwd must win:

it('qwen/settings/getCore prefers explicit cwd over config target dir', async () => {
  mockConfig.getTargetDir = vi.fn().mockReturnValue('/worktree/project');
  const settings = makeCoreSettings();
  const { agent, agentPromise } = await bootCoreSettingsAgent(settings);

  await agent.extMethod('qwen/settings/getCore', { cwd: '/explicit/dir' });

  expect(loadSettings).toHaveBeenCalledWith('/explicit/dir');

  mockConnectionState.resolve();
  await agentPromise;
});
中文说明

这个 fallback 中 requestedCwd || 的一半没有任何测试覆盖：套件中全部 11 处 getCore/setCoreValue 调用都省略了 cwd，且 loadSettings 被 mock 为无论参数如何都返回同一对象。变异 requestedCwd || this.config.getTargetDir() → this.config.getTargetDir() 在整个 settings 套件中存活（已通过探针验证：Tests 25 passed | 440 skipped）。由于 desktop 客户端总是显式传入 cwd，这种回归会在套件全绿的情况下悄悄把 desktop 的 settings 调用重定向到 config target dir。建议为每个方法补一个"显式 cwd 必须优先"的变体测试（代码见上）。

— qwen3.8-max via Qwen Code /review (v0.22.0)


expect(loadSettings).toHaveBeenCalledWith(targetDir);

mockConnectionState.resolve();
await agentPromise;
});

it('qwen/settings/getCore prefers explicit cwd over config target dir', async () => {
mockConfig.getTargetDir = vi.fn().mockReturnValue('/worktree/project');
const settings = makeCoreSettings();
const { agent, agentPromise } = await bootCoreSettingsAgent(settings);

await agent.extMethod('qwen/settings/getCore', { cwd: '/explicit/dir' });

expect(loadSettings).toHaveBeenCalledWith('/explicit/dir');
expect(vi.mocked(ExtensionManager)).toHaveBeenCalledWith(
expect.objectContaining({ workspaceDir: '/explicit/dir' }),
);

mockConnectionState.resolve();
await agentPromise;
});

it('qwen/settings/getCore uses the session target dir when cwd is omitted', async () => {
const sessionId = '11111111-1111-1111-1111-111111111111';
const targetDir = '/relocated/worktree';
const innerConfig = await setupSessionMocks(sessionId);
innerConfig.getTargetDir = vi.fn().mockReturnValue(targetDir);
const settings = makeCoreSettings();
vi.mocked(loadSettings).mockReturnValue(settings);
const { agent, agentPromise } = await bootAcpAgent();

await agent.newSession({ cwd: '/launch/dir', mcpServers: [] });
await agent.extMethod('qwen/settings/getCore', { sessionId });

expect(loadSettings).toHaveBeenLastCalledWith(targetDir);
expect(vi.mocked(ExtensionManager)).toHaveBeenLastCalledWith(
expect.objectContaining({ workspaceDir: targetDir }),
);

mockConnectionState.resolve();
await agentPromise;
});

it('qwen/settings/setCoreValue falls back to config target dir when cwd is omitted', async () => {
const targetDir = '/worktree/.qwen';
mockConfig.getTargetDir = vi.fn().mockReturnValue(targetDir);
const settings = makeCoreSettings();
const { agent, agentPromise } = await bootCoreSettingsAgent(settings);

await agent.extMethod('qwen/settings/setCoreValue', {
scope: 'workspace',
key: 'general.outputLanguage',
value: 'Japanese',
});

expect(loadSettings).toHaveBeenCalledWith(targetDir);
expect(settings.setValue).toHaveBeenCalledWith(
'Workspace',
'general.outputLanguage',
'Japanese',
);

mockConnectionState.resolve();
await agentPromise;
});

it('qwen/settings/setCoreValue prefers explicit cwd over config target dir', async () => {
mockConfig.getTargetDir = vi.fn().mockReturnValue('/worktree/project');
const settings = makeCoreSettings();
const { agent, agentPromise } = await bootCoreSettingsAgent(settings);

await agent.extMethod('qwen/settings/setCoreValue', {
cwd: '/explicit/dir',
scope: 'workspace',
key: 'general.outputLanguage',
value: 'Japanese',
});

expect(loadSettings).toHaveBeenCalledWith('/explicit/dir');
expect(vi.mocked(ExtensionManager)).toHaveBeenCalledWith(
expect.objectContaining({ workspaceDir: '/explicit/dir' }),
);

mockConnectionState.resolve();
await agentPromise;
});

it('qwen/settings/setCoreValue uses the session target dir when cwd is omitted', async () => {
const sessionId = '11111111-1111-1111-1111-111111111111';
const targetDir = '/relocated/worktree';
const innerConfig = await setupSessionMocks(sessionId);
innerConfig.getTargetDir = vi.fn().mockReturnValue(targetDir);
const settings = makeCoreSettings();
vi.mocked(loadSettings).mockReturnValue(settings);
const { agent, agentPromise } = await bootAcpAgent();

await agent.newSession({ cwd: '/launch/dir', mcpServers: [] });
await agent.extMethod('qwen/settings/setCoreValue', {
sessionId,
scope: 'workspace',
key: 'general.outputLanguage',
value: 'Japanese',
});

expect(loadSettings).toHaveBeenLastCalledWith(targetDir);
expect(vi.mocked(ExtensionManager)).toHaveBeenLastCalledWith(
expect.objectContaining({ workspaceDir: targetDir }),
);

mockConnectionState.resolve();
await agentPromise;
});

it('qwen/settings/setCoreValue clears model.baseUrl when setting model.name', async () => {
const settings = makeCoreSettings();
const { agent, agentPromise } = await bootCoreSettingsAgent(settings);
Expand Down
37 changes: 33 additions & 4 deletions packages/cli/src/acp-integration/acpAgent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5691,6 +5691,27 @@ class QwenAgent implements Agent {
};
}

private resolveCoreSettingsCwd(
params: Record<string, unknown>,
requestedCwd: string | undefined,
): string {
if (requestedCwd) {
return requestedCwd;
}
const sessionId =
typeof params['sessionId'] === 'string' ? params['sessionId'] : undefined;
if (sessionId) {
const sessionTargetDir = this.sessions
.get(sessionId)
?.getConfig()
.getTargetDir();
if (sessionTargetDir) {
return sessionTargetDir;
}
}
return this.config.getTargetDir();
}

private syncLivePermissionManagers(
before: PermissionRuleSet,
after: PermissionRuleSet,
Expand Down Expand Up @@ -11138,9 +11159,13 @@ class QwenAgent implements Agent {
return { newSessionId, title, displayName: title };
}
case 'qwen/settings/getCore': {
const settings = loadSettings(cwd);
const coreSettingsCwd = this.resolveCoreSettingsCwd(
params,
requestedCwd,
);
const settings = loadSettings(coreSettingsCwd);
this.settings = settings;
return this.buildCoreSettings(settings, cwd);
return this.buildCoreSettings(settings, coreSettingsCwd);

@qwen-code-ci-bot qwen-code-ci-bot Aug 24, 2026

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] The diff changes four sites (loadSettings + buildCoreSettings in each handler), but these new tests assert only the loadSettings calls. buildCoreSettings feeds its second argument to new ExtensionManager({ workspaceDir: cwd, ... }), and the module-level ExtensionManager mock ignores constructor arguments while mockExtensionManagerState.extensions resets to [] — so the mutation buildCoreSettings(settings, coreSettingsCwd) → buildCoreSettings(settings, cwd) compiles (cwd is still in scope from extMethodInternal) and survives all tests (probe-verified). A partial revert would load settings from the worktree while resolving workspace extensions against process.cwd(), undetected. Assert the second argument as well:

expect(vi.mocked(ExtensionManager)).toHaveBeenCalledWith(
  expect.objectContaining({ workspaceDir: targetDir }),
);
中文说明

本次改动修改了四处（每个 handler 中的 loadSettings + buildCoreSettings），但这些新测试只断言了 loadSettings 的调用。buildCoreSettings 会把第二个参数传给 new ExtensionManager({ workspaceDir: cwd, ... })，而模块级 ExtensionManager mock 忽略构造参数，mockExtensionManagerState.extensions 又被重置为 [] —— 因此变异 buildCoreSettings(settings, coreSettingsCwd) → buildCoreSettings(settings, cwd) 可以编译（cwd 在 extMethodInternal 作用域内仍然可见）且在所有测试中存活（已通过探针验证）。部分回退时会从 worktree 加载 settings、却按 process.cwd() 解析 workspace 扩展，且不会被察觉。建议同时断言第二个参数（代码见上）。

— qwen3.8-max via Qwen Code /review (v0.22.0)

}
case 'qwen/settings/setCoreValue': {
const key = params['key'];
Expand All @@ -11153,7 +11178,11 @@ class QwenAgent implements Agent {
'Unsupported Qwen setting key',
);
}
const settings = loadSettings(cwd);
const coreSettingsCwd = this.resolveCoreSettingsCwd(
params,
requestedCwd,
);
const settings = loadSettings(coreSettingsCwd);
const settingKey = key as QwenCoreSettingKey;
const normalizedValue = normalizeCoreSettingValue(
settingKey,
Expand Down Expand Up @@ -11183,7 +11212,7 @@ class QwenAgent implements Agent {
// `setValue` already persisted to disk and recomputed the in-memory
// merged view, so reloading from disk here is redundant I/O.
this.settings = settings;
return this.buildCoreSettings(settings, cwd);
return this.buildCoreSettings(settings, coreSettingsCwd);
}
case 'qwen/settings/setMcpServer': {
const name = params['name'];
Expand Down
Loading