fix(telemetry): gate request_text/response_text on logPrompts - #11670
Conversation
When telemetry.logPrompts is false, skip serializing API request and response content into telemetry instead of emitting it to every sink. Also add request_text to the log-to-span bridge's sensitive-attribute denylist so traces-only telemetry cannot diverge from the native logger. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-issue-patrol/jmtx5dam9y8
|
Thanks for the PR — re-run at head Template looks good ✓ — every required heading is present and the Chinese translation is complete rather than abridged. Problem: observed, not theoretical, and now doubly evidenced. #11666 is open carrying Direction: the gate escalates telemetry to a maintainer mechanically, because what data leaves the product is a privacy contract and that is not a bot's call. I'm keeping that escalation, but it is not doubt about this change — it brings runtime behaviour into line with an already-documented setting rather than setting new policy, the project's own triage of #11666 prescribed precisely this direction, and with Size: core paths are touched, so the two-tier gate applies. 7 production logic lines ( Approach: scope feels right and is close to the minimum this fix could be. Gating inside the two private log helpers is the correct place: I verified there are exactly two production construction sites for Two things I'd have flagged last pass are now closed, and I want to record that rather than silently drop them:
Risk: no elevated risk signals — none of the changed files match the high-risk paths from the revert-history analysis (checked mechanically, not by eye). Moving on to code review. 🔍 中文说明感谢贡献 —— 本次是在 head 模板完整 ✓ —— 所有必需标题都在,中文翻译完整、没有省略。 问题: 是已观测到的缺陷,不是理论性加固,且现在有双重证据。#11666 处于 open,带有 方向: 按 gate 规则,telemetry 一律上报给维护者,因为「产品对外输出哪些数据」属于隐私契约,不该由 bot 决定。这一上报我保留,但它并不代表对本改动有疑虑 —— 它是让运行时行为与已有的、已文档化的设置对齐,而非制定新策略;项目自身对 #11666 的分诊也给出了完全相同的方向;且 规模: 触及核心路径,适用两级门禁。生产逻辑 7 行( 方案: 范围合理,基本就是这个修复的最小形态。在两个私有日志辅助方法内加门禁是正确位置:我核实过全仓库构造 上一轮我会提出的两点现已关闭,我明确记录下来,而不是静默略过:
风险: 无升级风险信号 —— 改动文件均未命中 revert 历史分析得出的高风险路径(机械核查,非目测)。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewWhat I'd have written first. Reading only the title and the "why", before opening the diff: gate the two serialisation points inside No Critical findings, and no AGENTS.md violations. The consumer audit is what makes the 7 production lines safe, so here it is in full rather than as an assertion:
One consistency point worth a follow-up, not a change here. Emitting Relatedly, the PR description says the fields "never reach the native OTLP log exporters" — true of the content, not of the key. The docs text in the diff ("contains request content only when One more non-blocking observation. Test evidencePer the skill's rules I did not build, run, or execute anything from this PR — the review is static, and test evidence comes from the PR's own CI via the API, the isolated CI at the reviewed head. The branch was pushed three times during this review, which cancelled the previous head's Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The cancelled runs on Two independent end-to-end verifications now agree, which is more than most privacy fixes get. Sandboxed Maintainer verification (
One correction to how the test count could be over-read, which the maintainer flagged and I'd rather repeat than let stand: the new Does all this still apply at the current head? I checked rather than assumed. What is not verified, and what would settle it. The unit suite and lint have no recorded result at Residual — measured, pre-existing, and now with a maintainer recommendation attached. With Also worth a release-note line, per the same verification: with 中文说明代码审查我原本会怎么写。 只看标题和「为什么需要」、尚未打开 diff 时,我的方案是:在 无 Critical 意见,也无 AGENTS.md 违规。 让消费者核查完整呈现(而不是作为一个结论断言),因为这 7 行生产代码的安全性正来自它:
一处值得后续处理、但不必在本 PR 中改的一致性问题。 发出 与此相关,PR 描述中说这两个字段「never reach the native OTLP log exporters」—— 对内容成立,对键不成立。diff 中的文档措辞("contains request content only when 另一条不阻塞的观察: 测试证据按 skill 规则,我没有构建、运行或执行本 PR 的任何代码 —— 审查是静态的,测试证据来自 PR 自身 CI(经 API)、隔离的 被审查 head 上的 CI。 审查期间分支被推送了三次,导致上一个 head 的
两份彼此独立的端到端验证现已结论一致,这对一个隐私修复来说是超出常规的证据强度。 沙箱 维护者验证(
有一处可能被过度解读的地方需要更正,维护者已指出,我宁愿复述也不愿让它留着:新增的 这些结论在当前 head 上是否仍然适用? 我核实了,而不是假定。 哪些未验证,以及什么能确立它。 单元测试与 lint 在 残留 —— 已实测、属既有问题,且现在附有维护者的建议。 在 PR head 上以 同一份验证还提出一点值得写进 release note:在 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 3/5 — the change is clean and its central claim is now proven twice end-to-end, including by a maintainer who calls it merge-ready. The cap is purely formal: telemetry is a human call by rule, no human has yet expressed that call as a review, and the merge is blocked by a stale bot artifact. Not doubt about the code. Stepping back. My independent proposal and this PR landed on the same design, and the reason is that there is only one sensible place to put this gate — I confirmed that rather than assuming it, since two production construction sites for these events exist repo-wide and both are inside the two helpers the PR touches. The gate is therefore complete by construction, not by luck. What changed my confidence is that this pass stopped relying on argument from source. Two independent end-to-end verifications now agree: the sandboxed All three of my prior reservations are closed, and I want to record that explicitly rather than let it read as if nothing moved:
Why I am still not approving — and it is now only mechanism, not judgement:
Lesser: unit suite and lint are still in progress at Two things I'd ask whoever merges this to carry forward, neither blocking:
What turns this into a merge: one formal approval from someone with CODEOWNERS standing, dismissal (or superseding) of the stale 中文说明Confidence: 3/5 —— 改动干净,其核心主张现已两次被端到端证明,其中一次来自一位称其「可以合并」的维护者。封顶纯属形式原因:按规则 telemetry 由人决定,而这一决定尚未以 review 的形式表达,且合并正被一条过期的 bot 产物阻塞。这不是对代码的疑虑。 退一步看。我独立的方案与本 PR 的设计一致,原因在于这个门禁只有一个合理位置 —— 这一点我是核实过的,不是假设:全仓库构造这两个 event 的生产代码只有两处,且都在本 PR 触及的两个辅助方法内。因此门禁是由结构保证完备,而非碰巧完备。 真正改变我信心的是,本轮不再依赖「从源码推导」。两份彼此独立的端到端验证现已结论一致:沙箱 我此前三条保留意见已全部关闭,我明确记录下来,以免读起来像什么都没变:
为什么仍然没有 approve —— 而且现在只剩机制问题,不是判断问题:
较小一点:单元测试与 lint 在 有两件事希望合并者带下去,均不阻塞:
怎样才算可以合并: 一位具备 CODEOWNERS 身份的人提交一次正式批准;dismiss(或取代)那条过期的 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
⏸️ Deferring to @zjunothing (assigned owner) and @wenshao (who has already verified this end-to-end and called it merge-ready). The deterministic resolver had nothing to pick from — Re-run at The sandboxed My own static review found no correctness, security, or regression issue: two production construction sites for these events exist repo-wide and both are gated; All three of my prior reservations are closed. The So the ask is now narrow and purely formal — two items, neither a code defect:
Smaller notes, none blocking:
Nothing is waiting on the author. Needs a human click, not more code. 中文说明⏸️ 转交 @zjunothing(PR 的 assignee)与 @wenshao(已对本 PR 做端到端验证并判定可以合并)。确定性解析器没有可选项 —— 本次是在 沙箱 我自己的静态审查未发现任何正确性、安全性或回归问题:全仓库构造这两个 event 的生产代码只有两处且均已加门禁; 我此前三条保留意见已全部关闭。 #11666 中 因此所求现在很窄、且纯属形式 —— 两项,均非代码缺陷:
较小几点,均不阻塞:
没有任何事情在等作者。需要的是人点一下,而不是更多代码。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory; 223 passed — this review observed 25527, 2054, 30788, 1016, 2000, 569, 7430 passed.
中文说明
Test Plan(非阻断):src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory; 223 passed — this review observed 25527, 2054, 30788, 1016, 2000, 569, 7430 passed。
— qwen3.8-max via Qwen Code /review (v0.23.3)
Prettier-formats the two new generateContent logPrompts tests so the Lint & Static gate passes, and adds two streaming counterparts driving generateContentStream so the request_text/response_text stripping invariant is pinned on the path that carries interactive traffic. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtxcihx2yk
Adding request_text to SENSITIVE_ATTRIBUTE_KEYS makes the retained set on log-to-span bridge spans larger than what two doc statements enumerate. Update both enumerations to four fields, carry the log_prompts_enabled condition into the api_request/api_response attribute reference, and widen the logPrompts descriptions to cover API request/response text. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtxcihx2yk
|
@qwen-code /triage |
|
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: 110 passed · 0 failed · 110 total Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:110 通过 · 0 失败 · 110 总计 抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #11670 — fix(telemetry): gate request_text/response_text on logPromptsVerdict: The central claim is proven load-bearing on three real telemetry destinations (outfile, native OTLP/HTTP logs over real sockets, traces-only log-to-span bridge): with 中文摘要
Central claim + A/BCentral claim. When Each cell is one real
Witnesses: Secondary claim (b), census. Grepping Corrections
FindingsOrdered by severity. None is a regression introduced by this PR; none blocks it. 1. Suggestion (pre-existing, scope):
|
| settings | SDK's own endpoint resolution | bridge spans on the wire |
|---|---|---|
only otlpTracesEndpoint set |
traces=…/v1/traces, logs=http://localhost:4317/v1/logs, metrics=http://localhost:4317/v1/metrics |
0 |
same + otlpEndpoint: '' |
traces=…/v1/traces, logs=none, metrics=none |
14 |
Cause: Config.getTelemetryOtlpEndpoint() returns telemetrySettings.otlpEndpoint ?? DEFAULT_OTLP_ENDPOINT (packages/core/src/config/config.ts:7795-7796, DEFAULT_OTLP_ENDPOINT = 'http://localhost:4317' at packages/core/src/telemetry/index.ts:13), and the bridge is created only in the else if (tracesUrl) branch of createHttpExporters (packages/core/src/telemetry/sdk-exporters-http.ts:56-62), i.e. only when logsUrl is falsy. So an operator who follows the docs ("bridge spans … used when HTTP traces are exported without a logs endpoint") and sets only otlpTracesEndpoint gets a native log exporter pointed at a default localhost endpoint instead of the bridge — and, pre-PR, would have been shipping request_text there. This bounds how much of the PR's second claim is reachable in practice; the denylist fix itself is correct and now measured (C9–C14). Pre-existing behavior, not caused by this PR.
Minor observation (non-blocking, no behavior impact)
responseText is still computed (extractResponseText, capped at 4096 chars) and the streaming path still consolidates responses before _logApiResponse discards the value. Gating one step earlier would skip that work when logPrompts: false; bounded memory either way. Noted only for completeness.
Not covered
- Per-commit attribution. Shallow clone (depth 2):
git rev-list HEAD^1..HEAD^2returns 1 commit while the metadata snapshot lists 3 (22aec43a,d04bb9e9,d04fb917), so only the PR head is locally reachable. Verified the aggregateHEAD^1..HEADdiff; per-commit claims were not individually exercised. - Trial merge into current
main. The snapshot'sbaseRefOid(78bbd9f55c5a8ba9aab96bad91f42e8c83025737) is not present locally (git cat-file -tfails) and differs from the merge ref's base (HEAD^1=28df8b8a…), somainhas moved since the merge ref was created. No network/token in this job → could not fetch, could not confirm a conflict-free merge or re-run the suite on merged main. - gRPC OTLP path (
otlpProtocol: 'grpc'): not exercised; the bridge does not exist on that path. - Repo-wide test suite: not run; targeted gates only (3 affected test files, typecheck, prettier, eslint on the changed files).
--resumereplay of the persistedui_telemetrymirror afterresponse_textis omitted: not exercised. The no-reader claim rests on the census; the mirror still carries token counts, which is what the replay consumes perusageHistoryService.- Interactive TUI mode: all cells are headless (
-p). The gates sit in a shared producer, so mode should not matter, but it was not measured. - The
thoughtSignature/encrypted_contentpolicy question the PR explicitly declares out of scope: not evaluated. - Windows/macOS: Linux container only.
- Two discarded harness iterations, kept as raw evidence:
matrix-v1-flawed-oracle.log(byte-substring oracle over OTLP payloads, which false-positives on the empty-AnyValuekey residue of finding 3) andmatrix-v2-c14-mislabelled.log(C14 expectation mislabelledbridge-cleaninstead ofbridge-leak). Both were corrected and re-run; only the final run feedsassertions.json.
Methodology
Environment: the CI verify container (node:22-bookworm), merge-ref checkout at depth 2, npm ci + npm run build already done at head. Artifact dir tmp/pr11670-verify-20260911-205902/ holds every harness (.mjs), raw logs (matrix.log, mutation.log, typecheck.log, bridge-reachability.log, sibling-head-lp*.log, mutation-*.json, results.json, gates.json, assertions-detail.json) and evidence/*.png.
Control construction. Base sources were taken with git show HEAD^1:<path> for exactly the two changed production files and compiled with the real npm run build -w packages/core (53 s). The head rebuild reproduced the CI-built head bytes exactly (diff -q identical for both modules), and .d.ts output is byte-identical across arms, so swapping only the two compiled .js modules is an exact arm switch; every cell re-asserts the landed arm by grepping the compiled module before spawning. Module sha256 (base 69d2f61c…/e3973cca…, head d5222129…/90937df9…, full values plus the realpath and build-reproduction checks in provenance.txt). Internal workspace links were asserted before trusting any control: readlink -f node_modules/@qwen-code/qwen-code-core → /__w/qwen-code/qwen-code/packages/core (the head tree), which is why the base arm swaps compiled modules in place rather than using a second worktree whose node_modules would resolve into the head tree.
How the harnesses drove the code. Each cell spawns the real CLI (node packages/cli/dist/index.js --no-chat-recording --yolo … --auth-type openai --openai-base-url …) with an isolated QWEN_HOME/cwd settings file, against a loopback OpenAI-compatible server that returns marker content (and, in the sibling probe, a tool call whose arguments carry a distinct marker). Destinations are real: telemetry.outfile (FileLogExporter JSON), a loopback OTLP/HTTP receiver for /v1/logs, and a traces-only receiver for /v1/traces feeding the log-to-span bridge. OTLP payloads here are JSON, so oracles parse resourceLogs…attributes / resourceSpans…attributes and judge values, not key substrings. Bridge cells hold the process alive past the bridge's 5 s unref'd flush tick (model-side delay) and assert that bridge spans actually reached the wire.
Vacuity/mutation. Six mutations reverted one guard at a time (plus a 1+3 combination row and a same-file positive control) in the TypeScript source and ran the two affected test files via vitest's JSON reporter; every red is quoted with its expected-vs-actual message in mutation.log / 05-mutation-matrix.png. Gate liveness was proven by planting a formatting break (prettier exit 1) and a type error (tsc exit 2) and restoring; the unit-test gate's liveness is the mutation matrix itself (M1/M2/M5 land in the mutated file). The working tree and packages/core/dist were verified clean / at head afterwards (git status --porcelain empty; diff -q against the head build).
Flakiness gate log
rounds=5 files=2 skipped=0
file packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/loggingContentGenerator/loggingContentGenerator.test.ts
file packages/core/src/telemetry/log-to-span-processor.test.ts: (cd packages/core) npx --no-install vitest run ./src/telemetry/log-to-span-processor.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: PPPPP
packages/core/src/telemetry/log-to-span-processor.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/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 1 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 2 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 2 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 3 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 3 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 4 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 4 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 5 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 5 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 1a": runtime confirmation that an undefined log attribute reaches the OTLP payload as { key: 'request_text', value: {} } — two node --input-type=module -e prob…; "agent 6c": confirming that @opentelemetry/core 's addAttribute drops undefined attribute values (i.e. that "excluded" is literally key-absent on the OTLP wire rather ….
Test Plan (not a blocker): src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory; 223 passed — this review observed 25528, 2054, 30788, 1016, 2000, 569, 7430 passed.
中文说明
未探索到全部深度(达到工具调用预算):"agent 1a":runtime confirmation that an undefined log attribute reaches the OTLP payload as { key: 'request_text', value: {} } — two node --input-type=module -e prob…;"agent 6c":confirming that @opentelemetry/core 's addAttribute drops undefined attribute values (i.e. that "excluded" is literally key-absent on the OTLP wire rather …。
Test Plan(非阻断):src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory; 223 passed — this review observed 25528, 2054, 30788, 1016, 2000, 569, 7430 passed。
— qwen3.8-max via Qwen Code /review (v0.23.3)
The prior "excluded if log_prompts_enabled is false" phrasing over-promised presence for response_text (it is also absent for internal prompt ids and for thought-only/tool-call-only turns) and misdescribed request_text (the key ships empty on native OTLP log export, and is only absent from telemetry.outfile and log-to-span bridge spans). Qualify the includeSensitiveSpanAttributes bridge-span list so prompt / request_text / response_text note they additionally require telemetry.logPrompts. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtxixz7myu
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not explored to full depth (tool budget reached): "agent 3a": verifying telemetry.md:645/648's claim that request_text / response_text are "empty on native OTLP log export" when logPrompts is false — node_modules/@ope…; "agent 3c": could not directly verify the "key is empty on native OTLP log export" sub-claim for request_text / response_text ( telemetry.md:645,648 ) — node_modules is…; "agent 2": could not execute the OTLP log exporter to observe the empty- AnyValue wire shape directly — node_modules is absent from this review worktree, so that one li…; "agent reverse-audit (round 1)": did not read the three remaining non-streaming response converters (packages/core/src/core/openaiContentGenerator/converter.ts:1294 and :1426, packages/core/src….
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Test Plan (not a blocker): src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
docs/users/configuration/settings.md:664 — [probe] logPrompts description widened in two of five user-facing places; settings.md:805 (the env-var row), settings.md:881 and the --telemetry-log-prompts help string still scope it to user promp…
Convergence: round 3 posted 4 inline comment(s), 3 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: docs/developers/development/telemetry.md (findings in rounds 1, 2; 3 more now). 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. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查,审查缺口已披露。
未探索到全部深度(达到工具调用预算):"agent 3a":verifying telemetry.md:645/648's claim that request_text / response_text are "empty on native OTLP log export" when logPrompts is false — node_modules/@ope…;"agent 3c":could not directly verify the "key is empty on native OTLP log export" sub-claim for request_text / response_text ( telemetry.md:645,648 ) — node_modules is…;"agent 2":could not execute the OTLP log exporter to observe the empty- AnyValue wire shape directly — node_modules is absent from this review worktree, so that one li…;"agent reverse-audit (round 1)":did not read the three remaining non-streaming response converters (packages/core/src/core/openaiContentGenerator/converter.ts:1294 and :1426, packages/core/src…。
未审查:反向审计——评审时间预算不足,未能开始第 4 轮。
Test Plan(非阻断):src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 3 轮发布了 4 条行内评论,其中 3 条是首次提出;上一轮发布了 4 条(其中 4 条首次提出)。发现反复回到同一批文件:docs/developers/development/telemetry.md(第 1、2 轮已出过发现,本轮又有 3 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.23.3)
…t findings Address four review findings on the logPrompts gating of request_text/response_text: - R1-1/R2-1: rewrite the api_request/api_response attribute docs so the three suppression causes (internal prompt id, no visible text, logPrompts off) all collapse to "key present but empty on native OTLP log export, absent from telemetry.outfile and log-to-span bridge spans", and note that bridge spans additionally require includeSensitiveSpanAttributes for both keys. - R2-2: record that opaque thoughtSignature provider payload is exported verbatim with request_text, with the policy decision tracked in #11682. - R3-1: add loggers.test.ts assertions that request_text/response_text are present-but-undefined keys when logPrompts is off, and a FileLogExporter test that undefined-valued keys are dropped from the outfile JSON. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtxpdgjtz5
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 1a": end-to-end OTLP wire probe — a LoggerProvider harness against a local collector to observe whether request_text: undefined really arrives as a present-but-emp…; "agent 5": end-to-end verification that the OTLP exporter renders an undefined-valued log attribute as a present-but-empty key (the @opentelemetry/otlp-transformer AnyV…; "agent 1c": the doc clause "opaque thoughtSignature provider payload is exported verbatim as part of request_text " is unverified — I confirmed request_text is JSON.s…; "agent 1c": the three new tests were not executed (no node_modules in this review worktree); their pass/fail is reasoned from source only.; "agent 1c": did not verify that GitHub issue #11682 exists or concerns thoughtSignature policy, as the doc line asserts., and 2 more.
Test Plan (not a blocker): src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory; 223 passed — this review observed 25530, 2054, 30788, 1016, 2000, 569, 7430 passed.
Convergence: round 4 posted 5 inline comment(s), 5 of them reported for the first time; the previous round posted 4 (3 new). Findings keep coming back to the same files: docs/developers/development/telemetry.md (findings in rounds 1, 2, 3; 2 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.)
中文说明
未探索到全部深度(达到工具调用预算):"agent 1a":end-to-end OTLP wire probe — a LoggerProvider harness against a local collector to observe whether request_text: undefined really arrives as a present-but-emp…;"agent 5":end-to-end verification that the OTLP exporter renders an undefined-valued log attribute as a present-but-empty key (the @opentelemetry/otlp-transformer AnyV…;"agent 1c":the doc clause "opaque thoughtSignature provider payload is exported verbatim as part of request_text " is unverified — I confirmed request_text is JSON.s…;"agent 1c":the three new tests were not executed (no node_modules in this review worktree); their pass/fail is reasoned from source only.;"agent 1c":did not verify that GitHub issue #11682 exists or concerns thoughtSignature policy, as the doc line asserts.,另有 2 条。
Test Plan(非阻断):src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory; 223 passed — this review observed 25530, 2054, 30788, 1016, 2000, 569, 7430 passed。
收敛情况:第 4 轮发布了 5 条行内评论,其中 5 条是首次提出;上一轮发布了 4 条(其中 3 条首次提出)。发现反复回到同一批文件:docs/developers/development/telemetry.md(第 1、2、3 轮已出过发现,本轮又有 2 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.23.3)
|
⏳ Historical-head review — head moved to 中文说明⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 |
…ge gating docs - loggers.test.ts: drop the inert getTelemetryLogPromptsEnabled overrides; the two present-but-empty tests now name and pin the native OTLP log export leg (key present with an empty value) instead of a flag neither function reads. - file-exporters.test.ts: drive FileLogExporter.export() with a real ReadableLogRecord rather than poking the inherited private serialize. - telemetry.md: log-to-span bridge spans keep function_args under includeSensitiveSpanAttributes alone, plus prompt/request_text/response_text only when logPrompts is also enabled. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtxvsxv1ze
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
Not explored to full depth (tool budget reached): "agent 1a": whether the OTel logs SDK keeps undefined -valued attributes on the emitted ReadableLogRecord (i.e. whether the OTLP/JSON exporters omit request_text entir…; "agent 1a": a full packages/core unit-test run — I ran only the four touched test files plus contentGenerator.test.ts and loggers.test.ts , so an unrelated core test t….
Test Plan (not a blocker): src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory; 223 passed — this review observed 25525, 2054, 30787, 1016, 2000, 569, 7430 passed.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
docs/users/configuration/settings.md:666 — [probe] Two rows describing telemetry.includeSensitiveSpanAttributes now disagree: this diff qualified the env-var row at :807 on logPrompts but left the settings row at :666 promising bridge-span …
Convergence: round 5 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 5 (5 new). Findings keep coming back to the same files: docs/developers/development/telemetry.md (findings in round 4; 2 more now). 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. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查。 建议见行内评论。
未探索到全部深度(达到工具调用预算):"agent 1a":whether the OTel logs SDK keeps undefined -valued attributes on the emitted ReadableLogRecord (i.e. whether the OTLP/JSON exporters omit request_text entir…;"agent 1a":a full packages/core unit-test run — I ran only the four touched test files plus contentGenerator.test.ts and loggers.test.ts , so an unrelated core test t…。
Test Plan(非阻断):src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory; 223 passed — this review observed 25525, 2054, 30787, 1016, 2000, 569, 7430 passed。
收敛姿态下延后(第 5 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 5 轮发布了 3 条行内评论,其中 3 条是首次提出;上一轮发布了 5 条(其中 5 条首次提出)。发现反复回到同一批文件:docs/developers/development/telemetry.md(第 4 轮已出过发现,本轮又有 2 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.23.3)
|
@qwen-code /triage |
|
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: 131 passed · 0 failed · 131 total Flakiness gate: ✅ 3 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:131 通过 · 0 失败 · 131 总计 抖动门:✅ 3 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #11670 — fix(telemetry): gate request_text/response_text on logPromptsVerdict: This is a follow-up round. The previous round verified head The central claim is proven load-bearing on three real telemetry destinations at the new head. The sharpest single cell, on the real OTLP/HTTP wire with 中文摘要
Previous-finding status at the new headRe-measured, not diffed. "Stands" means the behaviour was reproduced at
Central claim + A/BCentral claim. When Each cell is one real Destination 1 —
|
| cell | arm | logPrompts | oracle (what the file actually contains) | result |
|---|---|---|---|---|
| OF-base-lpOFF | base | false | request_text PRESENT, response_text PRESENT; 71,250 B |
LEAKS (control) |
| OF-head-lpOFF | head | false | both keys ABSENT; 54,303 B | clean |
| OF-base-lpON / OF-head-lpON | both | true | both keys present, attribute key sets byte-identical | no regression |
The 16,947-byte drop between the two lpOFF cells is the removed conversation content.
Destination 2 — native OTLP/HTTP /v1/logs (real loopback socket)
| cell | arm | logPrompts | request_text value on the wire |
response_text value |
|---|---|---|---|---|
| OT-base-lpOFF | base | false | [{"role":"user","parts":[{"text":"<system-reminder>… — 6,829 chars (2nd record 9,541) |
SENSITIVE_RESPONSE_MARKER_OT-base-lpOFF |
| OT-head-lpOFF | head | false | <EMPTY AnyValue> |
<EMPTY AnyValue> |
| OT-base-lpON | base | true | 6,826 / 9,531 chars | marker present |
| OT-head-lpON | head | true | 6,826 / 9,531 chars — identical to base | marker present |
Witness: 01-otlp-wire-ab-base-leaks-head-empty.png. Note the base leak is not just "conversation text" — it includes the system-reminder block, i.e. prompt scaffolding the operator never typed.
Destination 3 — traces-only log-to-span bridge
Oracle is the log.bridge attribute the processor itself sets (log-to-span-processor.ts:191), so bridge spans cannot be confused with native trace spans. Every cell asserts a validity control (bridge spans reached the wire = 14, bridge produced api_request spans = 2) on both arms, so "absent" cannot pass vacuously.
| cell | arm | logPrompts | sensitive | api_request bridge spans carrying request_text |
result |
|---|---|---|---|---|---|
| BR-base-lpON-sensOFF | base | true | false | 2 of 2 | LEAKS (control) — the denylist gap this PR closes |
| BR-head-lpON-sensOFF | head | true | false | 0 of 2 | stripped — secondary claim (a) proven |
| BR-base-lpON-sensON | base | true | true | 2 of 2 | retained |
| BR-head-lpON-sensON | head | true | true | 2 of 2 | retained — docs claim holds |
| BR-base-lpOFF-sensON | base | false | true | 2 of 2 | LEAKS (control) — base got zero protection from logPrompts: false here |
| BR-head-lpOFF-sensON | head | false | true | 0 of 2 | clean (producer gate) |
Witness: 02-outfile-and-bridge-ab.png.
Secondary claim (b), census — widened past the previous round
Grepping every property access of the two fields across packages/*/src finds no non-test reader: only the producers (types.ts:290, types.ts:412), the OTLP attribute writer (loggers.ts:640-641), and the bridge denylist (log-to-span-processor.ts:64). Every other hit is a test file.
The previous round left the persisted ui_telemetry mirror uncovered, so I closed it: logApiResponse does hand the event to recordUiTelemetryEventToChat → chatRecordingService.recordUiTelemetryEvent (chatRecordingService.ts:2458), which persists { uiEvent } verbatim into the chat record. That mirror does have readers — but the only one touching uiEvent is the session-id remap at sessionService.ts:4587, which reads prompt_id alone and spreads the rest through unchanged. Nothing reads response_text off disk. Claim (b) holds; this is a census, not an end-to-end --resume run (see Not covered).
The delta since the previous round (commits 4–8)
Scoped new probes to what actually changed: one new test file and docs wording.
The delta's only new test is not vacuous. file-exporters.test.ts gained FileLogExporter omits undefined-valued attributes (logPrompts off). Mutating the mechanism its comment names — making safeJsonStringify's replacer return null for undefined — turns exactly that test red with expected '{\n "body": "api response",…' not to contain 'request_text' (V1, 03-mutation-matrix-zero-survivors.png). The unmutated control is green (4/4).
The delta also removed tests, so I checked for lost coverage. Commit 8fb0f8d9 added loggers.test.ts assertions pinning "present-but-undefined keys"; head commit 76de1e23 ("avoid pinning exporter empty-key internals") removed them — loggers.test.ts is absent from the final diff. Measured consequence: the mutation matrix still has 0 survivors and both positive controls still fire, so no guard lost coverage. What was lost is a pin on finding 3's informational empty-key residue. I agree with the removal: {"key":"request_text","value":{}} is an OTLP serialization detail of every undefined optional attribute (subagent_name behaves identically in the same payload), and pinning it would freeze a wire-format accident. Recording it here so the unpinned behaviour is a known choice rather than a silent gap.
Docs claims in the delta, tested rather than read.
- "opaque
thoughtSignatureprovider payload is included" — true by construction and confirmed:request_textisJSON.stringify(this.toContents(req.contents)), andthoughtSignatureis an own enumerable part property, so it serializes verbatim (JSON.stringifyof a part carrying it reproduces the payload). - bridge bullet: "keep their existing
function_argsfield, plusprompt,request_text, andresponse_textwhenlogPromptsis also enabled" — accurate, and the separation is the subtle part:promptis gated (loggers.ts:271),function_argsis not (logToolCallhas no gate, confirmed by finding 1's measurement). The sentence correctly putsfunction_argsoutside thelogPromptscondition. - "carries no content for internal prompt ids" — consistent with the call sites:
logApiRequestruns only insideif (!isInternal)(loggingContentGenerator.ts:436,:602). - "tracked in Track thoughtSignature/encrypted_content telemetry-export policy decision (#11666 second clause) #11682" — an issue reference; not verifiable without network (see Not covered).
Suggestion (nit, test naming). The new test's name promises a logPrompts scenario but its fixture is a hand-built ReadableLogRecord with request_text: undefined — no Config, no getTelemetryLogPromptsEnabled, no LoggingContentGenerator. The comment above it is honest about this ("when logPrompts is off the producer emits…"), and the assertion is genuinely load-bearing, so this costs nothing functionally; the name just buys a little more confidence than the fixture pays for. A name like omits undefined-valued attributes (as produced when logPrompts is off) would match. Not blocking.
Mutation matrix
One row per guard, plus a combination row, plus two positive controls landed in the same file as the mutant, plus the vacuity check on the delta's new test. Every red is the intended assertion with a behavioural expected-vs-actual message — no red came from a broken import or fixture.
| id | what was reverted / perturbed | suite | red | quoted failure |
|---|---|---|---|---|
| M0 | (none — unmutated control) | all 3 files | 0 / 142 | green, as required |
| M1 | guard #1: request_text producer gate |
lcg | 2 / 83 | expected '[{"role":"user","parts":[{"text":"SEN…' to be undefined |
| M2 | guard #2: response_text producer gate |
lcg | 2 / 83 | expected 'SENSITIVE_RESPONSE_MARKER' to be undefined |
| M3 | guard #3: 'request_text' denylist entry |
l2s | 1 / 55 | expected { Object (request_text, …) } to not have property "request_text" |
| M4 | combination M1 + M3 together | both | 3 / 138 | union of M1 and M3 — the guards are independent, no layered defence hiding either |
| M5 | positive control, same file as M3: drop the pre-existing 'response_text' entry |
l2s | 1 / 55 | expected { …(4) } to not have property "response_text" |
| M6 | positive control, same file as M1/M2: MAX_RESPONSE_TEXT_LENGTH 4096 → 4095 |
lcg | 1 / 83 | expected 'xxx…' to have a length of 4096 but got 4095 |
| V1 | vacuity check on the delta's new test: safeJsonStringify replacer undefined → null |
fx | 1 / 4 | expected '{\n "body": "api response",…' not to contain 'request_text' |
Survivors: 0. Both positive controls went red, so the harness can make these suites fail; the unmutated control is green, so the kills mean something. M1's message reproduces the PR body's own claimed "Before" evidence verbatim. Working tree verified clean after every restore (git status --porcelain empty), and packages/core/dist left at head (d5222129…).
Corrections
- The PR body's test count is stale (carried forward). The body reports
# 223 passed (3 files); measured at the verified head: 225 passed (3 files). Description accuracy only — the tests pass. log_prompts_enabledis not an attribute (carried forward, now wider). The delta's docs wording writescontains request content only when `log_prompts_enabled` is true; no attribute or setting by that name exists anywhere inpackages/**(grep: docs only, now 3 occurrences). This is a pre-existing docs convention — already used forpromptattelemetry.md:621— that the PR extends consistently, so matching it is defensible. Flagged so the next reader does not grep for a field that does not exist.- Confirming, not correcting: the body's claim that the two fields "never reach the native OTLP log exporters, the traces-only log-to-span bridge, or
telemetry.outfile" is true on all three destinations at the new head, and the no-internal-consumer claim holds (census above, now including the persistedui_telemetrymirror). - One correction to the previous round's framing of finding 4, in the direction of worse: it reported the consequence as "gets a native log exporter pointed at a default localhost endpoint instead of the bridge". Measured with the
log.bridgeoracle, the traces-only shape yields 0 bridge spans and 0 log records delivered to any reachable receiver — the log telemetry is silently dropped, not merely rerouted. Still pre-existing, still not caused by this PR, but it bounds the PR's second claim more tightly than the previous round stated.
Findings
Ordered by severity. None is a regression introduced by this PR; none blocks it. Findings 1, 2 and 4 are re-measurements of carried-forward items and are tabulated above; the substance is restated here only where the new head changed it.
1. Suggestion (pre-existing, scope): logPrompts: false still exports verbatim tool arguments via tool_call.function_args
Same bug class, adjacent door — and the door the PR's own motivation names ("tool arguments/results, file content"). At head with logPrompts: false, in a single run where api_request correctly has no request_text key:
tool_call record: function_name="run_shell_command"
function_args="{\n \"command\": \"printf '%s\\n' 'SENSITIVE_TOOL_OUTPUT_MARKER'\",\n \"description\": \"SENSITIVE_TOOL_ARG_MARKER\"\n}"
Cause: logToolCall (loggers.ts:302) spreads the normalized event and sets function_args: safeJsonStringify(...) with no gate; shouldLogUserPrompts has exactly one call site (loggers.ts:271). function_args is in the bridge denylist, so only the native-log and outfile paths carry it. Base arm is identical → pre-existing. Repro: node tmp/pr11670-verify-20260912-073825/probes.mjs (section A), witness 04-sibling-tool-args-still-ungated.png. Worth naming for whoever closes #11666, since the issue's stated symptom survives through a different key.
2. Suggestion (pre-existing, scope): subagent_execution.result still exports model output text
The residual-carrier scan reports, for every head logPrompts: false cell, exactly which attributes still carry the model's response marker: qwen-code.subagent_execution.result on the outfile and OTLP legs, and additionally the native GenAI span attributes (interaction.gen_ai.output.messages, llm_request.gen_ai.input.messages, llm_request.gen_ai.output.messages) on the bridge leg with includeSensitiveSpanAttributes: true. Each is asserted present on the matching base cell too, so none is introduced here. Cause: logSubagentExecution has no logPrompts gate and result is not in SENSITIVE_ATTRIBUTE_KEYS.
3. Informational: the native OTLP leg keeps an empty-valued key, and nothing pins it any more
At head with logPrompts: false: request_text present=2 empty=2, response_text present=2 empty=2 — {"key":"request_text","value":{}}. No content crosses (the census finds no marker in any api_request/api_response value), and this matches how every undefined optional attribute already serializes (subagent_name in the same payload). The outfile sink drops the key entirely, which is what the delta's new test pins. Practical consequence only: a wire audit grepping for the key name still sees it. Now unpinned by any test after 76de1e23 — deliberately, and I agree (see "The delta").
4. Informational (pre-existing): in the documented traces-only configuration, log telemetry is silently dropped
| settings | bridge spans (log.bridge) |
native spans | log records delivered |
|---|---|---|---|
only otlpTracesEndpoint set |
0 | 5 | 0 |
same + otlpEndpoint: '' |
14 | 5 | n/a (bridged) |
Cause, read from source and confirmed by the table: Config.getTelemetryOtlpEndpoint() returns telemetrySettings.otlpEndpoint ?? DEFAULT_OTLP_ENDPOINT (DEFAULT_OTLP_ENDPOINT = 'http://localhost:4317', telemetry/index.ts:13), so logsUrl stays truthy and the bridge is never constructed — it lives only in the else if (tracesUrl) branch of createHttpExporters (sdk-exporters-http.ts:56-62). An operator following the docs ("used when HTTP traces are exported without a logs endpoint") who sets only otlpTracesEndpoint gets log records aimed at an unreachable default and loses them without any signal. Pre-existing; it bounds how much of this PR's second claim is reachable in practice. Witness 05-bridge-reachability-log-bridge-oracle.png.
Minor observation (non-blocking, unchanged)
responseText is still computed (extractResponseText, capped at 4096) and the streaming path still consolidates responses before _logApiResponse discards the value. Gating one step earlier would skip that work when logPrompts: false; bounded memory either way.
Not covered
- Per-commit attribution. Shallow clone (
git rev-parse --is-shallow-repository→true):git rev-list HEAD^1..HEAD^2returns 1 commit while the metadata snapshot lists 8 (22aec43a,d04bb9e9,d04fb917,7f9f68b0,8fb0f8d9,3e2fd8e3,b51e0af0,76de1e23), and the previous round's headd04fb917is not present locally (git cat-file -tfails). Verified the aggregateHEAD^1..HEADdiff; the delta's contents were inferred from the final diff plus commit messages, not from per-commit diffs. No per-commit table is presented. - Trial merge into current
main. The snapshot'sbaseRefOid(78bbd9f5…) is not present locally and differs from the merge ref's base (HEAD^1=05a54fc3…), somainhas moved since the merge ref was created. No network/token in this job → could not fetch, could not confirm a conflict-free merge, could not re-run on mergedmain. Mitigating evidence: the two changed production files compile to bytes identical to the previous round's base, i.e.maindid not touch them between28df8b8aand05a54fc3. #11682, referenced by the new docs wording for thethoughtSignaturepolicy decision: an issue number, unverifiable offline. ThethoughtSignatureserialization claim was verified; the tracking issue's existence and content were not.- gRPC OTLP path (
otlpProtocol: 'grpc'): not exercised; the bridge does not exist on that path. - Repo-wide test suite: not run. Targeted gates only — the 3 PR-named files (225), the new test file (4), the whole
src/telemetrysuite (1002 passed / 30 files),tsc --noEmitforpackages/core, prettier and eslint on the changed files.packages/cliwas not run. --resumeend-to-end replay of the persistedui_telemetrymirror: closed by census only (the sole reader ofsystemPayload.uiEventissessionService.ts:4587, which readsprompt_idand spreads the rest). No resume was actually driven.- Interactive TUI mode: all cells are headless (
-p). The gates sit in a shared producer, so mode should not matter, but it was not measured. - The
thoughtSignature/encrypted_contentpolicy question the PR explicitly declares out of scope: not evaluated. - Windows/macOS: Linux container only.
- Six superseded assertions, kept as raw evidence. The first
probes.mjsrun scored 11/14; the 3 reds were my harness's fault, not the PR's — a crude "total span count" oracle that counted native trace spans as bridge spans, plus a debug-line oracle (QWEN_DEBUG_LOG_FILE) that never produced a file. Corrected to thelog.bridgeoracle and re-run asreachability.mjs(9/9); the 6 superseded entries are excluded fromassertions.jsonbybuild-assertions.mjsand the exclusion count is recorded inassertions-detail.json. An earliermatrix.mjsiteration likewise scored 70/86 becauseotlpLogRecordskeyednameoff the record body and OTLPAnyValueobjects were compared as strings; both were fixed and the matrix re-run clean at 92/92. Only the final runs feed the counts.
Methodology
Environment: the CI verify container (node:22-bookworm, no zstd, $RUNNER_TEMP unset in-shell), merge-ref checkout at depth 2, npm ci + npm run build already done at head. Artifact dir tmp/pr11670-verify-20260912-073825/ holds every harness (harness.mjs, matrix.mjs, probes.mjs, reachability.mjs, mutations.mjs, wire-summary.mjs, build-assertions.mjs, gates.sh), raw logs (matrix.log, probes.log, reachability.log, mutation.log, gates.log, wire-summary.log, build-*.log), per-cell JSON (cells/*.json), machine-readable results (matrix-results.json, probes.json, reachability.json, mutation-rows.json, assertions.json, assertions-detail.json, provenance.txt) and evidence/*.png.
Control construction. Internal workspace links were asserted before trusting any control: readlink -f node_modules/@qwen-code/qwen-code-core → /__w/qwen-code/qwen-code/packages/core, i.e. the head tree — so a second worktree would have silently loaded head code. Base sources were therefore taken with git show HEAD^1:<path> for exactly the two changed production files and compiled with the real npm run build -w packages/core (31 s); the arm switch swaps the two compiled .js modules in place, and every cell re-asserts the landed arm by grepping the compiled bytes (gateCount, denylisted) and recording both sha256s in the cell JSON before spawning. The head rebuild reproduced the CI-built head bytes exactly (diff -q identical for both modules), so swapping modules is an exact arm switch. The PR leaves package.json/package-lock.json untouched, so reusing the root node_modules for both arms is a clean control.
How the harnesses drove the code. Each cell spawns the real CLI with an isolated HOME/cwd settings.json against a loopback OpenAI-compatible server returning marker content (and, in the sibling probe, a tool call whose arguments carry a distinct marker). Destinations are real: telemetry.outfile, a loopback OTLP/HTTP receiver for /v1/logs, and a traces-only receiver for /v1/traces feeding the bridge. Oracles judge values, not key substrings: OTLP AnyValues are unwrapped and an empty one is distinguished from an absent key, which is exactly the false positive a substring oracle produces (finding 3). Bridge cells use a 6 s model-side delay to outlive the bridge's unref'd flush tick and assert on the processor's own log.bridge marker. The outfile is safeJsonStringify(data, 2) — pretty-printed concatenated objects, not JSONL — so it is parsed with a string-aware brace scanner. Universal validity controls (CLI exited 0, model was actually called, api_request records/spans were produced) run on both arms, so no "absent" result can pass vacuously.
Counts. assertions.json is generated by build-assertions.mjs from the result files, never hand-counted: matrix 92, sibling probe 8, reachability 9, mutation rows 9, gates 13 → 131 pass / 0 fail. Gate liveness was proven by planting a formatting break (prettier exit 1), a lint violation (no-explicit-any + no-console, eslint exit 1) and a type error (TS2322, tsc exit 2), then restoring; the unit-test gate's liveness is the mutation matrix itself (M5/M6 land in the mutated files). Working tree and packages/core/dist verified clean/at-head afterwards.
Flakiness gate log
rounds=5 files=3 skipped=0
file packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/loggingContentGenerator/loggingContentGenerator.test.ts
file packages/core/src/telemetry/file-exporters.test.ts: (cd packages/core) npx --no-install vitest run ./src/telemetry/file-exporters.test.ts
file packages/core/src/telemetry/log-to-span-processor.test.ts: (cd packages/core) npx --no-install vitest run ./src/telemetry/log-to-span-processor.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: PPPPP
packages/core/src/telemetry/file-exporters.test.ts: PPPPP
packages/core/src/telemetry/log-to-span-processor.test.ts: PPPPP
verdict: pass
summary: 3 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 1 · packages/core/src/telemetry/file-exporters.test.ts: P (exit 0)
round 1 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 2 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 2 · packages/core/src/telemetry/file-exporters.test.ts: P (exit 0)
round 2 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 3 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 3 · packages/core/src/telemetry/file-exporters.test.ts: P (exit 0)
round 3 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 4 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 4 · packages/core/src/telemetry/file-exporters.test.ts: P (exit 0)
round 4 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 5 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 5 · packages/core/src/telemetry/file-exporters.test.ts: P (exit 0)
round 5 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Resolve-comments scope ledgerScope corrected: production scope stays at two files, +5/-2, identical to the first reviewed head. Existing review rounds expanded tests from 78 to 188 changed lines and added two documentation files. This audit removed 29 lines pinning exporter internals; the remaining tests cover generation, bridge filtering and real file export. 229 focused tests pass and current-head CI is green. Further policy coverage belongs to #11682; the final documentation-only commit 4db3f55 corrects the existing bridge-key inventory without changing behavior. Prettier and diff checks pass; the new-head CI snapshot is pending. Triage has been requested. |
|
@qwen-code /triage |
Maintainer verification — real CLI × real telemetry sinksI built a local end-to-end rig and ran 18 real Verdict: the fix does what it says on all three sinks, with no default-config change. Merge-ready. Two non-blocking observations and one follow-up suggestion below.
HarnessResults
The bug reproduces on the merge-base for all three sinks, and is gone on the PR head for all three. The env-var override path (R/Q) works too. Default config is byte-identical (P vs O: same What actually leaked pre-fix is worth seeing — it is not just the prompt: system reminders, the working directory, the folder listing, every prior turn, and the managed-memory subagent instruction block, 6.8 KB on the first request and 9.5 KB on the second, with Tests are load-bearing, and typecheck is cleanReverting only the 7 production lines to the merge-base while keeping the PR's tests fails 3 of them; restoring the production lines makes all 229 pass. Note the new I could not run ESLint locally ( Two observations (neither blocks)1. On the native OTLP log exporter the key still ships, with an empty value. // arm F, PR head, logPrompts: false, received by the OTLP receiver
{"key":"request_text","value":{}}
{"key":"response_text","value":{}}No content escapes (
A two-line follow-up in // logApiRequest
if (event.request_text === undefined) delete attributes['request_text'];and the same for 2. Arms I → J are a deliberate behaviour change for existing traces-only users. With Residual: same class of gap, outside this PR's scopeWith
None of this blocks the PR — its stated scope is Coverage noteThe real-CLI arms exercise the streaming path, which is what Reproducegit worktree add head <PR head> && git worktree add --detach base 78bbd9f55c
# per arm: isolated QWEN_HOME with
# {"security":{"auth":{"selectedType":"openai"}},
# "telemetry":{"enabled":true,"logPrompts":false,
# "outfile":"…" # sink 1
# /* or */ "otlpProtocol":"http","otlpEndpoint":"http://127.0.0.1:PORT" # sink 2
# /* or */ "otlpProtocol":"http","otlpTracesEndpoint":"http://127.0.0.1:PORT/v1/traces","otlpEndpoint":"" # sink 3 = bridge
# }}
OPENAI_BASE_URL=http://127.0.0.1:FAKE/v1 OPENAI_API_KEY=x OPENAI_MODEL=fake-model \
node <arm>/scripts/dev.js --approval-mode yolo -p "Say the phrase SENSITIVE_REQUEST_MARKER and nothing else"
jq -c '..|.attributes?|arrays|.[]|select(.key=="request_text" or .key=="response_text")' otlp/logs.jsonlSink 3 (the bridge) needs a traces endpoint and no logs endpoint — set 中文版维护者验证 —— 真实 CLI × 真实遥测 sink我在本地搭建了端到端验证环境,用真实 OTLP/HTTP 接收端与真实 结论:三个 sink 上修复都真实生效,默认配置行为无变化,可以合并。 下面是两点不阻塞的观察和一条后续建议。
验证环境结果
三个 sink 在 merge-base 上都能复现该缺陷,在 PR head 上都已修复;环境变量覆盖路径(R/Q)同样生效;默认配置逐字节一致(P vs O: 修复前实际泄漏的内容值得一看,远不止用户 prompt:system reminder、当前工作目录、目录结构、此前每一轮对话,以及 managed-memory 子代理的完整指令块 —— 在显式设置 测试确实「吃劲」,类型检查干净只把 7 行生产代码回退到 merge-base、保留 PR 的测试,会有 3 个用例失败;恢复生产代码后 229 个用例全绿。 需要说明:新增的 我这里跑不了 ESLint( 两点观察(都不阻塞)1. 在原生 OTLP 日志导出上,key 仍然会发出,只是值为空。 // arm F,PR head,logPrompts: false,OTLP 接收端收到的内容
{"key":"request_text","value":{}}
{"key":"response_text","value":{}}没有任何内容外泄(
在 // logApiRequest
if (event.request_text === undefined) delete attributes['request_text'];
2. arm I → J 对现有 traces-only 用户是一次有意的行为变更。 在 残留:同类问题,但不在本 PR 范围内在 PR head 上、
这些都不阻塞本 PR —— 它声明的范围就是 覆盖范围说明真实 CLI 的所有 arm 走的是流式路径,也就是 |
Add a regression test asserting the thoughtSignature provider payload survives toContents serialization into request_text. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmty28f6zzp
…t_text" R5-1 is declined and deferred to #11682, which owns the enabled-policy thoughtSignature decision. This PR only changes the logPrompts=false privacy gate; a test that freezes the opposite policy widens scope. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmty28f6zzp
|
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: 189 passed · 0 failed · 189 total Flakiness gate: ✅ 3 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:189 通过 · 0 失败 · 189 总计 抖动门:✅ 3 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #11670 — fix(telemetry): gate request_text/response_text on logPromptsVerdict: This is a follow-up round. The previous round verified head Two facts bound the delta cheaply, and both are measured rather than assumed. The compiled head modules are byte-identical to the previous round's ( The central claim is proven load-bearing on three real telemetry destinations at the new head. The sharpest number: with 中文摘要
Previous-finding status at the new headRe-measured, not diffed. "Stands" means the behaviour was reproduced at
Central claim + A/BCentral claim. When Each cell is one real Destination 1 —
|
| cell | arm | logPrompts | oracle (what the file actually contains) | result |
|---|---|---|---|---|
| OF-base-lpOFF | base | false | request_text PRESENT, lengths 6,772 and 9,510 chars; response_text present |
LEAKS (control) |
| OF-head-lpOFF | head | false | both keys ABSENT | clean |
| OF-base-lpON / OF-head-lpON | both | true | both present, lengths 6,769 / 9,500 on both arms, carrier sets identical | no regression |
Δ at logPrompts: false = −16,282 characters. That delta is the removed conversation content.
Destination 2 — native OTLP/HTTP /v1/logs (real loopback socket)
| cell | arm | logPrompts | request_text on the wire |
response_text |
result |
|---|---|---|---|---|---|
| OT-base-lpOFF | base | false | 2 present, 2 with content, lengths 6,772 / 9,510 | 2 with content | LEAKS (control) |
| OT-head-lpOFF | head | false | 2 present, 2 EMPTY AnyValue, 0 with content |
2 present, 2 empty, 0 with content | clean |
| OT-base-lpON / OT-head-lpON | both | true | 2 with content, lengths 6,769 / 9,500 on both arms | identical | no regression |
Oracles judge values, not key substrings: AnyValues are unwrapped and an empty one is distinguished from an absent key — exactly the false positive a substring oracle produces (finding 3).
Destination 3 — traces-only log-to-span bridge
Oracle is the log.bridge attribute the processor itself sets (log-to-span-processor.ts:191), so bridge spans cannot be confused with native trace spans. Every cell asserts validity controls (bridge spans reached the wire = 14, bridge produced api_request spans = 2) on both arms, so "absent" cannot pass vacuously.
| cell | arm | logPrompts | sensitive | api_request bridge spans carrying request_text |
result |
|---|---|---|---|---|---|
| BR-base-lpON-sensOFF | base | true | false | 2 of 2 | LEAKS (control) — the denylist gap this PR closes |
| BR-head-lpON-sensOFF | head | true | false | 0 of 2 | stripped — secondary claim (a) proven |
| BR-base-lpON-sensON | base | true | true | 2 of 2 (6,790 / 9,570 chars) | retained |
| BR-head-lpON-sensON | head | true | true | 2 of 2, identical lengths and carrier set | retained — docs claim holds |
| BR-base-lpOFF-sensON | base | false | true | 2 of 2 (6,793 / 9,580) | LEAKS (control) — base got zero protection from logPrompts: false here |
| BR-head-lpOFF-sensON | head | false | true | 0 of 2 | clean (producer gate) |
The last pair is the cleanest isolation of the producer gate: includeSensitiveSpanAttributes is true on both arms, so the denylist cannot explain the difference — only the new gate can. Δ = −16,373 characters.
Secondary claim (b), census
Re-run at the new head: every property access of the two fields across packages/*/src resolves to a producer (types.ts), the OTLP attribute writer (loggers.ts), or the bridge denylist (log-to-span-processor.ts:64). No non-test reader. The persisted ui_telemetry mirror is the one path with on-disk readers, and the only reader touching uiEvent reads prompt_id alone and spreads the rest through unchanged — so nothing reads response_text off disk. Claim (b) holds; this remains a census, not an end-to-end --resume run (see Not covered).
The delta since the previous round (commits 9–11)
New probes were scoped to what actually changed: one docs sentence and one test that was added then reverted.
Commit 9's docs claim, tested rather than read. The bridge bullet now enumerates the retained set as function_args, error, error.message, error_message, plus prompt, request_text, response_text when logPrompts is also enabled. That is a completeness claim about SENSITIVE_ATTRIBUTE_KEYS, so it was checked behaviourally against the real compiled LogToSpanProcessor, not by reading the Set literal: a log record carrying all 7 candidate keys plus 4 controls was driven through at includeSensitiveSpanAttributes: false and true (03-delta-docs-census-and-thoughtsignature.png).
- sensitive OFF → dropped exactly
["error","error.message","error_message","function_args","prompt","request_text","response_text"]; kept exactly["error_type","model","duration_ms","safe_control"]. - sensitive ON → all 11 retained verbatim (11/11), so nothing is lost on the accept path.
- The docs enumeration and the behaviourally-derived set are equal, and the census runs both directions: no key the bridge drops is missing from the docs list (completeness), and no docs-listed key is actually retained when sensitive is off (soundness). Commit 9's sentence is accurate and complete.
- The
logPromptsqualifier is also accurate, and the reason is worth stating because it is not in the processor: the bridge filter is key-only and has nologPromptsawareness, soprompt/request_text/response_textdisappear atlogPrompts: falsepurely because the producer omits them. The matrix shows this directly —qwen-code.user_prompt:promptis a residual carrier in everylpONcell and in nolpOFFcell, whilefunction_argsis present atlpOFF(finding 1). The sentence correctly placesfunction_argsoutside thelogPromptscondition.
Commits 10 + 11 are a clean net-zero. thoughtSignature appears once in the final diff, inside commit 9's docs sentence; no test file carries it. Confirmed by the byte-identical head modules and by the mutation matrix, which still has 0 survivors — so the revert removed a test that was pinning behaviour nothing else needed pinned, and lost no guard coverage.
The declined R5-1, re-measured. toPart is a pass-through for non-thought parts and a spread ({...part}) for thought parts, so every own enumerable property survives — including an opaque thoughtSignature. Driving the real compiled toContents (the methods are stateless, so a prototype-created instance exercises real code with no Config) with two parts carrying the signature yields a request_text serialization containing it 2×, verbatim. The behaviour stands exactly as the declined finding said.
I agree with the decline, for a reason the revert message does not state: the PR's docs sentence now discloses it ("opaque thoughtSignature provider payload is included, with its policy decision tracked in #11682"). A passing test would freeze a policy the author is explicitly not deciding here, whereas the docs make the behaviour discoverable and #11682 owns the decision. That is the better of the two outcomes for a PR whose scope is the logPrompts: false gate. Note the boundary: at logPrompts: false this payload does not ship — the gate removes it, which is the third delta assertion.
Carried nit (unchanged, non-blocking). file-exporters.test.ts's FileLogExporter omits undefined-valued attributes (logPrompts off) still names a logPrompts scenario while its fixture is a hand-built ReadableLogRecord with request_text: undefined — no Config, no LoggingContentGenerator. The comment above it is honest about this and the assertion is genuinely load-bearing (V1 kills it), so the cost is only that the name buys slightly more confidence than the fixture pays for.
Mutation matrix
One row per guard, a combination row for layered defence, two positive controls landed in the same file as the mutant, and the vacuity check on the PR's newest test file. Every red is the intended behavioural assertion with an expected-vs-actual message — no red came from a broken import or fixture. Ground truth is vitest's own exit code, cross-checked against parsed counts (countsAgree=true on all 8 rows). Witness: 06-mutation-matrix-zero-survivors.png.
| id | what was reverted / perturbed | suite | red | quoted failure |
|---|---|---|---|---|
| M0 | (none — unmutated control) | all 3 files | 0 / 142 | green, as required |
| M1 | guard #1: request_text producer gate |
lcg | 2 / 83 | expected '[{"role":"user","parts":[{"text":"SEN…' to be undefined |
| M2 | guard #2: response_text producer gate |
lcg | 2 / 83 | expected 'SENSITIVE_RESPONSE_MARKER' to be undefined |
| M3 | guard #3: 'request_text' denylist entry |
l2s | 1 / 55 | expected { Object (request_text, error_type, …) } to not have property "request_text" |
| M4 | combination M1 + M3 together | both | 3 / 138 | union of M1 (2) and M3 (1) — the guards are independent, no layered defence hiding either |
| M5 | positive control, same file as M3: drop the pre-existing 'response_text' entry |
l2s | 1 / 55 | expected { …(4) } to not have property "response_text" |
| M6 | positive control, same file as M1/M2: MAX_RESPONSE_TEXT_LENGTH 4096 → 4095 |
lcg | 1 / 83 | expected 'xxx…' to have a length of 4096 but got 4095 |
| V1 | vacuity check: safeJsonStringify replacer returns null for undefined |
fx | 1 / 4 | expected '{\n "body": "api response",…' not to contain 'request_text' |
Survivors: 0. Both positive controls went red, so the harness can make these suites fail; the unmutated control is green (142), so the kills mean something. M1's message reproduces the PR body's own claimed "Before" evidence verbatim. git status --porcelain verified empty after every row (asserted per row, not just at the end), and packages/core/dist left at head (d5222129… / 90937df9…).
Corrections
- Refinement of the previous round's finding-4 wording, in the direction of more precise, not worse. The previous round reported that in the documented traces-only configuration the operator "loses the logs", with 0 log records delivered to any reachable receiver. Measured with a receiver bound to the default OTLP port, the mechanism is narrower and more useful: the 14 log records are produced and are delivered — to
http://localhost:4317, the unconfigured default thatgetTelemetryOtlpEndpoint()falls back to. Nothing was emitted in-process and dropped; the records were routed to an endpoint the operator never named. In production, where nothing listens on 4317, the observable outcome is the same silence the previous round described, so its conclusion stands — but the cause is routing, not production, and that distinction matters to whoever fixes it: the fix is thelogsUrl/tracesUrlbranch condition increateHttpExporters, not anything in the logging path. Still pre-existing, still not caused by this PR. - The PR body's test count is stale (carried forward). The body reports
# 223 passed (3 files); measured at the verified head: 225 passed (3 files). Description accuracy only — the tests pass. log_prompts_enabledis not an attribute (carried forward, count unchanged). Commit 9's docs wording again writesonly when `log_prompts_enabled` is true. No attribute or setting by that name exists anywhere inpackages/**(0 TypeScript hits); it appears only intelemetry.md, 3 times. This is a pre-existing docs convention — already used forpromptat line 621 — that the PR extends consistently, so matching it is defensible. Flagged so the next reader does not grep for a field that does not exist.- Confirming, not correcting: the body's claim that the two fields "never reach the native OTLP log exporters, the traces-only log-to-span bridge, or
telemetry.outfile" is true on all three destinations at the new head, and the no-internal-consumer claim holds.
Findings
Ordered by severity. None is a regression introduced by this PR; none blocks it. Findings 1, 2, 4 and R5-1 are re-measurements of carried-forward or newly-declined items and are tabulated above; the substance is restated here only where it changed.
1. Suggestion (pre-existing, scope): logPrompts: false still exports verbatim tool arguments via tool_call.function_args
Same bug class, adjacent door — and the door the PR's own motivation names ("tool arguments/results, file content"). At head with logPrompts: false, in a single run where api_request correctly has no request_text key:
tool_call record: function_name="run_shell_command"
function_args="{\n \"command\": \"printf '%s\\n' 'SENSITIVE_TOOL_OUTPUT_MARKER'\",\n \"description\": \"SENSITIVE_TOOL_ARG_MARKER\"\n}"
The base arm produced a byte-identical function_args, so this is pre-existing and not attributable to the PR. Cause: logToolCall spreads the normalized event and sets function_args: safeJsonStringify(…) with no gate; shouldLogUserPrompts has exactly one call site. function_args is in the bridge denylist, so only the native-log and outfile legs carry it. Repro: PROBE_SET=siblings node tmp/pr11670-verify-20260912-082150/harness.mjs, witness 04-sibling-tool-args-still-ungated.png. Worth naming for whoever closes #11666, since the issue's stated symptom survives through a different key.
2. Suggestion (pre-existing, scope): subagent_execution.result still exports model output text
The residual-carrier scan reports, for every head logPrompts: false cell, exactly one surviving carrier of the model's response marker: qwen-code.subagent_execution:result, whose value is the marker verbatim ("SENSITIVE_RESPONSE_MARKER_OF-head-lpOFF"). The same carrier appears on the matching base cell, asserted per cell, so it is not introduced here. On the bridge leg with includeSensitiveSpanAttributes: true the native GenAI span attributes join it. Cause: logSubagentExecution has no logPrompts gate and result is not in SENSITIVE_ATTRIBUTE_KEYS.
3. Informational: the native OTLP leg keeps an empty-valued key, and nothing pins it
At head with logPrompts: false: request_text present=2 empty=2 withContent=0, response_text present=2 empty=2 withContent=0 — i.e. {"key":"request_text","value":{}}. No content crosses (the carrier scan finds no marker in any api_request/api_response value), and this matches how every undefined optional attribute already serializes. The outfile sink drops the key entirely, which is what file-exporters.test.ts pins. Practical consequence only: a wire audit grepping for the key name still sees it. Unpinned by design since 76de1e23, and I agree with that choice — pinning it would freeze an OTLP serialization accident.
4. Informational (pre-existing): in the documented traces-only configuration, log telemetry is routed to an endpoint the operator never configured
| settings | bridge spans (log.bridge) |
native spans on the configured endpoint | log records at the default :4317 |
|---|---|---|---|
only otlpTracesEndpoint set |
0 | 5 | 14 (incl. api_request) |
same + otlpEndpoint: '' |
14 of 19 | 5 | n/a (bridged) |
Cause, read from source and confirmed by the table: getTelemetryOtlpEndpoint() returns telemetrySettings.otlpEndpoint ?? DEFAULT_OTLP_ENDPOINT ('http://localhost:4317'), so logsUrl stays truthy and the bridge is never constructed — it lives only in the else if (tracesUrl) branch of createHttpExporters. An operator following the docs ("used when HTTP traces are exported without a logs endpoint") who sets only otlpTracesEndpoint gets log records aimed at a default they never named. Pre-existing; it bounds how much of this PR's second claim is reachable in practice, since the bridge — and therefore the denylist hunk — does not exist on that path. Repro: PROBE_SET=reach node …/harness.mjs, witness 05-bridge-reachability-default-endpoint-oracle.png.
Minor observation (non-blocking, unchanged)
responseText is still computed (extractResponseText, capped at 4096) and the streaming path still consolidates responses before the value is discarded by the gate. Gating one step earlier would skip that work when logPrompts: false; bounded memory either way.
Not covered
- Per-commit attribution. Shallow clone (
git rev-parse --is-shallow-repository→true):git rev-list HEAD^1..HEAD^2returns 1 commit while the metadata snapshot lists 11 (22aec43a,d04bb9e9,d04fb917,7f9f68b0,8fb0f8d9,3e2fd8e3,b51e0af0,76de1e23,4db3f55a,ac00d3ee,34c8eb1c). The previous round's head76de1e23and the revertedac00d3eeare both unreachable locally, so commit 10's test content could not be read — its net-zero effect was established from the final diff and the byte-identical modules instead. Verified the aggregateHEAD^1..HEADdiff; no per-commit table is presented. - Trial merge into current
main. The snapshot'sbaseRefOid(78bbd9f5…) is not present locally and differs from the merge ref's base (HEAD^1=3b2283ee0d…), somainhas moved since the merge ref was created. No network/token in this job → could not fetch, could not confirm a conflict-free merge, could not re-run on mergedmain. Mitigating evidence: the base-side compiled modules are byte-identical to the previous round's, i.e.maindid not touch either changed file between05a54fc3and3b2283ee0d. #11682, referenced by commit 9's docs wording for thethoughtSignaturepolicy decision: an issue number, unverifiable offline. ThethoughtSignatureserialization claim was verified behaviourally; the tracking issue's existence and content were not.- gRPC OTLP path (
otlpProtocol: 'grpc'): not exercised; the bridge does not exist on that path. - Repo-wide test suite: not run. Targeted gates only — the 3 PR-named files (225),
file-exporters.test.ts(4), the wholesrc/telemetrysuite (1002 passed / 30 files),tsc --noEmitforpackages/core, prettier and eslint on the 7 changed files.packages/cliwas not run. --resumeend-to-end replay of the persistedui_telemetrymirror: closed by census only. No resume was driven.- Interactive TUI mode: all cells are headless (
-p). The gates sit in a shared producer, so mode should not matter, but it was not measured. - The
thoughtSignature/encrypted_contentpolicy question itself — whether that payload should ship when prompt logging is enabled — is explicitly out of scope for this PR and was not evaluated as a policy matter. Only the behaviour and the accuracy of the docs sentence describing it were measured. - Windows/macOS: Linux container only.
- Ten superseded harness failures, kept as raw evidence and excluded from
assertions.json. All ten were my harness's fault, not the PR's, and each was fixed and re-run clean: (a) 2 from the outfile oracle keying the event name off the record body instead ofattributes["event.name"], which theapi_request records producedvalidity control caught; (b) 1 from the docs-census regex sweeping up`logPrompts`— a setting name in that sentence, not an attribute key; (c) 7 from two mutation-harness bugs:git checkout HEAD -- <rel>used paths relative topackages/corefrom the repo root, so restores silently failed and mutations stacked (the run aborted, the tree was cleaned and its source hashes re-verified against the pre-mutation values before re-running), and vitest summary counts were parsed without stripping ANSI, reportingfailed=0for rows that had actually gone red. Ground truth for the matrix is now vitest's exit code, cross-checked by a counts-agree assertion. Exclusions and reasons are recorded inassertions-detail.json; only the final runs feed the counts.
Methodology
Environment: the CI verify container (node:22-bookworm, node v22.23.2, $RUNNER_TEMP unset in-shell), merge-ref checkout at depth 2, npm ci + npm run build already done at head. Artifact dir tmp/pr11670-verify-20260912-082150/ holds every harness (harness.mjs, model-server.mjs, otlp-receiver.mjs, delta-probes.mjs, mutations.mjs, summary-table.mjs, build-assertions.mjs, gates.sh), raw logs (matrix.log, delta-probes.log, mutation.log, mutation-summary.txt, mutation-raw/*.log, gates.log, summary-table.log, build-base.log, gates-stdout.log), per-cell JSON and raw sinks (cells/<id>/{cell.json,outfile.json,otlp.jsonl,otlp-default-4317.jsonl,cli.out,cli.err}), machine-readable results (matrix-results{,-main,-siblings,-reach}.json, delta-probes.json, mutation-rows.json, assertions.json, assertions-detail.json, provenance.txt) and evidence/*.png.
Control construction. Internal workspace links were asserted before trusting any control: readlink -f node_modules/@qwen-code/qwen-code-core → /__w/qwen-code/qwen-code/packages/core, i.e. the head tree — so a second worktree would have silently loaded head code. Base sources were therefore taken with git show HEAD^1:<path> for exactly the two changed production files and compiled with the real npm run build -w packages/core; the arm switch swaps the two compiled .js modules in place, and every cell re-asserts the landed arm two ways — by grepping the compiled bytes for the gate and the denylist entry (gateCount, denylisted) and by recording both sha256s in the cell JSON before spawning. The head rebuild reproduced the CI-built head bytes exactly (d5222129… / 90937df9…), so swapping modules is an exact arm switch. packages/cli/dist/index.js is a 245-byte shim, not a bundle, which is what makes the swap observable at all. The PR leaves package.json/package-lock.json untouched, so reusing the root node_modules for both arms is a clean control.
How the harnesses drove the code. Each cell spawns the real CLI with an isolated HOME/QWEN_HOME/cwd outside the repo against a loopback OpenAI-compatible server returning per-cell marker content (and, in the sibling probe, a tool call whose arguments carry a distinct marker, emitted on the first request only so the CLI does not loop). Destinations are real: telemetry.outfile, a loopback OTLP/HTTP receiver for /v1/logs, a loopback traces receiver feeding the bridge, and — for the reachability probe — a second receiver bound to the default port 4317 so the fate of diverted records is observed, not inferred. Bridge cells use a 6 s model-side delay to outlive the bridge's unref'd flush tick and assert on the processor's own log.bridge marker. The outfile is safeJsonStringify(data, 2) — pretty-printed concatenated objects, not JSONL — so it is parsed with a string-aware brace scanner. Universal validity controls (CLI exited 0, model was actually called, api_request records/spans were produced, bridge spans reached the wire) run on both arms, so no "absent" result can pass vacuously — and in one case did exactly its job, catching my own broken oracle before it could produce a confident negative. The delta probes import the real compiled LogToSpanProcessor and LoggingContentGenerator from dist/ with only a capturing span exporter and a prototype-created instance as collaborators, so the units under test are never stubbed.
Counts. assertions.json is generated by build-assertions.mjs from the result files, never hand-counted: main matrix 119, sibling probe 20, reachability probe 14, delta probes 16, mutation rows 8 + final tree clean 1, gates 11 → 189 pass / 0 fail. Gate liveness was proven by planting a formatting break (prettier exit 1), a lint violation (no-explicit-any, eslint exit 1) and a type error (TS2322, tsc exit 2), then restoring; the unit-test gate's liveness is the mutation matrix itself (M5/M6 land in the mutated files). Working tree verified clean after every mutation row and after all gates; packages/core/dist left at head; scratch run dirs and the smoke cell removed; git worktree list shows only the main checkout.
Flakiness gate log
rounds=5 files=3 skipped=0
file packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: (cd packages/core) npx --no-install vitest run ./src/core/loggingContentGenerator/loggingContentGenerator.test.ts
file packages/core/src/telemetry/file-exporters.test.ts: (cd packages/core) npx --no-install vitest run ./src/telemetry/file-exporters.test.ts
file packages/core/src/telemetry/log-to-span-processor.test.ts: (cd packages/core) npx --no-install vitest run ./src/telemetry/log-to-span-processor.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: PPPPP
packages/core/src/telemetry/file-exporters.test.ts: PPPPP
packages/core/src/telemetry/log-to-span-processor.test.ts: PPPPP
verdict: pass
summary: 3 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 1 · packages/core/src/telemetry/file-exporters.test.ts: P (exit 0)
round 1 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 2 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 2 · packages/core/src/telemetry/file-exporters.test.ts: P (exit 0)
round 2 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 3 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 3 · packages/core/src/telemetry/file-exporters.test.ts: P (exit 0)
round 3 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 4 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 4 · packages/core/src/telemetry/file-exporters.test.ts: P (exit 0)
round 4 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
round 5 · packages/core/src/core/loggingContentGenerator/loggingContentGenerator.test.ts: P (exit 0)
round 5 · packages/core/src/telemetry/file-exporters.test.ts: P (exit 0)
round 5 · packages/core/src/telemetry/log-to-span-processor.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": enumerating every telemetry event type's fields for a content-bearing key outside the 7-key denylist (e.g. a spread result attribute reaching bridge spans ung…; "agent 6c": whether OTLP log export renders the undefined -valued request_text / response_text attributes that ...event spreads onto api_request/api_response records (….
Test Plan (not a blocker): src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory.
中文说明
未发现问题。LGTM!✅
未探索到全部深度(达到工具调用预算):"agent reverse-audit (round 1)":enumerating every telemetry event type's fields for a content-bearing key outside the 7-key denylist (e.g. a spread result attribute reaching bridge spans ung…;"agent 6c":whether OTLP log export renders the undefined -valued request_text / response_text attributes that ...event spreads onto api_request/api_response records (…。
Test Plan(非阻断):src/core/loggingContentGenerator/loggingContentGenerator.test.ts — no such file or directory; src/telemetry/log-to-span-processor.test.ts — no such file or directory; src/telemetry/loggers.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.23.3)
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
APPROVE
已核对 head 34c8eb1cc2373550d5681e9feb48f8b0dec8230f(vs merge-base 78bbd9f55c)。产代码只有三处:logApiRequest 在 getTelemetryLogPromptsEnabled() 为假时不再 JSON.stringify(contents) 而是传 undefined(loggingContentGenerator.ts:238-240)、_logApiResponse 同口径处理 responseText(:273)、log-to-span-processor.ts 的 SENSITIVE_ATTRIBUTE_KEYS 补上 request_text。
独立复查未发现 Critical:
- 发布点收敛完整:
new ApiRequestEvent(、new ApiResponseEvent(在全仓只有这两处(loggingContentGenerator.ts:243/266),没有绕过开关的第三条构造路径;ApiErrorEvent(:307)不带正文。types.ts里request_text?: string本就是可选字段,undefined是合法值。 - 另一条会写完整请求/响应的通道是
logOpenAIInteraction(:1083-1108),它只在generatorConfig.enableOpenAILogging为真时才存在(:200),是独立的显式开关与本 PR 语义无关,不属于这里要收的门。 - 三sink契约与文案一致:文件导出会把
undefined值的属性丢掉(FileLogExporter omits undefined-valued attributes (logPrompts off))、OTLP 原生日志侧键存在但为空、log-to-span 桥接跨度按 denylist 丢弃。
本地验证:log-to-span-processor.test.ts + file-exporters.test.ts 在该 head Tests 59 passed (59),并做了一处变异——从 SENSITIVE_ATTRIBUTE_KEYS 删掉新增的 'request_text' —— LogToSpanProcessor > drops sensitive attributes before exporting bridged spans 立即变红,说明这一行的效果有见证而非靠文案。
说明一处环境限制:loggingContentGenerator.test.ts 与 loggers.test.ts 在本机无法收集(本地 node_modules 缺 @modelcontextprotocol/client,src/tools/mcp-client.ts 在导入链上,与本 PR 无关),因此那 4 条「开关 × 一元/流式」四象限用例的绿色以 CI 的 Test (ubuntu-latest, Node 22.x)(本 head 已 success)为准;我逐行读过这 4 条用例,断言的是 request_text/response_text 的 toBeUndefined() 与 toContain('KEEP_*_MARKER'),方向与开关一致。
CI:required 全部 success(Test (ubuntu-latest, Node 22.x)、Lint & Static、Integration Tests (no-AK, No Sandbox)、web-shell E2E Smoke),无失败无 pending。
历史项:第 1~5 轮共 19 条线程全部关闭,其中三条 Prettier/Critical(R1-4、R1-1 fix-induced、R4-1)在当前 head 均已落地(3e2fd8e 之后格式检查转绿,文档矩阵改写为「三个抑制原因 + present-but-empty」口径);R5-1(thoughtSignature 的导出策略)按作者说明移交 #11682,本 head 的最新一笔正是该断言的回退,与移交一致。
一条留给后续的事实性提醒(不阻塞):两处文档现在枚举的是 4 个键(prompt/function_args/request_text/response_text),而 SENSITIVE_ATTRIBUTE_KEYS 实际有 7 个(还含 error、error.message、error_message),R5-2/R5-3 的这一点成立;只是「桥接跨度保留哪几个敏感键」的说明,按现状会少列 3 个,可在后续一并补齐。
qqqys
left a comment
There was a problem hiding this comment.
APPROVE
核对基线:head 34c8eb1cc2373550d5681e9feb48f8b0dec8230f。
历史阻塞问题:已确认消除
本 PR 历史上有两条 Critical,我在当前 head 上分别核对:
- R1-1(
telemetry.md的response_text行同时发布两种互斥的线上形状)—— 已关闭。 第 4 轮 review 的 ledger 自身已把它记为closed,且当前 head 上该文件的对应线程为isResolved: true。 - R4-1(新增测试块未经 Prettier 格式化,会让仓库格式检查失败)—— 已消除。 该问题锚定的
packages/core/src/telemetry/loggers.test.ts已不在当前 PR 的变更文件列表里;同时当前 head 的Lint & Static (ubuntu-latest, Node 22.x)为 pass,格式门禁实际通过。
第 5 轮 review 只留下 3 条 Suggestion(文档中 denylist 键枚举不完整等),按本渠道策略不作为合入门禁。当前 PR 上所有 review thread 均为 isResolved: true。
本轮独立扫描:未发现 Critical
这是一个隐私门禁改动,我按「关闭状态下是否还有内容外泄」这条主线核了完整调用链:
- 门禁本身正确。
loggingContentGenerator.ts现在把requestText与responseText都改成getTelemetryLogPromptsEnabled() ? ... : undefined。该访问器定义在config.ts:7452,与telemetry/loggers.ts:149、telemetry/qwen-logger/qwen-logger.ts:1100用的是同一个门禁,口径一致,没有引入第二套判断。 - 类型安全。
ApiRequestEvent构造器的request_text?: string(telemetry/types.ts:283)与ApiResponseEvent的response_text?: string(:396)本来就是可选参数,传undefined合法;当前 head 的Lint & Static(含类型检查)pass 也印证了这一点。 - span 侧的补漏是真实缺口,不是防御性冗余。
log-to-span-processor.ts:176-190的属性拷贝循环只在includeSensitiveSpanAttributes为真时才放行SENSITIVE_ATTRIBUTE_KEYS中的键,而该开关默认是false(config.ts:2606、:7461)。改动前request_text不在这个集合里(response_text已在),因此默认配置下带request_text的 bridge 日志记录会被原样拷进 span —— 这正是本 PR 补上的那条。补入后与prompt、function_args、response_text同级处理,语义一致。 - 两层门禁叠加后没有残留写入口。 关闭
logPrompts时字段为undefined,拷贝循环的value !== undefined前置条件(:180)会直接跳过该键,因此既不会写出空字符串,也不会留下一个值为undefined的键。
CI:Test (ubuntu-latest, Node 22.x)、Lint & Static、Integration Tests (no-AK, No Sandbox)、web-shell E2E Smoke、review-pr 在当前 head 上均为 pass,没有由本 PR 引入的失败。
结论:历史阻塞问题已确认消除,本轮未发现可证明的 Critical,提交 APPROVE。
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
LGTM。已核对 head 34c8eb1c(vs origin/main merge-base),CI 全绿。
logPrompts 关闭时 request_text / response_text 不再进入任何 sink:生产侧只有两个产出点,都过了 gate(packages/core/src/core/loggingContentGenerator/loggingContentGenerator.ts:238-240、:273);bridge 侧 request_text / response_text 已进 SENSITIVE_ATTRIBUTE_KEYS(packages/core/src/telemetry/log-to-span-processor.ts:58-66),默认不再复制。
一条非阻塞观察(不影响 approve):gate 是把字段设成 undefined,不是删除键。OTLP 序列化时 toAnyValue(undefined) 不产出任何分支(@opentelemetry/otlp-transformer 的 common/internal.js:31-51),属性键仍会留在记录里、只是值为空,而不是 key-absent——仓库既有约定恰好相反,packages/core/src/telemetry/loggers.ts:169-172 的注释写明 error 字段是 delete 而不是 set undefined,为的就是让下游看到 key-absent。要不要对齐可以另开一个,不阻塞本次。
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings. Approval blockers: none.
Scope: loggingContentGenerator.ts, log-to-span-processor.ts, loggers.ts, types.ts. NOT reviewed — the OTLP exporter internals, FileLogExporter runtime behaviour (only its test pinning undefined-key serialization was checked), and the full monorepo test suite.
Checked:
- Symmetric gating: Both
request_textandresponse_textare gated viagetTelemetryLogPromptsEnabled()at the producer inloggingContentGenerator.ts(non-streaming and streaming paths both route throughlogApiRequest/_logApiResponse, so there is no bypass). - Defense-in-depth layering: producer (
undefinedwhen off) → bridgeonEmitshort-circuits onundefined/null→SENSITIVE_ATTRIBUTE_KEYSdenylist now includesrequest_text→JSON.stringifyin file exporter dropsundefinedkeys. No single layer failure exposes the content. request_textconsumer claim verified: assigned inApiRequestEventconstructor, spread into OTel attribute map inloggers.ts, read nowhere for business logic. The PR description is accurate.- Denylist addition is correct:
request_textnow treated consistently withprompt/response_textwhenincludeSensitiveSpanAttributes=false. - Tests: four new test cases cover non-streaming omit, non-streaming keep, streaming omit, and streaming keep — all four branches directly exercised.
Not covered: whether opaque thoughtSignature (encrypted_content) should be exported as request text even when prompt logging is explicitly enabled — the PR description correctly flags this as a separate policy question, not a defect in this change.
Reviewed with AI assistance.





















What this PR does
When
telemetry.logPromptsisfalse, the API request/response logging path inLoggingContentGeneratorno longer serializes conversation content.request_textandresponse_textare omitted fromApiRequestEvent/ApiResponseEvent, so they never reach the native OTLP log exporters, the traces-only log-to-span bridge, ortelemetry.outfile. It also addsrequest_textto the bridge's sensitive-attribute denylist so that, when prompt logging is enabled but sensitive span attributes are disabled, the bridge stripsrequest_textconsistently withprompt/response_text.Why it's needed
Fixes #11666. An operator can set
telemetry.logPrompts: falseand still export full conversation text, tool arguments/results, file content, and opaque reasoning replay material throughapi_request.request_text(andresponse_texton native export / outfile). The denylist omission also let the traces-only bridge retainrequest_textwhile stripping the other sensitive attributes. This closes the gap between the explicit privacy setting and what actually ships to configured telemetry destinations.Reviewer Test Plan
How to verify
Unit-level reproduction, no OTLP server required. Two new tests in
loggingContentGenerator.test.tsexerciseLoggingContentGenerator.generateContentdirectly and assert on theApiRequestEvent/ApiResponseEventhanded to the mockedlogApiRequest/logApiResponse:logPrompts: false→request_textandresponse_textareundefined.logPrompts: true→request_textcontains the request marker andresponse_textequals the response marker.The
log-to-span-processor.test.tssensitive-attribute test now includesrequest_textand asserts it is dropped by default and kept whenincludeSensitiveSpanAttributes: true.Evidence (Before & After)
Before the fix, the red tests fail:
request_textis"[{\"role\":\"user\",\"parts\":[{\"text\":\"SENSITIVE_REQUEST_MARKER\"}]}]"instead ofundefined, and the bridge keepsrequest_text: "secret request". After the fix, both test files pass.Tested on
Environment (optional)
N/A — unit tests only.
Risk & Scope
request_texthas no internal consumers (only produced, never read), so omitting it when prompt logging is off is type-safe and breaks nothing in-tree. Same forresponse_text.thoughtSignaturereplay data (encrypted_content) should be exported as request text even when prompt logging is explicitly enabled — that is a separate decision, not part of this fix. The full monorepo test suite was not run; only the three affectedpackages/coretest files plustsc --noEmitwere exercised.Linked Issues
Fixes #11666
中文说明
这个 PR 做了什么
当
telemetry.logPrompts为false时,LoggingContentGenerator的 API 请求/响应日志路径不再序列化对话内容。request_text与response_text不再写入ApiRequestEvent/ApiResponseEvent,因此不会流向原生 OTLP 日志导出器、traces-only 的 log-to-span bridge 或telemetry.outfile。同时将request_text加入 bridge 的敏感属性黑名单,使「开启 prompt 日志但关闭敏感 span 属性」时,bridge 对request_text的处理与prompt/response_text保持一致。为什么需要
修复 #11666。运营者可显式设置
telemetry.logPrompts: false,却仍通过api_request.request_text(以及在原生日志导出与 outfile 上的response_text)导出完整对话文本、工具参数/结果、文件内容以及不透明的推理回放数据。黑名单的遗漏也让 traces-only bridge 在过滤掉其他敏感属性的同时保留了request_text。本次修复弥合了「显式隐私设置」与「实际导出数据」之间的缺口。Reviewer 测试计划
如何验证
单元级复现,无需真实 OTLP 服务器。
loggingContentGenerator.test.ts中新增两个测试,直接调用LoggingContentGenerator.generateContent,并断言交给 mock 的logApiRequest/logApiResponse的事件:logPrompts: false→request_text与response_text均为undefined。logPrompts: true→request_text包含请求标记、response_text等于响应标记。log-to-span-processor.test.ts的敏感属性测试现在包含request_text,断言默认被丢弃、开启includeSensitiveSpanAttributes: true时被保留。证据(修复前后)
修复前红测试失败:
request_text是"[{\"role\":\"user\",\"parts\":[{\"text\":\"SENSITIVE_REQUEST_MARKER\"}]}]"而非undefined,bridge 保留了request_text: "secret request"。修复后两个测试文件均通过。测试环境
环境(可选)
N/A —— 仅单元测试。
风险与范围
request_text无内部消费者(只被生产、从不被读取),因此在关闭 prompt 日志时省略它是类型安全且不影响仓库内任何逻辑的。response_text同理。thoughtSignature回放数据(encrypted_content)作为请求文本导出」的策略问题——这是单独决策,不属于本次修复。未运行整个 monorepo 测试套件;仅运行了三个受影响的packages/core测试文件及tsc --noEmit。关联 Issue
Fixes #11666