-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(core): separate hook context from transcript display #7948
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1b83ebb
a5d71df
eca5832
10283d9
0f86770
b9eaf55
762ba8b
ee7ee23
3879b70
00d06c8
3052b25
f45208d
eab31c8
5e80f24
aa66dc1
891ee3c
8920e28
d808b4f
ad0d857
f90c51d
a4be4f6
d66130b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -634,6 +634,56 @@ When both fields are present, prompt-hook payloads contain overlapping text and | |||||||||||||
|
|
||||||||||||||
| Sequential UserPromptSubmit hooks can append `additionalContext` to `prompt`; `submitted_prompt` continues to represent the captured submission. Function hooks are trusted same-process code and are not constrained by an immutability guarantee. | ||||||||||||||
|
|
||||||||||||||
| When the final hook output contains non-empty `additionalContext`, Qwen first | ||||||||||||||
| sanitizes the value and then sends it to the model as a separate text part: | ||||||||||||||
|
|
||||||||||||||
| ```xml | ||||||||||||||
| <qwen:user-prompt-submit-context> | ||||||||||||||
| sanitized hook context | ||||||||||||||
| </qwen:user-prompt-submit-context> | ||||||||||||||
| ``` | ||||||||||||||
|
|
||||||||||||||
| The tag tells the model and transcript consumers that the part came from a | ||||||||||||||
| configured hook rather than from the user prompt. It is a provenance marker, | ||||||||||||||
| not authentication, authorization, or a general trust boundary. | ||||||||||||||
|
|
||||||||||||||
| For a `UserQuery` with this added context, the session JSONL record preserves | ||||||||||||||
| the model-bound parts, including the tagged part, and adds the following | ||||||||||||||
| `systemPayload`: | ||||||||||||||
|
|
||||||||||||||
| ```json | ||||||||||||||
| { | ||||||||||||||
| "displayText": "pre-hook display projection", | ||||||||||||||
| "hookContext": "sanitized hook context" | ||||||||||||||
| } | ||||||||||||||
| ``` | ||||||||||||||
|
|
||||||||||||||
| This two-field payload is written only for this kind of user-prompt record. | ||||||||||||||
| `hookContext` intentionally duplicates the tagged part so offline and | ||||||||||||||
| third-party consumers can identify its provenance without parsing model text. | ||||||||||||||
| `displayText` is the pre-hook display projection and never includes the hook | ||||||||||||||
| context. For a supported interactive TUI submission it is the raw composer | ||||||||||||||
| projection carried by `submitted_prompt`; ACP, headless, `serve`, SDK, remote | ||||||||||||||
| input, and other paths without that provenance record the expanded pre-hook | ||||||||||||||
| prompt instead. | ||||||||||||||
|
|
||||||||||||||
| Transcript display consumers treat `displayText` as this user-prompt projection | ||||||||||||||
| when `systemPayload.hookContext` is a string. For compatibility with released | ||||||||||||||
| `displayText`-only user-prompt records, a complete tagged context in the final | ||||||||||||||
| part after at least one other part is equivalent pairing evidence. Notification, | ||||||||||||||
| cron, and mid-turn records can also have `displayText`, but those values are | ||||||||||||||
| compact display labels and must not be substituted for their model-bound text | ||||||||||||||
| without that evidence. | ||||||||||||||
|
Comment on lines
+674
to
+676
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Suggestion] This sentence is ambiguous and contradicts first-party behavior: notification/cron/mid-turn records can never carry the pairing evidence defined above (hooks never fire for Cron/Notification — verified in
Suggested change
中文说明该句存在歧义且与第一方行为矛盾:notification/cron/mid-turn 记录永远不可能携带上文定义的配对证据( — qwen3.8-max via Qwen Code /review (v0.21.7) |
||||||||||||||
| Legacy bare-context records keep their model-bound display behavior because the | ||||||||||||||
| context cannot be separated reliably. For metadata-free records that use the | ||||||||||||||
| current tagged shape, compatibility consumers may remove the same complete | ||||||||||||||
| final tagged part; they must not infer that arbitrary tag-like user text is hook | ||||||||||||||
| provenance. | ||||||||||||||
|
|
||||||||||||||
| Sensitive prompt telemetry attributes, when enabled, and managed auto-memory | ||||||||||||||
| recall both use the pre-hook prompt. They do not include | ||||||||||||||
| `UserPromptSubmit`-added context. | ||||||||||||||
|
|
||||||||||||||
| **Output Options**: | ||||||||||||||
|
|
||||||||||||||
| - `decision`: "allow", "deny", "block", or "ask" | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] This overclaims the ACP/serve/SDK record shape. The spec says every
UserQuerywith added hook context preserves the tagged part and adds{displayText, hookContext}, but ACP-session turns record the raw pre-expansion input BEFORE the hook fires (Session.ts:3178, the onlyrecordUserMessagecall in that path) and send throughGeminiChat.sendMessageStreamdirectly, bypassingGeminiClient.sendMessageStream— the only placeuserPromptRecordPayloadis written. Verified at the reviewed commit: headless genuinely matches the claim, TUI matches viasubmitted_prompt, but an ACP/serve/SDK JSONL user record has neithersystemPayloadnor tagged part. — Failure scenario: in a serve/ACP/SDK session whose hook injectsadditionalContext, the model sees the hook context but the JSONL record holds only the raw input text, while this spec tells offline/third-party consumers both are present — a consumer built on the spec finds no provenance on ACP/serve-originated sessions and misattributes or discards those turns. — Suggested fix: scope the sentences to the surfaces that write the payload, or extend the ACP path to write it.中文说明
此处对 ACP/serve/SDK 的记录形状表述过度。规范声称每个带 Hook 上下文的
UserQuery都会保留标签 part 并附加{displayText, hookContext},但 ACP 会话轮次在 Hook 触发之前就已记录原始未展开输入(Session.ts:3178,该路径唯一的recordUserMessage调用),且直接经GeminiChat.sendMessageStream发送,绕过了GeminiClient.sendMessageStream——写入userPromptRecordPayload的唯一位置。已在被审提交上核实:headless 与描述相符,TUI 通过submitted_prompt相符,但 ACP/serve/SDK 的 JSONL 用户记录既无systemPayload也无标签 part。— 失败场景:在 Hook 注入additionalContext的 serve/ACP/SDK 会话中,模型看到了 Hook 上下文,但 JSONL 记录只有原始输入文本,而本规范告诉离线/第三方消费者两者都存在——按规范实现的消费者在 ACP/serve 来源的会话中找不到任何来源信息,会错误归因或直接丢弃这些轮次。— 建议修复:把表述限定到实际写入 payload 的界面,或扩展 ACP 路径使其写入 payload(见上方 suggestion,为前一种方案)。— qwen3.8-max via Qwen Code /review (v0.21.7)