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
433 changes: 433 additions & 0 deletions docs/design/skill-nudge/skill-nudge.md

Large diffs are not rendered by default.

10 changes: 8 additions & 2 deletions packages/cli/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
NativeLspService,
isBareMode,
isToolEnabled,
type ConfigParameters,
type MCPServerConfig,
} from '@qwen-code/qwen-code-core';
import { extensionsCommand } from '../commands/extensions.js';
Expand Down Expand Up @@ -1171,7 +1172,7 @@ export async function loadCliConfig(

const modelProvidersConfig = settings.modelProviders;

const config = new Config({
const configParams: ConfigParameters = {
sessionId,
sessionData,
embeddingModel: DEFAULT_QWEN_EMBEDDING_MODEL,
Expand Down Expand Up @@ -1301,6 +1302,9 @@ export async function loadCliConfig(
? false
: (settings.memory?.enableManagedAutoMemory ?? true),
enableManagedAutoDream: settings.memory?.enableManagedAutoDream ?? false,
enableAutoSkill: bareMode
? false
: (settings.memory?.enableAutoSkill ?? false),
fastModel: settings.fastModel || undefined,
// Use separated hooks if provided, otherwise fall back to merged hooks
userHooks: bareMode
Expand Down Expand Up @@ -1338,7 +1342,9 @@ export async function loadCliConfig(
: undefined,
}
: undefined,
});
};

const config = new Config(configParams);

if (lspEnabled) {
try {
Expand Down
10 changes: 10 additions & 0 deletions packages/cli/src/config/settingsSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1329,6 +1329,16 @@ const SETTINGS_SCHEMA = {
'Enable automatic consolidation (dream) of collected memories.',
showInDialog: false,
},
enableAutoSkill: {
type: 'boolean',
label: 'Enable Auto Skill',
category: 'Memory',
requiresRestart: false,
default: false,
description:
'Enable background review for reusable project skills after tool-heavy sessions.',
showInDialog: false,
},
},
},

Expand Down
14 changes: 14 additions & 0 deletions packages/cli/src/nonInteractiveCli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ describe('runNonInteractive', () => {
sendMessageStream: Mock;
getChatRecordingService: Mock;
getChat: Mock;
consumePendingMemoryTaskPromises: Mock;
recordCompletedToolCall: Mock;
};
let mockGetDebugResponses: Mock;

Expand Down Expand Up @@ -148,6 +150,8 @@ describe('runNonInteractive', () => {

mockGeminiClient = {

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.

[Critical] TS2353: consumePendingMemoryTaskPromises 在 mock 对象类型中不存在。GeminiClient 新增了此方法(第 762 行),但 mock 类型未包含它,导致 tsc --noEmit 编译失败。

Suggested change
mockGeminiClient = {
consumePendingMemoryTaskPromises: vi.fn().mockReturnValue([]),
recordCompletedToolCall: vi.fn(),

— DeepSeek/deepseek-v4-pro via Qwen Code /review

sendMessageStream: vi.fn(),
consumePendingMemoryTaskPromises: vi.fn().mockReturnValue([]),
recordCompletedToolCall: vi.fn(),
getChatRecordingService: vi.fn(() => ({
initialize: vi.fn(),
recordMessage: vi.fn(),
Expand Down Expand Up @@ -193,6 +197,7 @@ describe('runNonInteractive', () => {
getCronScheduler: vi.fn().mockReturnValue(null),
setModelInvocableCommandsProvider: vi.fn(),
setModelInvocableCommandsExecutor: vi.fn(),
getAutoSkillEnabled: vi.fn().mockReturnValue(false),
getDisabledSlashCommands: vi.fn().mockReturnValue([]),
getBackgroundTaskRegistry: vi
.fn()
Expand Down Expand Up @@ -404,6 +409,15 @@ describe('runNonInteractive', () => {
{ type: SendMessageType.ToolResult },
);
expect(processStdoutSpy).toHaveBeenCalledWith('Final answer\n');
// Verify recordCompletedToolCall is called with the tool name and args.
expect(mockGeminiClient.recordCompletedToolCall).toHaveBeenCalledWith(
'testTool',
{ arg1: 'value1' },
);
// Verify consumePendingMemoryTaskPromises is called at the end of the session.
expect(
mockGeminiClient.consumePendingMemoryTaskPromises,
).toHaveBeenCalled();
});

it('should handle error during tool execution and should send error back to the model', async () => {
Expand Down
18 changes: 18 additions & 0 deletions packages/cli/src/nonInteractiveCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,12 @@ export async function runNonInteractive(
}

adapter.emitToolResult(finalRequestInfo, toolResponse);
config
.getGeminiClient()
.recordCompletedToolCall(
finalRequestInfo.name,
finalRequestInfo.args as Record<string, unknown>,
);

if (toolResponse.responseParts) {
toolResponseParts.push(...toolResponse.responseParts);
Expand Down Expand Up @@ -674,6 +680,12 @@ export async function runNonInteractive(
}

adapter.emitToolResult(requestInfo, toolResponse);
config
.getGeminiClient()
.recordCompletedToolCall(
requestInfo.name,
requestInfo.args as Record<string, unknown>,
);

if (toolResponse.responseParts) {
itemToolResponseParts.push(...toolResponse.responseParts);
Expand Down Expand Up @@ -798,6 +810,12 @@ export async function runNonInteractive(
await new Promise((r) => setTimeout(r, 100));
}

const memoryTaskPromises = config
.getGeminiClient()
.consumePendingMemoryTaskPromises();
if (memoryTaskPromises.length > 0) {
await Promise.allSettled(memoryTaskPromises);
}
finalizeOneShotMonitors();

const metrics = uiTelemetryService.getMetrics();
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/ui/hooks/useGeminiStream.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const MockedGeminiClientClass = vi.hoisted(() =>
this.sendMessageStream = mockSendMessageStream;
this.addHistory = vi.fn();
this.consumePendingMemoryTaskPromises = vi.fn().mockReturnValue([]);
this.recordCompletedToolCall = vi.fn();
this.getChatRecordingService = vi.fn().mockReturnValue({
recordThought: vi.fn(),
initialize: vi.fn(),
Expand Down
7 changes: 7 additions & 0 deletions packages/cli/src/ui/hooks/useGeminiStream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1898,6 +1898,13 @@ export const useGeminiStream = (
(t) => !t.request.isClientInitiated,
);

for (const toolCall of geminiTools) {
geminiClient?.recordCompletedToolCall(

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] geminiTools 遍历中调用 recordCompletedToolCall 时未过滤已取消(cancelled)的工具调用。已取消的工具从未执行任何实际工作,但会计入 toolCallCount,人工降低技能提炼阈值。且 cancelled 的 write_file/edit 也会设置 skillsModifiedInSession = true,即使用户从未实际提交写入。

Suggested change
geminiClient?.recordCompletedToolCall(
for (const toolCall of geminiTools.filter(tc => tc.status !== 'cancelled')) {
geminiClient?.recordCompletedToolCall(
toolCall.request.name,
toolCall.request.args as Record<string, unknown>,
);
}

— DeepSeek/deepseek-v4-pro via Qwen Code /review

toolCall.request.name,
toolCall.request.args as Record<string, unknown>,
);
}

if (geminiTools.length === 0) {
return;
}
Expand Down
14 changes: 13 additions & 1 deletion packages/core/src/agents/runtime/agent-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,9 @@ export class AgentCore {
const toolsList: FunctionDeclaration[] = [];

const excludedFromSubagents = EXCLUDED_TOOLS_FOR_SUBAGENTS;
// When a subagent has an explicit tools list (not wildcard), only the

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] recursionGuardOnly 的注释与实际逻辑矛盾。顶部注释说「When a subagent has an explicit tools list...only the recursive-spawn guard is enforced」,但 else 分支内部却写着「Wildcard tool list...only block recursive spawning」——而这段代码实际执行的是显式工具列表路径。两个注释互相矛盾,会误导未来的维护者。

Suggested change
// When a subagent has an explicit tools list (not wildcard), only the
// Explicit tool list: only block recursive agent spawning.
// Other control-plane tools (cron, task, send_message) are
// allowed since the explicit list already bounds the agent.
const recursionGuardOnly = new Set<string>([ToolNames.AGENT]);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

// recursive-spawn guard (AgentTool) is enforced.
const recursionGuardOnly = new Set<string>([ToolNames.AGENT]);

if (this.toolConfig) {
const asStrings = this.toolConfig.tools.filter(
Expand All @@ -388,13 +391,22 @@ export class AgentCore {
.filter((t) => !(t.name && excludedFromSubagents.has(t.name))),
);
} else {
// Explicit tool list: apply the full subagent exclusion set (not just
// the recursion guard). This prevents control-plane tools
// (CRON_CREATE, TASK_STOP, SEND_MESSAGE, etc.) from leaking into
// explicitly-configured subagents that happen to list them.
toolsList.push(

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] For subagents with explicit tool lists, the exclusion set was narrowed from EXCLUDED_TOOLS_FOR_SUBAGENTS (blocks AGENT, CRON_CREATE, CRON_LIST, CRON_DELETE, TASK_STOP, SEND_MESSAGE) to just recursionGuardOnly (only AGENT). This means control-plane tools like TASK_STOP and CRON_CREATE could leak to explicitly-configured subagents. While the skill review agent doesn't declare these, this weakens the safety net for future subagent configs.

Consider restoring the full exclusion or documenting the rationale.

— deepseek-v4-pro via Qwen Code /review

...toolRegistry.getFunctionDeclarationsFiltered(
asStrings.filter((name) => !excludedFromSubagents.has(name)),
),
);
}
toolsList.push(...onlyInlineDecls);
// Also filter inline FunctionDeclaration[] passed directly in toolConfig.
toolsList.push(

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] 内联 FunctionDeclaration[]onlyInlineDecls)仅按 recursionGuardOnly(只含 AGENT)过滤,而显式字符串工具列表按完整的 excludedFromSubagents(含 CRON_CREATETASK_STOPSEND_MESSAGE 等)过滤。如果通过内联方式传入控制面工具,将绕过子 agent 排除过滤器。建议对 onlyInlineDecls 使用相同的 excludedFromSubagents 集合,保持一致性。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

...onlyInlineDecls.filter(
(d) => !(d.name && recursionGuardOnly.has(d.name)),
),
);
} else {
// Inherit all available tools by default when not specified.
toolsList.push(
Expand Down
8 changes: 8 additions & 0 deletions packages/core/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,8 @@ export interface ConfigParameters {
enableManagedAutoMemory?: boolean;
/** Enable managed auto-dream consolidation separately from extraction. Defaults to true. */
enableManagedAutoDream?: boolean;
/** Enable automatic project skill review after tool-heavy sessions. Defaults to false. */
enableAutoSkill?: boolean;
/**
* Lightweight model for background tasks (memory extraction, dream, /btw side questions).
* When set and valid for the current auth type, forked agents use this model instead of
Expand Down Expand Up @@ -768,6 +770,7 @@ export class Config {
private readonly defaultFileEncoding: FileEncodingType | undefined;
private readonly enableManagedAutoMemory: boolean;
private readonly enableManagedAutoDream: boolean;
private readonly enableAutoSkill: boolean;
private fastModel?: string;
private readonly disableAllHooks: boolean;
/** User-level hooks (always loaded regardless of trust) */
Expand Down Expand Up @@ -969,6 +972,7 @@ export class Config {
});
this.enableManagedAutoMemory = params.enableManagedAutoMemory ?? true;
this.enableManagedAutoDream = params.enableManagedAutoDream ?? false;
this.enableAutoSkill = params.enableAutoSkill ?? false;
this.fastModel = params.fastModel || undefined;
this.disableAllHooks = params.disableAllHooks ?? false;
// Store user and project hooks separately for proper source attribution
Expand Down Expand Up @@ -2294,6 +2298,10 @@ export class Config {
return this.enableManagedAutoDream && !this.getBareMode();
}

getAutoSkillEnabled(): boolean {
return this.enableAutoSkill && !this.getBareMode();
}

/**
* Return the MemoryManager instance created for this Config.
* Use this to share background-task state (registry, drainer) with memory
Expand Down
Loading
Loading