Skip to content

fix(acp): Preserve submitted prompt provenance for auto recall - #11455

Merged
doudouOUC merged 5 commits into
mainfrom
codex/daemon-auto-recall-submitted-prompt
Sep 10, 2026
Merged

doudouOUC merged 5 commits into
mainfrom
codex/daemon-auto-recall-submitted-prompt

Conversation

@doudouOUC

@doudouOUC doudouOUC commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Carries original submitted text to UserPromptSubmit hooks through an explicit per-request declaration, enabling configured Auto Recall for Web Shell user submissions and opted-in ACP/daemon clients. Web Shell captures the original composer text before host preparation, slash-command rewriting, and attachment expansion, and preserves it separately in ordinary queues. Background dispatches, retries, channel deliveries, and requests without a valid declaration do not publish submitted_prompt.

Why it's needed

Released daemon sessions invoke the hook without submitted_prompt, so provenance-gated Auto Recall skips retrieval. Inferring this field from every fresh non-channel request would also trigger retrieval for scheduled tasks, sub-session spawns, and model-authored Live tasks. Explicit declarations close that gap without inferring user provenance when a channel marker is removed or a display projection is available.

Reviewer Test Plan

Verify declared user input and undeclared machine input independently. Confirm that host-prepared text, attachments, model-only context, and slash-command expansion never replace the original declaration. Check that existing legacy hook invocation and recording behavior remain intact.

How to verify

Configure the existing Auto Recall profile in an isolated launcher with a loopback provider and observing hook. Submit a declared original question with different request text; the hook must preserve the declaration, search once, and add the returned context to the model input. Repeat without a declaration, with forged private metadata, and with invalid, blank, channel, or display metadata; no submitted field or provider search should occur. A real tool-result continuation must not repeat retrieval. In Web Shell, verify direct, host-prepared, queued, and converted /skills submissions preserve the original text; generic actions and manual scheduled runs must not declare it automatically.

Validation: real rebuilt daemon/REST/bridge/ACP-child/hook/provider chain passed nine scenarios, and the original failing Session assertion passed unchanged. Affected bridge, ACP agent, Session, REST server, ACP HTTP, Web Shell action, App, ChatPane, and queue tests passed. Root build, bundle, typecheck, lint, and changed-file formatting passed. Independent code review and two self-audit passes found no remaining substantive issue. After merging main at 1097b9fe0a to refresh the CI gate, root build, bundle, typecheck, lint, formatting, all 1,383 Web Shell tests in the five affected suites, and 50 targeted backend tests passed again. The same nine real daemon scenarios also passed again on the clean merged head b4ee50261e.

Evidence (Before & After)

N/A for visual changes; this PR changes hook behavior. Before correction at 2b4f46a, the reproduction asserted omission but received submitted_prompt for an undeclared machine request. After correction, that assertion passes; the real daemon probe reports no search for undeclared or invalid input, and one search for explicitly declared input. A real read_file continuation produced three model requests with one hook invocation and one search. The earlier four real Holo scenarios predate this correction and are historical evidence only.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

macOS 26.6.2, Node.js 22.22.3, npm 10.9.8. Rebuilt local daemon and packaged hook with a controlled model and loopback provider; no cloud credentials were used in this correction. Build used an 8 GiB Node heap. Browser component tests used jsdom with Node experimental Web Storage disabled to avoid a local storage-global conflict.

Risk & Scope

  • Main risk or tradeoff: eligible text reaches every configured hook and may be sent to an explicitly configured memory provider. The declaration is caller-supplied provenance, not human authentication or DLP. Both human and automated channel messages remain excluded; realtime voice handoffs omit declarations because their request text is model-generated.
  • Not validated / out of scope: Windows/Linux execution, real browser UI, actual cron/Live dispatcher execution, channel-worker restart races, real direct ACP transport, PolarDB/cloud revalidation, and per-workspace profile routing. These admission boundaries have focused unit coverage; the real daemon probe exercises REST. Core TUI editor provenance rules are not added to ACP: re-submitting restored editor content is a new Web Shell submission. The Direct Profile managed launcher remains TTY-only.
  • Breaking changes / migration notes: ACP/daemon clients opt in per eligible request with _meta: { "qwen.submittedPrompt": "original submitted text" }; ordinary SDK transports must not add it globally. Existing clients without the optional declaration retain legacy hook behavior and skip provenance-gated recall. The private daemon-to-child key cannot be supplied by external callers. Default MCP registration and provider protocols are unchanged. Strict hook decoders must allow or explicitly handle the optional submitted_prompt field.

Design: English · 简体中文. Both versions have matching decisions, constraints, acceptance criteria, and evidence boundaries.

Linked Issues

No issue is closed by this PR. Follow-up validation after #11397 exposed this independent ACP hook gap.

中文说明

本 PR 做了什么

通过逐请求显式声明,将原始提交文本传递给 UserPromptSubmit hook,让 Web Shell 用户提交及显式启用的 ACP/daemon 客户端能够使用已配置的 Auto Recall。Web Shell 在宿主准备、斜杠命令改写和附件展开之前捕获原始输入框文本,并在普通队列中独立保存。后台派发、重试、channel 投递以及缺少有效声明的请求不发布 submitted_prompt

为什么需要

已发布的 daemon 会话调用 hook 时不携带 submitted_prompt,依赖来源信息的 Auto Recall 因此跳过检索。如果从每个全新非 channel 请求推断该字段,定时任务、子会话派生和模型撰写的 Live task 也会触发检索。显式声明修复这一缺口,并避免在 channel 标记被删除或存在显示投影时推断用户来源。

审阅者测试计划

分别验证有声明的用户输入和无声明的机器输入。确认宿主准备后的文本、附件、模型专用上下文和斜杠命令展开不会替换原始声明。检查既有旧 hook 调用和录制行为保持不变。

如何验证

在隔离启动环境中配置已有 Auto Recall profile、本地 provider 和观察 hook。声明原始问题,同时使用不同的请求文本;hook 必须保留声明、搜索一次,并将返回上下文加入模型输入。分别测试无声明、伪造私有元数据、非法值、空白值、channel 或显示元数据;不得出现提交字段或 provider 搜索。真实工具结果续轮不得重复检索。在 Web Shell 验证直接提交、宿主准备、排队和转换后的 /skills 提交均保留原文;通用 action 和定时任务手动运行不得自动声明。

验证:真实重新构建的 daemon/REST/bridge/ACP 子进程/hook/provider 链路九项场景通过,原先失败的 Session 断言保持原期望并通过。相关 bridge、ACP agent、Session、REST server、ACP HTTP、Web Shell action、App、ChatPane 和队列测试通过。全仓构建、打包、类型检查、lint 及修改文件格式检查通过。独立代码审查和两轮自审未发现剩余实质问题。合入 1097b9fe0a 处的 main 以更新 CI 门禁后,全仓构建、打包、类型检查、lint、格式检查、五个相关 Web Shell 测试文件的全部 1,383 项测试及 50 项后端定向测试再次通过。同样的九项真实 daemon 场景也在工作树干净的合并提交 b4ee50261e 上再次通过。

证据(前后对比)

视觉变化为 N/A;本 PR 改变 hook 行为。修正前的 2b4f46a 上,复现断言要求省略字段,却收到无声明机器请求的 submitted_prompt。修正后该断言通过;真实 daemon 探针在无声明或非法输入时不搜索,显式声明输入时搜索一次。真实 read_file 续轮产生三次模型请求,仅调用一次 hook 和搜索。此前四项真实 Holo 场景早于本次修正,仅作为历史证据。

测试平台

OS 状态
🍏 macOS ✅ 已测试
🪟 Windows ⚠️ 未测试
🐧 Linux ⚠️ 未测试

环境(可选)

macOS 26.6.2、Node.js 22.22.3、npm 10.9.8。重新构建的本地 daemon 和打包 hook,配合受控模型及本地 provider;本次修正未使用云凭证。构建使用 8 GiB Node 堆。浏览器组件测试使用 jsdom,并关闭 Node 实验性 Web Storage,以避开本机存储全局变量冲突。

风险与范围

  • 主要风险或取舍:符合条件的文本会传递给每个已配置 hook,并可能发送到显式配置的记忆 provider。声明是调用方提供的来源信息,不是人工身份认证或 DLP。人工和自动 channel 消息均继续排除;实时语音委派因请求文本由模型生成而不提供声明。
  • 未验证或范围外:Windows/Linux 执行、真实浏览器 UI、实际 cron/Live 派发器执行、channel-worker 重启竞争、真实直连 ACP 传输、PolarDB/云端重新验证及按 workspace 路由 profile。相关准入边界有定向单元测试覆盖;真实 daemon 探针走 REST。不向 ACP 增加 core TUI 编辑器来源规则:重新提交恢复到输入框的内容属于新的 Web Shell 提交。Direct Profile 托管启动器仍仅支持 TTY。
  • 破坏性变更或迁移说明:ACP/daemon 客户端通过 _meta: { "qwen.submittedPrompt": "original submitted text" } 在符合条件的请求上逐次启用;普通 SDK 传输层不得全局添加它。缺少这个可选声明的现有客户端保留旧 hook 行为,并跳过依赖来源的召回。外部调用方不能提供 daemon 到子进程的私有键。默认 MCP 注册和 provider 协议不变。严格 hook 解码器必须允许或显式处理可选 submitted_prompt 字段。

设计:English · 简体中文。两个版本的决策、约束、验收标准及证据边界一致。

关联 Issue

本 PR 不关闭任何 issue。在 #11397 后续验证中发现了这个独立的 ACP hook 缺口。

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

E2E verification report

Verified the submitted source in 4fb7d2f0a9 on base 1919ff97f5. Tests ran before committing the same working-tree implementation; both local and cloud runs used CLI bundle SHA256 4e22afca94f37ae738c0e38c91c752c61e8e7e468c3830e92b85acb6a451152d.

The global CLI and pre-fix daemon reproduced the missing-field failure: UserPromptSubmit ran with no submitted_prompt, the shipped Auto Recall hook returned {}, provider requests were zero, and the actual model request had no synthetic memory. Adding the regression assertions before the fix produced 4 failed / 7 passed focused hook tests.

After the fix, a real daemon invoked the packaged hook through an observing wrapper that passed its JSON input unchanged. No context_search MCP tool was configured. The model was a controlled local OpenAI-compatible endpoint; assertions inspected its actual requests.

Scenario Loopback provider Real Holo
Fresh A question automatically injects target and control memory PASS PASS
Real read_file tool continuation executes UserPromptSubmit only once PASS PASS
Workspace B outside the configured root receives no A memory PASS PASS
Fresh A after target deletion receives control only PASS PASS

Each run used four different session IDs. The tool scenario made three model requests but one hook invocation. Loopback provider request counts were 1/1/0/1. Cloud search HTTP calls were not independently intercepted, so the cloud result establishes hook invocation count and actual injected context, not a separately measured HTTP count.

Holo used a fresh isolated scope and two synthetic records. Seed/read/delete verified exact ID, scope, and original content; both records ended with DELETE 200 followed by exact GET 404, with no residual IDs. Seed and deletion were direct Holo API calls for this hook-specific acceptance; the earlier explicit MCP write/delete acceptance is separate. B tests repository containment, not per-workspace profile routing. Temporary daemons, local servers, and configuration directories were cleaned up; no credentials or persistent user configuration were added.

Validation: 869/869 session tests; root build, bundle, typecheck; focused lint; formatting and diff checks passed on macOS 26.6.2 / Node 22.22.3 / npm 10.9.8. Windows and Linux were not tested locally.

中文说明

验证对应提交 4fb7d2f0a9 的源码,基线为 1919ff97f5。测试在提交前对同一工作区实现执行;本地与云端使用相同 CLI bundle SHA256:4e22afca94f37ae738c0e38c91c752c61e8e7e468c3830e92b85acb6a451152d

全局 CLI 和修复前 daemon 均复现缺少字段的问题:UserPromptSubmit 执行但没有 submitted_prompt,打包 Auto Recall hook 返回 {},provider 请求为零,实际模型请求没有合成记忆。在修复前加入回归断言,定向 hook 测试为 4 失败 / 7 通过。

修复后,真实 daemon 通过观察 wrapper 调用打包 hook,JSON 输入原样转发。未配置 context_search MCP 工具。模型是受控本地 OpenAI 兼容服务,断言检查其实际收到的请求。

场景 本地 provider 真实 Holo
新 A 会话普通提问自动注入目标和控制记忆 PASS PASS
真实 read_file 工具续接仅执行一次 UserPromptSubmit PASS PASS
配置根之外的 workspace B 不获得 A 记忆 PASS PASS
删除目标后的新 A 会话仅获得控制记忆 PASS PASS

每轮使用四个不同会话 ID。工具场景有三次模型请求,但只有一次 hook 执行。本地 provider 请求次数为 1/1/0/1。未独立拦截云端搜索 HTTP 请求,因此云端结果证明 hook 执行次数和实际注入上下文,不单独证明 HTTP 次数。

Holo 使用全新隔离 scope 和两条合成记录。创建、读取、删除时校验精确 ID、scope 和原文;两条记录最终均 DELETE 200 后精确 GET 404,无残留 ID。本轮针对 hook,直接调用 Holo API 创建和删除记录;此前的显式 MCP 写入和删除验收是独立测试。B 验证仓库包含关系,不验证按 workspace 路由 profile。临时 daemon、本地服务和配置目录均已清理;未添加凭证或持久用户配置。

验证:869/869 会话测试、全仓构建、打包、类型检查、定向 lint、格式和 diff 检查通过。环境为 macOS 26.6.2 / Node 22.22.3 / npm 10.9.8。未在本地测试 Windows 和 Linux。

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the rework — this is a materially different PR from the one I requested changes on, and the difference is the right one.

Template looks good ✓

Problem: observed and reproduced, not theoretical. The design doc names the commit it was reproduced on: a normal daemon prompt on main 1919ff97f5 runs the configured UserPromptSubmit hook but omits submitted_prompt, so Mem0 Auto Recall returns {} without searching. The over-emission of the first fix was reproduced too — a failing Session/Hook probe at 2b4f46a showed scheduled tasks, sub-session spawns and Live task dispatches also satisfied that condition. Both directions of the bug carry evidence, which is more than most PRs bring.

Direction: aligned. Web Shell is a first-party client whose users can register Auto Recall and then silently get no retrieval — a real gap — and the fix expands no deployment contract (Direct Profile stays TTY-only, default MCP registration and provider protocols unchanged). Last run I escalated to @wenshao the question of whether ACP producer eligibility was a product decision already made; no human has ruled on it, but this rework answers it in the conservative direction — opt-in per request, no inference — and records the decision in a bilingual design doc. The upstream (claude-code) CHANGELOG has no direct reference to submitted_prompt provenance, but hook payload correctness is an actively maintained area there, so the territory is clearly in scope.

Size: cross-package (acp-bridge, cli, core, web-shell), so Stage 0 applies. Breakdown: 165 production code lines · 146 docs/markdown lines · 504 test lines (815 total, no generated/schema files). The only packages/core/src/** file is chatRecordingService.ts — a 5-line JSDoc correction, so there is no production logic change in core. Under the 500-line escalation threshold and under the 1000-line advisory; fix type, so no Tier 1 question. Author has write access, so Tier 2 evaluates rather than blocks.

Approach: the scope feels right and I found nothing to cut. Every edit traces to the stated goal: the 9 locale files are one translated string that had to move because its English source moved; the two integration READMEs describe the producer contract this PR changes; hooks.md is the public contract; constants.ts is the /hooks UI description that quoted "interactive TUI", now false. No drive-by refactors, no formatting churn, no unrelated files. The two-key scheme (public qwen.submittedPrompt for callers, private qwen.daemon.submittedPrompt for the daemon-to-child hop) is not new machinery either — it mirrors how promptDisplayText, modelPrompt and the channel key already work here, which is what I would have proposed independently.

Risk: Stage 1e matched — packages/cli/src/acp-integration/acpAgent.ts and packages/cli/src/acp-integration/session/Session.ts sit on the acp-integration high-revert path. That raises review depth rather than blocking: no Stage 2 enrichment skipped, CI evidence required before approving, and a named sandboxed lane. Both are in Stage 2.

Moving on to code review. 🔍

中文说明

感谢这轮重写 —— 与我上次请求修改的那个 PR 已有实质差别,而且差别正是该有的方向。

模板完整 ✓

问题: 已观测且已复现,不是理论性问题。设计文档写明了复现所在的 commit:main 1919ff97f5 上的一次普通 daemon 提问会执行已配置的 UserPromptSubmit hook,但不携带 submitted_prompt,因此 Mem0 Auto Recall 返回 {} 而不检索。首轮修复的过度发射同样有复现 —— 2b4f46a 上一个失败的 Session/Hook 探针表明定时任务、子会话派生与 Live task 派发也满足该条件。这个 bug 的两个方向都有证据,这比多数 PR 带来的要多。

方向: 对齐。Web Shell 是第一方客户端,其用户可以注册 Auto Recall 却静默地得不到任何检索 —— 这是一个真实缺口 —— 而本次修复没有扩大任何部署契约(Direct Profile 仍仅支持 TTY,默认 MCP 注册与 provider 协议不变)。上一轮我把「ACP 生产方资格是否是既定产品决策」升级给了 @wenshao;没有人对此作出裁定,但这次重写以保守方向回答了它 —— 逐请求显式启用、不做推断 —— 并把该决策记录在双语设计文档中。上游(claude-code)CHANGELOG 没有直接提到 submitted_prompt 来源信息,但 hook 载荷正确性在那里是持续维护的领域,所以这块地界显然在范围内。

规模: 跨包(acp-bridgeclicoreweb-shell),因此适用 Stage 0。拆分为:生产代码 165 行 · 文档/markdown 146 行 · 测试 504 行(合计 815,无生成/schema 文件)。唯一的 packages/core/src/** 文件是 chatRecordingService.ts —— 一处 5 行 JSDoc 更正,因此 core 中没有生产逻辑变更。低于 500 行升级阈值,也低于 1000 行大 PR 建议;类型为 fix,因此不涉及 Tier 1。作者具有写权限,所以 Tier 2 是评估而非阻断。

方案: 范围合理,我没有找到可以砍掉的部分。每处改动都能追溯到既定目标:9 个语言文件是一条翻译字符串,因其英文源串变动而必须跟随;两个 integration README 描述的正是本 PR 改动的生产方契约;hooks.md 本身就是公开契约;constants.ts/hooks 界面描述,其中引用的「interactive TUI」现已不成立。没有顺手重构,没有格式化噪声,没有无关文件。双键方案(对调用方公开的 qwen.submittedPrompt,用于 daemon 到子进程一跳的私有 qwen.daemon.submittedPrompt)也不是新机制 —— 它与本仓库中 promptDisplayTextmodelPrompt 和 channel 键的既有做法一致,这也正是我独立会提出的方案。

风险: Stage 1e 命中 —— packages/cli/src/acp-integration/acpAgent.tspackages/cli/src/acp-integration/session/Session.ts 位于 acp-integration 高回滚率路径上。这会加深审查深度,而不是阻断:不跳过任何 Stage 2 补充项、批准前必须有 CI 证据、并点名沙箱验证通道。两者都在 Stage 2 中。

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 46dc4382ed1a08b7509426d8ca429d681aec83a6 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Code review

I wrote my proposal before opening the diff: the ACP path has no in-band signal separating "a human typed this" from "a machine composed this" — scheduled tasks, Live dispatches, sub-session spawns, retries and channel deliveries all funnel through the same session.prompt() — so provenance has to be declared by the caller that actually knows, carried as trusted metadata an outsider cannot forge, with no fallback to request text and no inference from a display projection. That is what this PR does, including the two-key split I would have arrived at for the same reason the codebase already uses it for promptDisplayText and the channel key. No simpler path was missed.

The Critical I requested changes on is closed, and I verified that rather than taking it on faith. Session.ts now derives the value solely from the declaration (typeof declaredSubmission === 'string' ? declaredSubmission : '') — the fallback to promptText is gone, so the field requires a positive declaration instead of surviving a denylist. I then enumerated the population the old gate leaked on: there are exactly six bridge.sendPrompt callers in the repo. Two are in this diff (the REST prompt route and the ACP HTTP dispatch, both guarded). The other four — serve/create-sub-session.ts:932, serve/live/live-task-service.ts:1081, serve/live/live-session-coordinator.ts:1666, serve/conversations/standalone-session-service.ts:3144 — I read at this head. All four pass a context carrying only {promptId, onPromptAdmitted, …} and a request with no _meta at all. BridgeClientRequestContext.submittedPrompt is a new field, so those four cannot set it; the bridge injects nothing, acpAgent re-stamps nothing, Session emits no field. Closed by construction, not by enumeration — which is the property that makes the next dispatcher safe by default.

Three places I checked because they were where this could plausibly still go wrong:

  • Both guards are independently load-bearing, not belt-and-braces. isUserSubmissionTurn (turn class) and the nonblank-declaration test (presence) fail on different rows — the retry and channel rows carry declarations and must still omit the field, so dropping the turn-class guard breaks them even with the declaration gate intact. Neither term is redundant.
  • The trusted-parent ternary fails in the right direction. acpAgent.ts reads the private key when privateParentState === 'trusted' and the public key otherwise, so a direct ACP client cannot forge the daemon hop and a trusted parent cannot silently fall back to the public key. I independently counted the mint sites for PRIVATE_PARENT_CAPABILITY_META_KEY: exactly two in non-test code (acp-bridge/src/bridge.ts, channels/base/src/AcpBridge.ts), and the channel bridge builds _meta from scratch rather than spreading caller input — so that invariant is not currently violable.
  • The Web Shell submittedPrompt = text default is safe. This is the client-side mirror of the original bug and the first thing I went looking for. enqueuePrompt receives the original composer text as text and queues the prepared text separately, so the default declares the original, not the rewrite. Manual scheduled runs go through the REST scheduled-tasks route, which never sets it — and App.test.tsx now pins that with expect.not.objectContaining. Server-restored summary-only queue rows drop the declaration at useQueuedPrompts.ts:624, correctly, since that text is not the original composer text.

Nothing rises to a blocker. Three non-blocking items, all completeness rather than correctness:

  1. Suggestion — the design doc promises "a trusted parent cannot use a public key as a fallback when the private declaration is missing". The code implements it; no test pins it. The new acpAgent.test.ts case covers the untrusted direction only. The mirror fixture (a trusted session whose meta carries only qwen.submittedPrompt, asserting no submitted_prompt reaches the hook) would close it. Defence-in-depth for a documented property, not a live hole — census above.
  2. Nice to have — the bridge's !isPromotedMidTurn clause cannot decide any outcome today, because the only producer of promotedMidTurn never sets submittedPrompt. Worth keeping (it becomes load-bearing the moment a promotion producer declares), but the doc sentence "channel and promoted mid-turn dispatches omit it" should not be read as test-backed on its second half.
  3. Nice to haveacpAgent's channel guard on re-stamp is redundant for the same reason: every ingress that could reach it already drops the declaration when a channel marker is present.

Public-contract note, not a defect. qwen.submittedPrompt is a new externally-suppliable key, and newly-eligible ACP/daemon payloads now carry submitted_prompt. That is bounded rather than open-ended: every ingress accepting the key already accepts an arbitrary params.prompt from the same caller, it cannot forge the private key, and the marker guards test === undefined so they fail closed (adding a channel, display or worker-auth marker suppresses the declaration rather than enabling it). The PR states plainly in the body, the design doc and hooks.md that this is caller-supplied provenance and not authentication or DLP, and warns admins whose hooks reject unknown fields. That is how a contract change should ship. Both predecessor design docs it links to exist at this head and the #compatibility-and-migration anchor resolves.

sequenceDiagram
    participant P1 as Web Shell composer
    participant P2 as REST prompt route
    participant P3 as ACP bridge
    participant P4 as ACP child admission
    participant P5 as Session
    participant P6 as UserPromptSubmit hook
    participant P7 as Auto Recall provider
    P1->>P1: capture original text before prepareSubmit
    P1->>P2: prompt plus _meta qwen.submittedPrompt
    P2->>P2: strip public and private keys from the forwarded body
    P2->>P3: context.submittedPrompt, only with no channel, worker-auth or display marker
    P3->>P3: strip both keys, re-inject the private key from trusted context
    P3->>P4: _meta qwen.daemon.submittedPrompt
    P4->>P4: trusted parent reads the private key only, direct client the public key only
    P4->>P5: _meta qwen.daemon.submittedPrompt
    P5->>P5: emit only on a fresh non-channel turn with a nonblank declaration
    P5->>P6: prompt plus optional submitted_prompt
    P6->>P7: one search, only when submitted_prompt is present
Loading

Internal dispatchers (scheduled tasks, Live tasks, sub-sessions, standalone sessions) enter at P3 with no context declaration and no _meta, so they reach P6 with prompt only — that is the path the old gate leaked.

Files changed (38 total — key paths)
File What changed
packages/cli/src/acp-integration/session/Session.ts The polarity inversion: value comes only from the declaration, emitted only on a fresh non-channel turn when nonblank
packages/cli/src/acp-integration/acpAgent.ts Trusted parent reads the private key, direct client the public key; both stripped then conditionally re-stamped
packages/acp-bridge/src/bridge.ts Strips both keys from every request, re-injects the private key only from trusted context, not on promoted mid-turn or channel turns
packages/acp-bridge/src/bridgeTypes.ts Adds the context field and the two meta key constants
packages/cli/src/serve/routes/session.ts REST admission reads the public key, deletes both from the forwarded body, fails closed on channel, worker-auth or display markers
packages/cli/src/serve/acp-http/dispatch.ts Same admission shape for the ACP HTTP/WebSocket path
packages/web-shell/client/App.tsx Threads the original composer text through direct, prepared, queued and converted slash-command submissions
packages/web-shell/client/components/ChatPane.tsx Declares the original text on both the send and the enqueue branch
packages/web-shell/client/hooks/useQueuedPrompts.ts Stores the declaration separately from the queued payload, drops it on server-restored summary-only rows, includes it in queue equality
packages/web-shell/client/daemon/session/actions.ts Puts the declaration on the wire as the public meta key
packages/web-shell/client/daemon/session/types.ts Adds the option to the public send-prompt contract
packages/web-shell/client/components/QueuedPromptDisplay.tsx Carries the field on the queued-prompt shape
packages/core/src/services/chatRecordingService.ts JSDoc only — corrects the recording payload comment for the ACP path
packages/cli/src/ui/components/hooks/constants.ts Hook description no longer claims the field is TUI-only
packages/cli/src/i18n/locales/*.js 9 files, one translated string each, following the English source
docs/users/features/hooks.md Rewrites the public contract: positive declaration, ACP invocation policy, strict-decoder warning
docs/design/daemon-user-prompt-submit-provenance.md + .zh-CN.md New bilingual design doc, synchronized section for section
integrations/external-context*/README.md 2 files, bring the Auto Recall producer contract in line with the new eligibility
…and 10 test files (504 lines) Session 14-row provenance table, bridge stripping, admission spoofing, REST and ACP HTTP admission, Web Shell actions, App, ChatPane, queued prompts

Test evidence — the PR's own CI

This is an unattended CI run, so I built and executed nothing from this PR. The evidence below is the PR's own CI on the reviewed commit, read through the API, plus the completed sandboxed verification already posted in this thread.

All five pull_request-event workflow runs on this head are complete and green; nothing is pending. Test (ubuntu-latest, Node 22.x) is the unit line that actually ran — Test (windows-latest, …) and Test (macos-latest, …) were skipped, as was Integration Tests (CLI, No Sandbox). No check is red, so there is no failing-job log to excerpt and no pre-existing-infra-noise call to make.

Check Conclusion
Qwen Code CI — Test (ubuntu-latest, Node 22.x) success
Qwen Code CI — Lint & Static (ubuntu-latest, Node 22.x) success
Qwen Code CI — Integration Tests (no-AK, No Sandbox) success
Qwen Code CI — web-shell E2E Smoke (ubuntu-latest, Node 22.x) success
Qwen Code CI — Capture web-shell visuals (ubuntu-latest, Node 22.x) success
Qwen Code CI — Test (windows-latest, Node 22.x) skipped
Qwen Code CI — Test (macos-latest, Node 22.x) skipped
Qwen Code CI — Integration Tests (CLI, No Sandbox) skipped
Serve A/B (ubuntu-latest, Node 22.x) success
Web-shell Visuals success
tui-parity — TUI parity snapshots (ink vs opentui) success
tui-parity — OpenTUI no-flicker gate success
SDK Java — ubuntu-latest / Java 11, 17, 21 success
SDK Java — macos-latest / Java 21 success
SDK Java — windows-latest / Java 21 success
Desktop Shell (ubuntu-22.04) success
Desktop Shell (windows-2022) success
Real daemon E2E / Java 11 success

Two automated probes on this head are worth reading alongside that table, because they bear directly on the finding I raised: Serve A/B built the PR base and this head, drove a fixed endpoint set against each and reported no response changes across 12 scenarios — consistent with a change that adds an opt-in field rather than altering existing responses. Web-shell Visuals reported no changed screenshot while naming three render-shaping files; by that check's own framing an empty result there is a coverage gap, not a clean bill of health.

Sandboxed lane

A /verify run already completed on this exact head, and its report is in this thread. Verdict merge-ready, 67/67 scripted assertions. What makes it worth more than a green suite is the control arm: it rebuilt the old denylist polarity as a mutant and ran the same 20-row probe across three arms. Head was green on all 20; the denylist arm went red on 11 rows including all five blast-radius rows of the finding I raised (scheduled-task fire, manual run-now, model-authored sub-session, send_message_to_thread, daemon-MCP prompt tool). A consumer wire oracle then fed each arm's real emitted hook bytes to the shipped auto-recall.js against a loopback provider and recorded 0 outbound searches for head versus 5 for the denylist arm. Mutation M5 — restore the fallback to request text — is killed 12/20. A 14-mutant matrix came back 11 killed / 3 survived, with every survivor having a killed control in its own file.

That is the A/B I said last run I would want before dropping the finding, so I am dropping it.

What it did not settle, and what I am therefore not claiming:

  • No single end-to-end run. Each hop was verified separately (REST route, ACP HTTP dispatch, bridge, child admission, Session emission, hook consumer), but one request was never driven REST → bridge → a real spawned child → Session → hook in a single pass. A hop-by-hop chain can hide a seam mismatch; that is the shape of the gap, not evidence one exists.
  • The multi-turn continuation claim is the author's, not re-run. "A real tool-result continuation produced three model requests with one hook invocation and one search" is macOS-only, on the author's machine, and needs a live model loop the verify container does not have. The author's nine real daemon scenarios are likewise untestable in a credential-free container. I have attributed both as claims throughout, never as evidence.
  • Not verified: Windows and Linux execution (author tested macOS only, and CI skipped both, so ubuntu is the only unit line that ran), real cron/Live dispatcher execution, channel-worker restart races, real direct ACP stdio transport, real browser UI, and PolarDB/cloud revalidation. The flakiness gate timed out at 2 of 5 rounds, though the completed rounds agreed.

@qwen-code /tmux would not close the largest of these — it drives the TUI, and this surface is the Web Shell browser. The honest ask there is a scenario in packages/web-shell/client/e2e/visuals/screenshots.spec.ts that seeds a queued submission, so the visuals check can actually see the new path instead of reporting empty. The end-to-end seam is the one thing a further @qwen-code /verify round could target explicitly (another is in flight on this head as of this run); I would not block on either.

中文说明

代码审查

我在打开 diff 之前先写了自己的方案:ACP 路径上没有带内信号能区分「人输入的」与「机器组装的」—— 定时任务、Live 派发、子会话派生、重试与 channel 投递都汇入同一个 session.prompt() —— 因此来源信息必须由真正知情的调用方声明,以外部无法伪造的可信元数据承载,且不回退到请求文本、不从显示投影推断。本 PR 做的正是这件事,包括我本来也会采用的双键拆分(理由与本仓库对 promptDisplayText 和 channel 键的既有做法相同)。没有遗漏更简的路径。

我上次请求修改的那条 Critical 已关闭,而且我是核实过的,不是采信。 Session.ts 现在只从声明中取值(typeof declaredSubmission === 'string' ? declaredSubmission : '')—— 回退到 promptText 的分支已删除,因此该字段需要正向声明才能成立,而不是靠排除清单幸存。随后我清点了旧闸门会泄漏的那一类调用方:仓库中 bridge.sendPrompt 的调用点恰好六处。两处在本次 diff 中(REST prompt 路由与 ACP HTTP dispatch,都有守卫)。另外四处 —— serve/create-sub-session.ts:932serve/live/live-task-service.ts:1081serve/live/live-session-coordinator.ts:1666serve/conversations/standalone-session-service.ts:3144 —— 我在本 head 上逐一读过。四者传入的 context 只带 {promptId, onPromptAdmitted, …},请求完全不带 _metaBridgeClientRequestContext.submittedPrompt 是新增字段,因此这四处不可能设置它;bridge 不注入,acpAgent 不重新打标,Session 不发出该字段。是由构造关闭,而非由枚举关闭 —— 而这正是让下一个派发器默认安全的性质。

我重点检查了三处最可能仍然出错的地方:

  • 两个守卫各自都承重,不是双重保险。 isUserSubmissionTurn(回合类别)与非空白声明判定(是否声明)在不同用例行上失败 —— retry 行与 channel 行带有声明却仍必须省略该字段,所以即便声明闸门完好,去掉回合类别守卫也会让它们变红。两项都不冗余。
  • 可信父进程的三元判定朝正确方向失败。 acpAgent.tsprivateParentState === 'trusted' 时读私有键,否则读公开键,因此直连 ACP 客户端无法伪造 daemon 一跳,可信父进程也无法静默回退到公开键。我独立清点了 PRIVATE_PARENT_CAPABILITY_META_KEY 的铸造点:非测试代码中恰好两处(acp-bridge/src/bridge.tschannels/base/src/AcpBridge.ts),且 channel bridge 是从零构建 _meta 而非展开调用方输入 —— 所以该不变量当前不可被违反。
  • Web Shell 的 submittedPrompt = text 默认参数是安全的。 这是原 bug 在客户端的镜像,也是我最先去查的地方。enqueuePrompt 收到的 text 是输入框原文,而排队的是准备后的文本,两者分开,所以默认值声明的是原文而非改写后的文本。手动定时任务运行走 REST scheduled-tasks 路由,该路由从不设置它 —— App.test.tsx 现在用 expect.not.objectContaining 钉住了这一点。服务端恢复的 summary-only 队列行在 useQueuedPrompts.ts:624 丢弃声明,这是对的,因为那段文本不是输入框原文。

没有构成阻塞的问题。三项非阻塞事项,都属于完备性而非正确性:

  1. 建议 —— 设计文档承诺「可信父进程缺少私有声明时不能回退到公开键」。代码实现了,但没有测试钉住它。新增的 acpAgent.test.ts 用例只覆盖了不可信方向。镜像 fixture(一个可信会话,meta 只带 qwen.submittedPrompt,断言 hook 收不到 submitted_prompt)即可关闭。这是对一个成文属性的纵深防御,不是活的漏洞 —— 清点见上。
  2. 可选 —— bridge 的 !isPromotedMidTurn 子句当前无法决定任何结果,因为 promotedMidTurn 唯一的生产方从不设置 submittedPrompt。值得保留(一旦有提升生产方携带声明它就会承重),但文档中「channel 与提升为普通回合的 mid-turn 派发省略该字段」这句的后半部分不应被当作有测试支撑。
  3. 可选 —— acpAgent 重新打标时的 channel 守卫同理属于冗余防御:任何能到达它的入口都已在存在 channel 标记时丢弃声明。

公开契约说明,不是缺陷。 qwen.submittedPrompt 是新的、外部可提供的键,且新获得资格的 ACP/daemon 载荷会携带 submitted_prompt。这是有界的而非开放式的:接受该键的每个入口本来就接受同一调用方任意提供的 params.prompt;它无法伪造私有键;标记守卫用 === undefined 判定,因此失败关闭(增加 channel、display 或 worker-auth 标记会抑制声明,而不是启用它)。PR 在正文、设计文档与 hooks.md 中都明确写出:这是调用方提供的来源信息,不是身份认证或 DLP,并提醒 hook 会拒绝未知字段的管理员。契约变更就应当这样发布。它链接的两份前序设计文档在本 head 上都存在,#compatibility-and-migration 锚点也可解析。

时序图与文件表见上方英文部分,此处不重复。内部派发器(定时任务、Live task、子会话、独立会话)在 P3 处进入,既无 context 声明也无 _meta,因此到达 P6 时只带 prompt —— 这正是旧闸门泄漏的那条路径。

测试证据

本次为无人值守 CI 运行,因此我没有构建或执行任何来自本 PR 的代码。下方证据是通过 API 读取的、本 PR 自身在被审 commit 上的 CI 结果,以及本线程中已完成的沙箱验证。

本 head 上五个 pull_request 事件的 workflow run 全部完成且全绿,无待决项。Test (ubuntu-latest, Node 22.x) 是真正执行的单元测试线 —— Test (windows-latest, …)Test (macos-latest, …)跳过Integration Tests (CLI, No Sandbox) 同样被跳过。没有红色检查,因此没有失败作业日志可摘录,也不需要做「既有基础设施噪声」的判断。CI 表格见上方英文部分(已用机器可读区域标记包裹,供 finalize 任务在 CI 落定后原地更新)。

本 head 上有两项自动化探针与该表格值得一并阅读,因为它们直接关系到我提出的发现:Serve A/B 构建了 PR base 与本 head,对两者驱动固定端点集,报告 12 个场景无响应变化 —— 与「新增一个可选启用字段、而非改变既有响应」的变更相符。Web-shell Visuals 报告没有截图变化,同时点名了三个影响渲染的文件;按该检查自身的表述,那里的空结果是一个覆盖缺口,而不是健康状况良好的证明。

沙箱验证通道

一次 /verify 已在完全相同的 head 上完成,报告就在本线程中。结论 merge-ready,67/67 条脚本断言。它比绿色测试套件更有价值的地方在于对照臂:它把旧的排除清单极性重建为变异体,并在三臂上运行同一份 20 行探针。head 在 20 行上全绿;排除清单臂在 11 行上变红,其中包含我提出的发现的全部五个爆炸半径行(定时任务触发、手动立即运行、模型撰写的子会话、send_message_to_thread、daemon-MCP prompt 工具)。随后消费端线神谕把每一臂真实发出的 hook 字节喂给已发布的 auto-recall.js,对接本地回环 provider,记录到 head 外发检索 0 次,排除清单臂 5 次。变异体 M5 —— 恢复回退到请求文本 —— 被杀 12/20。14 个变异体的矩阵结果为 11 杀 / 3 存活,且每个存活者在其自身文件中都有被杀的控制项。

这正是我上一轮说的、在撤回发现之前想要的那次 A/B,所以我撤回该发现。

没有定论的部分,也就是我不主张的部分:

  • 没有单次端到端运行。 每一跳分别验证过(REST 路由、ACP HTTP dispatch、bridge、子进程准入、Session 发射、hook 消费端),但从未在一次运行中把一个请求驱动过 REST → bridge → 真实派生的子进程 → Session → hook。逐跳链路可能掩盖接缝不匹配;这是缺口的形状,不是断言它存在。
  • 多轮续轮的结论属于作者,未重跑。「真实 read_file 续轮产生三次模型请求,仅一次 hook 调用与一次检索」仅在 macOS 上、在作者机器上完成,且需要验证容器不具备的真实模型循环。作者的九项真实 daemon 场景同样无法在无凭证容器中测试。两者我一律记为作者的结论,从不作为证据。
  • 未验证: Windows 与 Linux 执行(作者仅测 macOS,且 CI 跳过了两者,因此 ubuntu 是唯一执行的单元测试线)、真实 cron/Live 派发器执行、channel-worker 重启竞争、真实直连 ACP stdio 传输、真实浏览器 UI,以及 PolarDB/云端重新验证。抖动门在 5 轮中只完成 2 轮即超时,但已完成的轮次结论一致。

@qwen-code /tmux 无法关闭其中最大的那一项 —— 它驱动的是 TUI,而本 PR 的界面是 Web Shell 浏览器。那里真正该提的要求是:在 packages/web-shell/client/e2e/visuals/screenshots.spec.ts 中补一个播种排队提交的场景,让视觉检查能真正看到这条新路径,而不是报告空结果。端到端接缝是唯一可以被再一次 @qwen-code /verify 明确瞄准的项(本轮运行时已有一次在同 head 上进行中);两者我都不作为阻塞条件。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 46dc4382ed1a08b7509426d8ca429d681aec83a6 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — the finding I blocked on is fixed, and fixed measurably; what keeps this off 5/5 is verification completeness, not doubt about the code.

Going back to the proposal I wrote before reading the diff: it matched this PR on every load-bearing choice — declare positively, carry it as trusted metadata, never fall back to request text, never infer from a display projection. I did not find a simpler path it missed, and the two-key split I would have designed is the one the codebase already uses for promptDisplayText and the channel key, so it adds convention rather than machinery. 165 production lines against 504 test lines is the ratio I want to see on a trust-boundary change.

What moved me from 2/5 last run is not that the author argued with the finding — it is that the fix inverted the polarity instead of extending the denylist. Last run I wrote that a denylist works only for the machine-driven producers someone remembered to enumerate, and that the class arriving with no _meta at all was missing from the list. There is no list now. Provenance requires a positive declaration, so the four internal dispatchers I enumerated this run are excluded by not participating rather than by being named. That is the difference between a fix and a patch, and it is why I am comfortable approving without having executed anything myself.

The evidence bar moved with it. Last run I was explicit that my finding was read-verified rather than executed, and that I would drop it if a /verify A/B came back clean. It came back clean on this exact head — with a control arm rebuilt to the old polarity so the green cells actually mean something, and a wire oracle counting real outbound searches against a loopback provider. I am dropping it, plainly, as promised. I also re-derived the two censuses that carry the most weight rather than accepting them: all six bridge.sendPrompt call sites, and the two mint sites of the private-parent capability key. Both held.

Reservations, all non-blocking and all named in Stage 2: the trusted-parent invariant the design doc promises is unpinned by a test; two guards are currently unreachable and their doc sentence should not be read as test-backed; nothing drove one request through every hop in a single run; and the multi-turn continuation claim plus the nine real daemon scenarios are the author's, on macOS, which I have attributed as claims throughout rather than counting as evidence. Windows and Linux never ran — CI skipped both, so ubuntu is the only unit line. None of that is a reason to hold the PR; it is a list of what nobody has observed yet.

The one thing I cannot settle from the diff, tests and description is not technical. This PR widens a documented public contract: a new externally-suppliable meta key, and submitted_prompt becoming reachable on ACP-path hook invocations. Last run I put that to @wenshao as a product question; he engaged the autofix loop without ruling on it. The rework answers it conservatively — third-party clients opt in per request, existing clients see no change, no deployment contract expands — and the author is himself a maintainer, so I do not think it needs my veto. But main wants two approvals, and this deserves a human yes on the contract specifically, not only on the code. So I am approving on the code, which I am confident in, and leaving the contract sign-off where it belongs.

For the record on the review state: my earlier CHANGES_REQUESTED at 2b4f46a cited a finding that is now fixed, so this approval supersedes it — it is not a second opinion coexisting with the first.

@doudouOUC — the ask from last run is met, and met in the stronger of the two ways it could have been. You inverted the gate instead of adding rows to it, and you kept the failing assertion as the acceptance criterion rather than rewriting it to pass. The PR body also discloses its own limits more carefully than the review asked for — the macOS-only coverage, the deferred design docs, and the historical Holo evidence explicitly marked as predating the correction. The three non-blocking items in Stage 2 are yours to take or leave; item 1 is the only one I would bother with, and it is a short mirror of a fixture you already wrote.

@wenshao — this is the bot's vote, not the contract's. If you are fine with ACP/daemon clients opting into submitted_prompt per request, a second approval lands it. If you would rather it stayed first-party-only, the place to say so is the opt-in paragraph in hooks.md, and narrowing it later is a small change — the declaration is already per-request, so removing the public ingress would not disturb Web Shell.

CI on this head is complete and green with nothing pending, so this run's approval is pinned to 46dc4382ed1a08b7509426d8ca429d681aec83a6 rather than deferred.

中文说明

Confidence: 4/5 —— 我此前阻塞的那条发现已修复,而且是可度量地修复;让它没到 5/5 的是验证的完备性,不是对代码本身的怀疑。

回到我在读 diff 之前写下的方案:它在每一个承重的选择上都与本 PR 一致 —— 正向声明、以可信元数据承载、绝不回退到请求文本、绝不从显示投影推断。我没有找到它遗漏的更简路径,而我本来会设计的双键拆分,正是本仓库对 promptDisplayText 与 channel 键已在使用的方案,所以它增加的是既有惯例而非新机制。在一处信任边界变更上,165 行生产代码对 504 行测试,是我希望看到的比例。

让我从上一轮 2/5 改变判断的,不是作者与该发现争辩,而是这次修复反转了极性,而不是延长排除清单。上一轮我写过:排除清单只对有人记得枚举出来的机器驱动生产方有效,而那一类完全不带 _meta 到达的回合不在清单里。现在没有清单了。来源信息需要正向声明,因此我这轮清点出的四个内部派发器是因为不参与而被排除,不是因为被点名。这是修复与打补丁的区别,也是我在自己没有执行任何代码的情况下仍愿意批准的原因。

证据的门槛也随之提高。上一轮我明确说过,我的发现是走查核实而非实跑核实的,如果一次 /verify A/B 回来是干净的,我就会撤回它。它在这个完全相同的 head 上回来是干净的 —— 并且带有重建为旧极性的对照臂,因此那些绿色单元格是有意义的;还有一个线神谕,对本地回环 provider 统计真实的外发检索次数。我撤回该发现,明确地,如约。权重最大的两次清点我也自己重新推导过,而不是采信:全部六处 bridge.sendPrompt 调用点,以及私有父进程能力键的两处铸造点。两者都成立。

保留意见,均为非阻塞且都已在 Stage 2 中点名:设计文档承诺的可信父进程不变量没有测试钉住;两个守卫当前不可达,其文档句子不应被当作有测试支撑;没有任何一次运行把单个请求驱动过所有跳;以及多轮续轮结论与九项真实 daemon 场景属于作者、在 macOS 上完成 —— 我一律记为作者的结论,而非证据。Windows 与 Linux 从未运行 —— CI 跳过了两者,因此 ubuntu 是唯一的单元测试线。这些都不是扣住 PR 的理由;它们是一张「尚无人观测过什么」的清单。

我无法从 diff、测试与描述中定论的那一件事不是技术性的。本 PR 拓宽了一份成文公开契约:一个外部可提供的新元数据键,以及 submitted_prompt 在 ACP 路径的 hook 调用上变得可达。上一轮我把这个问题作为产品问题交给 @wenshao;他启动了 autofix 循环,但没有对此作出裁定。这次重写以保守方式回答了它 —— 第三方客户端逐请求显式启用,既有客户端看不到任何变化,没有部署契约被扩大 —— 而作者本人就是维护者,所以我不认为它需要我的否决。但 main 需要两个批准,而这件事值得一个人类针对契约本身点头,而不只是针对代码。所以我基于代码批准(这部分我有信心),把契约签署留在它该在的地方。

关于评审状态的记录:我早先在 2b4f46a 上的 CHANGES_REQUESTED 所引用的发现现已修复,因此本次批准取代它 —— 不是与它并存的第二种意见。

@doudouOUC —— 上一轮的要求已经达成,而且是以两种可能方式中更强的那一种达成的。你反转了闸门,而不是给它增加用例行;你把那条失败的断言保留为验收标准,而不是改写它让它通过。PR 正文对自身局限的披露也比评审所要求的更仔细 —— 仅 macOS 的覆盖、延期的设计文档,以及明确标注为早于本次修正的历史 Holo 证据。Stage 2 中三项非阻塞事项由你决定取舍;只有第 1 项我会去动,而它只是你已写好的一个 fixture 的简短镜像。

@wenshao —— 这是 bot 的一票,不是契约的一票。如果你认可 ACP/daemon 客户端逐请求启用 submitted_prompt,第二个批准就能让它落地。如果你更希望它仅限第一方,该表态的位置是 hooks.md 中的显式启用段落,而且日后收窄是个小改动 —— 声明本来就是逐请求的,因此移除公开入口不会扰动 Web Shell。

本 head 上的 CI 已完整且全绿、无待决项,所以本轮的批准钉在 46dc4382ed1a08b7509426d8ca429d681aec83a6 上,而非延迟生效。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at 46dc4382ed1a08b7509426d8ca429d681aec83a6 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not explored to full depth (tool budget reached): "agent 3c": I did not confirm whether a pre-existing test elsewhere in Session.test.ts pins that the hook does not fire at all on continue / restoreAskUserQuestion /runt…; "agent reverse-audit (round 1)": locating the daemon serve app's JSON body limit for the prompt route — only express.raw({ type: '*/*', limit: '8mb' }) on POST /session/:id/attachments ( se….

中文说明

仅完成部分审查,审查缺口已披露。

未审查(原文为英文):build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

未探索到全部深度(达到工具调用预算):"agent 3c"I did not confirm whether a pre-existing test elsewhere in Session.test.ts pins that the hook does not fire at all on continue / restoreAskUserQuestion /runt…"agent reverse-audit (round 1)"locating the daemon serve app's JSON body limit for the prompt route — only express.raw({ type: '*/*', limit: '8mb' }) on POST /session/:id/attachments ( se…

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

Comment thread packages/cli/src/acp-integration/session/Session.ts Outdated
Comment thread packages/cli/src/acp-integration/session/Session.ts Outdated
Comment thread docs/design/daemon-user-prompt-submit-provenance.md Outdated

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

Partially reviewed — gaps disclosed.

2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • R1-2 ACP producer contract assertions left unreconciled — already reported (comment 3967113105)
  • R1-7 acceptance bullet claims continuation coverage no test provides — already reported (comment 3967113122)

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not explored to full depth (tool budget reached): "agent 1c": did not confirm a shipped producer that sets qwen.daemon.promptDisplayText to a bare media placeholder for an inbound media-only message — the [image] / [aud…; "agent 1c": did not confirm end-to-end admission of a channel-wrapped prompt when channel-worker authorization or the sourceType === 'channel' check fails (the preconditi…; "agent 5": did not verify whether any in-repo channel adapter actually composes hidden context into text while leaving displayText unset — finding 3's trigger rests on…; "agent 5": mutation verdicts above are reading-based; I executed only the unmutated run (8 passed / 861 skipped) and made no edit in this shared worktree.; "agent 6c": did not execute packages/cli/src/acp-integration/session/Session.test.ts to confirm the new it.each block passes at HEAD — findings above rest on reading th….

Not reviewed: reverse audit — stopped before round 3 by the review time budget.

中文说明

仅完成部分审查,审查缺口已披露。

本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

未审查(原文为英文):build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

未探索到全部深度(达到工具调用预算):"agent 1c"did not confirm a shipped producer that sets qwen.daemon.promptDisplayText to a bare media placeholder for an inbound media-only message — the [image] / [aud…"agent 1c"did not confirm end-to-end admission of a channel-wrapped prompt when channel-worker authorization or the sourceType === 'channel' check fails (the preconditi…"agent 5"did not verify whether any in-repo channel adapter actually composes hidden context into text while leaving displayText unset — finding 3's trigger rests on…"agent 5"mutation verdicts above are reading-based; I executed only the unmutated run (8 passed / 861 skipped) and made no edit in this shared worktree."agent 6c"did not execute packages/cli/src/acp-integration/session/Session.test.ts to confirm the new it.each block passes at HEAD — findings above rest on reading th…

未审查:反向审计——评审时间预算不足,未能开始第 3 轮。

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

Comment thread packages/cli/src/acp-integration/session/Session.ts Outdated
Comment thread docs/design/daemon-user-prompt-submit-provenance.md Outdated
@wenshao

wenshao commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Sep 9, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the autofix/takeover label (or comment @qwen-code /takeover stop) to release.

中文说明

🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 autofix/takeover 标签(或评论 @qwen-code /takeover stop)即可释放。

@qwen-code-dev-bot

qwen-code-dev-bot commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

⚠️ AutoFix round 2 ended without publishing a reportview run.

中文说明

⚠️ AutoFix 第 2 轮结束但未发布报告 —— 查看运行

Channel turns are machine-relayed deliveries (loop jobs, webhook tasks,
adapter-synthesized events), so nothing on them crossed a submission
boundary. Since submitted_prompt presence alone gates Auto Recall's
outbound provider search, emitting it there ran retrieval on unattended
turns and could publish composed wrapper text as user provenance. Gate
emission on a positive user-submission condition instead.

Also reconcile the producer-set contract this PR changed: hooks.md, the
Auto Recall READMEs, the in-product hook description with its locale
mirrors, and the recording payload comment now state the actual rule,
and the new design doc records the channel exclusion plus the
strict-decoder rollout warning.
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈(第 1/100 轮)。改动内容与我反驳保留之处如下:

Address-review round — PR #11455

Feedback dispositions

1. R1-1 Critical — submitted_prompt emitted on channel-classified machine turns (rc:3967113092, rc:3967295210) — Fixed

Reproduced first. Added the two rows the finding asked for to the preserves submission provenance table — a channel turn with qwen.daemon.promptDisplayText: 'Issue assigned: broken build' and a channel turn without a display projection, both carrying _meta: { [CHANNEL_PROMPT_META_KEY]: true }. Pre-fix run: exactly those 2 rows failed (8 existing rows passed); the hook input carried submitted_prompt: "composed channel wrapper" — confirming both the projection path and the wrapper-text fallback leak.

Fix. Session.ts now gates emission on a positive user-submission condition: isUserSubmissionTurn = isFreshUserTurn && !channelTurn, beside the existing turn classification. isFreshUserTurn itself is untouched, so managed memory recall at the same site is unaffected, and non-channel daemon/serve/ACP turns (the incident this PR fixes) keep the field — the 'trusted display projection' row still asserts submitted_prompt: 'original question'. The narrower promptDisplayText ?? (channelTurn ? '' : promptText) variant was not added: with the emission gate, the fallback value is unobservable on channel turns, so the extra ternary would be dead defense.

Mutation probe. Temporarily removed !channelTurn from the gate and re-ran the focused tests: exactly the 2 new channel rows failed. Restored; all 10 rows green. The gate is witnessed.

Docs. The channel rule is recorded in both language versions of the new design doc (Design section), naming the label producers (loop jobs, webhook tasks, adapter-synthesized events) and stating honestly that human channel messages share the transport marker and are excluded with the class in this version, rather than distinguished by a display-projection heuristic.

2. R1-2 Suggestion — producer-set contract drift (rc:3967113105) — Partially fixed; one slice deferred

Fixed the statements that are wrong after this PR:

  • docs/users/features/hooks.md: the field description ("supported submission boundary", not TUI-only), the producer paragraph (positive rule: interactive TUI submission, first-turn headless UserQuery, fresh non-channel ACP session user turn; explicit channel-turn exclusion; the stale "ACP, headless, serve, SDK, and remote-input paths do not produce it" sentence removed), and the record-displayText clause (dropped the stale non-producer enumeration; behavior unchanged by this PR).
  • integrations/external-context/README.md and integrations/external-context-mem0/README.md: the Auto Recall producer list now includes the ACP session path (daemon and serve hosts).
  • packages/cli/src/ui/components/hooks/constants.ts + all 9 locale mirrors (en, zh, zh-TW, ja, fr, de, pt, ru, ca): the in-product hook description key reworded and translated together, so no orphaned locale entries. constants.test.ts:238 pinned the old wording ('interactive TUI'); updated to 'submission boundary' — content evidence: the pinned string was the defect this finding reports.
  • packages/core/src/services/chatRecordingService.ts: the UserPromptRecordPayload.displayText comment now covers headless and the trusted ACP display projection.

Deferred (see deferred-findings.json): amending docs/design/submitted-prompt-provenance.md (mermaid node, eligibility row) and docs/design/direct-external-context-auto-recall.md. Both are English-only; editing either triggers docs/design/README.md's pairing rule, i.e. two full new zh-CN translations (~400 lines) of pre-existing design docs whose staleness is half pre-existing (headless/SDK) and whose remaining statements are historically scoped to their own change ("No producer is added in this change"). The current ACP producer rule is recorded in this PR's new design doc (both languages), which cross-links the predecessor doc. The thread is left open with a reply explaining the split.

3. R1-7 Suggestion — acceptance bullet overclaims continuation coverage (rc:3967113122) — Fixed

Applied the suggested remedy: the bullet no longer claims continuation coverage. It now reads "…blank input, retry, and channel-turn exclusion" in both language versions — accurate against the updated it.each table, which gained the two channel rows this round.

4. R2-1 Suggestion — missing strict-decoder rollout warning (rc:3967295223) — Fixed

Added the supplied guidance to "Consumers and compatibility" in both docs/design/daemon-user-prompt-submit-provenance.md and its zh-CN twin: ACP/daemon/serve UserPromptSubmit payloads now carry submitted_prompt; administrators whose hooks reject unknown fields (e.g. additionalProperties: false) must re-test before rollout because strict-decoder failure changes fail-open vs fail-closed; cross-references hooks.md and the predecessor design's Compatibility section. Identifiers are preserved verbatim in the Chinese version.

Conflict notes

--conflict false; no merge performed.

Verification

  • vitest Session.test.ts -t "preserves submission provenance" pre-fix — 2 failed / 8 passed (the two new channel rows; reproduction).
  • Same command post-fix — 10 passed.
  • Mutation probe (!channelTurn removed) — 2 failed / 8 passed (both new rows red); restored and re-ran green.
  • npm run build — passed.
  • vitest packages/cli src/acp-integration/session/Session.test.ts (full file) — 871 passed.
  • vitest packages/cli src/i18n/89 passed (locale key consistency holds).
  • vitest packages/core src/services/chatRecordingService.test.ts125 passed.
  • vitest packages/cli src/ui/components/hooks/149 passed (after updating the stale wording assertion; 1 failed before that update).
  • npm run typecheck — passed.
  • npm run lint — passed.
  • Pre-commit hook (lint-staged) — passed on commit.

Not run: integration/E2E suites — the changed behavior is exercised directly by the ACP session unit tests, and the finding's acceptance criteria were the table rows plus the mutation probe.

中文说明

评审处理轮次 —— PR #11455

反馈处理结果

1. R1-1 Critical —— channel 分类的机器回合携带了 submitted_prompt(rc:3967113092、rc:3967295210)—— 已修复

先复现。 按发现要求向 preserves submission provenance 表格新增两行:一行是带 qwen.daemon.promptDisplayText: 'Issue assigned: broken build' 的 channel 回合,一行是不带显示投影的 channel 回合,两者都携带 _meta: { [CHANNEL_PROMPT_META_KEY]: true }。修复前运行:恰好这两个新用例失败(8 个既有用例通过);hook 输入携带了 submitted_prompt: "composed channel wrapper" —— 同时证实了投影路径与包装文本回退两条泄漏路径。

修复。 Session.ts 现在以正向的用户提交条件判定发射:在既有回合分类旁新增 isUserSubmissionTurn = isFreshUserTurn && !channelTurnisFreshUserTurn 本身未改动,因此同一位置的托管记忆召回不受影响;非 channel 的 daemon/serve/ACP 回合(即本 PR 要修复的事故)仍保留该字段 —— 'trusted display projection' 用例仍断言 submitted_prompt: 'original question'。未采用更窄的 promptDisplayText ?? (channelTurn ? '' : promptText) 写法:有了发射门之后,该回退值在 channel 回合上不可观测,额外的三元表达式属于无效防御。

变异探针。 临时从判定中移除 !channelTurn 并重跑定向测试:恰好两个新增 channel 用例失败。恢复后 10 行全绿。该判定有测试见证。

文档。 channel 规则已记入新设计文档的两个语言版本(设计章节),点名了标签型生产者(定时任务、webhook 任务、适配器合成事件),并如实说明:人类 channel 消息使用相同的传输标记,本版本中与该类一并排除,而不是依靠显示投影启发式区分。

2. R1-2 Suggestion —— 生产者集合契约漂移(rc:3967113105)—— 部分修复;一部分延期

修复了本 PR 之后确实变错的表述:

  • docs/users/features/hooks.md:字段描述(“受支持的提交边界”,不再仅限 TUI)、生产者段落(正向规则:交互式 TUI 提交、首回合 headless UserQuery、ACP 会话路径上的全新非 channel 用户回合;明确排除 channel 回合;删除已过时的“ACP、headless、serve、SDK 和 remote-input 路径不产生该字段”一句),以及记录侧 displayText 子句(删除过时的非生产者枚举;该行为未被本 PR 改变)。
  • integrations/external-context/README.mdintegrations/external-context-mem0/README.md:Auto Recall 生产者清单现在包含 ACP 会话路径(daemon 与 serve 宿主)。
  • packages/cli/src/ui/components/hooks/constants.ts 及全部 9 个语言镜像(en、zh、zh-TW、ja、fr、de、pt、ru、ca):产品内 hook 描述的键已改写并同步翻译,不存在孤立的语言条目。constants.test.ts:238 钉住的是旧措辞('interactive TUI'),已更新为 'submission boundary' —— 内容证据:被钉住的字符串正是该发现所报告的缺陷。
  • packages/core/src/services/chatRecordingService.tsUserPromptRecordPayload.displayText 注释现在覆盖 headless 与可信 ACP 显示投影。

延期(见 deferred-findings.json):修订 docs/design/submitted-prompt-provenance.md(mermaid 节点、资格表行)与 docs/design/direct-external-context-auto-recall.md。两者均只有英文版;按 docs/design/README.md 的配对要求,编辑其中任何一个都需要补齐完整的 zh-CN 译文(约 400 行),而这两份既有设计文档的过时之处有一半在本 PR 之前就已存在(headless/SDK 部分),其余表述又是针对其自身变更的历史范围限定(“本次不新增生产方”)。当前 ACP 生产者规则已记入本 PR 的新设计文档(双语),并交叉链接了前序文档。该评审线程保持开放,并已回复说明拆分原因。

3. R1-7 Suggestion —— 验收条目夸大了续跑覆盖(rc:3967113122)—— 已修复

采纳了建议的修法:该条目不再声称覆盖续跑行为。两个语言版本现在均为“……空白输入、重试以及 channel 回合排除”,与本轮新增两个 channel 用例后的 it.each 表格一致。

4. R2-1 Suggestion —— 缺少严格解码器上线前警告(rc:3967295223)—— 已修复

已在 docs/design/daemon-user-prompt-submit-provenance.md 及其中文版的“消费方与兼容性”章节加入所建议的说明:ACP、daemon 与 serve 宿主的 UserPromptSubmit payload 现在会携带 submitted_prompt;部署了会拒绝未知字段的 hook(例如 additionalProperties: false)的管理员必须在上线前重新测试,因为严格解码器失败会改变失败放行与失败关闭的走向;并交叉引用 hooks.md 与前序设计的 Compatibility 章节。中文版本中原样保留了各技术标识符。

冲突说明

--conflict false;未执行合并。

验证

  • vitest Session.test.ts -t "preserves submission provenance"(修复前)—— 2 失败 / 8 通过(两个新增 channel 用例;复现)。
  • 同一命令(修复后)—— 10 通过
  • 变异探针(移除 !channelTurn)—— 2 失败 / 8 通过(两个新用例变红);恢复后重跑为绿。
  • npm run build —— 通过。
  • vitest packages/cli src/acp-integration/session/Session.test.ts(全文件)—— 871 通过
  • vitest packages/cli src/i18n/ —— 89 通过(语言键一致性保持)。
  • vitest packages/core src/services/chatRecordingService.test.ts —— 125 通过
  • vitest packages/cli src/ui/components/hooks/ —— 149 通过(更新过时措辞断言后;更新前 1 失败)。
  • npm run typecheck —— 通过。
  • npm run lint —— 通过。
  • 提交时的 pre-commit 钩子(lint-staged)—— 通过。

未运行:集成/E2E 套件 —— 变更行为由 ACP 会话单元测试直接覆盖,且该发现的验收标准就是表格用例加变异探针。

🧭 Gate advisory — this round modified areas outside the PR footprint (machine-measured, not agent-authored):

  • integrations/external-context
  • integrations/external-context-mem0
  • packages/core
    Review the expansion deliberately; the footprint gate is in advisory mode. · 本轮改动了 PR 足迹之外的区域(门自动测量,非 agent 文本),当前足迹门为 advisory 模式,请有意识地审阅该扩张。

Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。

🧵 Resolved all 4 selected review thread(s). · 已关闭全部选中的 4 条评审线程。

Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审;第 100 轮后本 bot 停止并将 PR 交给人工。


🧠 Handled by Qwen Code · model/模型 kimi-k3 · CLI 0.23.2

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 55 passed · 0 failed · 55 total

Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:55 通过 · 0 失败 · 55 总计

抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence

Verification report

verify-pr 11455 — fix(acp): Preserve submitted prompt provenance for auto recall

Verdict: findings — 55/55 scripted assertions passed, 0 unexpected failures; one load-bearing scope finding against the PR's own stated invariant. Verified head: 2b4f46aca55bd4c9eb04d478dd0d064beffddc4a (git rev-parse HEAD^2). First round (no previous-report.md in the context directory).

中文摘要
  • 结论: findings。中心主张已用 A/B 证明是承重的:基线(HEAD^1Session.ts 原始 blob)在 14 个判别单元中 7 个不发射 submitted_prompt,PR head 全部发射;消费端线神谕(real hook 子进程 + 真实 loopback socket)的外发检索次数从 0/4 变为 3/4(channel 回合两臂均为 0,即 PR 的 channel 闸门有效)。55 条脚本断言全部通过,0 条意外失败。
  • 主要 finding(F1,Critical,new-surface):提交说明称"改为按正向用户提交条件闸门",但实现是对五种机器回合标记的否定清单,默认值为"params.prompt 里的文本即来源"。因此跨会话机器派发(无 _meta 标记)仍被判定为用户提交:实测 per_run 定时任务触发(完全无人值守、文本为机器拼装 wrapper,含内部指令句 "This is a scheduled task run. Execute the instructions below now.")、手动 run-now、create_sub_session / send_message_to_thread / daemon-MCP prompt(模型撰写文本)都会发射 submitted_prompt,并触发 Auto Recall 的真实外发检索、且检索结果以 additionalContext 回流到模型输入。这与提交说明自述的危害("在未值守回合上跑检索、把拼装 wrapper 文本当作提交来源发布")完全同形,也与新版 hooks.md"cron / teammate 流量省略该字段"的句子矛盾。边界:非安全边界(设计文档自述),需管理员显式启用 Auto Recall,非数据丢失;基线不存在该路径(new-surface)。
  • 其余 finding:F2 hooks.md 新句子对 cron/per_run 与 teammate 类比路径为假(同根因,文档侧);F3 实时语音委派被当作用户提交(PR 自己的测试钉为有意,仅提示);F4 hooks.md 录制段落"otherwise the expanded pre-hook prompt"对 ACP 不精确(观察级)。
  • 未覆盖:真实 daemon/Holo 验收、多回合"工具续接不重复召回"、Windows/Linux、per-commit 归因(浅克隆仅 3 个提交)、delivery 无 channel 标记的兄弟形状(仅读码)。

Central claim and A/B

Central claim. A fresh ordinary ACP user submission supplies submitted_prompt to UserPromptSubmit hooks (so an explicitly configured Auto Recall hook fires), taken from the trusted display projection when present else the request's pre-expansion text blocks; retries, channel-classified turns, blank text and resource-only submissions omit it.

Control construction. The only behaviorally changed production file in the emission path is packages/cli/src/acp-integration/session/Session.ts (constants.ts is a description string, chatRecordingService.ts a comment). The base arm therefore swaps that one file for its exact HEAD^1 blob — diff <(git show HEAD^1:<path>) <path> empty, sha256 51fb91f6… vs head 9d4e2720…, both quoted inside the captures — and runs the identical probe test file. Everything else (test file, node_modules, config, all other sources) is byte-identical between arms, so no internal-workspace-link confound exists; readlink -f node_modules/@qwen-code/qwen-code-corepackages/core (unchanged by this PR) recorded for the record.

cell (probe row) oracle base (Session.ts = HEAD^1 blob) head (2b4f46a)
text blocks without resource bodies hook input submitted_prompt absent (fail) ' check this\n'
trusted display projection hook input absent (fail) 'original question'
empty display projection hook input absent (pass) absent (pass)
model-only delegation hook input absent (fail) 'original question'
blank text / resource-only hook input absent (pass) absent (pass)
legacy retry / daemon retry hook input absent (pass) absent (pass)
channel turn ± display projection hook input absent (pass) absent (pass)
CONTROL ordinary user question hook input absent (fail) 'why does the nightly build fail?'
GAP per_run scheduled-task fire hook input absent (fail) composed wrapper emitted
GAP manual run-now hook input absent (fail) composed wrapper emitted
GAP model-authored sub-session hook input absent (fail) model text emitted

7 of 14 discriminating cells flip absent→present; the 7 omission cells hold on both arms. Witness: evidence/01-producer-ab-base-arm-no-submitted-prompt.png (7 red) and evidence/02-producer-ab-head-arm-submitted-prompt-emitted.png (15 green, sha256 of the file under test in frame).

Consumer-side A/B (wire oracle). The real emitted bytes were then fed to the real shipped hook bundle integrations/external-context/dist/auto-recall.js, spawned as a child process against a real loopback HTTP provider that records every request:

scenario base outbound POST /v1/context/search head
ordinary user question 0 1, query verbatim
per_run scheduled-task fire 0 1, query = composed wrapper incl. internal instruction
model-authored sub-session 0 1, query = model text
channel turn 0 0

Witness: evidence/03-consumer-oracle-head-unattended-search-fires.png and evidence/04-consumer-oracle-base-zero-searches.png. On head the returned provider item also came back as hook additionalContext ({"untrusted_external_context":…VERIFY-MEMORY…}), i.e. the retrieved memory reaches the turn's model input — including on the unattended timer fire.

Findings

F1 — Critical (new-surface): the gate is a denylist where the commit promises a positive condition, so unattended and model-authored cross-session dispatches publish provenance and fire retrieval

The second commit states: "Gate emission on a positive user-submission condition instead." The implementation (Session.ts:5616-5643) is the opposite polarity — isUserSubmissionTurn = !isRetry && !isContinue && !isRestoreAskUserQuestion && goalTurn?.origin !== 'runtime' && !channelTurn over a default of submittedPrompt = promptDisplayText ?? promptText. Every machine prompt that reaches Session.prompt() without one of those five markers inherits provenance. The core and headless producers use the positive form (client.ts:3199-3222 allowlists SendMessageType.UserQuery + caller-supplied text; nonInteractiveCli.ts:2474-2478 requires isFirstTurn && UserQuery), so ACP is now the only producer that defaults to "present".

Measured end-to-end (harness consumer-wire-oracle.mjs, real child process + real socket):

outbound query: "Scheduled task: Nightly triage sweep Task ID: task-verify-1 Schedule: 7 9 * * *
Triggered at: 2026-09-10T09:07:00.000Z Trigger: scheduled Session: new chat for this run
This is a scheduled task run. Execute the instructions below now. Do not create or modify a
schedule unless the instructions explicitly ask you to. Check the nightly build and summarise failures."

Reproducing commands:

# producer: real Session code, no classifying marker, wrapper built by the real producer
cd packages/cli && npx vitest run src/acp-integration/session/Session.verifyprobe.test.ts \
  -t "provenance"            # artifact copy: tmp/pr11455-verify-*/Session.verifyprobe.test.ts
# consumer: real shipped hook bundle over a real loopback socket
node tmp/pr11455-verify-*/consumer-wire-oracle.mjs tmp/pr11455-verify-*/logs/probe-head.json head

Blast radius (all dispatch through AcpSessionBridge.sendPrompt with no marker; all classified isUserSubmissionTurn === true):

  1. Session.ts:8800-8835serve/create-sub-session.ts:932-940per_run scheduled-task fire: timer-driven, fully unattended, composed buildScheduledTaskRunPrompt wrapper. Highest severity; matches the commit message's own failure description.
  2. serve/routes/scheduled-tasks.ts:566-586 — manual "run now", same composed wrapper (human clicked, text still machine-composed).
  3. Session.ts:3320-3340create-sub-session.ts:869/932, serve/conversations/standalone-session-service.ts:3144-3157create_sub_session tool: model-authored prompt into a fresh child.
  4. serve/live/live-task-service.ts:982-1090send_message_to_thread / create_thread: the daemon analogue of teammate send_message, which core explicitly excludes via SendMessageType.Teammate (client.ts:3208); ACP has no equivalent.
  5. sdk-typescript/src/daemon-mcp/serve-bridge/tools/agent.ts:52-54 — daemon-MCP prompt tool: a model in one agent prompts a daemon session.

What this is NOT (bounded, per the round's own measurements): not a security or DLP boundary — the design doc states the field "is not an authentication or DLP boundary"; not data loss; requires an administrator to have explicitly registered Auto Recall with credentials and repository binding; the wrapper text is admin-authored task text plus one fixed instruction sentence, and the sanitizer still redacts credential shapes; at base no ACP turn carried the field at all, so this is new surface created by this PR, not a regression of existing behaviour. The in-session cron/loop/notification drains (#executeCronPromptInner, #executeBackgroundNotificationPromptInner) never fire the hook and remain safe — the leak is only the cross-session dispatch.

Suggested direction (not applied, not measured this round — a policy decision for the author). Either honour the commit's stated polarity by requiring a positive trusted marker on the ACP path, or close the unattended class with the metadata that already exists: config.getSessionSourceType()/getSessionSourceId() (config.ts:4476/4480) plus isScheduledTaskRunSource() (acp-bridge/session-source.ts:30) identify the per_run child exactly. I did not ship a patch because the design doc's "an ACP text block is the host's submitted text, not proof of human authorship" sentence may mean the author deliberately accepts machine text on these paths — in which case the commit message and hooks.md (F2) are what need amending, not the code. The fixture that would pin whichever choice is made is a probe row asserting absence for a session whose source is scheduled_task_run:*; today no test distinguishes that session from an ordinary one.

F2 — Suggestion: the new hooks.md sentence is falsified by the measured behaviour for "cron" and "teammate traffic"

docs/users/features/hooks.md (this PR) now says the field "is omitted for unsupported producers and machine-driven paths such as same-turn steering, tool-result continuations, retries, cron, notifications, teammate traffic, and channel-classified turns". Measured: a per_run scheduled task (user-facing "cron", created by cron_create / the Web Shell scheduled-tasks UI) fires into a fresh child session that does emit the field and does perform an outbound search (F1 table); send_message_to_thread is the daemon teammate analogue and likewise emits it. The sentence is true only for the in-session cron drain. Same root cause as F1; it makes the doc a false coverage claim that the next reader will trust. Fix is one sentence once F1's policy is settled.

F3 — Nice to have (awareness): realtime voice delegation is a model-relayed delivery treated as a user submission

serve/live/live-session-coordinator.ts:1336-1341/1666-1676 dispatches the voice model's background_agent function-call argument (qwen-realtime-session.ts:1230) as params.prompt. It carries no marker, so it emits submitted_prompt = the relayed request. The PR's own test (Session.test.ts "model-only delegation excluded", expecting submitted: 'original question') pins this as intended, and it is defensible (a human spoke; the model relayed their words). Flagged only because it is the one adapter-synthesized delivery inside the "user submission" class, and because the sibling delivery-without-qwen.channel.prompt shape (Session.ts:4889-4900 deliberately does not classify on it) was read but not driven this round.

F4 — Nice to have (doc reading, not measured as a defect): the recording paragraph's fallback clause is imprecise for ACP

hooks.md says paths without provenance "record the expanded pre-hook prompt instead". On ACP, recordUserMessage receives displayText: promptDisplayText ?? promptText only when a projection or attachment refs exist (Session.ts:5456-5465); without either, no systemPayload is written at all, and promptText is pre-expansion, not "expanded pre-hook". Pre-existing phrasing the PR's rewrite inherited; no behavioural claim of this round depends on it.

Reviewer Test Plan, step by step

plan step result
fresh ordinary ACP submission supplies its original text verified — CONTROL row + consumer oracle (query verbatim)
resource bodies excluded verified — row text blocks without resource bodies (PRIVATE RESOURCE absent from submitted_prompt)
model-only delegation excluded verified — wrapper absent, original text emitted
trusted display text takes precedence, including empty verified — rows trusted display projection / empty display projection without internal fallback
retries must not carry submitted_prompt verified — rows legacy retry, daemon retry
configured hook context reaches the actual model input partially verified — hook input reaches a real subprocess and a returned item comes back as additionalContext; the wrap-into-parts step is pre-existing Session code not re-proven here
tool continuation does not repeat automatic recall not verified — needs a multi-turn model loop (see Not covered)
"How to verify" daemon + workspace A/B + Holo scenarios not verifiable in this container — no credentials/network; the author's E2E comment is the only evidence

Not covered

  • Real daemon end-to-end (workspace A/B binding, Holo/Mem0 records, deletion-then-fresh-session): requires provider credentials and network; this container has neither. The consumer oracle substitutes a loopback provider and says so.
  • Multi-turn behaviour: "tool continuation runs the hook once", Stop-hook re-entry, mid-turn steering — read in code (Session.ts:6317-6329, bridge enqueueMidTurnMessage) but not driven.
  • delivery-carrying prompts without qwen.channel.prompt (F3 sibling): read only.
  • Per-commit attribution: the checkout is depth-2 (git rev-parse --is-shallow-repository = true); commit 4fb7d2f0 from the snapshot's commits array is unreachable, so only the aggregate HEAD^1..HEAD diff was exercised. The snapshot's baseRefOid (1919ff97…) is also unreachable; the merge ref's HEAD^1 (33a40625…) was used as base throughout.
  • Trial merge into current main: not performed (base drift not measured beyond the OID mismatch above).
  • Windows/Linux execution paths (author-declared untested).
  • The producer probe drives Session.prompt() with a recording message-bus seam and a stubbed model stream — the unit under test (hook-input construction) is real compiled-from-source code, but the ACP transport above prompt() is not exercised by it; the consumer oracle covers the seam below it with real processes and sockets.

Methodology

Environment: the CI verify container (node v22.23.2), merge-ref checkout at depth 2, npm ci + npm run build pre-run at HEAD. Producer arm: the PR's own Session.test.ts provenance table copied to a scratch probe file with four added rows (one control, three machine-dispatch shapes whose text is built by the real buildScheduledTaskRunPrompt), run under vitest against head source and against the exact HEAD^1 blob of Session.ts (sha256-verified swap, restored and re-verified; git status clean afterwards). Consumer arm: consumer-wire-oracle.mjs spawns the shipped integrations/external-context/dist/auto-recall.js as a real child process, feeds it the exact stdin JSON hookEventHandler.fireUserPromptSubmitEvent would build from the producer's real emitted bytes, and points its generic-http-search-v1 provider at a loopback server that records method/path/auth/body; nothing on the tested path is stubbed. Gates: Session.test.ts + constants.test.ts = 934/934 green; tsc --noEmit clean in packages/cli and packages/core; vacuity proven by reverting constants.ts (1 red, intended message) and by the base arm's 7 reds naming expected-vs-actual hook input. Raw logs, probe dumps, harness sources and the four captures live in tmp/pr11455-verify-20260910-043903/ (logs/, evidence/, assertions-aggregate.mjs, consumer-wire-oracle.mjs, Session.verifyprobe.test.ts, run-producer-arm.sh).

Flakiness gate log

rounds=5 files=2 skipped=0
file packages/cli/src/acp-integration/session/Session.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/Session.test.ts
file packages/cli/src/ui/components/hooks/constants.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/components/hooks/constants.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/cli/src/acp-integration/session/Session.test.ts: PPPPP
  packages/cli/src/ui/components/hooks/constants.test.ts: PPPPP

verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/components/hooks/constants.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/components/hooks/constants.test.ts: P (exit 0)
round 3 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 3 · packages/cli/src/ui/components/hooks/constants.test.ts: P (exit 0)
round 4 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 4 · packages/cli/src/ui/components/hooks/constants.test.ts: P (exit 0)
round 5 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 5 · packages/cli/src/ui/components/hooks/constants.test.ts: P (exit 0)

Evidence images

01-producer-ab-base-arm-no-submitted-prompt

02-producer-ab-head-arm-submitted-prompt-emitted

03-consumer-oracle-head-unattended-search-fires

04-consumer-oracle-base-zero-searches

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

Requesting changes on the exclusion gate added this round — one Critical survives it, and it is verified against 2b4f46ac, not inferred. Full chain, line by line, in the Stage 2 comment above.

The short version: isUserSubmissionTurn = isFreshUserTurn && !channelTurn closes the channel-classified producers (webhook tasks, /loop jobs, GitHub issue events) — that part I confirmed is correct and trustworthy, since bridge.ts:10502/:10530 strip the key from callers and re-arm it only for the authenticated channel-worker route at serve/routes/session.ts:6912.

But round 1 of the /review named a fourth population, and it is still open: daemon scheduled-task dispatch and Live task runs. Those call bridge.sendPrompt(sessionId, { sessionId, prompt: [...] }, undefined, { promptId, onPromptAdmitted })create-sub-session.ts:932-940, live-task-service.ts:1081-1092 — with a context that carries no channelPrompt and a request that carries no _meta at all. So channelTurn is false, every isFreshUserTurn exclusion term is false, isUserSubmissionTurn is true, and submittedPrompt falls back to promptText — the machine-composed task text — which is then emitted at Session.ts:5633-5640. create-sub-session.ts:7-14 documents both producers as the create_sub_session tool inside an agent turn and the isolated scheduled-task dispatch; neither is a human submission.

The consequence is the one the original Critical traced: auto-recall.ts gates an outbound provider search solely on a nonblank submitted_prompt, so an admin who registered Auto Recall on a daemon pays a search on every scheduled lap, every Live task dispatch and every sub-session spawn, with the result injected into that unattended turn's model input — and a hook written to the documented presence-implies-human pattern can block and kill the task at Session.ts:5641-5652.

Two reasons this is a code fix rather than a docs fix, and why I am not treating it as settled:

  • Both sibling producers gate positively and this gate is now wider than either, which round 2 explicitly asked it not to be: core/client.ts:3225-3230 requires messageType === SendMessageType.UserQuery plus a producer-supplied options.submittedPrompt; nonInteractiveCli.ts:2475-2477 requires isFirstTurn && sendType === SendMessageType.UserQuery.
  • The hooks.md sentence this PR rewrites still promises the field is omitted for "cron, notifications" — and an isolated scheduled-task dispatch is the daemon's cron. Shipped doc contradicts shipped behaviour on a line this PR touched.

I ruled out the two explanations that would have made this moot: sub-sessions do not disable hooks (disableAllHooks has zero non-test hits under packages/cli/src/serve/ or packages/acp-bridge/src/), and sourceType === 'channel' would not rescue it either, because getChannelPromptDisplayText (bridge.ts:411-416) needs a string displayText regardless and channelPrompt — not sourceType — is what classifies the turn.

Suggested direction, same as round 1's: gate positively instead of subtracting. Either let the producer declare provenance through a trusted meta key admitted alongside DAEMON_PROMPT_DISPLAY_TEXT_META_KEY, or add a UserQuery-equivalent classification on the ACP path and require it. Acceptance criterion that actually pins it: a table row driving session.prompt() with an empty _meta — the shape those two call sites really send — asserting the hook input deep-equals { prompt: ... } with no submitted_prompt, while 'trusted display projection' keeps asserting submitted_prompt: 'original question'; then remove the gate and confirm the new row goes red. No current row has an empty _meta, which is why all ten pass in both directions.

To be explicit about what I am not asking for: the original capture point, the ??-not-|| choice, the projection parity with headless, the trust boundary, the retry handling and the locale mirroring are all correct and I verified each rather than assuming it. The refusal of the channelTurn ? '' : promptText ternary as dead defense was also right — submittedPrompt is read only inside the short-circuit at :5637/:5638. This is about the gate's shape, not the rest of the change.

Honest limits on this finding: it is a read-verified chain, not an executed probe — this gate does not run PR-derived code, so I have no runtime evidence of the field leaving the process on a scheduled-task dispatch. @qwen-code /verify would settle it with an A/B against the base build and a wire oracle on the hook stdin; if that comes back clean I will drop the finding. Also unresolved and escalated to @wenshao in the Stage 3 comment: whether ACP producer eligibility is a product decision already made, in which case this becomes a hooks.md correction instead of a gate change, and whether the two deferred design docs (submitted-prompt-provenance.md, direct-external-context-auto-recall.md, both still rowing ACP as Absent) should be reconciled in this PR or a follow-up.

中文说明

针对本轮新增的排除闸门请求修改 —— 有一条 Critical 在该闸门之后依然存在,且已在 2b4f46ac 上逐行核实,不是推断。完整链路见上方 Stage 2 评论。

简述:isUserSubmissionTurn = isFreshUserTurn && !channelTurn 关闭了 channel 分类的生产方(webhook 任务、/loop 任务、GitHub issue 事件)—— 这部分我确认是正确且可信的,因为 bridge.ts:10502/:10530 会剥离调用方传入的该键,仅在 serve/routes/session.ts:6912 的已认证 channel-worker 路由上重新注入。

/review 第 1 轮点名的第四类生产方仍然敞开:daemon 定时任务派发与 Live task 运行。它们调用 bridge.sendPrompt(sessionId, { sessionId, prompt: [...] }, undefined, { promptId, onPromptAdmitted }) —— create-sub-session.ts:932-940live-task-service.ts:1081-1092 —— context 不带 channelPrompt,请求完全不带 _meta。于是 channelTurn 为 false,isFreshUserTurn 的每一项排除条件都为 false,isUserSubmissionTurn 为 true,submittedPrompt 回退到 promptText —— 机器组装的任务文本 —— 并在 Session.ts:5633-5640 被发出。create-sub-session.ts:7-14 明确记录这两类生产方是 agent 回合内部的 create_sub_session 工具与 isolated 定时任务派发;两者都不是人类提交。

后果正是原 Critical 追溯过的那一个:auto-recall.ts 仅凭非空 submitted_prompt 就决定是否发起对外 provider 检索,因此在 daemon 上注册了 Auto Recall 的管理员,每一次定时触发、每一次 Live task 派发、每一次子会话派生都会产生一次检索,结果还会注入该无人值守回合的模型输入 —— 而按「字段存在即代表人类」这一成文约定编写的 hook,可能在 Session.ts:5641-5652 返回 block 并终止该任务。

有两点使它属于代码修复而非文档修复,也是我不把它当作已定论的原因:

  • 两个同级生产方都是正向判定,而本闸门现在比二者都宽 —— 这正是第 2 轮明确要求不得发生的:core/client.ts:3225-3230 要求 messageType === SendMessageType.UserQuery 且由 producer 提供 options.submittedPromptnonInteractiveCli.ts:2475-2477 要求 isFirstTurn && sendType === SendMessageType.UserQuery
  • 本 PR 改写的那句 hooks.md 仍承诺该字段对「cron、notifications」省略 —— 而 isolated 定时任务派发正是 daemon 的 cron。在本 PR 亲手改过的那一行上,已发布的文档与已发布的行为相互矛盾。

我排除了两种会使本发现失效的解释:子会话并未禁用 hook(disableAllHookspackages/cli/src/serve/packages/acp-bridge/src/ 下的非测试代码中命中数为零);sourceType === 'channel' 也救不了它,因为 getChannelPromptDisplayTextbridge.ts:411-416)无论如何都需要字符串 displayText,而对回合做分类的是 channelPrompt,不是 sourceType

建议方向与第 1 轮一致:以正向判定代替逐项排除。要么由 producer 通过一个与 DAEMON_PROMPT_DISPLAY_TEXT_META_KEY 同样只在受信父级放行的 meta 键来声明来源,要么在 ACP 路径上引入等价于 UserQuery 的分类并要求其成立。真正能钉住它的验收标准:补一行以_meta 驱动 session.prompt() 的用例 —— 即上述两处调用点实际发送的形状 —— 断言 hook 输入严格等于 { prompt: ... } 且不含 submitted_prompt,同时保留 'trusted display projection' 行断言 submitted_prompt: 'original question';随后移除该闸门,确认新用例变红。当前没有任何一行的 _meta 为空,这正是十行用例在两个方向上都通过的原因。

需要明确说明我没有要求什么:最初的捕获时机、?? 而非 || 的选择、与 headless 一致的投影构造、可信边界、重试处理以及语言镜像同步都是正确的,我逐项核实而非假定。把 channelTurn ? '' : promptText 三元表达式作为无效防御加以拒绝也是对的 —— submittedPrompt 只在 :5637/:5638 的短路之内被读取。本次意见针对的是闸门的形状,不是这次改动的其余部分。

本发现的诚实边界:它是代码走查得出的链路,不是实跑探针 —— 本闸门不执行 PR 衍生的代码,因此我没有运行时证据证明该字段会在一次定时任务派发中离开进程。@qwen-code /verify 可以通过针对基线构建的 A/B 与 hook stdin 上的线上 oracle 定论此事;如果结果是干净的,我会撤回该发现。另有一项未决事项已升级给 @wenshao(见 Stage 3 评论):ACP 生产方资格是否属于既定的产品决策 —— 若是,这就变成一次 hooks.md 更正而非闸门改动;以及两处延期的设计文档(submitted-prompt-provenance.mddirect-external-context-auto-recall.md,二者仍把 ACP 标记为 Absent)应在本 PR 还是后续 PR 中对齐。

Qwen Code · qwen3.8-max-2026-09-02

@chiga0 chiga0 left a comment

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.

Scope: reviewed packages/cli/src/acp-integration/session/Session.ts (all changed hunks + surrounding context to ~5640), packages/cli/src/serve/create-sub-session.ts, packages/cli/src/serve/live/live-task-service.ts, packages/acp-bridge/src/bridgeTypes.ts, packages/cli/src/serve/routes/session.ts (meta-key stripping logic), packages/cli/src/ui/components/hooks/constants.ts, i18n locale patches (titles only), chatRecordingService.ts (comment update). Working tree unavailable — test execution not run.


Blocker

F1 — isUserSubmissionTurn does not exclude daemon sub-session and Live task dispatches

isUserSubmissionTurn = isFreshUserTurn && !channelTurn. Inside the hook-fire block, isFreshUserTurn reduces to !isRetry (the other three terms are already excluded by the outer if). channelTurn is set only for turns that arrive via the authenticated channel-worker route, which strips and re-injects the CHANNEL_PROMPT_META_KEY flag.

Two callers bypass this path entirely:

  1. packages/cli/src/serve/create-sub-session.ts:932 — calls bridge.sendPrompt(sessionId, { sessionId, prompt: [{ type: 'text', text: info.prompt }] }, undefined, { promptId, onPromptAdmitted }) — no channelPrompt in options, no _meta in the request.
  2. packages/cli/src/serve/live/live-task-service.ts:dispatchPrompt — calls bridge.sendPrompt(sessionId, { sessionId, prompt: [{ type: 'text', text: prompt }] }, undefined, { promptId, ... }) — same shape, no channelPrompt.

For both: isRetry = false, channelTurn = false, so isUserSubmissionTurn = true and submittedPrompt falls back to promptText (the machine-composed task text). submittedPrompt.trim().length > 0 is almost certainly true for a non-trivial task, so submitted_prompt is emitted to the hook.

Consequence: a daemon session with Auto Recall configured pays an outbound memory-provider search on every sub-session spawn and every Live task dispatch. Per the hook implementation, submitted_prompt presence is the only gate before the search fires; the result is then injected into that unattended machine turn's model input. A blocking hook written to the documented "presence implies human submission" pattern can also kill the automated task at the hook blocking-decision check.

Contrast with sibling producers:

  • core/client.ts — requires messageType === SendMessageType.UserQuery plus an explicit options.submittedPrompt
  • nonInteractiveCli.ts — requires isFirstTurn && sendType === SendMessageType.UserQuery

Both gate on a UserQuery type. The ACP path has no equivalent; the channelTurn guard only covers channel-worker authenticated HTTP-originated turns, leaving internal bridge dispatches unchecked.

Verified against head 2b4f46ac by reading both callers. This finding was also filed as Critical R1-1 by the ci-bot across two review rounds; I confirm it independently.


Suggestions (non-blocking)

S1 (Session.ts:5299) — The ACP path is now a submitted_prompt producer, but cross-file contract assertions (e.g., docs/design/submitted-prompt-provenance.md, hooks.md producer table) were partially updated in this commit (hooks.md, both Auto Recall READMEs are updated ✓) but the predecessor design doc submitted-prompt-provenance.md rows ACP as Absent ("No producer added in this change"). The dev-bot confirms partial reconciliation; verify the predecessor design and any strict-decoder rollout documentation are fully updated.

S2 (docs/design/daemon-user-prompt-submit-provenance.md:30) — Acceptance criteria state "retry and continuation behavior" is unit-tested, but no test in the new it.each block drives a continue, isRestoreAskUserQuestion, or runtime-goal turn against a registered UserPromptSubmit hook — only legacy and daemon retries are covered. Continuation exclusions are exercised only implicitly by the outer if guard (not as explicit test cases).


Cross-check against existing reviews

  • qwen-code-ci-bot R1-1 (Critical, three rounds): independently confirmed above — create-sub-session.ts and live-task-service.ts both confirmed as callers that do not set channelPrompt. Finding stands.
  • qwen-code-ci-bot R1-2 / R2-1 (Suggestion): docs partially reconciled in this commit (hooks.md, integration READMEs updated). Predecessor design doc check per S1 above.
  • qwen-code-ci-bot R1-7 (Suggestion): continuation test gap confirmed per S2 above.
  • qwen-code-dev-bot comment on R1-2: confirms partial doc reconciliation. The doc changes in this commit address the forward-looking producer description; the backward-compat/strict-decoder concern in R2-1 is still open.

Unreviewed dimensions

  • Test execution at current head (working tree unavailable).
  • Whether isolated scheduled-task dispatch also bypasses channelTurn (same pattern expected based on the architecture, but not individually confirmed).
  • Integration Tests (CLI, No Sandbox) noted as skipped in CI.

No approval. Blocker: machine-dispatched sub-session and Live task turns incorrectly receive submitted_prompt, triggering Auto Recall on unattended automated turns.

Reviewed with AI assistance.

Comment thread packages/cli/src/acp-integration/session/Session.ts

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 8)": whether channel-worker-supervisor.ts:1072 's opts.workspace and serve/routes/session.ts:6849 's runtime.workspaceCwd can differ by path normalization (sym…; "agent reverse-audit (round 2)": whether a Web Shell/daemon client actually resubmits rewind-restored or composer-history text through the prompt route — I confirmed POST /session/:id/rewind ….

Not reviewed: reverse audit — stopped before round 9 by the review time budget.

Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:

  • packages/cli/src/acp-integration/session/Session.ts:5299 — [review] D3-1 third copy of the same projection expression in one closure (:5299, :5368, :5453) — deferred by the code-age rule, the line is unchanged since the previous round

Convergence: round 3 posted 9 inline comment(s), 8 of them reported for the first time; the previous round posted 2 (1 new). Findings keep coming back to the same files: docs/design/daemon-user-prompt-submit-provenance.md (findings in round 2; 1 more now); packages/cli/src/acp-integration/session/Session.ts (findings in round 1; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

仅完成部分审查,审查缺口已披露。

未审查(原文为英文):build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 8)"whether channel-worker-supervisor.ts:1072 's opts.workspace and serve/routes/session.ts:6849 's runtime.workspaceCwd can differ by path normalization (sym…"agent reverse-audit (round 2)"whether a Web Shell/daemon client actually resubmits rewind-restored or composer-history text through the prompt route — I confirmed POST /session/:id/rewind …

未审查:反向审计——评审时间预算不足,未能开始第 9 轮。

收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。

收敛情况:第 3 轮发布了 9 条行内评论,其中 8 条是首次提出;上一轮发布了 2 条(其中 1 条首次提出)。发现反复回到同一批文件:docs/design/daemon-user-prompt-submit-provenance.md(第 2 轮已出过发现,本轮又有 1 条);packages/cli/src/acp-integration/session/Session.ts(第 1 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。(仅为观察——本轮评审未因此扣留任何内容。)

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

Comment thread packages/cli/src/acp-integration/session/Session.ts
Comment thread docs/design/daemon-user-prompt-submit-provenance.md Outdated
Comment thread packages/cli/src/acp-integration/session/Session.ts Outdated
Comment thread packages/core/src/services/chatRecordingService.ts Outdated
Comment thread docs/users/features/hooks.md
Comment thread docs/users/features/hooks.md Outdated
Comment thread integrations/external-context/README.md Outdated
Comment thread integrations/external-context-mem0/README.md Outdated
Comment thread docs/users/features/hooks.md Outdated
@doudouOUC doudouOUC added the autofix/skip Not eligible for the scheduled autofix agent label Sep 10, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🤖 AutoFix crashed before it could evaluate the feedback (attempt 2/100) — it will retry on the next scan.

AutoFix failed before producing a verified commit (the run crashed or timed out before it could explain why).

中文说明

🤖 AutoFix 在评估反馈之前崩溃(第 2/100 次尝试)—— 将在下次扫描时重试。

Run log: https://github.com/QwenLM/qwen-code/actions/runs/34441347168


🧠 Handled by Qwen Code · model/模型 kimi-k3

Carry original Web Shell composer text separately through admission and queues; keep background dispatches and undeclared requests ineligible. Reconcile hook and integration contracts and add regression coverage.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Review corrections and verification for 2438732.

Feedback Action
F1 / R1-1: unattended dispatches incorrectly acquire user provenance Fixed with explicit per-request declaration, owner-scoped admission, trusted bridge context and no prompt/display fallback. Web Shell captures original text before preparation; internal dispatches remain undeclared.
R1-2 / R2-1: stale producer contracts and predecessor-design pointer Updated current bilingual design and hook/integration documentation; narrowed the historical design reference to the strict-decoder migration note. A wholesale rewrite of the predecessor designs remains out of scope.
R3-1: unreachable display-projection preference Removed display projection from provenance selection; retained its recording behavior.
R3-2 / R3-3: recording and legacy prompt semantics Documented core/headless versus ACP behavior, including pre-expansion ACP text and optional recording payload.
R3-4: human channel messages Explicitly documented that both human and automated channel messages are excluded.
R3-5 / R3-6: integration deployment versus producer coverage Kept Direct Profile managed launching TTY-only; documented per-request ACP/daemon opt-in and Web Shell support.
R3-7: TUI composer rules and validation example scope Limited TUI-specific provenance rules to TUI and documented the example’s non-TUI execution/blocking behavior.

Verification report:

  • Before correction, the independent Session reproduction failed because an undeclared machine request emitted submitted_prompt. The same assertion passed after the fix without changing its expected result.
  • Nine real local daemon → REST → bridge → ACP child → observing command hook → packaged Auto Recall → loopback provider scenarios passed. Explicit original text was preserved and searched once; undeclared input, forged private metadata, empty/blank/invalid declarations and channel/display combinations produced no submitted field and no search. A real read_file continuation made three model requests but only one hook invocation/search.
  • Full affected suites passed: bridge 921; ACP agent 624; Session 875; REST server 1,259; ACP HTTP 370; Web Shell actions 189; ChatPane 144; queue DOM 74; queue reconciliation 103; queue helper 6. Full App suite passed 817 tests before the final queued /skills correction; five targeted App tests subsequently passed, covering both idle/busy /skills, host preparation, prepared queues, and manual scheduled runs.
  • Root build, bundle, typecheck, full lint, changed-file formatting and diff checks passed. Two self-audit passes and independent review found no remaining substantive issue.
  • Validation used macOS / Node 22.22.3 and a controlled local model. No cloud credentials were used. The real probe did not exercise actual cron/Live dispatchers, a channel-worker restart race, direct ACP transport, or browser UI. Those limits are recorded in the updated PR body. Prior Holo evidence predates this correction and is not claimed as current validation.

The temporary autofix/skip label will be removed after thread cleanup, restoring the previous remote takeover setup. The hourly Codex babysit task remains active. Resolving addressed threads does not dismiss the outstanding CHANGES_REQUESTED review or replace a fresh review of this commit.

@doudouOUC doudouOUC added autofix/skip Not eligible for the scheduled autofix agent and removed autofix/skip Not eligible for the scheduled autofix agent labels Sep 10, 2026
Incorporate main at 1097b9f so the PR is validated by the current lint gate. The provenance correction is preserved without manual conflict resolution.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Synchronized main in b4ee502 to address the new CI gate failure.

Item Result
Lint & Static failure on 2438732 check-lint-gate-freshness rejected the branch before ESLint because main updated .github/workflows/ci.yml at 17990c3. This was neither a provenance lint error nor a transient failure; no blind rerun was used.
Correction Merged main at 1097b9f without manual conflict resolution. Independent review confirmed no semantic conflict with original-text capture, admission, or recall eligibility.
Merged-head validation Build, bundle, typecheck, full lint and PR-diff formatting passed. Five full Web Shell suites passed 1,383 tests; targeted bridge and CLI regressions passed 50 tests.
Real daemon verification The same nine scenarios passed again on the clean merged head. Provider search counts were [1,0,0,0,0,0,0,0,1]; only declared ordinary input and declared input followed by a real tool continuation searched once. No cloud credentials or residual test processes/records. Bundle SHA-256: 28e4bd72cac54b8ed83910adccfe93422c7c3c1d2a7a1cde2746b0368af67d44.
Review threads The 11 addressed threads remain resolved. Original fix replies reference ancestor 2438732, which remains intact in this merge. Outstanding CHANGES_REQUESTED requires a fresh reviewer decision; it was not dismissed.

The temporary autofix pause is being removed and the hourly babysit task remains enabled. New-head CI results are pending.

@doudouOUC doudouOUC removed the autofix/skip Not eligible for the scheduled autofix agent label Sep 10, 2026
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 46dc438. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

ℹ️ No screenshot changed against the PR base — but this PR edits 3 render-shaping files:

  • packages/web-shell/client/App.tsx
  • packages/web-shell/client/components/ChatPane.tsx
  • packages/web-shell/client/components/QueuedPromptDisplay.tsx

Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to packages/web-shell/client/e2e/visuals/screenshots.spec.ts that seeds whatever state the UI is gated on; it then appears here as a head-only (NEW) capture.

Full-resolution recordings (.webm) are attached to the workflow run.

Qwen Code · web-shell visuals

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head 46dc438, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

No response changes against the PR base across 12 scenario(s).

Qwen Code · serve A/B

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Synchronized main in 46dc4382ed to clear a Test failure this PR did not cause.

Item Result
Test (ubuntu-latest, Node 22.x) failure on b4ee502 Not from this PR. Brand PR #11244 landed on top of #11470 and left two web-shell suites failing on main (tracked as #11525): the sidebar brand tests queried a version tooltip hidden below the 344px compact footer breakpoint, and the workspace-session loading test pinned an exact request list that no longer accounts for GET /brand. Neither file is in this PR's diff, and the branch does not touch either suite.
Why no rerun Main fixed both suites in 2488d12d8b (#11530), which the previous sync at main 1097b9fe0a predates, so the broken files were inherited into the merged tree and the failure was deterministic. A blind rerun would have reproduced the same 8 assertions and burned a CI cycle.
Merge target Merged main at 2488d12d8b — the newest main commit with a fully green CI run — rather than tip 0ef35351e9, whose CI was still in progress and which itself registers daemon-managed sessions, adjacent to this PR's provenance surface. Held that pull back rather than importing unverified risk into a red-CI recovery.
Merge integrity Clean merge, no conflicts. The merge introduced only main's 29 files; this PR's diff is still exactly 38 files, and none of them were altered by the merge. SendPromptOptions.submittedPrompt and main's own daemon/session/types.ts addition coexist.
Verification on the merged head The 8 previously failing assertions now pass: 16 tests across both suites. Provenance-adjacent suites passed 738 tests (daemon session actions, DaemonSessionProvider including main's new cases, ChatPane, queue DOM); the full App suite passed 862 tests. Root typecheck clean. Validated on macOS / Node 22.22.3, matching the CI Node line.
Review threads All 15 threads remain resolved; none outstanding.

The outstanding CHANGES_REQUESTED review still needs a fresh reviewer decision on this head — this sync does not dismiss it and makes no production behavior change. New-head CI results are pending.

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 67 passed · 0 failed · 67 total

Flakiness gate: ⚠️ timeout — only 2 of 5 rounds fit the 15-minute budget; the completed rounds agreed

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:67 通过 · 0 失败 · 67 总计

抖动门:⚠️ timeout — only 2 of 5 rounds fit the 15-minute budget; the completed rounds agreed

Verification report

verify-pr 11455 — fix(acp): Preserve submitted prompt provenance for auto recall

Verdict: merge-ready — 67/67 scripted assertions passed, 0 unexpected failures. Verified head: 46dc4382ed1a08b7509426d8ca429d681aec83a6 (git rev-parse HEAD^2). Follow-up round: previous-report.md was present and substantive (round 1 verdict findings, head 2b4f46ac). Round 1's Critical (F1) is fixed, re-measured with the same instrument; F2/F3/F4 are all fixed too. Three new non-blocking coverage items are reported below.

中文摘要
  • 结论: merge-ready。67 条脚本断言全部通过,0 条意外失败。已验证 head:46dc4382。本轮为复验轮(上一轮 head 为 2b4f46ac,结论 findings)。
  • 上一轮 finding 状态(详见下方 Previous-finding status 表):F1(Critical,机器回合默认取得 provenance)已修复;F2(hooks.md 关于 cron / teammate 的错误句子)已修复;F3(实时语音委派被当作用户提交)已修复;F4(录制段落对 ACP 不精确)已修复。四项全部用新 head 重新实测,未对比旧报告文字。
  • A/B 结论(详见 Central claim and A/B 表):三臂对照(base HEAD^1 blob / denylist 变异体=上一轮 head 的极性 / head)。20 个判别行中,head 全绿;base 在 4 个「应发射」行红(功能缺失,符合预期);denylist 变异体在 11 行红,其中包含 F1 的全部 5 个爆炸半径行——证明这些行确实能区分 F1 极性,不是空绿。
  • 消费端线神谕:把 head 与 denylist 两臂真实发射的 hook 字节喂给已发布的 hook bundle(真实子进程 + 真实 loopback provider)。head 在 F1 的 5 行上外发检索 0 次;denylist 对照臂 5 次,其中无人值守 per_run 定时任务触发的检索 query 就是机器拼装的 wrapper 原文——即上一轮 F1 最尖锐的那个数字,本轮归零。
  • 变异矩阵:14 个变异,11 killed / 3 survived(M6、M8、M10)。每个 survivor 都在同一文件里有已 killed 的正对照(M7、M9/M11),故 harness 有效。三者经穷举上下游后判定为非承重(冗余防御 / 当前不可达子句),属完整性报告,非合并条件。
  • 门禁:4 组受影响套件全绿 —— packages/cli 2009/2009 + 1270/1270、packages/acp-bridge 925/925、packages/web-shell 1280/1280。
  • 未覆盖:真实 daemon/Holo 端到端、逐提交归因(浅克隆 depth-2,快照 5 个提交本地仅 1 个可达)、跨 hop 的真实全链路(REST→bridge→子进程→Session→hook 逐跳验证,未端到端串起)、Windows/Linux、_meta 转发路径的穷举(仅穷举了 PR 自述的三处入口与两处 trusted parent)。

Previous-finding status (round 1 → this head)

Every row was re-measured at 46dc4382, not diffed against the old report. The one input-closure shortcut used is named in Methodology.

# round-1 finding sev (round 1) status at 46dc4382 evidence
F1 Gate was a denylist where the commit promised a positive condition, so unattended per_run fires, manual run-now, create_sub_session, send_message_to_thread and daemon-MCP prompt published provenance and fired retrieval Critical (new-surface) fixed Polarity inverted: Session.ts now defaults to '' and requires qwen.daemon.submittedPrompt. All 5 blast-radius rows absent at head, PRESENT under the denylist mutant that reproduces the old polarity; consumer oracle 0 vs 5 outbound searches. 01-producer-ab-three-arm-base-denylist-head.png, 03-consumer-oracle-head-zero-searches-vs-denylist-five.png
F2 New hooks.md sentence was falsified for "cron" and "teammate traffic" Suggestion fixed The false sentence is gone. hooks.md now states the positive contract ("a fresh turn must carry an explicit submission declaration") and explicitly warns "scheduled tasks, Live task runs, sub-session spawns, model-authored cross-session messages, and promoted mid-turn messages must not acquire it automatically" — which matches the measured behaviour
F3 Realtime voice delegation treated as a user submission Nice to have fixed That path supplies no declaration, so nothing is stamped upstream and Session sees none. Now documented: "Realtime voice handoffs do not declare provenance because their request text comes from model-generated tool arguments." The probe row model-only delegation excluded only emits because the test itself declares
F4 Recording paragraph's "otherwise the expanded pre-hook prompt" imprecise for ACP Nice to have fixed hooks.md now reads "ACP records the trusted display projection or raw request text before expansion when a projection or attachment references require a payload; otherwise it records the user message without systemPayload or displayText" — exactly the behaviour round 1 described. The UserPromptRecordPayload.displayText doc comment was corrected to match

No round-1 finding stands, worsened, or was declined.

Central claim and A/B

Central claim. On the ACP/daemon path, UserPromptSubmit receives submitted_prompt only when a supported producer explicitly declares it via _meta["qwen.submittedPrompt"] (promoted to the private qwen.daemon.submittedPrompt for the daemon-to-child hop). Undeclared machine dispatch — the exact class round 1's Critical leaked on — never acquires it, and the value is never reconstructed from request text or a display label.

Control construction. Three arms differing only in packages/cli/src/acp-integration/session/Session.ts; the probe file, node_modules, config and every other source are byte-identical across arms, so no internal-workspace-link confound exists (readlink -f node_modules/@qwen-code/qwen-code-corepackages/core, a package this PR touches only in a doc comment). Arms: base = the exact HEAD^1 blob (sha256 51fb91f6e5c7ebda…byte-identical to round 1's base blob, so main did not touch this file between the two base tips); denylist = head with one expression reverted to the previously verified head 2b4f46ac's polarity (submittedPrompt defaults to promptDisplayText ?? promptText); head = 46dc4382 (sha256 c7480297d7b14c7d…). Each arm's restore is sha256-verified.

Probe = the PR's own 14-row provenance table plus 6 rows I added: the 5 F1 blast-radius shapes with text built by the real buildScheduledTaskRunPrompt, and one declared CONTROL row. 20 rows × 3 arms.

cell (probe row) oracle base denylist (prev head) head
text blocks without resource bodies hook input submitted_prompt RED (absent) green ' check this\n' green ' check this\n'
explicit submission overrides unrelated display text hook input RED green green 'original question'
display projection cannot declare provenance hook input green (absent) RED — leaks 'display text without a declaration' green (absent)
model-only delegation excluded hook input RED green green
blank text / resource-only hook input green green green
legacy retry / daemon retry hook input green green green
channel turn ± display projection hook input green green green
machine dispatch without a declaration hook input green RED — leaks green (absent)
empty / blank / invalid declaration hook input green RED ×3 — falls back to request text green (absent)
GAP per_run scheduled-task fire hook input green RED — composed wrapper emitted green (absent)
GAP manual run-now hook input green RED — composed wrapper emitted green (absent)
GAP model-authored sub-session hook input green RED — model text emitted green (absent)
GAP send_message_to_thread hook input green RED — model text emitted green (absent)
GAP daemon-MCP prompt tool hook input green RED — model text emitted green (absent)
CONTROL ordinary declared user question hook input RED (absent) green green 'why does the nightly build fail?'

Head 20/20 green; base red on exactly the 4 emission rows (feature absent — an intended control red, encoded as a passing assertion); denylist red on 11 rows including all 5 GAP rows. Witness: 01-producer-ab-three-arm-base-denylist-head.png.

Consumer-side A/B (wire oracle). The bytes each arm's real Session code emitted were dumped from the hook seam and fed, unmodified, to the shipped integrations/external-context/dist/auto-recall.js — spawned as a real child process against a real loopback generic-http-search-v1 provider recording method/path/auth/body:

arm outbound POST /v1/context/search on the 5 F1 rows on the declared CONTROL row total
head 46dc4382 0 1, query = declaration verbatim 4
denylist (prev head polarity) 5 — incl. per_run with query "Scheduled task: Nightly triage sweep Task ID: task-verify-1 …" 1 14

Search count tracked submitted_prompt presence exactly on all 20 rows in both arms. Witness: 03-consumer-oracle-head-zero-searches-vs-denylist-five.png. This is round 1's headline number re-measured on the same instrument: the unattended timer fire went 1 search → 0.

Mutation matrix (vacuity + guard completeness)

14 single-expression mutants of the guards this PR adds, each run against the suite that should catch it, with the file restored and sha256-re-verified after every cell. All 5 unmutated controls green first. 11 killed / 3 survived. Every survivor has a killed control in its own file (M7 in acpAgent.ts, M9/M11 in bridge.ts), so "survived" cannot mean "my command never collected that file". Witness: 02-mutation-matrix-11-killed-3-survived.png.

id guard mutated verdict red test / classification
M1 Session: drop isUserSubmissionTurn from the emission condition KILLED 16/20 legacy retry, daemon retry, channel turn …
M2 Session: drop the typeof === 'string' guard (coerce) KILLED 19/20 invalid declaration
M3 Session: drop the nonblank guard KILLED 8/20 12 rows incl. blank text, resource-only
M4 Session: drop !channelTurn KILLED 18/20 both channel rows
M5 Session: fall back to request text (the F1 polarity) KILLED 12/20 8 rows incl. all 5 GAP rows
M6 acpAgent: let a trusted parent fall back to the public key SURVIVED see N1
M7 acpAgent: stop deleting the forged private key KILLED 1/2 strips forged invocation metadata from an untrusted ACP client
M8 acpAgent: re-stamp even on a channel turn SURVIVED see N3
M9 bridge: stop stripping the forged keys KILLED 0/1 forwards only explicitly declared submission text…
M10 bridge: drop !isPromotedMidTurn SURVIVED see N2
M11 bridge: drop the channelPrompt guard KILLED 0/1 forwards only explicitly declared submission text…
M12 ACP HTTP dispatch: drop the 3 marker guards KILLED 3/4 admits only public submission declarations over ACP HTTP
M13 REST route: drop the 3 marker guards KILLED 0/1 requires explicit submission provenance…
M14 REST route: stop deleting the forged keys KILLED 0/1 requires explicit submission provenance…

The design doc's own acceptance criterion — "Remove the new declaration gate and confirm omission cases fail" — is M5, and it fails 8 rows. The PR's new tests are not vacuous.

Findings

All three are completeness reporting, not merge conditions — none is load-bearing, and each is named with the pinned sibling that closes the same hazard today.

N1 — Suggestion: a documented invariant about trusted parents is unpinned

docs/design/daemon-user-prompt-submit-provenance.md states "A trusted parent cannot use a public key as a fallback when the private declaration is missing." The code implements it (a ternary on privateParentState === 'trusted' in acpAgent.ts), but M6 — turning the ternary into meta[DAEMON_…] ?? meta[SUBMITTED_…] — survives both relevant tests green. The new acpAgent.test.ts case covers the untrusted direction only (setupSessionMocks('untrusted-session')).

I enumerated every trusted parent rather than inferring it, and the invariant is not currently violable: PRIVATE_PARENT_CAPABILITY_META_KEY is minted in exactly two places — packages/acp-bridge/src/bridge.ts:4593 (daemon bridge), which strips both submission keys before the child sees meta (M9, pinned), and packages/channels/base/src/AcpBridge.ts:124 (channel bridge), which builds _meta from scratch at line 334 — only CHANNEL_PROMPT_META_KEY: true plus an optional display-text key, never a caller-controlled spread — and always marks the turn as a channel turn, which Session excludes (M4, pinned). So this is defence-in-depth for a property the doc promises, not a live hole. The fixture that would pin it is the mirror of the existing one: a trusted session whose meta carries only qwen.submittedPrompt, asserting no submitted_prompt reaches the hook.

N2 — Nice to have: the bridge's !isPromotedMidTurn clause cannot currently decide any outcome

The doc claims "channel and promoted mid-turn dispatches omit it." The channel half is pinned (M11 killed). The mid-turn half is not: M10 survives because the only producer of promotedMidTurnbridge.ts:9352 — builds its context as { promptId, promotedMidTurn: { originatorClientId }, onPromptAdmitted } and never sets submittedPrompt, so typeof context?.submittedPrompt === 'string' is already false on that path. Classification: currently-unreachable clause / redundant defence, correct exactly as it stands and worth keeping (it becomes load-bearing the moment a promotion producer carries a declaration). Reported so the doc sentence is not read as test-backed.

N3 — Nice to have: acpAgent's channel guard on re-stamp is redundant defence

M8 (re-stamp qwen.daemon.submittedPrompt even when suppliedChannelPrompt === true) survives. Bounded by census: the trusted path is closed upstream by the bridge's own channelPrompt guard (M11, pinned), and the HTTP/REST paths drop the declaration whenever a channel marker is present (M12/M13, pinned). The only directly observable case left is an untrusted direct-ACP client that self-labels qwen.channel.prompt: true — and for an untrusted caller that label is itself discarded (the sibling block re-arms CHANNEL_PROMPT_META_KEY only when privateParentState === 'trusted'), so the turn is not a channel turn downstream anyway. No hazard in either direction; the guard is belt-and-braces and should stay.

Bounded: what the new public declaration key is NOT

qwen.submittedPrompt is a new externally-suppliable meta key — before this PR no ACP path could produce submitted_prompt at all. I checked the scary readings and they do not hold: it is not a privilege escalation, because every ingress that accepts it (REST prompt route, ACP HTTP dispatch, direct ACP stdio) already accepts an arbitrary params.prompt from the same caller, which is strictly more powerful; it cannot forge the private key (all three ingresses delete qwen.daemon.submittedPrompt, M7/M9/M14 pinned); and the marker guards fail closed (they test === undefined, so adding a channel/display/worker-auth marker suppresses the declaration rather than enabling it). What remains is exactly what the PR body and design doc already state: caller-supplied provenance that is not authentication or DLP, gated behind an administrator explicitly registering Auto Recall with credentials.

Web Shell submittedPrompt = text default parameter — checked, benign

App.tsx:10308 gives enqueuePrompt a default of submittedPrompt = text, which would silently declare provenance for any caller that omits the argument — the mirror of F1 on the client side. Census of all call sites: App.tsx:13971 (passes the original text explicitly while enqueueing a rewritten slash-command commandText — the intended behaviour) and App.tsx:15110 (omits it, but its first argument is text, the composer text). ChatPane.tsx passes text explicitly on both branches. No machine path inherits the default, and the Web Shell's scheduled-task UI goes through the REST scheduled-tasks route, which never sets submittedPrompt — so the test-plan claim "generic actions and manual scheduled runs must not declare it automatically" holds. useQueuedPrompts.ts:624 also correctly drops the declaration when a summary-only queued row is restored from the server, since that text is not the original composer text.

Reviewer Test Plan, step by step

plan step result
"Verify declared user input and undeclared machine input independently" verified — 20-row × 3-arm table; declared rows emit, all 5 undeclared machine rows absent, and the denylist control proves the rows discriminate
"host-prepared text, attachments, model-only context, and slash-command expansion never replace the original declaration" verified at the Session/hook seam (text blocks without resource bodies keeps PRIVATE RESOURCE out; explicit submission overrides unrelated display text; model-only delegation excluded) and at the client seam by the PR's actions.test.ts / useQueuedPrompts.dom.test.tsx rows, which I ran (1280/1280) but did not independently mutate
"existing legacy hook invocation and recording behavior remain intact" verified — 2009/2009 in the four packages/cli suites, 925/925 bridge, and prompt is still emitted on every row including all 16 absence rows
"Repeat without a declaration, with forged private metadata, and with invalid, blank, channel, or display metadata; no submitted field or provider search should occur" verified — every one of those shapes is a probe row, and the consumer oracle shows 0 outbound searches for all 16 absence rows
"A real tool-result continuation must not repeat retrieval" not verified — needs a multi-turn model loop (see Not covered)
"In Web Shell, verify direct, host-prepared, queued, and converted /skills submissions preserve the original text" partially verified — by call-site census plus the PR's own green tests; I drove no real browser
"generic actions and manual scheduled runs must not declare it automatically" verified by census — see the bounded note above
"real rebuilt daemon/REST/bridge/ACP-child/hook/provider chain passed nine scenarios" not verifiable in this container — no credentials or network; the author's claim is untested here. My chain evidence is hop-by-hop, not end-to-end

Not covered

  • Real daemon end-to-end (the author's nine scenarios, Holo/Mem0 records, workspace A/B binding): requires provider credentials and network; this container has neither. The consumer oracle substitutes a loopback provider and says so.
  • End-to-end chain across all hops. I verified each hop separately — REST route (server.test.ts, M13/M14), ACP HTTP dispatch (transport.test.ts, M12), bridge (bridge.test.ts, M9/M10/M11), ACP child admission (acpAgent.test.ts, M6/M7/M8), Session emission (my 3-arm probe), hook consumer (real bundle + real socket) — but never drove one request through REST → bridge → a real spawned child → Session → hook in a single run. A hop-by-hop chain can hide a seam mismatch; this is the shape of the gap, not a claim that one exists.
  • Multi-turn behaviour: "a tool-result continuation does not repeat retrieval", Stop-hook re-entry, mid-turn steering. Read in code, not driven.
  • Per-commit attribution: the checkout is depth 2 (git rev-parse --is-shallow-repository = true). git rev-list HEAD^1..HEAD^2 returns 1 commit while the snapshot's commits array lists 5; 4fb7d2f0, 2b4f46ac, 2438732a and b4ee5026 are all unreachable (git cat-file -t fails on each). Only the aggregate HEAD^1..HEAD diff was exercised, so I cannot attribute the polarity inversion to 2438732a by measurement — only by its headline.
  • Base OID drift: the snapshot's baseRefOid is 2488d12d…, which is unreachable locally; the merge ref's HEAD^1 is 0ef3535…. Per the merge-ref contract I used HEAD^1 throughout. Mitigating fact: the base Session.ts blob is byte-identical to round 1's (51fb91f6…), so the file under test did not move between the two base tips.
  • Trial merge into current main: not performed.
  • Exhaustive _meta-forwarding census. I enumerated the three ingresses the PR names and both trusted parents, and grepped packages/channels/ for the new keys (zero hits). I did not prove that no other code path in the repo forwards a caller-controlled _meta to an ACP child.
  • Windows/Linux execution paths (author-declared untested); no real browser UI.
  • No repo-wide gate: typecheck, lint, bundle and the unaffected workspaces were not run — the PR's own CI covers those and this round's budget went to the A/B.

Methodology

Environment: the CI verify container (node v22.23.2), merge-ref checkout at depth 2, npm ci + npm run build pre-run at HEAD. Producer arm (ab-driver.mjs): the PR's own Session.test.ts provenance table copied to a scratch Session.verifyprobe.test.ts with 6 added rows — 5 F1 blast-radius shapes whose text is built by the real buildScheduledTaskRunPrompt imported from src/runtime/scheduled-task-run.ts, plus one declared CONTROL row — and a messageBus seam that dumps every real hook input to JSONL. The driver swaps only Session.ts per arm (base = git show HEAD^1:…; denylist = head with one expression replaced by the previously verified head's polarity), sha256-verifies each swap and each restore, runs vitest with --reporter=json, and zips results positionally against the known row order because vitest renders it.each's $name quoted and truncated — the zip itself is validated per arm by asserting each rendered fragment is a prefix of the expected row name. Expected red cells are encoded as passing assertions, so fail counts only unexpected outcomes. Mutation arm (mutation-matrix.mjs): 14 single-expression mutants across 5 files in 3 packages, each preceded by an unmutated green control for its own runner, each restored and sha256-re-verified. Consumer arm (consumer-wire-oracle.mjs): spawns the shipped integrations/external-context/dist/auto-recall.js as a real child process, feeds it the exact dumped bytes with hook_event_name/prompt/submitted_prompt/cwd, points its generic-http-search-v1 provider at a loopback server recording method/path/auth/body, and attributes requests per spawn; nothing on the tested path is stubbed. Gates (run-gates.sh): the four affected workspace suites. The single input-closure shortcut taken: round 1's base blob was reused without re-deriving it, justified by the sha256 of HEAD^1:packages/cli/src/acp-integration/session/Session.ts being identical to the one round 1 quoted (51fb91f6…) — and everything that measurement consumes (the probe file, node_modules, config) was rebuilt or re-run this round regardless. Raw logs, JSONL dumps, harness sources and the three captures live in tmp/pr11455-verify-20260910-102405/ (logs/, evidence/, ab-driver.mjs, mutation-matrix.mjs, consumer-wire-oracle.mjs, run-gates.sh, plus the complete Session.verifyprobe.test.ts — copy it back to packages/cli/src/acp-integration/session/ to rerun either arm). The probe file was removed from the working tree after the runs: git status is clean and no scratch worktree remains.

Flakiness gate log

rounds=5 files=10 skipped=0
file packages/acp-bridge/src/bridge.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/bridge.test.ts
file packages/cli/src/acp-integration/acpAgent.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/acpAgent.test.ts
file packages/cli/src/acp-integration/session/Session.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/Session.test.ts
file packages/cli/src/serve/acp-http/transport.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/acp-http/transport.test.ts
file packages/cli/src/serve/server.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server.test.ts
file packages/cli/src/ui/components/hooks/constants.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/components/hooks/constants.test.ts
file packages/web-shell/client/App.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/App.test.tsx
file packages/web-shell/client/components/ChatPane.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/ChatPane.test.tsx
file packages/web-shell/client/daemon/session/actions.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/actions.test.ts
file packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/hooks/useQueuedPrompts.dom.test.tsx


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/acp-bridge/src/bridge.test.ts: PPP
  packages/cli/src/acp-integration/acpAgent.test.ts: PPP
  packages/cli/src/acp-integration/session/Session.test.ts: PP
  packages/cli/src/serve/acp-http/transport.test.ts: PP
  packages/cli/src/serve/server.test.ts: PP
  packages/cli/src/ui/components/hooks/constants.test.ts: PP
  packages/web-shell/client/App.test.tsx: PP
  packages/web-shell/client/components/ChatPane.test.tsx: PP
  packages/web-shell/client/daemon/session/actions.test.ts: PP
  packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: PP

verdict: timeout
summary: only 2 of 5 rounds fit the 15-minute budget; the completed rounds agreed

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/acp-http/transport.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/components/hooks/constants.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/ChatPane.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/actions.test.ts: P (exit 0)
round 1 · packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: P (exit 0)
round 2 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/acp-http/transport.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/components/hooks/constants.test.ts: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/ChatPane.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/daemon/session/actions.test.ts: P (exit 0)
round 2 · packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: P (exit 0)
round 3 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 3 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)

Evidence images

01-producer-ab-three-arm-base-denylist-head

02-mutation-matrix-11-killed-3-survived

03-consumer-oracle-head-zero-searches-vs-denylist-five

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@doudouOUC
doudouOUC enabled auto-merge September 10, 2026 11:20
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 144 passed · 0 failed · 144 total

Flakiness gate: ⚠️ timeout — the 15-minute budget elapsed before two full rounds completed (1 done) — no flakiness signal either way

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:144 通过 · 0 失败 · 144 总计

抖动门:⚠️ timeout — the 15-minute budget elapsed before two full rounds completed (1 done) — no flakiness signal either way

Verification report

<!-- qwen-triage:verify -->
<!-- qwen-triage:verify-substantive -->

Sandboxed verification: ✅ passed — merge-ready (agent verdict)

Ran the PR in an isolated, token-free container: A/B against the base build on the real production chain, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 144 passed · 0 failed · 144 total

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(真实生产链路上的 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:144 通过 · 0 失败 · 144 总计

Verification report

verify-pr 11455 (round 3) — fix(acp): Preserve submitted prompt provenance for auto recall

Verdict: merge-ready — 144/144 scripted assertions passed, 0 unexpected failures. Verified head: 46dc4382ed1a08b7509426d8ca429d681aec83a6 (git rev-parse HEAD^2).

Follow-up round, zero code delta. previous-report.md (round 2) verified head 46dc4382ed1a… — the same OID as this round's HEAD^2 — and HEAD^1 is the same base tip 0ef35351e9…. Both Session.ts blob hashes match round 2's quoted values byte-for-byte (base 51fb91f6e5c7ebda…, head c7480297d7b14c7d…). Git OIDs are content hashes of the full tree, so the entire source closure round 2 measured is provably identical here; that is the strongest form of the input-closure shortcut, and it is why this round spends its budget on the gaps round 2 itself named rather than re-deriving identical numbers. Those gaps are now closed: the real end-to-end chain (round 2: "never drove one request through REST → bridge → a real spawned child → Session → hook in a single run") and the core-path tool-result continuation Test-Plan step (round 2: "not verified").

中文摘要
  • 结论: merge-ready。144 条脚本断言全部通过,0 条意外失败。已验证 head:46dc4382。本轮为复验轮,且与上一轮代码零差异HEAD^2 与上一轮已验证 head 的 OID 完全相同,Session.ts 两侧 blob sha256 逐字节一致),因此上一轮的全部测量在本 head 上依然成立;本轮预算用于关闭上一轮自己点名的两个缺口。
  • 上一轮 finding 状态(详见 Previous-finding status 表):F1(Critical)已修复,且本轮在真实生产链路上复测(真实 bridge → 真实 spawn 的 qwen --acp 子进程 → acpAgent → Session → 真实 hook 子进程):未声明的机器派发、伪造私钥、仅线上公钥三种形状在 head 臂下 hook 输入均 submitted_prompt,而 denylist 对照臂(把第一轮极性补丁进编译产物)三种全部泄漏 —— 10/10 对 10/10。F2/F3/F4 已修复(文档句子与实测行为一致)。N1/N2/N3 维持(M6b/M10/M8 复测仍 survived,各自同文件正对照 M7/M9 仍 killed);其中 N1 的证据句需要一处更正(见 Corrections)。
  • A/B 结论(详见 Central claim and A/B 表与 03-real-chain-ab-head-absent-vs-denylist-leaks.png):真实链路上 head 臂 4 行中 3 行缺省、1 行按声明原样发射;denylist 对照臂 3 行泄漏机器/伪造/线上文本。链路每一跳均为真实代码,无 mock。
  • 新关闭的缺口:(a) Reviewer Test Plan 中"tool-result 续跑不得重复检索"一步(上一轮标 not verified)—— core 生产者门(client.ts)只对 UserQuery 附加 submitted_promptToolResult/Hook 发送不可能携带该字段;真实 auto-recall bundle 对无该字段的输入0 次外发检索,对有声明的输入 1 次(正对照),见 01-core-path-toolresult-no-retrieval-vs-declared-search.png;(b) REST 一跳(上一轮仅以变异 M12–M14 钉住)—— 真实无 token 回环 daemon 上 4 个用例各自独立 session、经 SSE 等到 turn_complete:声明原样到达 hook;未声明/伪造私钥/公钥+显示标记 均缺省;且一个回合内 5 次模型往返只触发 1 次 hook,见 04-rest-daemon-chain-declared-vs-undeclared-forged-marker.png
  • 新 finding(非阻塞,Suggestion,文档):PR 重写的 hooks.md core 路径句子漏列 Goal(代码排除 6 种发送类型,文档列 5 种)。该遗漏早于本 PR,但 PR 改写了这句话且其 ACP 半句确实点名了 runtime-goal,故作为完整性记录。
  • 门禁:packages/acp-bridge 925/925;packages/cli 5 个 PR 触及的测试文件 3279/3279;packages/web-shell 4 个 PR 触及的测试文件 1280/1280。另新增 i18n 键契约 47/47(上一轮未检查的区域)。
  • 未覆盖被驱动的工具执行续跑(fake OpenAI 的工具调用形状始终未让子进程回传 tool 角色消息,故"tool-result 续跑"以生产者门+消费者神谕与"每回合仅 1 次 hook"两项证据覆盖,而非真实工具回路)、逐提交归因(浅克隆 depth-2)、trial merge 进 main、全仓 typecheck/lint/bundle、Windows、真实浏览器。

Previous-finding status (rounds 1–2 → this head)

Every row was re-measured at 46dc4382, not diffed against the old report. Because both parent OIDs of the merge ref are byte-identical to round 2's, the closure argument is the whole tree; each measurement below was nonetheless re-executed this round on the real chain or by re-running the mutant.

# finding sev (origin) status at 46dc4382 evidence this round
F1 Denylist polarity let unattended per_run fires, run-now, sub-session spawns, send_message_to_thread, daemon-MCP prompt publish provenance and fire retrieval Critical (round 1) fixed Real-chain A/B: undeclared machine dispatch, forged private key on the wire, and public-key-only-on-the-wire all yield hook input with no submitted_prompt at head; the denylist control (round-1 polarity patched into the compiled Session.js) leaks all three ("scheduled machine dispatch text", "forged wire text", "public wire declaration"). 10/10 vs 10/10. 03-real-chain-ab-head-absent-vs-denylist-leaks.png
F2 hooks.md sentence falsified for "cron" and "teammate traffic" Suggestion (round 1) fixed Re-read at this head: the positive contract sentence is present and matches measured behaviour (machine dispatch absent on the real chain)
F3 Realtime voice delegation treated as a user submission Nice to have (round 1) fixed Doc states the omission; no producer supplies a declaration on that path (census: 3 public-key producers, all Web Shell client)
F4 Recording paragraph imprecise for ACP Nice to have (round 1) fixed chatRecordingService.ts doc comment and hooks.md now state the ACP projection/raw-text contract; unchanged behaviour re-confirmed by the 3279/3279 cli gate
N1 "A trusted parent cannot use a public key as a fallback" unpinned Suggestion (round 2) stands, with a correction The trusted-only fallback mutant (M6b) survives again at this head; the census bounding it re-confirmed (bridge strips both keys; channel bridge builds _meta from scratch and always marks a channel turn). But round 2's quoted mutant expression does not reproduce as a survivor — see Corrections
N2 Bridge !isPromotedMidTurn clause cannot decide any outcome today Nice to have (round 2) stands M10 survives again; census re-confirmed: single promotedMidTurn producer (bridge.ts:9354) builds its context without submittedPrompt
N3 acpAgent channel guard on re-stamp is redundant defence Nice to have (round 2) stands M8 survives again; upstream guards (bridge channelPrompt, HTTP/REST marker guards) re-confirmed pinned by M9/M11-class controls in round 2 and by the real-chain case C/D this round

No round-1 or round-2 finding is worsened or declined.

Central claim and A/B

Central claim. On the ACP/daemon path, UserPromptSubmit receives submitted_prompt only when a supported producer explicitly declares it; undeclared machine dispatch, forged private metadata, and wire-supplied public metadata never acquire it, and the value is never reconstructed from request text or a display label.

This round's A/B runs on the real production chain, closing round 2's largest named gap. No code on the tested path is mocked: the bridge is the real createAcpSessionBridge from the built dist; the channel is the real createSpawnChannelFactory, so the child is a real node packages/cli/dist/index.js --acp process (the same spawn the daemon uses, pid observed in its own stderr); the hook is a real command hook in a real QWEN_HOME/settings.json, executed by the child as a real subprocess; the oracle is the exact JSON that hook subprocess received on stdin. The model endpoint is deliberately a dead loopback port: the hook fires before the model round-trip, so a failing model call still leaves the hook input on disk (each sendPrompt rejects with JSON-RPC -32603 Internal error, details Connection error., as designed and caught).

case (real chain) head (PR polarity) denylist control (round-1 polarity in compiled Session.js)
A: trusted bridge context declares submittedPrompt hook input submitted_prompt = "why does the nightly build fail?" verbatim same (positive control for both arms)
B: undeclared machine dispatch absent (present=false) leaks "scheduled machine dispatch text"
C: forged qwen.daemon.submittedPrompt on the wire absent — bridge strips it before the child leaks "forged wire text" (forged key still stripped; fallback is the wire text)
D: qwen.submittedPrompt on the wire, no trusted context absent — wire key alone never establishes provenance leaks "public wire declaration"

Both arms exit green (10/10 each) because each arm's expectations match its own build; the A/B is the pair of observed payloads. The control arm's patch is one expression in the compiled Session.js (: '': (promptDisplayText ?? promptText)), sha256-verified applied and restored (62e98bdf…45378664…62e98bdf…). The child's own stderr independently shows the bridge re-injecting qwen.daemon.submittedPrompt from the trusted context on case A — a second, instrument-free corroboration of the hop. Witness: 03-real-chain-ab-head-absent-vs-denylist-leaks.png.

Core-path consumer oracle (round 2's unverified Test-Plan step). The core producer gate (packages/core/src/core/client.ts) attaches submitted_prompt only when messageType === SendMessageType.UserQuery, so a ToolResult or Hook send — the shapes a tool-result continuation produces — cannot carry it. Replaying exactly those payload shapes through the shipped integrations/external-context/dist/auto-recall.js against a real loopback generic-http-search-v1 provider: 0 outbound POST /v1/context/search for ToolResult, Hook, whitespace-only, and undeclared ACP shapes; 1 search with the declaration verbatim for the two declared positive controls. Witness: 01-core-path-toolresult-no-retrieval-vs-declared-search.png. This reproduces the shape the core path can emit plus the live consumer reaction; it is not a live multi-turn model loop (see Not covered).

REST-hop chain on a real tokenless daemon. A background research pass established that qwen serve on 127.0.0.1 with no token is trusted-loopback auth-free mode, so the whole REST surface is reachable credential-free. daemon-rest-chain.ts then boots a real daemon (node packages/cli/dist/index.js serve --port 0 --hostname 127.0.0.1 --no-web) against a loopback fake OpenAI server, with a real UserPromptSubmit command hook in a scratch QWEN_HOME, and drives four cases — each in its own session (the default scope attaches to an existing session, which silently merges cases), each awaited to turn_complete over the SSE stream, each oracle line filtered by session_id:

case over REST observed hook input
R1 _meta: {"qwen.submittedPrompt": "why does the nightly build fail?"} submitted_prompt = the declaration verbatim; exactly 1 hook fire although the turn made 5 model round-trips
R2 no _meta absent
R3 _meta: {"qwen.daemon.submittedPrompt": "forged"} absent — the route strips the private key
R4 _meta: {"qwen.submittedPrompt": …, "qwen.daemon.promptDisplayText": …} absent — the marker combination silently drops the declaration, and the prompt still returns 202

18/18 assertions. This closes the REST hop round 2 pinned only by mutation, end to end: REST route → runQwenServecreateAcpSessionBridge → real spawned qwen --acp child → Session → real hook subprocess. Witness: 04-rest-daemon-chain-declared-vs-undeclared-forged-marker.png.

Corrections

Round 2's N1 evidence sentence does not reproduce as quoted. Round 2 stated that M6 — "turning the ternary into meta[DAEMON_…] ?? meta[SUBMITTED_…]" — "survives both relevant tests green". Measured at this identical head, that exact expression is killed by two tests, with assertion diffs naming the security-relevant values: strips forged invocation metadata from an untrusted ACP client (pre-existing test the PR extended with 'qwen.daemon.submittedPrompt': 'forged submission'; under the mutant the spy receives the forged private key) and the PR's new admits an explicit public submission without trusting private ACP metadata (under the mutant the public declaration is dropped in favour of the forged private value). The mutant that genuinely survives is the trusted-only fallback — trusted ? (meta[PRIVATE] ?? meta[PUBLIC]) : meta[PUBLIC] — which is what N1's prose describes and what its census bounds; it survives again here (M6b). Consequence: N1 stands in its correct, narrower scope (the doc invariant about trusted parents remains unpinned), and the untrusted-forgery direction is in fact test-pinned by the PR's own tests — a stronger position than round 2's table implied. This is a correction of the previous report's evidence, not a defect in the PR. Witness: 02-mutation-remeasure-m6a-killed-m6b-m8-m10-survive.png.

Findings

R3-1 — Suggestion (non-blocking, documentation): the rewritten hooks.md core-path sentence under-enumerates by one send type

The PR rewrote the UserPromptSubmit purpose sentence to scope it per path: "On the core/headless path, the event currently covers UserQuery, ToolResult, and Hook sends, while Retry, Steer, Cron, Notification, and Teammate sends are skipped." SendMessageType has nine values and client.ts excludes six from the hook — the five named plus Goal (messageType !== SendMessageType.Goal). The "covers" half is exactly right; the "skipped" half omits one.

The omission predates this PR (the base sentence at HEAD^1 is identical, and client.ts is untouched by the diff), so this is not a regression. It is reported because the PR rewrote that very sentence, and because its own ACP half does name the analogue ("continue, restored-question, and runtime-goal turns do not"), making the asymmetry visible in one paragraph. Goal sends are precisely the machine-generated re-entries this PR's subject matter cares about.

Reproduce:

grep -n 'Teammate` sends are skipped' docs/users/features/hooks.md
grep -n 'messageType !== SendMessageType.Goal' packages/core/src/core/client.ts

Suggested fix (one word, doc-only): add "Goal" to the skipped list, or phrase it as "the remaining machine/re-entry send types". Not load-bearing; no code change implied.

R3-2 — Nice to have (completeness, seam to watch): a second in-process entry into Session.prompt bypasses the acpAgent sanitizer

packages/cli/src/ui/daemon/daemon-tui-adapter.ts:798 calls this.session.prompt({ prompt: promptBlocks }) directly, in-process, around QwenAgent.prompt()'s sanitize block. It passes no _meta at all, so at head it fails closed (no provenance); under the denylist polarity it would have leaked the typed text as provenance. Census: DaemonTuiAdapter has zero production importers — only its own test file and a comment reference in ToolGroupMessage.tsx — so this is not a live gap today. It becomes one the moment a caller wires it up with caller-controlled metadata, because that path would skip the admission choke point entirely. Recorded so the seam is not mistaken for covered surface; no change requested.

Bounded: what the new public declaration key is NOT (re-confirmed by census at this head)

Census of every writer/reader of both keys across packages/ and integrations/: public key producers = Web Shell actions.ts only (2 sites); ingresses that read it = REST prompt route, ACP HTTP dispatch, acpAgent admission; private key writers = bridge (from trusted context.submittedPrompt, after deleting both keys) and acpAgent re-stamp (after deleting both keys); private key readers = Session.ts:5306 only. Session.prompt has exactly two in-process callers (the sanitized acpAgent choke point, and the dead adapter above); the bridge has exactly one child-prompt door with the strip inside its single promptRequest builder; the channel bridge builds _meta from scratch and never sets either key. The scary readings still do not hold: no privilege escalation (every ingress already accepts arbitrary params.prompt from the same caller), no private-key forgery (all three ingresses delete it; real-chain case C), and the marker guards fail closed.

Not covered

  • A driven tool-execution continuation. The fake OpenAI server's tool-call shape never made the child send a tool-role message back (every model request in R1 ended with role user, across 5 round-trips), so no real tool result re-entered the turn in-container. The Test-Plan step is therefore covered by two independent measurements instead: the core producer gate makes a ToolResult send incapable of carrying the field (plus the live consumer oracle), and the REST chain shows exactly one UserPromptSubmit fire per turn even with five model round-trips. A real tool loop remains undriven here.
  • Per-commit attribution. The checkout is depth 2 (git rev-parse --is-shallow-repository = true); git rev-list HEAD^1..HEAD^2 returns 1 while the snapshot's commits array lists 5. Only the aggregate HEAD^1..HEAD diff was exercised.
  • Trial merge into current main, repo-wide typecheck/lint/bundle, and the unaffected workspaces' suites — the PR's own CI covers these; this round's budget went to the chain.
  • Web Shell client seams beyond the PR's own green tests (1280/1280 across the four touched files) — no real browser.
  • Windows execution paths; author-declared untested.
  • The daemon mechanics behind the REST chain (tokenless trusted-loopback auth on 127.0.0.1, QWEN_HOME user-level hook loading, the reusable integration-tests/fake-openai-server.ts) came from a background research agent whose report arrived after the bridge-level chain was already built and measured; the REST chain reuses those findings. No in-repo test combines daemon + hooks, so this combination is new coverage rather than a rerun.

Methodology

Environment: the CI verify container (node v22.23.2), merge-ref checkout at depth 2, npm ci + npm run build pre-run at HEAD; worktree clean before and after every mutating step. Real-chain A/B (e2e-chain.mjs + hook-dump.mjs): real createAcpSessionBridge + real createSpawnChannelFactory (child = node packages/cli/dist/index.js --acp, QWEN_HOME pointed at a scratch settings dir holding one UserPromptSubmit command hook that appends its stdin to JSONL); four cases per arm (declared / undeclared / forged private key on wire / public key on wire); arm selected by EXPECT_POLARITY, with the denylist arm produced by patching one expression in the compiled Session.js and sha256-verifying apply and restore. Core-path oracle (core-path-consumer-oracle.mjs): static-chain assertions over real source plus the shipped auto-recall.js spawned per row against a loopback provider recording method/path/auth/body; declared rows are built-in positive controls. Mutation re-measure (mutation-remeasure.mjs): six cells (M6a stronger unconditional-??, M6b trusted-only fallback, M7/M9 in-file killed controls, M8, M10), each with a uniqueness check, an apply-hash check, and a sha256-verified restore. i18n contract (i18n-key-contract.mjs): imports all nine locale tables and asserts key presence/removal/translation. Gates (logs/gate-targeted.log): the ten PR-touched test files across three packages. REST chain (daemon-rest-chain.ts, run under tsx): boots a real tokenless qwen serve on an ephemeral loopback port against integration-tests/fake-openai-server.ts, one session per case (sessionScope: 'thread', because the default scope attaches to an existing session), SSE turn_complete awaited per case, oracle = the hook subprocess's JSONL filtered by session_id. Harness defects found and fixed before counting: an early version of the REST oracle read the {pid, input} wrapper instead of .input, which made three absence assertions pass vacuously and one presence assertion fail — caught by the failing assertion and now guarded by prompt-presence checks on every case; and an early version reused one session across cases, so the per-session FIFO merged neighbouring turns' fires — caught by the session-scoped filter. Neither defective run contributes to assertions.json. Raw logs, harness sources and the four captures live in tmp/pr11455-verify-20260910-115121/ (logs/, evidence/). No scratch worktree remains and git status is clean.

Flakiness gate log

rounds=5 files=10 skipped=0
file packages/acp-bridge/src/bridge.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/bridge.test.ts
file packages/cli/src/acp-integration/acpAgent.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/acpAgent.test.ts
file packages/cli/src/acp-integration/session/Session.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/session/Session.test.ts
file packages/cli/src/serve/acp-http/transport.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/acp-http/transport.test.ts
file packages/cli/src/serve/server.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server.test.ts
file packages/cli/src/ui/components/hooks/constants.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/components/hooks/constants.test.ts
file packages/web-shell/client/App.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/App.test.tsx
file packages/web-shell/client/components/ChatPane.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/ChatPane.test.tsx
file packages/web-shell/client/daemon/session/actions.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/daemon/session/actions.test.ts
file packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/hooks/useQueuedPrompts.dom.test.tsx


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/acp-bridge/src/bridge.test.ts: PP
  packages/cli/src/acp-integration/acpAgent.test.ts: PP
  packages/cli/src/acp-integration/session/Session.test.ts: PP
  packages/cli/src/serve/acp-http/transport.test.ts: PP
  packages/cli/src/serve/server.test.ts: P
  packages/cli/src/ui/components/hooks/constants.test.ts: P
  packages/web-shell/client/App.test.tsx: P
  packages/web-shell/client/components/ChatPane.test.tsx: P
  packages/web-shell/client/daemon/session/actions.test.ts: P
  packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: P

verdict: timeout
summary: the 15-minute budget elapsed before two full rounds completed (1 done) — no flakiness signal either way

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/acp-http/transport.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/components/hooks/constants.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/ChatPane.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/daemon/session/actions.test.ts: P (exit 0)
round 1 · packages/web-shell/client/hooks/useQueuedPrompts.dom.test.tsx: P (exit 0)
round 2 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/session/Session.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/acp-http/transport.test.ts: P (exit 0)

Evidence images

01-core-path-toolresult-no-retrieval-vs-declared-search

02-mutation-remeasure-m6a-killed-m6b-m8-m10-survive

03-real-chain-ab-head-absent-vs-denylist-leaks

04-rest-daemon-chain-declared-vs-undeclared-forged-marker

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

LGTM — the Critical I requested changes on at 2b4f46a is fixed, and fixed by inverting the gate to a positive declaration rather than extending the denylist. This approval supersedes that review.

Verified at this head: all six bridge.sendPrompt call sites enumerated, and the four internal dispatchers (sub-session, Live task, Live session coordinator, standalone session) carry no context declaration and no _meta, so they cannot acquire provenance by construction. Both mint sites of the private-parent capability key counted independently. The full chain, three non-blocking items, and what remains unobserved are in the Stage 2 comment; my confidence and the contract sign-off I am leaving to a human are in Stage 3.

✅ Approving on the code. The widened public contract (a new externally-suppliable meta key, and submitted_prompt becoming reachable on ACP-path hook invocations) still deserves a maintainer's explicit yes — main wants two approvals.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

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.

⚠️ Downgraded from Approve to Comment: CI still running. Partially reviewed — gaps disclosed.

Not reviewed: test-efficacy probe — the mutation and hunk-necessity harness could not execute in this environment (harnessValidated: null, mutants not run, 46 hunks skippedForCap, 6 skippedForBaseline); a hand scan of every removed test line found no test weakened, disabled or deleted to let new behaviour through.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": did not walk the daemon-side promotion route for a mid-turn insert ( enqueueMidTurnMessage in packages/cli / packages/core) to check the design doc's "promoted…; "agent reverse-audit (round 2)": whether the --input-file remote-input submit path passes options.submittedPrompt — the deleted hooks.md line explicitly listed "remote-input" as a non-produ….

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.

17 Suggestion(s) were drafted inline past the resolved critical posting floor — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s); the CLI moved them into the deferral list below (floor enforcement).

Deferred under the convergence posture (round 4, not a blocker) — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s) — recorded, not requested in this round:

  • docs/design/daemon-user-prompt-submit-provenance.md:35 — [review] R2-1: (fix-induced) The round-3 fix closed what was reported — the link now targets only the predecessor design's Compatibility and migration section for the strict-decoder n…
  • docs/users/features/hooks.md:613 — [review] R4-1: The core/headless invocation list this sentence rewrites omits Goal sends, which client.ts also skips — and because the ACP clause added in the very same sentence explicitly names "runti…
  • docs/users/features/hooks.md:624 — [review] R4-11: This rewritten eligibility paragraph leaves two paths misclassified, and both are fixable in one editing pass. First, "Retries … omit it" is not true of a caller-supplied retry flag on a …
  • integrations/external-context-mem0/README.md:263 — [review] R4-14: This paragraph gains the ACP/daemon producer surface, but the third stale producer enumeration in this doc set — docs/design/external-context-mem0-auto-recall.md:108-113 , …
  • packages/acp-bridge/src/bridge.ts:10541 — [review] R4-15: The !isPromotedMidTurn conjunct in this guard — which enforces a documented hard requirement — is asserted by no test, while both of its sibling conjuncts are covered by the test t…
  • packages/acp-bridge/src/bridgeTypes.ts:977 — [review] R4-2: This new field is the only caller-populated member of BridgeClientRequestContext whose doc comment does not say where the value comes from, and its provenance is the inverse of e…
  • packages/cli/src/acp-integration/acpAgent.ts:6109 — [review] R4-3: The trusted arm of this admission ternary — the hop the entire daemon and Web Shell production path depends on — is exercised by no test anywhere in the repo; only the unt…
  • packages/cli/src/serve/acp-http/dispatch.ts:5875 — [review] R4-4: Two of this gate's suppression conjuncts — CHANNEL_PROMPT_META_KEY and DAEMON_PROMPT_DISPLAY_TEXT_META_KEY — have no test on the ACP transport, while the REST twin's equi…
  • packages/cli/src/serve/routes/session.ts:6989 — [review] R4-13: The declaration is admitted on typeof alone here, with no length bound at any of the four admission or re-arm sites, while the sibling caller-supplied trusted string on this …
  • packages/cli/src/serve/routes/session.ts:6992 — [review] R4-5: This provenance deny-list is hand-written twice — here over bound locals, and in packages/cli/src/serve/acp-http/dispatch.ts:5873-5878 over raw _meta lookups — with no share…
  • packages/cli/src/ui/components/hooks/constants.ts:247 — [review] R4-12: This in-product /hooks description still asserts absolutely that prompt is "the current model-bound prompt", which this round's hooks.md rewrite makes false on th…
  • packages/web-shell/client/daemon/session/actions.test.ts:29 — [review] R4-6: This new provenance test is nested inside describe('getConnectionAfterSessionClear', …) , a block whose subject is an unrelated pure function, while a describe('…
  • packages/web-shell/client/daemon/session/actions.ts:1125 — [review] R4-7: The _meta merge that carries the declaration together with inputAnnotations in one object is untested at both rewritten sites, so the combined shape — every @ -m…
  • packages/web-shell/client/daemon/session/types.ts:348 — [review] R4-10: One Web Shell user-submission boundary never declares provenance — the side-task initial prompt. packages/web-shell/client/components/artifacts/SideTaskPanel.tsx:304 …
  • packages/web-shell/client/hooks/useQueuedPrompts.ts:624 — [review] R4-9: This new fail-closed reset — dropping a stored declaration when a server restore rewrites a summary-only queued row's payload — has no test, so it is a guard with no w…
  • packages/web-shell/client/hooks/useQueuedPrompts.ts:1726 — [review] R4-16: The declaration is attached to the fallback row here and to the next-turn row at :2001 , but the mid-turn route that actually runs by default while the session is s…
  • packages/web-shell/client/hooks/useQueuedPrompts.ts:1725 — [review] R4-8: This restoreAdmission carry of the declaration — the only thing that keeps provenance on the mid-turn-rejection requeue — is asserted by no test in the repo. Delete…
中文说明

⚠️ 已从批准降级为评论:CI still running。 仅完成部分审查,审查缺口已披露。

未审查(原文为英文):test-efficacy probe — the mutation and hunk-necessity harness could not execute in this environment (harnessValidated: null, mutants not run, 46 hunks skippedForCap, 6 skippedForBaseline); a hand scan of every removed test line found no test weakened, disabled or deleted to let new behaviour through.

未审查(原文为英文):build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 2)"did not walk the daemon-side promotion route for a mid-turn insert ( enqueueMidTurnMessage in packages/cli / packages/core) to check the design doc's "promoted…"agent reverse-audit (round 2)"whether the --input-file remote-input submit path passes options.submittedPrompt — the deleted hooks.md line explicitly listed "remote-input" as a non-produ…

未审查:反向审计——在 5 轮的反审轮数上限内未收敛。

17 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论——发布下限因首次发现速率连续 2 轮未下降而提前生效;CLI 已将其移入下方延后清单(下限强制执行)。

收敛姿态下延后(第 4 轮,非阻断)——发布下限因首次发现速率连续 2 轮未下降而提前生效——已记录,本轮不要求修改:共 17 条(原文未翻译,列表见上方英文部分)。

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

@wenshao

wenshao commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Maintainer verification — real daemon, real hook, real browser

I built the PR head 46dc4382ed and its merge-base 2488d12d8b on Linux (the platform the PR marks as not tested) and ran both arms through the same harness: an isolated HOME, a real qwen serve daemon, the shipped integrations/external-context Auto Recall hook wired to a loopback context provider, and a scripted OpenAI endpoint. The hook records its raw stdin; the provider records every search. Nothing is mocked inside the product.

Verdict: the mechanism does what the PR says, on every path I could reach. I found one hardening gap (unbounded declaration size) and one UX consequence worth a conscious decision before merge. Neither blocks.

A/B over 16 prompt-admission cases

Aggregate: the UserPromptSubmit hook fired 18 times on each arm — identical invocation behaviour, so nothing regressed for existing hook consumers. Merge-base delivered submitted_prompt 0 times and made 0 provider searches; the PR head delivered it 7 times and made 7 searches, exactly on the declared, eligible, non-channel, non-retry turns.

That reproduces the reported problem (a configured Auto Recall profile is inert behind serve, on the exact release-shaped client in S1) and the fix.

The admission gates all hold under adversarial input. A forged private qwen.daemon.submittedPrompt (S3), a whitespace-only (S4), non-string (S5), empty (S13) or object (S14) declaration, a channel marker (S6), a display projection (S7), and a forged channel-worker authorization (S11) each produce no field and no search. S12 is worth calling out as correct rather than suspicious: forging qwen.daemon.privateParent does not promote the caller, so the declaration is still read from the public key and is still honoured — the private-parent state only chooses which key is read, and an unprivileged caller declaring on the public key is the documented opt-in.

S9 confirms the value is carried verbatim: " Original With Spacing and CASE " survives with its leading, trailing and interior whitespace intact, and only the hook's own sanitizer normalizes it into the provider query.

Focused probes

Web Shell, driven in real headless Chromium against a live daemon. The browser genuinely puts _meta: { "qwen.submittedPrompt": … } on POST /session/:id/prompt, the hook receives the composer text, and Auto Recall runs. This is the client half of the PR working end to end, not a unit-test assertion.

Direct qwen --acp. The documented per-request opt-in works on the non-daemon stdio transport too, and the private key is refused there as well — so the contract is transport-independent, which is what the docs now promise.

Genuine armed retry. This one needed staging to test honestly. A retry: true request on a session with nothing to retry is not classified as a retry (entry.retryAllowed is false), so it is admitted as an ordinary fresh turn and does carry the declaration — correct, and easy to misread as a leak. I failed a turn with a non-retryable upstream 400 so the daemon reported turnError and armed retry admission; the subsequent declared retry produced no submitted_prompt and no search. The declaration does cross the bridge and reach the child — Session's isFreshUserTurn is what drops it. Two independent gates, which is the right shape.

Tool-result continuation. S8 ran a real read_file round trip: three model requests, one hook invocation, one search. No re-retrieval on continuation.

Leakage. Neither qwen.submittedPrompt nor qwen.daemon.submittedPrompt appears anywhere in the captured outgoing model requests, and the recorded transcript stores the request text, not the declaration. The only thing that reaches the model is the provider's returned context, correctly wrapped in <qwen:user-prompt-submit-context> with the untrusted-data notice.

Finding — the declaration is unbounded

submittedPrompt is admitted on typeof value === 'string' alone. The neighbouring trusted field in the same admission block, modelPrompt, is validated by isValidTrustedModelPrompt at MAX_TRUSTED_MODEL_PROMPT_CHARS = 64 * 1024 and throws above it. The asymmetry runs the wrong way: the bounded field is internal-only ("HTTP routes never populate this from request input"), while the unbounded one is the externally supplied one.

Measured: 64 KiB, 1 MiB, 4 MiB and 8 MiB declarations are all accepted with 202, cross the daemon→ACP-child JSON-RPC hop, and land verbatim on hook stdin (8,389,016 bytes for the 8 MiB case). The ceiling is just express.json({ limit: '10mb' }).

This is authenticated, so it is not an unauthenticated DoS, and the bundled Auto Recall hook caps its own stdin at 1 MiB and bails — so nothing shipped breaks. But the PR's own risk note says "eligible text reaches every configured hook", and a third-party hook that does not bound its stdin inherits this. A MAX_SUBMITTED_PROMPT_CHARS mirroring the neighbouring constant, applied at the two route entry points, would close it in a few lines. I'd take it as a follow-up rather than a blocker.

Behaviour worth a conscious decision, not a bug

Typing into the Web Shell composer while a turn is running does not go through the prompt queue — the client sends it to POST /session/:id/mid-turn-message. The hook still fires for it, with no declaration, so Auto Recall does not serve it. That is consistent with the design doc ("promoted mid-turn messages must not acquire it automatically") and I am not asking to change it. But from the user's side it is the same act of typing a question and pressing send, and the recall behaviour silently differs. The useQueuedPrompts plumbing the PR adds is real and covered by its own tests — it just is not the path a user hits by typing during an active turn. Worth a sentence in the hooks doc so nobody debugs this later.

For the same reason, the "queue" row I could reach over REST (S15) is a second prompt queued behind a running turn, which is a genuine fresh user turn and correctly does carry the declaration.

Docs claim I checked because it was newly added

"A blocking result stops the affected invocation, including on these non-TUI paths." Confirmed: a hook returning {"decision":"block"} gated on submitted_prompt produced 0 model calls, while the same policy string appearing only in the request text (no declaration) let the turn through with 1 model call. That is what makes the field usable as a policy gate rather than telemetry, and it is worth having verified rather than assumed.

Scope I did not cover

Real cron/Live dispatcher execution, channel-worker restart races, per-workspace profile routing, and Windows. The exclusion of scheduled tasks, sub-session spawns and model-authored Live tasks is guaranteed structurally here — the field is opt-in per request, so a producer that does not declare cannot acquire it — but I did not exercise those producers.

Two unrelated environment notes so nobody chases them: packages/vscode-ide-companion fails to build on my box, and POST /session/:id/model returns 500 when the model id is a fake one. Neither touches this PR's diff.

Recommendation

Approve. The chain is real, the gates hold under forgery and malformed input, the invocation behaviour of existing hooks is byte-identical to merge-base, and the A/B is unambiguous about what the PR adds. I'd like the size bound as a follow-up and one doc sentence about the mid-turn path.

中文说明

维护者验证 —— 真实 daemon、真实 hook、真实浏览器

我在 Linux(PR 标注为未测试的平台)上分别构建了 PR head 46dc4382ed 与其 merge-base 2488d12d8b,两个 arm 跑同一套 harness:隔离的 HOME、真实的 qwen serve daemon、随仓库发布的 integrations/external-context Auto Recall hook(指向本地回环 context provider),以及脚本化的 OpenAI 端点。hook 记录它收到的原始 stdin,provider 记录每一次检索。产品内部没有任何 mock。

结论:机制在我能触达的每条路径上都与 PR 描述一致。发现一个加固缺口(声明长度无上限)和一个值得合并前明确取舍的 UX 后果,两者都不构成阻塞。

16 个准入场景的 A/B

聚合结果:UserPromptSubmit hook 在两个 arm 上各触发 18 次 —— 调用行为完全一致,既有 hook 消费方没有任何回归。merge-base 交付 submitted_prompt 0 次、provider 检索 0 次;PR head 交付 7 次、检索 7 次,恰好落在已声明、符合条件、非 channel、非 retry 的轮次上。

这同时复现了所报告的问题(在 S1 这个与已发布客户端完全一致的形态下,已配置的 Auto Recall profile 在 serve 后面确实不生效)和修复效果。

各准入门禁在对抗性输入下都成立:伪造私有键 qwen.daemon.submittedPrompt(S3)、纯空白(S4)、非字符串(S5)、空串(S13)、对象(S14)声明,以及 channel 标记(S6)、显示投影(S7)、伪造 channel-worker 授权(S11),均不产生字段、不触发检索。S12 需要特别说明它是正确而非可疑:伪造 qwen.daemon.privateParent 并不能提权,声明仍从公开键读取、仍被采纳 —— private-parent 状态只决定读哪个键,而非特权调用方用公开键声明本来就是文档规定的 opt-in 方式。

S9 证实取值是逐字携带的:" Original With Spacing and CASE " 的前导、尾随和中间空白全部保留,只有 hook 自身的 sanitizer 在生成 provider query 时做了归一化。

专项探针

Web Shell(真实无头 Chromium 驱动实时 daemon)。 浏览器确实在 POST /session/:id/prompt 上放置了 _meta: { "qwen.submittedPrompt": … },hook 收到输入框原文,Auto Recall 真实执行。这是 PR 客户端那一半的端到端验证,不是单测断言。

直连 qwen --acp 文档描述的逐请求 opt-in 在非 daemon 的 stdio 传输上同样有效,私有键在那里同样被拒绝 —— 说明这份契约与传输层无关,正是文档现在承诺的。

真正被 arm 的 retry。 这一项需要认真构造才能诚实地测。对一个没有可重试内容的会话发 retry: true 不会被判定为 retry(entry.retryAllowed 为 false),因此会作为普通新轮次准入、确实携带声明 —— 这是正确行为,但很容易被误读成泄漏。我用不可重试的上游 400 让一轮真正失败,daemon 报出 turnError 并 arm 了 retry 准入,随后携带声明的 retry 产生了submitted_prompt、零检索。声明确实穿过了 bridge 抵达子进程,是 SessionisFreshUserTurn 把它丢弃的。两道独立门禁,这个形状是对的。

工具结果续轮。 S8 跑了真实的 read_file 往返:三次模型请求、一次 hook 调用、一次检索。续轮不重复检索。

泄漏检查。 捕获到的出站模型请求中,qwen.submittedPromptqwen.daemon.submittedPrompt 均未出现;转录记录的是请求文本而非声明。唯一进入模型的是 provider 返回的上下文,且正确包裹在带"不可信数据"提示的 <qwen:user-prompt-submit-context> 中。

发现 —— 声明长度无上限

submittedPrompt 仅凭 typeof value === 'string' 就被准入。同一准入块中相邻的可信字段 modelPromptisValidTrustedModelPromptMAX_TRUSTED_MODEL_PROMPT_CHARS = 64 * 1024 处校验,超限直接抛错。这个不对称的方向是反的:有上限的那个是内部专用的("HTTP routes never populate this from request input"),没有上限的那个才是外部提供的。

实测:64 KiB、1 MiB、4 MiB、8 MiB 的声明全部以 202 被接受,穿过 daemon→ACP 子进程的 JSON-RPC 跳转,逐字落到 hook stdin 上(8 MiB 那次为 8,389,016 字节)。天花板只有 express.json({ limit: '10mb' })

这是需要鉴权的,因此不构成未授权 DoS;随仓库发布的 Auto Recall hook 也自行在 1 MiB 处截断并放弃,所以已发布的东西不会坏。但 PR 自己的风险说明写着"符合条件的文本会传递给每个已配置 hook",而不对 stdin 设限的第三方 hook 会继承这个问题。在两个路由入口加一个对齐相邻常量的 MAX_SUBMITTED_PROMPT_CHARS,几行即可关闭。我倾向把它作为后续项而非阻塞项。

值得明确取舍、但不是 bug 的行为

在一轮正在运行时往 Web Shell 输入框打字,走的不是提示词队列 —— 客户端发到 POST /session/:id/mid-turn-message。hook 仍会为它触发,但没有声明,所以 Auto Recall 不服务它。这与设计文档一致("promoted mid-turn messages must not acquire it automatically"),我不要求改动。但从用户视角看,这与打一个问题然后按发送是同一个动作,而召回行为却悄悄不同。PR 新增的 useQueuedPrompts 管线是真实且有自己的测试覆盖的 —— 只是用户在活跃轮次中打字时并不会走到那条路径。值得在 hooks 文档里补一句,免得日后有人为此排查。

同理,我通过 REST 能触达的"队列"那一行(S15)是排在运行中轮次后面的第二条提示词,它是真正的新用户轮次,正确地携带了声明。

因为是新增而特意核对的文档主张

"A blocking result stops the affected invocation, including on these non-TUI paths."已确认:一个基于 submitted_prompt 返回 {"decision":"block"} 的 hook 产生了零次模型调用;而同一策略字符串只出现在请求文本里(无声明)时,轮次放行并产生 1 次模型调用。这正是该字段能作为策略门禁而非仅仅遥测的原因,值得实测而非假定。

我没有覆盖的范围

真实 cron/Live 派发器执行、channel-worker 重启竞争、按 workspace 路由 profile,以及 Windows。定时任务、子会话派生和模型撰写的 Live task 的排除在这里是结构性保证的 —— 字段是逐请求 opt-in,不声明的生产者无法获得它 —— 但我没有实际驱动这些生产者。

两条与本 PR 无关的环境情况,避免有人去追:我这台机器上 packages/vscode-ide-companion 构建失败;POST /session/:id/model 在 model id 是伪造值时返回 500。两者都不涉及本 PR 的 diff。

建议

同意合并。链路是真实的,门禁在伪造与畸形输入下都成立,既有 hook 的调用行为与 merge-base 完全一致,A/B 对"这个 PR 增加了什么"给出了明确答案。希望把长度上限作为后续项补上,并为 mid-turn 路径补一句文档。

@doudouOUC
doudouOUC added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit cbd2cba Sep 10, 2026
106 checks passed
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.23.4.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants