mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-10-21 14:21:48 +00:00
fix(core,cli): close the round-3 findings on the deferred-coordination defaults
- client.ts: narrow the bridged-result exclusion to the bridge's refusal and cancellation prefixes — a bridged Agent that ran and failed carries a generic error, and the delegated work still forces the todo reminder, matching the direct-Agent branch and the seeding pass (R2-9). - Session.ts: skip the awaited bridge resolution for envelopes once the turn's abort signal has fired — a cancelled batch's classification work would be discarded (R4-3). - Session.test.ts: drive the production schema-review gate in the three ACP bridge cases (getReviewedDeclaration wired to the live fingerprint) plus a stale-fingerprint refusal pin, and add the live-PM allow arm of the bridge-enablement guard (R4-2/R4-4). - client.test.ts: pin that a non-Agent deferred summary entry does not count as Agent reachability — the mutant reading `deferredSummary.length > 0` goes red (R4-7). - nonInteractiveCli.test.ts: capture and assert the executed request keeps the envelope identity (R4-6), and pin that a malformed string-arguments envelope degrades to the bridge's own refusal instead of a synchronous JSON.parse throw (R4-5). - docs: §7 item 2 quotes the brackets prompts.test.ts actually asserts (900-1,500 / 250-450), a self-consistent component split, and the re-measured narrow-allowlist figures as descriptive, in both languages; the verification runbook's 818 row now names the reachable recipe (tools.disabled: ["agent"]) instead of an unreachable condition (R2-4/R4-1). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
jinjing.zzj
parent
6892866240
commit
34dd9b1408
8 changed files with 504 additions and 17 deletions
|
|
@ -134,7 +134,7 @@ bridge-reachable, so both lines still drop there.
|
|||
Items 1-4 are automated in this PR's `prompts.test.ts`, so every push re-checks them; item 5 needs a real session and is handed off in [`docs/verification/resident-tool-prompt-assembly/README.md`](../verification/resident-tool-prompt-assembly/README.md).
|
||||
|
||||
1. **Default-session regression (in CI).** The 17 existing full-prompt snapshots cover the no-snapshot path, and `renders identically when every tool is declared` covers the all-declared path. Together they are the guard that makes the change safe for the common case.
|
||||
2. **Effect, and no drift outside it (in CI).** The file-work bracket is pinned in the two states the reachability exception (§4.6) distinguishes: with `agentReachable` unset the allowlist must drop 900-1,400 characters (measured 1,135, ~284 tokens — delegation 448 + codebase search 368 + monitor 317, policy bullets only, since that allowlist keeps every example); with `agentReachable: true` — the default in practice, since the bridge pair is exempt from `tools.eager` — both Agent bullets survive and only the 317-character monitor policy drops. A narrower allowlist must drop 3,800-5,000 (measured 4,327, ~1,082 tokens, three example blocks included). A third asserts all four model-specific example notations are gated, not just the bracket form. Together they fail on a lost saving and on newly added ungated tool text. `changes nothing outside the two gated sections` strips `## Using Your Tools` and `# Examples` from both renders and asserts the remainder is identical.
|
||||
2. **Effect, and no drift outside it (in CI).** The file-work bracket is pinned in the two states the reachability exception (§4.6) distinguishes, and the pinned bounds below are exactly the ones `prompts.test.ts` asserts: with `agentReachable` unset the drop must stay within 900-1,500 characters (measured 1,135 — delegation 448 + codebase search 368 + monitor 317 of bullet text, plus the three dropped line breaks); with `agentReachable: true` — the default in practice, since the bridge pair is exempt from `tools.eager` — both Agent bullets survive and only the monitor policy drops (must stay within 250-450; measured 317). A narrower allowlist drops 3,158 characters (Agent unreachable) or 2,709 (reachable — the default this PR creates), two example blocks included, measured at this commit; no CI bracket pins the narrow case, so those figures are descriptive (the pre-exception measurement was 4,327 with three example blocks). A third test asserts all four model-specific example notations are gated, not just the bracket form. Together they fail on a lost saving and on newly added ungated tool text. `changes nothing outside the two gated sections` strips `## Using Your Tools` and `# Examples` from both renders and asserts the remainder is identical.
|
||||
3. **Invariant, both directions (in CI).** `never names an undeclared tool inside the gated sections` sweeps every `ToolNames` value against the gated text with a word-boundary match, and `gates every tool name the gated sections can mention, on every example set` makes that config-independent by withholding each of the 66 names in turn against all four example sets — the check that would have caught the model-specific notations going ungated. `keeps the policy text of every tool that is declared` pins the opposite direction so gating cannot over-reach. Scoped to those sections because of the residues in §6.
|
||||
4. **Reverse checks and plumbing (in CI).** `leaves CodeModeOnly guidance untouched by the declared set` asserts code mode renders identically with and without a snapshot, and `keeps Agent guidance when Agent is bridge-reachable` asserts `getMainSessionBaseSystemPrompt` reads the `Config` session snapshots (`getPromptToolSnapshot()`, plus `getPromptAgentReachable()` since #13033) — the property that keeps `/context` and the request on one source.
|
||||
5. **Token measurement (handed off).** On a session with a trimmed `tools.eager` allowlist, compare the system-prompt row before and after, anchored on the provider's `input_token_count` (the category ruler itself is being fixed in #12119). The brief also carries the three-way run that separates this change's saving from `tools.eager`'s own, and the weakened recall check that is all the repo's missing eval harness allows.
|
||||
|
|
|
|||
|
|
@ -124,7 +124,7 @@ getCoreSystemPrompt(
|
|||
第 1-4 项已在本 PR 的 `prompts.test.ts` 中自动化，每次推送都会重新检查；第 5 项需要真实会话，交接文档为 [`docs/verification/resident-tool-prompt-assembly/README.md`](../verification/resident-tool-prompt-assembly/README.md)。
|
||||
|
||||
1. **默认会话回归（已在 CI）。** 现有 17 份完整提示词快照覆盖"无快照"路径，`renders identically when every tool is declared` 覆盖"全部声明"路径。两者共同构成让改动对常见场景安全的守卫。
|
||||
2. **效果，以及效果之外不漂移（已在 CI）。** 文件七件套的节省区间按可达性例外（§4.6）区分的两种状态分别钉住：`agentReachable` 未设置时，白名单必须减少 900-1,400 字符（实测 1,135，约 284 token——委派 448 + 代码库搜索 368 + monitor 317，只有策略条目，因为该白名单保留了全部示例）；`agentReachable: true` 时——实践中这是默认状态，因为桥的两个半件免于 `tools.eager`——两条 Agent 条目都保留，只减少 317 字符的 monitor 策略。更窄的白名单必须减少 3,800-5,000 字符（实测 4,327，约 1,082 token，含三个示例块）。第三条断言四种按模型选择的示例写法都被门控，而不只是方括号写法。三者合起来能在收益丢失与新增未门控工具文案时失败。`changes nothing outside the two gated sections` 从两次渲染中剥掉 `## Using Your Tools` 与 `# Examples`，断言其余部分完全相同。
|
||||
2. **效果，以及效果之外不漂移（已在 CI）。** 文件七件套的节省区间按可达性例外（§4.6）区分的两种状态分别钉住，且写出的边界就是 `prompts.test.ts` 实际断言的边界：`agentReachable` 未设置时，减少量必须落在 900-1,500 字符（实测 1,135——委派 448 + 代码库搜索 368 + monitor 317 的条目正文，外加三条被删的换行）；`agentReachable: true` 时——实践中这是默认状态，因为桥的两个半件免于 `tools.eager`——两条 Agent 条目都保留，只减少 monitor 策略（必须落在 250-450；实测 317）。更窄的白名单减少 3,158 字符（Agent 不可达）或 2,709（可达——本 PR 造成的默认状态），含两个示例块，均为本提交实测；CI 没有为窄名单钉数值区间，这两个数字只是描述性的（例外引入前的实测是 4,327、含三个示例块）。第三条断言四种按模型选择的示例写法都被门控，而不只是方括号写法。三者合起来能在收益丢失与新增未门控工具文案时失败。`changes nothing outside the two gated sections` 从两次渲染中剥掉 `## Using Your Tools` 与 `# Examples`，断言其余部分完全相同。
|
||||
3. **不变量，双向（已在 CI）。** `never names an undeclared tool inside the gated sections` 以词边界匹配把每个 `ToolNames` 取值扫一遍被门控文本；`gates every tool name the gated sections can mention, on every example set` 把它变成与配置无关的检查——逐个withhold 全部 66 个名字、对四套示例模板各跑一遍，这正是能抓住"按模型选择的写法未被门控"的那条。`keeps the policy text of every tool that is declared` 钉住相反方向，防止门控过度。之所以限定这两段，是因为 §6 中的那些残留。
|
||||
4. **反向检查与接线（已在 CI）。** `leaves CodeModeOnly guidance untouched by the declared set` 断言 code mode 在有无快照时渲染完全一致；`keeps Agent guidance when Agent is bridge-reachable` 断言 `getMainSessionBaseSystemPrompt` 确实读取 `Config` 上的会话快照（`getPromptToolSnapshot()`，以及 #13033 起的 `getPromptAgentReachable()`）——这正是让 `/context` 与真实请求同源的性质。
|
||||
5. **Token 度量（已交接）。** 在设置了裁剪版 `tools.eager` 白名单的会话上，对比改动前后的系统提示词一行，以 provider 的 `input_token_count` 为基准（分类标尺本身正在 #12119 中修复）。交接文档还包含把本改动与 `tools.eager` 自身收益分离的三档跑法，以及在仓库缺少 eval 设施下只能做的弱化召回验证。
|
||||
|
|
|
|||
|
|
@ -35,14 +35,14 @@
|
|||
|
||||
被门控的只有两段：`## Using Your Tools` 的部分条目，以及 `# Examples` 中的示例块。以下是第二轮精简（PR #12546）后、**Todo 关闭**（未设 `tools.todoWrite.enabled`）时的 direct 模式口径；文件七件套指 read_file / write_file / edit / glob / grep_search / run_shell_command / skill，加上豁免工具：
|
||||
|
||||
| 对比基线 → 文件七件套白名单 | 节省字符 | 节省 UTF-8 字节 | 移除内容 |
|
||||
| --------------------------------------------- | -------: | --------------: | ------------------------------------------------------------ |
|
||||
| 无声明快照（假定全部工具可用） | 1,135 | 1,139 | Subagent Delegation、Codebase Search、Monitor Processes 三条 |
|
||||
| 基线已不声明 monitor，且白名单侧 Agent 不可达 | 818 | 822 | Subagent Delegation、Codebase Search 两条 |
|
||||
| 对比基线 → 文件七件套白名单 | 节省字符 | 节省 UTF-8 字节 | 移除内容 |
|
||||
| -------------------------------------------------------------------------------------------- | -------: | --------------: | ------------------------------------------------------------ |
|
||||
| 无声明快照（假定全部工具可用） | 1,135 | 1,139 | Subagent Delegation、Codebase Search、Monitor Processes 三条 |
|
||||
| 基线已不声明 monitor，且白名单侧 Agent 被 `tools.disabled: ["agent"]` 整体移除（桥保持完整） | 818 | 822 | Subagent Delegation、Codebase Search 两条 |
|
||||
|
||||
字符采用 JavaScript `string.length` 口径，字节采用 UTF-8 口径；`wc -c` 量的是字节，不能直接与字符数比较。两种对比都保留全部示例，因为示例只调用文件类工具和 shell。若继续移除 `edit`、`write_file`、`glob` 或 shell，示例也会被裁剪，必须按当前模型模板重新测量，不能沿用旧版范围。若 A/B 两边都开启 Todo，B 还会少 `- **Task Management:**` 一条（228 字符——该条按 `todo_write` 是否被**声明**门控，见 `prompts.ts` 的 `TOOL_GUIDANCE_LINE_GATES`，而 `todo_write` 不在文件七件套内）：无快照行差值变为 1,364 字符 / 1,368 字节，monitor 未声明行变为 1,047 字符 / 1,051 字节。
|
||||
|
||||
上表的白名单侧未启用 Agent 桥接可达性例外。#13033 保留可通过桥接调用的 Agent 指引，因此不能把这些静态数值作为当前默认配置与文件白名单的固定差值。
|
||||
上表第 2 行的白名单侧须额外在 settings 里设 `tools.disabled: ["agent"]` 才能让 Agent 真正不可达——`tools.eager` 名单做不到这一点（桥接两件套免于白名单降级，Agent 始终可通过桥接调用，可达性例外因此总是生效）。该行数字描述的是这一配置下的差值。
|
||||
|
||||
这些数字不是 token 计费数据，也不是所有真实 CLI 配置的固定收益。默认延迟加载的 `monitor` 若在 A/B 两边均未声明，就不能把它的 317 字符算入 A/B 差值。
|
||||
|
||||
|
|
@ -70,7 +70,7 @@ wc -c /tmp/prompt-default.md /tmp/prompt-eager.md
|
|||
diff /tmp/prompt-default.md /tmp/prompt-eager.md
|
||||
```
|
||||
|
||||
**预期：** A/B 必须使用同一构建、模型、output style、Todo 状态及其他配置，并记录实际声明集合与 Agent 可达性。默认配置与文件七件套都保留完整桥接，且 `agent` 已注册并列在延迟摘要中时，即使 B 不直接声明 Agent，也会保留 Subagent Delegation 和 Codebase Search 两条（设计 §4.6）。Todo 关闭且 `monitor` 在两边均未声明时，两份提示词可以完全相同，预期差值为 **0 字节**。只有 B 的 Agent 确实不可达时，才适用 §1 的历史 818 字符 / 822 字节差值。Todo 或更窄白名单造成的其他差异须按实际声明与 diff 逐项归因，不使用历史总量作通过阈值。
|
||||
**预期：** A/B 必须使用同一构建、模型、output style、Todo 状态及其他配置，并记录实际声明集合与 Agent 可达性。默认配置与文件七件套都保留完整桥接，且 `agent` 已注册并列在延迟摘要中时，即使 B 不直接声明 Agent，也会保留 Subagent Delegation 和 Codebase Search 两条（设计 §4.6）。Todo 关闭且 `monitor` 在两边均未声明时，两份提示词可以完全相同，预期差值为 **0 字节**。只有 B 侧把 Agent 整体移除（`tools.disabled: ["agent"]`，桥保持完整）时，才适用 §1 第 2 行的 818 字符 / 822 字节差值。Todo 或更窄白名单造成的其他差异须按实际声明与 diff 逐项归因，不使用历史总量作通过阈值。
|
||||
|
||||
**如果 B 和 A 一样大**，先用 `/tools` 及首个真实请求的 schema 确认设置是否被读取、白名单外工具是否按需加载，再核对 Agent 是否桥接可达。提示词相等本身不能证明 `tools.eager` 未被接受：声明裁剪仍可能生效，而 Agent 指引因可达性例外被保留。
|
||||
|
||||
|
|
|
|||
|
|
@ -3773,6 +3773,11 @@ describe('Session', () => {
|
|||
kind: core.Kind.Execute,
|
||||
displayName: 'Agent',
|
||||
description: 'Delegates work to a subagent',
|
||||
// The schema-review gate fingerprints name + parametersJsonSchema.
|
||||
schema: {
|
||||
name: 'Agent',
|
||||
parametersJsonSchema: { type: 'object', properties: {} },
|
||||
},
|
||||
build: vi.fn().mockReturnValue({
|
||||
params: {},
|
||||
execute,
|
||||
|
|
@ -3802,6 +3807,15 @@ describe('Session', () => {
|
|||
findTool(name),
|
||||
);
|
||||
mockToolRegistry.isDeferredAndHidden.mockReturnValue(true);
|
||||
// The production registry always defines the schema-review gate;
|
||||
// wire it so this arm exercises the gate instead of resolving
|
||||
// unconditionally — the recorded review matches the live tool.
|
||||
mockToolRegistry.getReviewedDeclaration = vi.fn((name: string) => {
|
||||
const tool = findTool(name);
|
||||
return tool && 'build' in tool
|
||||
? core.deferredDeclarationFingerprint(tool as never)
|
||||
: undefined;
|
||||
});
|
||||
}
|
||||
mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.YOLO);
|
||||
mockConfig.getDisableAllHooks = vi.fn().mockReturnValue(true);
|
||||
|
|
@ -37810,6 +37824,11 @@ describe('Session', () => {
|
|||
const goal = {
|
||||
name: core.ToolNames.GET_GOAL,
|
||||
kind: core.Kind.Other,
|
||||
// The schema-review gate fingerprints name + parametersJsonSchema.
|
||||
schema: {
|
||||
name: 'GetGoal',
|
||||
parametersJsonSchema: { type: 'object', properties: {} },
|
||||
},
|
||||
build: vi.fn((params: Record<string, unknown>) => ({
|
||||
params,
|
||||
getDefaultPermission: async () => 'allow',
|
||||
|
|
@ -37837,6 +37856,14 @@ describe('Session', () => {
|
|||
findTool(name),
|
||||
);
|
||||
mockToolRegistry.isDeferredAndHidden.mockReturnValue(true);
|
||||
// Exercise the production schema-review gate: the recorded review
|
||||
// matches the live target's fingerprint.
|
||||
mockToolRegistry.getReviewedDeclaration = vi.fn((name: string) => {
|
||||
const tool = findTool(name);
|
||||
return tool && 'build' in tool
|
||||
? core.deferredDeclarationFingerprint(tool as never)
|
||||
: undefined;
|
||||
});
|
||||
mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.YOLO);
|
||||
mockConfig.getPermissionManager = vi.fn().mockReturnValue(null);
|
||||
mockChat.sendMessageStream = vi
|
||||
|
|
@ -37867,6 +37894,170 @@ describe('Session', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('refuses a bridged call whose recorded review no longer matches', async () => {
|
||||
// The schema-review gate the other bridge cases now exercise: a
|
||||
// stale recorded fingerprint (the tool changed since tool_search
|
||||
// returned it) must refuse the call rather than run arguments
|
||||
// written against a schema the model no longer has.
|
||||
const { ToolCallTool } = await import(
|
||||
'@qwen-code/qwen-code-core/tools/tool-call.js'
|
||||
);
|
||||
const bridge = new ToolCallTool();
|
||||
const execute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'agent done',
|
||||
returnDisplay: 'agent done',
|
||||
});
|
||||
const agentTool = {
|
||||
name: core.ToolNames.AGENT,
|
||||
kind: core.Kind.Think,
|
||||
schema: {
|
||||
name: 'Agent',
|
||||
parametersJsonSchema: { type: 'object', properties: {} },
|
||||
},
|
||||
build: vi.fn().mockReturnValue({
|
||||
params: {},
|
||||
execute,
|
||||
getDefaultPermission: vi.fn().mockResolvedValue('allow'),
|
||||
getDescription: () => 'Agent',
|
||||
toolLocations: () => [],
|
||||
}),
|
||||
};
|
||||
const findTool = (name: string) =>
|
||||
name === core.ToolNames.TOOL_CALL
|
||||
? bridge
|
||||
: name === core.ToolNames.TOOL_SEARCH
|
||||
? { name }
|
||||
: name === core.ToolNames.AGENT
|
||||
? agentTool
|
||||
: undefined;
|
||||
mockToolRegistry.getTool.mockImplementation(findTool);
|
||||
mockToolRegistry.ensureTool.mockImplementation(async (name: string) =>
|
||||
findTool(name),
|
||||
);
|
||||
mockToolRegistry.isDeferredAndHidden.mockReturnValue(true);
|
||||
mockToolRegistry.getReviewedDeclaration = vi.fn(
|
||||
() => 'stale-fingerprint-from-an-older-schema',
|
||||
);
|
||||
mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.YOLO);
|
||||
mockConfig.getPermissionManager = vi.fn().mockReturnValue(null);
|
||||
mockChat.sendMessageStream = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(
|
||||
createStreamWithChunks([
|
||||
{
|
||||
type: core.StreamEventType.CHUNK,
|
||||
value: {
|
||||
functionCalls: [
|
||||
{
|
||||
id: 'call-stale',
|
||||
name: core.ToolNames.TOOL_CALL,
|
||||
args: { name: core.ToolNames.AGENT, arguments: {} },
|
||||
},
|
||||
],
|
||||
},
|
||||
},
|
||||
]),
|
||||
)
|
||||
.mockResolvedValueOnce(createEmptyStream());
|
||||
|
||||
await session.prompt({
|
||||
sessionId: 'test-session-id',
|
||||
prompt: [{ type: 'text', text: 'delegate' }],
|
||||
});
|
||||
|
||||
expect(execute).not.toHaveBeenCalled();
|
||||
const followUp = vi
|
||||
.mocked(mockChat.sendMessageStream)
|
||||
.mock.calls.at(-1)?.[1] as { message: Part[] };
|
||||
const response = followUp.message.find(
|
||||
(part) => part.functionResponse?.id === 'call-stale',
|
||||
)?.functionResponse?.response;
|
||||
expect(JSON.stringify(response)).toContain(
|
||||
'changed since tool_search last returned it',
|
||||
);
|
||||
});
|
||||
|
||||
it('stops bridge classification for the rest of the batch once the turn is cancelled', async () => {
|
||||
// The partition loop resolves each bridged envelope through awaited
|
||||
// registry work; once the turn is cancelled the remaining envelopes
|
||||
// must not pay that for a batch the execution path will discard.
|
||||
// Driver: the first resolution's ensureTool aborts the turn, so a
|
||||
// batch of two bridged delegations resolves exactly one with the
|
||||
// gate and both without it.
|
||||
const { ToolCallTool } = await import(
|
||||
'@qwen-code/qwen-code-core/tools/tool-call.js'
|
||||
);
|
||||
const bridge = new ToolCallTool();
|
||||
const execute = vi.fn().mockResolvedValue({
|
||||
llmContent: 'agent done',
|
||||
returnDisplay: 'agent done',
|
||||
});
|
||||
const agentTool = {
|
||||
name: core.ToolNames.AGENT,
|
||||
kind: core.Kind.Think,
|
||||
build: vi.fn().mockReturnValue({
|
||||
params: {},
|
||||
execute,
|
||||
getDefaultPermission: vi.fn().mockResolvedValue('allow'),
|
||||
getDescription: () => 'Agent',
|
||||
toolLocations: () => [],
|
||||
}),
|
||||
};
|
||||
const findTool = (name: string) =>
|
||||
name === core.ToolNames.TOOL_CALL
|
||||
? bridge
|
||||
: name === core.ToolNames.TOOL_SEARCH
|
||||
? { name }
|
||||
: name === core.ToolNames.AGENT
|
||||
? agentTool
|
||||
: undefined;
|
||||
mockToolRegistry.getTool.mockImplementation(findTool);
|
||||
const cancellation = new AbortController();
|
||||
let firstResolution = true;
|
||||
mockToolRegistry.ensureTool.mockImplementation(async (name: string) => {
|
||||
if (firstResolution) {
|
||||
firstResolution = false;
|
||||
// The cancel lands mid-partition, after the first envelope's
|
||||
// resolution has started.
|
||||
cancellation.abort();
|
||||
}
|
||||
return findTool(name);
|
||||
});
|
||||
mockToolRegistry.isDeferredAndHidden.mockReturnValue(true);
|
||||
mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.YOLO);
|
||||
mockConfig.getPermissionManager = vi.fn().mockReturnValue(null);
|
||||
|
||||
mockChat.sendMessageStream = vi.fn().mockImplementation(() =>
|
||||
createStreamWithChunks([
|
||||
{
|
||||
type: core.StreamEventType.CHUNK,
|
||||
value: {
|
||||
functionCalls: ['a', 'b'].map((id) => ({
|
||||
id,
|
||||
name: core.ToolNames.TOOL_CALL,
|
||||
args: { name: core.ToolNames.AGENT, arguments: {} },
|
||||
})),
|
||||
},
|
||||
},
|
||||
]),
|
||||
);
|
||||
|
||||
await session.prompt(
|
||||
{
|
||||
sessionId: 'test-session-id',
|
||||
prompt: [{ type: 'text', text: 'delegate twice' }],
|
||||
},
|
||||
undefined,
|
||||
cancellation.signal,
|
||||
);
|
||||
|
||||
const ensuredAgent = mockToolRegistry.ensureTool.mock.calls.filter(
|
||||
(call) => call[0] === core.ToolNames.AGENT,
|
||||
).length;
|
||||
expect(ensuredAgent).toBe(1);
|
||||
expect(execute).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('runs multiple Agent tool calls concurrently (issue #2516)', async () => {
|
||||
// Each Agent call has two controllable async boundaries:
|
||||
// - `called` — resolves *when* the test code reaches `execute()`
|
||||
|
|
@ -38024,6 +38215,12 @@ describe('Session', () => {
|
|||
const agentTool = {
|
||||
name: core.ToolNames.AGENT,
|
||||
kind: core.Kind.Think,
|
||||
// The schema-review gate fingerprints name +
|
||||
// parametersJsonSchema.
|
||||
schema: {
|
||||
name: 'Agent',
|
||||
parametersJsonSchema: { type: 'object', properties: {} },
|
||||
},
|
||||
build: vi
|
||||
.fn()
|
||||
.mockImplementation((args: Record<string, unknown>) => {
|
||||
|
|
@ -38063,6 +38260,16 @@ describe('Session', () => {
|
|||
async (name: string) => findTool(name),
|
||||
);
|
||||
mockToolRegistry.isDeferredAndHidden.mockReturnValue(true);
|
||||
// Exercise the production schema-review gate: the recorded
|
||||
// review matches the live target's fingerprint.
|
||||
mockToolRegistry.getReviewedDeclaration = vi.fn(
|
||||
(name: string) => {
|
||||
const tool = findTool(name);
|
||||
return tool && 'build' in tool
|
||||
? core.deferredDeclarationFingerprint(tool as never)
|
||||
: undefined;
|
||||
},
|
||||
);
|
||||
}
|
||||
mockConfig.getApprovalMode = vi
|
||||
.fn()
|
||||
|
|
@ -38150,6 +38357,121 @@ describe('Session', () => {
|
|||
},
|
||||
);
|
||||
|
||||
it('fans bridged envelopes out when a live permission manager allows the bridge', async () => {
|
||||
// The batcher consults pm.isToolEnabled(tool_call) before resolving
|
||||
// the envelope; every production session has a non-null PM. The
|
||||
// deny arm is pinned elsewhere — this pins the allow arm: resolution
|
||||
// must still happen and the bridged delegations must fan out.
|
||||
const { ToolCallTool } = await import(
|
||||
'@qwen-code/qwen-code-core/tools/tool-call.js'
|
||||
);
|
||||
const bridge = new ToolCallTool();
|
||||
const isToolEnabled = vi.fn().mockResolvedValue(true);
|
||||
mockConfig.getPermissionManager = vi.fn().mockReturnValue({
|
||||
isToolEnabled,
|
||||
findMatchingDenyRule: vi.fn().mockReturnValue(undefined),
|
||||
hasRelevantRules: vi.fn().mockReturnValue(false),
|
||||
evaluate: vi.fn().mockResolvedValue('default'),
|
||||
hasMatchingAskRule: vi.fn().mockReturnValue(false),
|
||||
});
|
||||
type Deferred<T> = { promise: Promise<T>; resolve: (v: T) => void };
|
||||
const makeDeferred = <T>(): Deferred<T> => {
|
||||
let resolve!: (v: T) => void;
|
||||
const promise = new Promise<T>((r) => {
|
||||
resolve = r;
|
||||
});
|
||||
return { promise, resolve };
|
||||
};
|
||||
const called: Record<string, Deferred<void>> = {
|
||||
'call-a': makeDeferred<void>(),
|
||||
'call-b': makeDeferred<void>(),
|
||||
};
|
||||
const result: Record<string, Deferred<core.ToolResult>> = {
|
||||
'call-a': makeDeferred<core.ToolResult>(),
|
||||
'call-b': makeDeferred<core.ToolResult>(),
|
||||
};
|
||||
const agentTool = {
|
||||
name: core.ToolNames.AGENT,
|
||||
kind: core.Kind.Think,
|
||||
schema: {
|
||||
name: 'Agent',
|
||||
parametersJsonSchema: { type: 'object', properties: {} },
|
||||
},
|
||||
build: vi.fn().mockImplementation((args: Record<string, unknown>) => {
|
||||
const id = args['_test_id'] as string;
|
||||
return {
|
||||
params: args,
|
||||
getDefaultPermission: vi.fn().mockResolvedValue('allow'),
|
||||
getDescription: vi.fn().mockReturnValue(`agent ${id}`),
|
||||
toolLocations: vi.fn().mockReturnValue([]),
|
||||
execute: vi.fn().mockImplementation(() => {
|
||||
called[id].resolve();
|
||||
return result[id].promise;
|
||||
}),
|
||||
};
|
||||
}),
|
||||
};
|
||||
const findTool = (name: string) =>
|
||||
name === core.ToolNames.TOOL_CALL
|
||||
? bridge
|
||||
: name === core.ToolNames.TOOL_SEARCH
|
||||
? { name }
|
||||
: name === core.ToolNames.AGENT
|
||||
? agentTool
|
||||
: undefined;
|
||||
mockToolRegistry.getTool.mockImplementation(findTool);
|
||||
mockToolRegistry.ensureTool.mockImplementation(async (name: string) =>
|
||||
findTool(name),
|
||||
);
|
||||
mockToolRegistry.isDeferredAndHidden.mockReturnValue(true);
|
||||
mockToolRegistry.getReviewedDeclaration = vi.fn((name: string) => {
|
||||
const tool = findTool(name);
|
||||
return tool && 'build' in tool
|
||||
? core.deferredDeclarationFingerprint(tool as never)
|
||||
: undefined;
|
||||
});
|
||||
mockConfig.getApprovalMode = vi.fn().mockReturnValue(ApprovalMode.YOLO);
|
||||
|
||||
const toolLoopState = {
|
||||
totalToolCalls: 0,
|
||||
invalidToolParamErrors: new Map<string, number>(),
|
||||
toolCallKeyCounts: new Map<string, number>(),
|
||||
maxToolCallKeyRepeat: 0,
|
||||
loopDetected: false,
|
||||
};
|
||||
const runPromise = (
|
||||
session as unknown as {
|
||||
runToolCalls: (
|
||||
abortSignal: AbortSignal,
|
||||
promptId: string,
|
||||
calls: FunctionCall[],
|
||||
loopState: typeof toolLoopState,
|
||||
) => Promise<{ parts: Part[] }>;
|
||||
}
|
||||
).runToolCalls(
|
||||
new AbortController().signal,
|
||||
'prompt-bridged-allow',
|
||||
['call-a', 'call-b'].map((id) => ({
|
||||
id,
|
||||
name: core.ToolNames.TOOL_CALL,
|
||||
args: {
|
||||
name: core.ToolNames.AGENT,
|
||||
arguments: { _test_id: id, subagent_type: 'explore' },
|
||||
},
|
||||
})),
|
||||
toolLoopState,
|
||||
);
|
||||
|
||||
// Serial classification would start call-b only after call-a's
|
||||
// execute resolves — the race below deadlocks in that shape.
|
||||
await Promise.all([called['call-a'].promise, called['call-b'].promise]);
|
||||
result['call-a'].resolve({ llmContent: 'A', returnDisplay: 'A' });
|
||||
result['call-b'].resolve({ llmContent: 'B', returnDisplay: 'B' });
|
||||
await runPromise;
|
||||
|
||||
expect(isToolEnabled).toHaveBeenCalledWith(core.ToolNames.TOOL_CALL);
|
||||
});
|
||||
|
||||
it('ignores malformed QWEN_CODE_MAX_TOOL_CONCURRENCY values', async () => {
|
||||
const previousMaxConcurrency =
|
||||
process.env['QWEN_CODE_MAX_TOOL_CONCURRENCY'];
|
||||
|
|
|
|||
|
|
@ -12882,7 +12882,10 @@ export class Session implements SessionContext {
|
|||
// where `task` is a live alias of the agent tool; concurrent batches
|
||||
// are therefore agent-only.
|
||||
let executionToolName = canonicalToolName(fc.name ?? '');
|
||||
if (executionToolName === ToolNames.TOOL_CALL) {
|
||||
// Skip the awaited bridge resolution once the turn is cancelled: the
|
||||
// execution path bails on the same signal before resolving, so work
|
||||
// spent here would be discarded anyway.
|
||||
if (executionToolName === ToolNames.TOOL_CALL && !abortSignal.aborted) {
|
||||
const pm = this.config.getPermissionManager?.();
|
||||
const bridgeEnabled =
|
||||
!pm ||
|
||||
|
|
|
|||
|
|
@ -4235,8 +4235,20 @@ describe('runNonInteractive', () => {
|
|||
const gate = new Promise<void>((resolve) => {
|
||||
openGate = resolve;
|
||||
});
|
||||
// R4-6: capture what production actually hands to executeToolCall —
|
||||
// the envelope must survive partitioning (a refactor forwarding the
|
||||
// computed unwrapped request would run the hidden target under its
|
||||
// own name and desync the provider's function-call/response names).
|
||||
const executedRequests: Array<{
|
||||
name: string;
|
||||
args: Record<string, unknown>;
|
||||
}> = [];
|
||||
mockCoreExecuteToolCall.mockImplementation(
|
||||
async (_config, request, _signal, options) => {
|
||||
executedRequests.push({
|
||||
name: request.name,
|
||||
args: request.args as Record<string, unknown>,
|
||||
});
|
||||
started += 1;
|
||||
if (started === total) openGate();
|
||||
await gate;
|
||||
|
|
@ -4295,6 +4307,11 @@ describe('runNonInteractive', () => {
|
|||
);
|
||||
|
||||
expect(started).toBe(total);
|
||||
expect(executedRequests).toHaveLength(2);
|
||||
for (const req of executedRequests) {
|
||||
expect(req.name).toBe(ToolNames.TOOL_CALL);
|
||||
expect(req.args['name']).toBe(targetName);
|
||||
}
|
||||
expect(bridgeEvents[0].value).toMatchObject({
|
||||
args: {
|
||||
arguments:
|
||||
|
|
@ -4313,6 +4330,71 @@ describe('runNonInteractive', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('degrades a malformed string-arguments envelope without throwing', async () => {
|
||||
// A truncated model output leaves `arguments` as unparseable text; the
|
||||
// partition predicate must pass the envelope through untouched (the
|
||||
// bridge's own refusal downstream) rather than throw synchronously and
|
||||
// abort the headless turn.
|
||||
setupMetricsMock();
|
||||
const targetName = 'mcp__docs__read';
|
||||
vi.mocked(mockToolRegistry.getTool).mockImplementation(
|
||||
(name: string) =>
|
||||
(name === targetName
|
||||
? { name: targetName, kind: Kind.Read }
|
||||
: undefined) as unknown as ReturnType<
|
||||
typeof mockToolRegistry.getTool
|
||||
>,
|
||||
);
|
||||
vi.mocked(mockToolRegistry.isDeferredAndHidden).mockImplementation(
|
||||
(name: string) => name === targetName,
|
||||
);
|
||||
|
||||
const executedNames: string[] = [];
|
||||
mockCoreExecuteToolCall.mockImplementation(async (_config, request) => {
|
||||
executedNames.push(request.name);
|
||||
return {
|
||||
responseParts: [
|
||||
{
|
||||
functionResponse: {
|
||||
id: request.callId,
|
||||
name: request.name,
|
||||
response: { output: 'ok' },
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
});
|
||||
|
||||
mockLlmClient.sendMessageStream
|
||||
.mockReturnValueOnce(
|
||||
createStreamFromEvents([
|
||||
{
|
||||
type: LlmEventType.ToolCallRequest,
|
||||
value: {
|
||||
callId: 'bridge-bad',
|
||||
name: ToolNames.TOOL_CALL,
|
||||
args: { name: targetName, arguments: '{"path":' },
|
||||
isClientInitiated: false,
|
||||
prompt_id: 'p-bridge-malformed',
|
||||
},
|
||||
},
|
||||
] as ServerLlmStreamEvent[]),
|
||||
)
|
||||
.mockReturnValueOnce(createStreamFromEvents(finishTurn));
|
||||
|
||||
await expect(
|
||||
runNonInteractive(
|
||||
mockConfig,
|
||||
mockSettings,
|
||||
'read',
|
||||
'p-bridge-malformed',
|
||||
),
|
||||
).resolves.not.toThrow();
|
||||
// The envelope survived to execution untouched (not unwrapped, not
|
||||
// thrown on): the bridge itself produces the graceful refusal.
|
||||
expect(executedNames).toEqual([ToolNames.TOOL_CALL]);
|
||||
});
|
||||
|
||||
it('finalizes concurrent results in request order despite out-of-order completion', async () => {
|
||||
setupMetricsMock();
|
||||
vi.mocked(mockToolRegistry.getTool).mockReturnValue({
|
||||
|
|
|
|||
|
|
@ -1822,6 +1822,26 @@ describe('Gemini Client (client.ts)', () => {
|
|||
expect(setReachable).toHaveBeenLastCalledWith(false);
|
||||
});
|
||||
|
||||
it('does not count a non-Agent deferred tool as Agent reachability', async () => {
|
||||
// Pins the second disjunct's name match: a mutant reading
|
||||
// `deferredSummary.length > 0` would record reachable here, and the
|
||||
// prompt would point the model at an uncallable tool.
|
||||
const reg = deferredToolRegistry(bridgeOnly);
|
||||
reg.getFunctionDeclarations.mockReturnValue([
|
||||
{ name: ToolNames.TOOL_SEARCH },
|
||||
{ name: ToolNames.TOOL_CALL },
|
||||
]);
|
||||
reg.getDeferredToolSummary.mockReturnValue([
|
||||
{ name: 'monitor', description: 'watch a process' },
|
||||
]);
|
||||
const setReachable = vi.mocked(mockConfig.setPromptAgentReachable);
|
||||
setReachable.mockClear();
|
||||
|
||||
await client.startChat();
|
||||
|
||||
expect(setReachable).toHaveBeenLastCalledWith(false);
|
||||
});
|
||||
|
||||
it('records Agent unreachable when an incomplete bridge withholds a permission-deferred Agent', async () => {
|
||||
// The incomplete-bridge fallback deliberately withholds permission-
|
||||
// deferred tools from the eager reveal, so this session can neither
|
||||
|
|
@ -2764,6 +2784,62 @@ describe('Gemini Client (client.ts)', () => {
|
|||
);
|
||||
});
|
||||
|
||||
it('forces the active todo reminder for a bridged Agent that ran and failed', async () => {
|
||||
// A bridged delegation that resolved and then threw carries a generic
|
||||
// `error` string with no refusal/cancellation prefix — real work ran,
|
||||
// so the reminder force must fire exactly as it does for the
|
||||
// direct-Agent path. Goes red if the guard skips any error-carrying
|
||||
// response instead of only the two prefixed ones.
|
||||
const reminder =
|
||||
'<system-reminder>unfinished todo: follow up on the delegated node</system-reminder>';
|
||||
vi.mocked(mockConfig.takeActiveTodoReminder).mockReturnValue(reminder);
|
||||
mockTurnRunFn.mockReturnValue(
|
||||
(async function* () {
|
||||
yield { type: LlmEventType.Content, value: 'response' };
|
||||
})(),
|
||||
);
|
||||
client.getChat().setHistory([
|
||||
{
|
||||
role: 'model',
|
||||
parts: [
|
||||
{
|
||||
functionCall: {
|
||||
id: 'call-bridged-agent-failed',
|
||||
name: ToolNames.TOOL_CALL,
|
||||
args: {
|
||||
name: 'agent',
|
||||
args: { description: 'd', prompt: 'p' },
|
||||
},
|
||||
},
|
||||
},
|
||||
],
|
||||
},
|
||||
]);
|
||||
|
||||
const stream = client.sendMessageStream(
|
||||
[
|
||||
{
|
||||
functionResponse: {
|
||||
id: 'call-bridged-agent-failed',
|
||||
name: ToolNames.TOOL_CALL,
|
||||
response: { error: 'subagent crashed with ECONNRESET' },
|
||||
},
|
||||
},
|
||||
],
|
||||
new AbortController().signal,
|
||||
'prompt-bridged-agent-failed',
|
||||
{ type: SendMessageType.ToolResult },
|
||||
);
|
||||
for await (const _ of stream) {
|
||||
// drain
|
||||
}
|
||||
|
||||
expect(mockConfig.takeActiveTodoReminder).toHaveBeenCalledWith(
|
||||
'prompt-bridged-agent-failed',
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
it('keeps the turn budget for tool results without an Agent execution', async () => {
|
||||
vi.mocked(mockConfig.takeActiveTodoReminder).mockReturnValue(undefined);
|
||||
|
||||
|
|
|
|||
|
|
@ -4703,15 +4703,19 @@ export class LlmClient {
|
|||
break;
|
||||
}
|
||||
if (response.name === ToolNames.TOOL_CALL && response.id) {
|
||||
// A refused or cancelled bridge call carries `error` (the
|
||||
// refusal/cancellation prefixes; `convertToFunctionErrorResponse`
|
||||
// moves `output` into `error`) — nothing was delegated on that
|
||||
// turn, so it must not force the reminder, mirroring the
|
||||
// refused/cancelled exclusion in the sibling seeding pass.
|
||||
// Only a refusal or cancellation means nothing was delegated —
|
||||
// both carry the bridge's prefix in `error`. A bridged Agent
|
||||
// that ran and then failed carries a generic error string: real
|
||||
// work advanced, so the reminder must still force, exactly like
|
||||
// the direct-Agent branch above and the sibling seeding pass
|
||||
// (which excludes only the two prefixes).
|
||||
const errorText = (
|
||||
response.response as Record<string, unknown> | undefined
|
||||
)?.['error'];
|
||||
if (
|
||||
(response.response as Record<string, unknown> | undefined)?.[
|
||||
'error'
|
||||
] !== undefined
|
||||
typeof errorText === 'string' &&
|
||||
(errorText.startsWith(DEFERRED_TOOL_CALL_REFUSAL_PREFIX) ||
|
||||
errorText.startsWith(DEFERRED_TOOL_CALL_CANCELLATION_PREFIX))
|
||||
) {
|
||||
continue;
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue