mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-10-21 22:39:45 +00:00
fix(core,cli): close the round-5 findings on the deferred-coordination defaults
- Session.ts: reveal propose_goal exactly while its turn key is armed and unreveal on clear — natively deferred, it was otherwise filtered out of every announcement and undiscoverable in ACP/daemon sessions (R5-1). - goal-tool-result-provenance.ts: narrow the goal_runtime stamp to select: lookups naming a Goal tool — keyword discovery about the objective's own work is evidence the verifier needs and must keep its ordinary stamp (R5-2). - Session.ts: align the ACP observation arm with the core client's exclusion — a cancelled bridged call means nothing was delegated and must not force the reminder (R5-3). - docs: fix the component-split arithmetic against the measured total, both languages (R2-4 carried). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
jinjing.zzj
parent
cfce41cf39
commit
b8df6af78d
6 changed files with 72 additions and 9 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, 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.
|
||||
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 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）区分的两种状态分别钉住，且写出的边界就是 `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`，断言其余部分完全相同。
|
||||
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 设施下只能做的弱化召回验证。
|
||||
|
|
|
|||
|
|
@ -609,6 +609,7 @@ describe('Session', () => {
|
|||
registerTool: ReturnType<typeof vi.fn>;
|
||||
registerPermissionDeferredFactory: ReturnType<typeof vi.fn>;
|
||||
revealDeferredTool: ReturnType<typeof vi.fn>;
|
||||
unrevealDeferredTool: ReturnType<typeof vi.fn>;
|
||||
pinDeferredToolReveal: ReturnType<typeof vi.fn>;
|
||||
warmAll: ReturnType<typeof vi.fn>;
|
||||
getFunctionDeclarationsFiltered: ReturnType<typeof vi.fn>;
|
||||
|
|
@ -967,6 +968,7 @@ describe('Session', () => {
|
|||
registerTool: vi.fn(),
|
||||
registerPermissionDeferredFactory: vi.fn(),
|
||||
revealDeferredTool: vi.fn(),
|
||||
unrevealDeferredTool: vi.fn(),
|
||||
pinDeferredToolReveal: vi.fn(),
|
||||
warmAll: vi.fn().mockResolvedValue(undefined),
|
||||
getFunctionDeclarationsFiltered: vi.fn((names: string[]) =>
|
||||
|
|
@ -39372,7 +39374,15 @@ describe('Session', () => {
|
|||
undefined,
|
||||
);
|
||||
expect(mockLlmClient.setTools).toHaveBeenCalledTimes(2);
|
||||
|
||||
// propose_goal is natively deferred: the arm/clear pair must also
|
||||
// reveal/unreveal it, or the tool is filtered out of every
|
||||
// announcement and no ACP/daemon model can ever discover it.
|
||||
expect(mockToolRegistry.revealDeferredTool).toHaveBeenCalledWith(
|
||||
'propose_goal',
|
||||
);
|
||||
expect(mockToolRegistry.unrevealDeferredTool).toHaveBeenCalledWith(
|
||||
'propose_goal',
|
||||
);
|
||||
vi.mocked(mockConfig.setGoalProposalTurnKey).mockClear();
|
||||
mockLlmClient.setTools.mockClear();
|
||||
await session.prompt({
|
||||
|
|
|
|||
|
|
@ -5557,6 +5557,9 @@ export class Session implements SessionContext {
|
|||
this.config.getGoalProposalHostSupported() &&
|
||||
this.config.setGoalProposalTurnKey(undefined)
|
||||
) {
|
||||
this.config
|
||||
.getToolRegistry()
|
||||
.unrevealDeferredTool(ToolNames.PROPOSE_GOAL);
|
||||
try {
|
||||
await this.config.getLlmClient().setTools();
|
||||
} catch (error) {
|
||||
|
|
@ -6037,6 +6040,12 @@ export class Session implements SessionContext {
|
|||
responseCapture.goalProposalTurn?.turnKey,
|
||||
)
|
||||
) {
|
||||
// propose_goal is natively deferred: the key arms only inside a
|
||||
// prompt, so without the reveal it would be filtered out of every
|
||||
// announcement and never be discoverable in ACP/daemon sessions.
|
||||
this.config
|
||||
.getToolRegistry()
|
||||
.revealDeferredTool(ToolNames.PROPOSE_GOAL);
|
||||
await this.config.getLlmClient().setTools();
|
||||
}
|
||||
const daemonPromptId = getInvocationContext()?.promptId;
|
||||
|
|
@ -9191,7 +9200,14 @@ export class Session implements SessionContext {
|
|||
toolRun.repeatedToolFailureBatch?.observations.some(
|
||||
(observation) =>
|
||||
canonicalToolName(observation.policyToolName ?? '') ===
|
||||
ToolNames.AGENT && observation.executionStatus !== 'not_started',
|
||||
ToolNames.AGENT &&
|
||||
// Match the core client's exclusion: a cancelled (or refused)
|
||||
// bridged call means nothing was delegated, so it must not force
|
||||
// the reminder. The observation carries no error text, so the
|
||||
// prefix check is not available here; 'cancelled' is the
|
||||
// observable half of it.
|
||||
observation.executionStatus !== 'not_started' &&
|
||||
observation.executionStatus !== 'cancelled',
|
||||
);
|
||||
const activeTodoReminder = carriesAgentToolResult
|
||||
? this.config.takeActiveTodoReminder(promptId, true)
|
||||
|
|
|
|||
|
|
@ -60,6 +60,27 @@ describe('goalToolResultProvenance', () => {
|
|||
).toEqual({ goalContext: permit, provenance: 'goal_runtime' });
|
||||
});
|
||||
|
||||
it('keeps keyword and non-Goal discovery as evidence during a Goal turn', () => {
|
||||
// The stamp is only for select:-of-a-Goal-tool bookkeeping: a keyword
|
||||
// search about the objective's own work (e.g. "wiki fetch" returning
|
||||
// "No tools found") is exactly the external_fact a blocked/infeasible
|
||||
// proposal is proved with, and must keep its ordinary stamp.
|
||||
expect(
|
||||
goalToolResultProvenance({
|
||||
name: ToolNames.TOOL_SEARCH,
|
||||
args: { query: 'wiki fetch' },
|
||||
goalContext: permit,
|
||||
}),
|
||||
).toEqual({ goalContext: permit });
|
||||
expect(
|
||||
goalToolResultProvenance({
|
||||
name: ToolNames.TOOL_SEARCH,
|
||||
args: { query: 'select:read_file' },
|
||||
goalContext: permit,
|
||||
}),
|
||||
).toEqual({ goalContext: permit });
|
||||
});
|
||||
|
||||
it.each([ToolNames.GET_GOAL, ToolNames.UPDATE_GOAL])(
|
||||
'marks a bridged %s result as the Goal’s own bookkeeping',
|
||||
(name) => {
|
||||
|
|
|
|||
|
|
@ -58,17 +58,33 @@ export function goalToolResultProvenance(
|
|||
if (
|
||||
lowerToolName === ToolNames.GET_GOAL ||
|
||||
lowerToolName === ToolNames.UPDATE_GOAL ||
|
||||
// Discovery is bookkeeping too: a `tool_search` schema lookup during a
|
||||
// Goal turn is the Goal's own navigation, not evidence about the world —
|
||||
// otherwise the finishing turn's `tool_search select:update_goal` enters
|
||||
// the catalog as an `external_fact` and reads as progress.
|
||||
requestName === ToolNames.TOOL_SEARCH
|
||||
// Only the discovery that IS Goal bookkeeping counts: a `select:` lookup
|
||||
// naming a Goal tool. An unconditional tool_search stamp would also
|
||||
// swallow keyword discovery about the objective's own work — e.g.
|
||||
// `tool_search wiki fetch` returning "No tools found" is exactly the
|
||||
// external_fact a blocked/infeasible proposal is proved with.
|
||||
(requestName === ToolNames.TOOL_SEARCH &&
|
||||
selectsGoalTool(request.args?.['query']))
|
||||
) {
|
||||
return { goalContext: { ...goalContext }, provenance: 'goal_runtime' };
|
||||
}
|
||||
return { goalContext: { ...goalContext } };
|
||||
}
|
||||
|
||||
/** Whether a tool_search query is a `select:` lookup naming a Goal tool. */
|
||||
function selectsGoalTool(query: unknown): boolean {
|
||||
if (typeof query !== 'string' || !query.startsWith('select:')) return false;
|
||||
const goalTools = new Set([
|
||||
ToolNames.GET_GOAL,
|
||||
ToolNames.UPDATE_GOAL,
|
||||
ToolNames.PROPOSE_GOAL,
|
||||
]);
|
||||
return query
|
||||
.slice('select:'.length)
|
||||
.split(',')
|
||||
.some((name) => goalTools.has(canonicalToolName(name.trim())));
|
||||
}
|
||||
|
||||
/**
|
||||
* The same rule for a host that executes tools straight from the model's
|
||||
* function calls and so has no `ToolCallRequestInfo` to read a permit from.
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue