mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-05-19 16:28:28 +00:00
fix(cli): apply /language output to running session without restart (#4143)
* fix(cli): apply /language output to running session without restart `/language output <lang>` wrote ~/.qwen/output-language.md and persisted the setting, but the rule reaches the model through the system instruction which is bound once when GeminiChat is created at startup. The command therefore had to tell the user "Please restart the application for the changes to take effect." Refresh hierarchical memory after writing the rule file so userMemory re-reads output-language.md, then rebuild and re-bind the main-session system instruction on the live chat via a new GeminiClient.refreshSystemInstruction() helper. The change takes effect on the next turn without restarting the session and without losing conversation history. Drop the restart notice from the success message. Fixes #4142 * test(cli): assert refresh order in /language output and cover failure path - The fix for /language output relies on refreshHierarchicalMemory running *before* refreshSystemInstruction; otherwise the new systemInstruction is rebuilt from stale userMemory and the language switch silently fails to take effect. Assert ordering with invocationCallOrder so a regression cannot pass review. - Add a test that the command still reports success when the in-session refresh throws — the setting is already persisted on disk, so the user-visible message should not surface the refresh failure. - Drop two stale "rule file is updated on restart" comments left over from the pre-fix behavior. --------- Co-authored-by: 秦奇 <gary.gq@alibaba-inc.com>
This commit is contained in:
parent
796de4dfef
commit
a86404e9ea
3 changed files with 114 additions and 11 deletions
|
|
@ -476,7 +476,7 @@ describe('languageCommand', () => {
|
|||
'output Chinese',
|
||||
);
|
||||
|
||||
// Verify setting was saved (rule file is updated on restart)
|
||||
// Verify the setting was persisted.
|
||||
expect(mockContext.services.settings?.setValue).toHaveBeenCalledWith(
|
||||
expect.anything(), // SettingScope.User
|
||||
'general.outputLanguage',
|
||||
|
|
@ -489,20 +489,80 @@ describe('languageCommand', () => {
|
|||
});
|
||||
});
|
||||
|
||||
it('should include restart notice in success message', async () => {
|
||||
it('should apply the new output language to the running session without requiring a restart', async () => {
|
||||
if (!languageCommand.action) {
|
||||
throw new Error('The language command must have an action.');
|
||||
}
|
||||
|
||||
const refreshHierarchicalMemory = vi.fn().mockResolvedValue(undefined);
|
||||
const refreshSystemInstruction = vi.fn().mockResolvedValue(undefined);
|
||||
const getGeminiClient = vi
|
||||
.fn()
|
||||
.mockReturnValue({ refreshSystemInstruction });
|
||||
(
|
||||
mockContext.services as unknown as {
|
||||
config: Record<string, unknown>;
|
||||
}
|
||||
).config = {
|
||||
getModel: vi.fn().mockReturnValue('test-model'),
|
||||
refreshHierarchicalMemory,
|
||||
getGeminiClient,
|
||||
};
|
||||
|
||||
const result = await languageCommand.action(
|
||||
mockContext,
|
||||
'output Japanese',
|
||||
);
|
||||
|
||||
expect(refreshHierarchicalMemory).toHaveBeenCalledTimes(1);
|
||||
expect(getGeminiClient).toHaveBeenCalledTimes(1);
|
||||
expect(refreshSystemInstruction).toHaveBeenCalledTimes(1);
|
||||
// Memory MUST be refreshed before the system instruction is rebuilt;
|
||||
// otherwise the new instruction would be built from stale userMemory
|
||||
// and the language switch would silently fail to take effect.
|
||||
const memoryOrder = refreshHierarchicalMemory.mock.invocationCallOrder[0];
|
||||
const instructionOrder =
|
||||
refreshSystemInstruction.mock.invocationCallOrder[0];
|
||||
expect(memoryOrder).toBeLessThan(instructionOrder);
|
||||
// The success message no longer asks the user to restart.
|
||||
expect(result).toEqual({
|
||||
type: 'message',
|
||||
messageType: 'info',
|
||||
content: expect.stringContaining('restart'),
|
||||
content: expect.not.stringContaining('restart'),
|
||||
});
|
||||
});
|
||||
|
||||
it('should still report success when applying to the running session fails', async () => {
|
||||
if (!languageCommand.action) {
|
||||
throw new Error('The language command must have an action.');
|
||||
}
|
||||
|
||||
const refreshHierarchicalMemory = vi
|
||||
.fn()
|
||||
.mockRejectedValue(new Error('boom'));
|
||||
(
|
||||
mockContext.services as unknown as {
|
||||
config: Record<string, unknown>;
|
||||
}
|
||||
).config = {
|
||||
getModel: vi.fn().mockReturnValue('test-model'),
|
||||
refreshHierarchicalMemory,
|
||||
// No getGeminiClient — refreshSystemInstruction must not be reached.
|
||||
};
|
||||
|
||||
const result = await languageCommand.action(mockContext, 'output Korean');
|
||||
|
||||
// The setting was still persisted; the user-facing message reports
|
||||
// success and does not surface the in-session refresh failure.
|
||||
expect(mockContext.services.settings?.setValue).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
'general.outputLanguage',
|
||||
'Korean',
|
||||
);
|
||||
expect(result).toEqual({
|
||||
type: 'message',
|
||||
messageType: 'info',
|
||||
content: expect.stringContaining('LLM output language set to'),
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -536,8 +596,7 @@ describe('languageCommand', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('should save setting without immediate rule file update', async () => {
|
||||
// Even though rule file updates happen on restart, the setting should still be saved
|
||||
it('should save the setting and report success on a valid argument', async () => {
|
||||
if (!languageCommand.action) {
|
||||
throw new Error('The language command must have an action.');
|
||||
}
|
||||
|
|
@ -868,7 +927,7 @@ describe('languageCommand', () => {
|
|||
|
||||
const result = await outputSubcommand.action(mockContext, 'French');
|
||||
|
||||
// Verify setting was saved (rule file is updated on restart)
|
||||
// Verify the setting was persisted.
|
||||
expect(mockContext.services.settings?.setValue).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
'general.outputLanguage',
|
||||
|
|
|
|||
|
|
@ -128,6 +128,11 @@ async function setUiLanguage(
|
|||
/**
|
||||
* Handles the /language output command, updating both the setting and the rule file.
|
||||
* 'auto' is preserved in settings but resolved to the detected language for the rule file.
|
||||
*
|
||||
* After persisting the change, hierarchical memory is reloaded so `output-language.md`
|
||||
* flows back into `userMemory`, and the live chat's system instruction is rebuilt
|
||||
* in place. The new language therefore takes effect on the next turn without
|
||||
* restarting the session and without losing conversation history.
|
||||
*/
|
||||
async function setOutputLanguage(
|
||||
context: CommandContext,
|
||||
|
|
@ -155,6 +160,22 @@ async function setOutputLanguage(
|
|||
}
|
||||
}
|
||||
|
||||
// Apply the new rule to the running session: refresh hierarchical memory
|
||||
// so output-language.md is re-read into userMemory, then rebuild and
|
||||
// re-bind the system instruction on the live chat.
|
||||
const config = context.services.config;
|
||||
if (config) {
|
||||
try {
|
||||
await config.refreshHierarchicalMemory();
|
||||
await config.getGeminiClient().refreshSystemInstruction();
|
||||
} catch (error) {
|
||||
debugLogger.warn(
|
||||
'Failed to apply output language to running session:',
|
||||
error,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Format display message
|
||||
const displayLang = isAuto
|
||||
? `${t('Auto (detect from system)')} → ${resolved}`
|
||||
|
|
@ -163,11 +184,7 @@ async function setOutputLanguage(
|
|||
return {
|
||||
type: 'message',
|
||||
messageType: 'info',
|
||||
content: [
|
||||
t('LLM output language set to {{lang}}', { lang: displayLang }),
|
||||
'',
|
||||
t('Please restart the application for the changes to take effect.'),
|
||||
].join('\n'),
|
||||
content: t('LLM output language set to {{lang}}', { lang: displayLang }),
|
||||
};
|
||||
} catch (error) {
|
||||
return {
|
||||
|
|
|
|||
|
|
@ -448,6 +448,33 @@ export class GeminiClient {
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Rebuilds the main-session system instruction from the current
|
||||
* `userMemory` / model / prompt overrides and re-binds it to the live chat.
|
||||
*
|
||||
* Use this after mutating inputs that feed into the system instruction
|
||||
* (e.g. user memory refreshed from `output-language.md`) so the change
|
||||
* takes effect on the next turn without restarting the session. No-op if
|
||||
* no chat has been started yet.
|
||||
*/
|
||||
async refreshSystemInstruction(): Promise<void> {
|
||||
if (!this.chat) {
|
||||
return;
|
||||
}
|
||||
const toolRegistry = this.config.getToolRegistry();
|
||||
await toolRegistry.warmAll();
|
||||
const deferredSummary = toolRegistry.getDeferredToolSummary();
|
||||
const toolSearchAvailable = !!toolRegistry.getTool(ToolNames.TOOL_SEARCH);
|
||||
const deferredTools = toolSearchAvailable
|
||||
? deferredSummary.filter(
|
||||
(t) => !toolRegistry.isDeferredToolRevealed(t.name),
|
||||
)
|
||||
: undefined;
|
||||
this.chat.setSystemInstruction(
|
||||
this.getMainSessionSystemInstruction(deferredTools),
|
||||
);
|
||||
}
|
||||
|
||||
async startChat(extraHistory?: Content[]): Promise<GeminiChat> {
|
||||
this.forceFullIdeContext = true;
|
||||
// Clear stale cache params on session reset to prevent cross-session leakage
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue