feat(core): tag UserPromptSubmit hook context and record display provenance - #7956
Conversation
|
|
github-actions
Bot
added
the
review/self-reported
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with solid evidence. Issue #7940 documents the transcript pollution clearly — hook-injected Direction: aligned. Session transcript integrity is core to qwen-code's reliability, and the approach is isomorphic to two patterns already in-tree — Size: core paths touched ( Approach: the scope feels right. Write path (tag + record payload), read path (three-shape resume fallback), and the ACP-bridge transcript-replay projection are all needed to close the issue end-to-end. The new commits since the last review add the Risk: Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，证据充分。Issue #7940 清楚记录了会话记录污染——hook 注入的 方向：对齐。会话记录完整性是 qwen-code 可靠性的核心，方案与代码库中已有的两个模式同构。 规模：触及核心路径。生产逻辑约 285 行（core 146，CLI 44，acp-bridge 95）。测试约 598 行。文档约 88 行。远低于 500 行关注阈值。 方案：范围合理。新提交补充了 风险： 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
Code reviewThe implementation is clean and follows existing patterns closely. My independent proposal for this problem was the same shape — tag the injection, record a display projection, fix the downstream consumers — so the approach feels well-calibrated rather than over-engineered. Write path ( Tag helper ( Resume projection ( ACP-bridge transcript-replay ( ACP Session ( Recording service ( No critical blockers found. No AGENTS.md violations. Files changed (18 of 18 shown)
TestingCI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 All CI checks green on the current head. The ubuntu unit suite (which gates macOS/Windows and integration tests) passed. Sandboxed verification lanes ( Not verified: live TUI resume behavior (CI path — no local build); ACP session path end-to-end (no sandboxed lane available). 中文说明代码审查实现干净，紧密遵循现有模式。我的独立方案与此 PR 同构——标签注入、记录显示投影、修复下游消费者——方案校准得当。 写入路径（ 标签 helper（ Resume 投影（ ACP-bridge transcript-replay（新提交）：填补了原始 diff 的缺口。多模态安全—— ACP Session（ 未发现关键阻塞项。无 AGENTS.md 违规。 测试CI 在当前 head 上全绿。沙箱验证通道不可用——维护者可触发 未验证：实时 TUI resume 行为；ACP 会话路径端到端。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 5/5 — clean, well-scoped fix that follows existing patterns; CI is fully green on the current head, and the new commits close the transcript-replay gap that was the only open question from the previous pass. This is a textbook fix for a transcript-integrity bug. The problem is real (issue #7940, before/after evidence, maintainer-labeled The new commits since the last review are the strongest signal of quality: rather than waiting for a reviewer to flag the missing ACP-bridge transcript-replay path, the author proactively added it — with the same three-shape fallback, multimodal-safe part handling, and a narrow subpath export that respects the browser/Node boundary. The ACP Session test closes the high-risk-path coverage gap. My independent proposal for this problem was the same shape. I looked for a simpler path and didn't find one — the write-path fix, the read-path fallback, and the transcript-replay projection are all necessary, and the ACP consistency change is two lines. The security argument is solid: One minor note: the PR description mentions 中文说明置信度：5/5 —— 干净、范围合理的修复，遵循现有模式；CI 在当前 head 上全绿，新提交填补了上次审查中唯一悬而未决的 transcript-replay 缺口。 这是修复会话记录完整性 bug 的典范。问题真实（issue #7940，before/after 证据，维护者标记 新提交是最强的质量信号：作者主动补充了 ACP-bridge transcript-replay 路径，而非等待审查者指出。多模态安全、窄子路径导出、ACP Session 测试——全部到位。 安全性论证可靠。唯一的小注意：PR 描述中的 before/after JSON 提到了 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
E2E Test ReportSetup: headless run ( Before (bundle built from {"type":"user","systemPayload":null,"parts":["hello marker test","E2E injected context marker-7940"]}After (bundle built from this branch): the injected part is wrapped in the reserved tag and the record carries {"type":"user","systemPayload":{"displayText":"hello marker test AFTER","hookContext":"E2E injected context marker-7940"},"parts":["hello marker test AFTER","<qwen:user-prompt-submit-context>\nE2E injected context marker-7940\n</qwen:user-prompt-submit-context>"]}Unit coverage (all passing on macOS, Node 22):
|
|
@qwen-code /resolve |
|
@qwen-code /review |
| _Qwen Code review request accepted. Review is queued in [workflow run](https://github.com/QwenLM/qwen-code/actions/runs/30417538235)._ |
|
Qwen Code attempted to resolve merge conflicts but the run did not complete successfully. address-summary.mdMerge conflict resolution summary — PR #7956Root causeThe Goal runtime feature landed on Textual or semanticSemantic. Both sides modified the same The this.config.getChatRecordingService()?.recordUserMessage(
request,
goalPermit, // from main (2nd param)
injectedHookContext !== undefined
? { displayText: preInjectionPromptText, hookContext: injectedHookContext }
: undefined, // from PR (3rd param)
);What is load-bearing
What I could not verify
中文说明合并冲突解决摘要 — PR #7956根本原因Goal 运行时功能合入 文本冲突还是语义冲突语义冲突。 双方都修改了同一个方法，且在 hook 输出处理和消息记录方面有重叠的关注点。 关键约束
无法验证的内容
Check the workflow run for full logs. |
…enance UserPromptSubmit additionalContext was appended to the request as a bare text part and persisted verbatim, so hook-injected text was indistinguishable from user-authored text in the transcript, polluted resumed sessions, telemetry, and auto-memory recall queries. - Wrap injected context in a reserved <qwen:user-prompt-submit-context> tag (hook output already escapes angle brackets, so the tag cannot be forged from inside). - Record the pre-injection user prompt as systemPayload.displayText plus the injected string as hookContext on the user record; the model-bound message stays verbatim for faithful resume replay. - Use the pre-injection prompt text for telemetry prompt attributes and managed auto-memory recall. - Resume projection prefers displayText, strips a trailing whole-part tagged block when no payload exists, and leaves legacy bare-injected records unchanged. - Apply the same tag wrapping on the ACP session injection path, which already records the pre-injection prompt. Closes QwenLM#7940 Co-authored-by: Cursor <cursoragent@cursor.com>
Document the conflict-resolution constraint that promptText must be declared before the injection assignment, and the sole-part read-path guard that keeps a user-authored whole-tag message intact. Co-authored-by: Cursor <cursoragent@cursor.com>
0d3b1e7 to
2e277fa
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
Rebase follow-upAddressed the blocking review item:
Verification after rebase: DingTalk doc wasn’t readable from this environment (login wall), so if there are additional minor doc nits beyond the TDZ/sole-part notes above, please paste them (or drop them on the PR) and I’ll land a follow-up. |
Confirm the at_command branch still prefers payload.userText when a paired user record carries a trailing tagged hook-context part, and falls back to the tag-stripping projection only when userText is absent. Co-authored-by: Cursor <cursoragent@cursor.com>
Minor follow-upRead the DingTalk CR doc; the only non-blocking item was:
Addressed in
No further open items from that CR note. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
| const payload = record.systemPayload as { displayText?: string } | undefined; | ||
| if (payload?.displayText) { | ||
| return payload.displayText; |
There was a problem hiding this comment.
[Suggestion] This anonymous structural cast as { displayText?: string } | undefined can use the named UserPromptRecordPayload type this PR defines (re-exported from @qwen-code/qwen-code-core). This file already imports the sibling SlashCommandRecordPayload / AtCommandRecordPayload types for the same systemPayload narrowing (lines 15-16), so the named type matches local convention. — Concrete cost: if UserPromptRecordPayload.displayText is renamed or retyped later, this cast still compiles against the stale shape; payload?.displayText becomes undefined at runtime, the function falls through to the tag-stripping path, and resumed sessions display hook-injected text as user-authored — the exact regression this PR fixes. The named type surfaces the mismatch at compile time. The fix spans the import and the cast:
// add to the existing import from '@qwen-code/qwen-code-core' (lines 11-18)
SlashCommandRecordPayload,
AtCommandRecordPayload,
UserPromptRecordPayload,
// and narrow with the named type
const payload = record.systemPayload as UserPromptRecordPayload | undefined;中文说明
[建议] 这里的匿名结构断言 as { displayText?: string } | undefined 可以改用本 PR 定义的具名类型 UserPromptRecordPayload（已从 @qwen-code/qwen-code-core 再导出）。本文件已经为相同的 systemPayload 窄化导入了同类的 SlashCommandRecordPayload / AtCommandRecordPayload 类型（第 15-16 行），使用具名类型符合本文件既有约定。— 具体代价：若未来重构将 UserPromptRecordPayload.displayText 改名或改类型，此断言仍会针对过时的形状通过编译；运行时 payload?.displayText 变为 undefined，函数回退到标签剥离分支，resume 时会把 hook 注入的文本显示为用户输入——正是本 PR 要修复的回归。具名类型能在编译期暴露这种不一致。修复涉及 import 与断言两处：
// 加入到现有从 '@qwen-code/qwen-code-core' 的 import（第 11-18 行）
SlashCommandRecordPayload,
AtCommandRecordPayload,
UserPromptRecordPayload,
// 并用具名类型窄化
const payload = record.systemPayload as UserPromptRecordPayload | undefined;— qwen3.8-max-preview via Qwen Code /review
| parts = [ | ||
| ...parts, | ||
| { text: wrapUserPromptSubmitContext(additionalContext) }, | ||
| ]; |
There was a problem hiding this comment.
[Suggestion] This ACP-path additionalContext tag-wrapping has no test coverage. The existing UserPromptSubmit hook tests in Session.test.ts mock output: {} (no additionalContext) and never exercise this branch. — Concrete cost: if this call is accidentally reverted to the pre-PR bare { text: additionalContext }, no test fails; ACP sessions would inject bare hook context into the model-bound record and resume would display it as user-authored — the regression this PR fixes for the interactive path. Consider adding a test in the describe('UserPromptSubmit hook') block that mocks hookSpecificOutput.additionalContext and asserts the last sent part satisfies isUserPromptSubmitContextPartText.
中文说明
[建议] ACP 路径的这处 additionalContext 标签包裹没有测试覆盖。Session.test.ts 中现有的 UserPromptSubmit hook 测试 mock 的是 output: {}（不含 additionalContext），从未执行到这个分支。— 具体代价：若此调用被意外回退为 PR 之前的裸 { text: additionalContext }，没有任何测试会失败；ACP 会话会把裸 hook 上下文注入发给模型的记录，resume 时会把它显示为用户输入——正是本 PR 为交互路径修复的回归。建议在 describe('UserPromptSubmit hook') 块中新增一个测试，mock hookSpecificOutput.additionalContext，并断言发送的最后一个 part 满足 isUserPromptSubmitContextPartText。
— qwen3.8-max-preview via Qwen Code /review
Local runtime verification (maintainer review)I built PR head Verdict: the core change does what it says, is well-guarded, and I recommend merging. Two follow-ups below — one coverage gap that leaves the tag visible on the ACP/export surfaces, and one test-teeth improvement. 1. Core write path — all claims confirmed
Recorded record, PR build: {"type":"user",
"systemPayload":{"displayText":"hello marker-7956, what is 2+2?",
"hookContext":"INJECTED-CTX-7956 repo policy: always cite file paths."},
"message":{"parts":[{"text":"hello marker-7956, what is 2+2?"},
{"text":"<qwen:user-prompt-submit-context>\nINJECTED-CTX-7956 repo policy: always cite file paths.\n</qwen:user-prompt-submit-context>"}]}}Same run on the parent build: 2. Resume display — before/after in a real terminalBoth arms recorded the same prompt with the same hook, then were resumed with
I also confirmed model-history fidelity survives resume: after resuming the PR-recorded session and sending a second turn, the captured request still carries turn 1's tagged part and turn 2's own tagged part — the transcript replays what the model actually saw. Cross-arm compatibility on real recorded sessions:
And the read-path guards, driven through the compiled
I also tried to break the whole-part guard from the outside: submitting 3. Suites, lint, mutation testing
Mutation testing (revert a behaviour, re-run the PR's own suites):
Finding 1 — the tag is still shown as user text on the ACP / export surfaces (follow-up)The TUI is fixed, but
Reproduced on the PR build with a real ACP client doing Surfaces affected: Zed / VS Code companion / web-shell / daemon TUI ( Suggested minimal change (either in this PR or a fast follow-up): in Related: Finding 2 — one new test has no teeth (1-fixture fix)
it('prefers displayText over the tag-strip fallback', () => {
const items = buildUserItems({
type: 'user',
message: {
parts: [{ text: 'my prompt' }, { text: 'expanded extra' }, { text: tagged }],
},
systemPayload: { displayText: 'my prompt', hookContext: 'injected hook context' },
});
expect(items).toEqual([{ id: 1_001, type: 'user', text: 'my prompt' }]);
});I ran this: 40/40 pass on the PR as-is, and the "delete the displayText branch" mutant goes from SURVIVED to CAUGHT. Finding 3 — informational: the ACP write path never persists the hook context
This is pre-existing and not introduced here, so it shouldn't block the merge — but after this PR the two paths have genuinely different transcript semantics, and the design doc's fidelity rationale only holds on one of them. Worth a tracking issue alongside the 中文版本地运行时验证（维护者评审）我把 PR head 结论：核心改动确实做到了它声称的事情，边界守卫也扎实，建议合入。 另有两点后续项 —— 一处覆盖缺口（ACP / export 侧仍会把标签当用户文本展示），以及一处单测"咬合力"的改进。 1. 写路径 —— 全部claim得到确认
同一 prompt 在父提交上运行： 2. Resume 展示 —— 真实终端下的 before/after两个 arm 用相同 prompt、相同 hook 录制会话，再用 我还确认了 resume 后模型历史的保真性：恢复 PR 录制的会话并发第二轮后，抓到的请求同时携带第一轮的标签 part 和第二轮自己的标签 part —— 会话记录确实回放了模型真正看到的内容。 跨 arm 兼容性（真实录制会话）：
读路径守卫（在两个 arm 的编译产物上驱动 我还从外部尝试攻破整块守卫：在没有配置 hook 的情况下提交 3. 测试、lint 与变异测试
变异测试（反向改回某个行为，再跑 PR 自己的用例）：7 个变异体中 6 个被捕获；唯独"整体删除
发现 1 —— ACP / export 侧仍把标签当用户文本展示（后续项）TUI 已修好，但 已在 PR 构建上复现：用真实 ACP 客户端对 PR 录制的会话做 受影响面：Zed / VS Code companion / web-shell / daemon TUI（ 建议的最小改动（本 PR 内或紧随其后的跟进均可）：在 相关： 发现 2 —— 有一个新增用例没有咬合力（补一条 fixture 即可）
我实测过：该 fixture 在当前 PR 上 40/40 通过，而"删除 displayText 分支"的变异体从 SURVIVED 变为 CAUGHT。 发现 3 —— 信息性：ACP 写路径从不持久化 hook 上下文
这是既有问题、非本 PR 引入，不应阻塞合入；但本 PR 之后两条路径的记录语义确实产生了分歧，设计文档里的保真性论证也只在其中一条上成立。建议与已列为 out of scope 的 |
Omit the optional UserPromptRecordPayload third arg when no hook injected, so Goal admission spies expecting two args stay exact and CI client-goal.test.ts passes. Project plain UserPromptSubmit-augmented records through transcript-replay with the same displayText / trailing-tag strip fallback as the TUI, covering ACP/export surfaces. Strengthen the displayText preference fixture so it disagrees with the tag-strip path, and use the named UserPromptRecordPayload type in resume. Co-authored-by: Cursor <cursoragent@cursor.com>
Review follow-up (
|
CLI and acp-bridge Vitest configs already map goalWire/transcriptRecords to TypeScript sources; without the same alias the new package export fails import analysis and breaks dozens of CLI suites. Co-authored-by: Cursor <cursoragent@cursor.com>
Preferring UserPromptSubmit displayText previously returned early and skipped projectMessageParts, dropping multimodal inlineData. Rebuild parts so displayText replaces text while images keep their order. Co-authored-by: Cursor <cursoragent@cursor.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
— qwen3.8-max-preview via Qwen Code /review
| if (!replaced) { | ||
| nextParts.push({ text: displayText }); | ||
| } |
There was a problem hiding this comment.
[Suggestion] The !replaced fallback in withUserPromptDisplayText — an image-only record carrying systemPayload.displayText with no text part to replace — is not exercised by any test. — Concrete cost: the only image fixture (transcript-replay.test.ts:200) also contains text parts, so replaced is always true and this append-at-end branch ships untested; if the placement were wrong (a downstream consumer expecting text before images), the ordering regression would fail no test. Suggested fix: add a transcript-replay.test.ts case whose parts are [{ inlineData: { data: 'abc', mimeType: 'image/png' } }, { text: tagged }] with systemPayload: { displayText: 'my image prompt' }, asserting the projection emits the image part followed by the displayText part.
— qwen3.8-max-preview via Qwen Code /review
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.7-max via Qwen Code /review
| /** Sanitized additional context injected by the hook (without the tag). */ | ||
| hookContext?: string; |
There was a problem hiding this comment.
[Suggestion] hookContext is declared in UserPromptRecordPayload, written to every hook-augmented JSONL record, and asserted in tests — but has zero read sites in the entire codebase.
Concrete cost: this field enters the persisted transcript format with no consumer and no documented purpose. A future maintainer modifying the JSONL schema cannot determine whether hookContext is load-bearing (an offline tool reads it), redundant (recoverable from the tagged part in message.parts), or dead. Dropping it later risks a backwards-incompatible format change if an external consumer was added silently. Per AGENTS.md: "For every added field, grep its read sites, including outside the diff."
Either remove hookContext from the payload (the tagged context in message.parts is already machine-parseable via isUserPromptSubmitContextPartText), or add a comment on the interface explaining which downstream reader it serves.
中文说明
hookContext 在 UserPromptRecordPayload 中声明，写入每条 hook 增强的 JSONL 记录，并在测试中断言——但在整个代码库中没有任何读取点。
具体成本：该字段进入了持久化的转录格式，但没有消费者，也没有文档说明其用途。未来维护者修改 JSONL 模式时无法判断 hookContext 是否是关键的（外部工具读取它）、冗余的（可从 message.parts 中的标签块恢复），还是死代码。日后删除它可能会因为外部消费者的悄悄添加而导致向后不兼容的格式变更。按照 AGENTS.md："对于每个新增字段，grep 其读取点，包括 diff 之外的。"
建议从 payload 中移除 hookContext（message.parts 中的标签上下文已经可以通过 isUserPromptSubmitContextPartText 机器解析），或者在接口上添加注释说明哪个下游读取者使用它。
— qwen3.7-max via Qwen Code /review
|
@qwen-code /resolve |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge resolution summary — PR #7956Root causeMain commit Textual, not semanticThe two sides were only adjacent — they modify different imports used by unrelated functions. Resolution keeps both imports: import { wrapUserPromptSubmitContext } from '../hooks/user-prompt-submit-context.js';
import { DEFAULT_TOKEN_LIMIT, tokenLimit } from './tokenLimits.js';
Both symbols are verified exported ( What is load-bearingNothing order-sensitive. The only invariant: the merged What I could not verifyNo build/typecheck/tests were run (out of scope for this command). The auto-merged files were resolved by git, not by hand; I confirmed via 中文说明根因main 的提交 文本冲突，非语义冲突双方仅相邻，修改的是不同导入，分别被互不相关的函数使用。解决方案是保留两个导入（见上方代码块）： 关键依赖无顺序敏感性。唯一不变量：合并后的 未能验证的部分本命令不运行 build/typecheck/测试。自动合并的文件由 git 处理，非手工解决；我已通过 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
— qwen3.8-max-preview via Qwen Code /review
UserPromptRecordPayload.hookContext had no read sites; keep displayText only and recover injected text from the tagged message part. Also cover the image-only !replaced append path and simplify the recording guard. Co-authored-by: Cursor <cursoragent@cursor.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
Share stripTrailingUserPromptSubmitContextPart between TUI resume and ACP replay, assert ACP Session tags additionalContext, and lock telemetry to the pre-injection prompt text. Co-authored-by: Cursor <cursoragent@cursor.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 建议见行内评论。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
| const displayText = | ||
| payload && typeof payload['displayText'] === 'string' | ||
| ? payload['displayText'] | ||
| : undefined; | ||
| yield* this.projectMessageParts( |
There was a problem hiding this comment.
[Suggestion] The systemPayload → displayText extraction here is duplicated verbatim from the first if branch above (the goal_runtime/notification/cron/mid_turn_user_message arm). — Concrete cost: both branches must agree on exactly what counts as a displayText (a string field on an object systemPayload); if that rule is later refined in one branch (e.g. trimming or rejecting empty strings) and the other is missed, plain user records and subtype records will project display text by different rules, producing inconsistent replay output that no single-branch test would catch. The first branch also derives backgroundTask from the same payload, so a shared helper both branches read from captures the whole shared computation:
private getSystemPayload(record: TranscriptRecordInput) {
return isObjectRecord(record.systemPayload)
? record.systemPayload
: undefined;
}中文说明
此处 systemPayload → displayText 的提取逻辑与上方第一个 if 分支（goal_runtime/notification/cron/mid_turn_user_message 分支）逐字重复。— 具体代价：两个分支必须对“什么算 displayText”（对象 systemPayload 上的字符串字段）保持完全一致；若之后只在一个分支精化该规则（如去空白、拒绝空串）而漏掉另一个，普通 user 记录与 subtype 记录将按不同规则投影显示文本，产生不一致的 replay 输出，只覆盖单分支的测试无法捕获。第一个分支还从同一 payload 派生 backgroundTask，因此抽出两个分支都读取的共享 helper 可覆盖全部共享计算。
— qwen3.8-max-preview via Qwen Code /review
|
@qwen-code /takeover |
|
@qwen-code /triage |
|
🚫 Takeover not engaged: fork takeover requires the PR author to hold write access on this repository (author 中文说明🚫 未接管：fork 托管要求 PR 作者在本仓库持有 write 及以上权限（作者 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
已审查。 未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max-preview via Qwen Code /review
|
Released in v0.21.2. |
6 participants
What this PR does
When a UserPromptSubmit hook returns
additionalContext, the injected text is now appended to the model-bound request as its own part wrapped in a reserved<qwen:user-prompt-submit-context>...</qwen:user-prompt-submit-context>tag, instead of as bare text. Hook output already has angle brackets escaped, so the tag cannot be forged from inside the injected content, and user-authored text is never rewritten.The user record in the session transcript keeps the exact model-bound parts (so resume replays what the model actually saw), and additionally stores the pre-injection user prompt as
systemPayload.displayTextplus the injected string assystemPayload.hookContext— the same displayText separation already used for mid-turn and notification records. Telemetry prompt attributes and managed auto-memory recall now use the pre-injection prompt text instead of the augmented request.On the read side, the resume projection restores user turns through a three-shape fallback: new records prefer
displayText; records that carry the tag but no payload drop a trailing part only when that part is, in its entirety, a tagged block (and never when it is the sole part, which can only be user-authored); legacy records with bare injected text keep the current concatenation behavior. The ACP session path, which already records the pre-injection prompt, gets the same tag wrapping on its model-bound injection for consistency.Why it's needed
Injected hook context was persisted indistinguishably from what the user typed. Resumed sessions displayed hook-injected context as if the user had written it, offline transcript analysis could not separate the two, and downstream consumers had to strip vendor-specific markers with fragile regexes. Telemetry and memory-recall queries were also polluted by the injected text. The live TUI was unaffected (it renders the pre-hook input), which made the transcript pollution easy to miss.
Reviewer Test Plan
How to verify
additionalContext, e.g. a command hook echoing{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"extra context"}}.message.partsentry should be the injected context wrapped in<qwen:user-prompt-submit-context>tags, andsystemPayloadshould carrydisplayText(the original prompt) andhookContext.--resume/ session picker): the user turn should display only the original prompt text, not the injected context.<qwen:user-prompt-submit-context>...</qwen:user-prompt-submit-context>block is still displayed verbatim on resume (sole-part guard).Unit coverage: tag helper tests, client tests for tagged injection + recording payload + pre-injection recall query, recording-service payload tests, and resume-projection fixtures for all three record shapes.
Evidence (Before & After)
Headless run with a UserPromptSubmit hook injecting
E2E injected context marker-7940, then inspecting the recorded user record.Before:
{"type":"user","systemPayload":null,"parts":["hello marker test","E2E injected context marker-7940"]}After:
{"type":"user","systemPayload":{"displayText":"hello marker test AFTER","hookContext":"E2E injected context marker-7940"},"parts":["hello marker test AFTER","<qwen:user-prompt-submit-context>\nE2E injected context marker-7940\n</qwen:user-prompt-submit-context>"]}Tested on
Environment (optional)
Unit tests via vitest; headless JSONL evidence via the bundled CLI (
npm run bundle,node dist/cli.js -p ...).Risk & Scope
SubagentStartviacontextState) needs its own investigation; desktop/web transcript viewers can adoptdisplayTextin follow-ups.recordUserMessagegains an optional trailing parameter; the payload is only written when a hook actually injected context, and old records remain readable through the fallback.Linked Issues
Closes #7940
中文说明
本 PR 做了什么
当 UserPromptSubmit hook 返回
additionalContext时，注入文本现在会作为独立 part 追加到发给模型的请求中，并用保留标签<qwen:user-prompt-submit-context>...</qwen:user-prompt-submit-context>包裹，而不再是裸文本。hook 输出中的尖括号本就会被转义，因此注入内容无法伪造闭合标签；用户自己输入的文本不会被改写。会话记录中的 user 记录仍保存与模型所见完全一致的 parts（resume 必须回放模型实际看到的内容），同时把注入前的用户原文存入
systemPayload.displayText、注入串存入systemPayload.hookContext—— 与 mid-turn / notification 记录已有的 displayText 分离模式同构。遥测的 prompt 属性和自动记忆召回查询改用注入前的原文。读路径上，resume 投影按三种记录形态回退：新记录优先用
displayText；只有标签没有 payload 的记录，仅当末尾 part 整体是一个标签块时才剥离（且该 part 不是唯一 part —— 唯一 part 只可能是用户输入，保留原样）；旧的裸注入记录保持现有拼接行为不变。ACP 会话路径本就单独记录注入前原文，这里为一致性给它的模型侧注入也加上相同的标签包裹。为什么需要
hook 注入的上下文此前与用户输入无法区分地落盘：resume 时把注入内容当作用户输入显示，离线分析无法分离两者，下游只能用脆弱的正则剥离私有标记；遥测与记忆召回查询也被注入文本污染。交互式 TUI 因为用 pre-hook 输入渲染而不受影响，这正是该问题容易被忽视的原因。
审阅者验证方案
additionalContext的 UserPromptSubmit hook（例如 echo 上述 JSON 的 command hook）。message.parts的最后一个条目应是被标签包裹的注入内容，systemPayload应包含displayText（原文）与hookContext。<qwen:user-prompt-submit-context>...</qwen:user-prompt-submit-context>块时，resume 仍原样显示（唯一 part 守卫）。单测覆盖：标签 helper、client 的打标注入 + 录制 payload + 注入前召回查询、录制服务 payload、以及三种记录形态的 resume 投影 fixture。
风险与范围
SubagentStart经contextState）需要单独调查；desktop / web 端的记录查看器可在后续跟进采用displayText。recordUserMessage新增可选尾参；仅在 hook 实际注入时才写入 payload，旧记录通过回退逻辑保持可读。关联 Issue
Closes #7940