Skip to content

feat(serve): Complete legacy session workspace telemetry - #7003

Merged
doudouOUC merged 5 commits into
QwenLM:mainfrom
doudouOUC:agent/legacy-session-workspace-telemetry
Jul 17, 2026
Merged

feat(serve): Complete legacy session workspace telemetry#7003
doudouOUC merged 5 commits into
QwenLM:mainfrom
doudouOUC:agent/legacy-session-workspace-telemetry

Conversation

@doudouOUC

Copy link
Copy Markdown
Collaborator

What this PR does

This PR adds a declarative telemetry catalog for all 48 explicit legacy /session, /sessions, and /permission routes, with canonical route matching and an audited 41 handler-resolved / 7 primary-bound attribution split.

Handler-resolved requests now bind qwen-code.workspace.hash to the runtime actually selected by session creation, restore, transcript, or live-owner routing. The middleware no longer performs a duplicate live-owner lookup, and unresolved or ambiguous requests omit the workspace hash instead of falling back to the primary workspace.

Request-span and metric settlement now distinguishes the long-lived session event stream from bounded request-scoped generation SSE, while preserving the existing heartbeat and daemon-status dashboard exclusions. The design and daemon observability documentation describe the coverage boundary, failure semantics, and intentionally daemon-global metrics.

Why it's needed

PR #6833 completed owner routing for additional legacy session operations but intentionally left telemetry as a follow-up to #6378. Before this change, many explicit legacy routes had no canonical request span, owner-routed routes could be attributed to the primary workspace, and rewind/shell requests could resolve the live owner twice.

Completing the catalog and late-binding workspace attribution makes multi-workspace traces reliable without changing HTTP contracts, SDK behavior, session execution, or metric cardinality.

Reviewer Test Plan

How to verify

  • Run the four targeted telemetry and resolver test files and confirm all 59 tests pass, including the bidirectional Express catalog drift guard, canonical matcher cases, late attribution, failure paths, resolver seams, and SSE metric behavior.
  • Start a daemon with primary and secondary workspaces plus telemetry.outfile; confirm an owner-routed secondary request records the secondary hash, a primary-bound exception records the primary hash, and an unresolved multi-workspace request has no qwen-code.workspace.hash.
  • Inspect the telemetry JSONL and confirm request spans contain only workspace hashes, never raw workspace paths.
  • Run build, typecheck, lint, and npm run verify:pr; all deterministic checks should pass. Locally, verify:pr passed dependency installation, build, audit, lockfile, isolation, lint, formatting, i18n, schema, typecheck, fast-path bundle, and the complete 739-test serve regression before the long full-unit step hit two unrelated flaky cases: the multi-workspace archived-listing file passed 88/88 on immediate full-file rerun, and the core user-memory timeout passed 1/1 on immediate isolated rerun. Remote CI should confirm the clean-run result.

Evidence (Before & After)

N/A — observability-only change with no UI surface.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

macOS, Node.js 22.22.3, npm 10.9.8, local dual-workspace daemon, JSONL telemetry outfile, sandbox disabled.

Risk & Scope

  • Main risk or tradeoff: Route-catalog or publication-seam drift could omit or misattribute a span; bidirectional route enumeration, exact attribution-set assertions, matcher tests, and resolver-seam tests guard those boundaries.
  • Not validated / out of scope: Per-workspace HTTP metric labels, access-log workspace fields, rate-limit path normalization, implicit HEAD/OPTIONS, pre-parser rejection spans, workspace session-group and workspace-qualified organization gaps, ACP/WebSocket telemetry, and enabling secondary branch/fork/cd execution remain out of scope. Windows and Linux were not tested locally.
  • Breaking changes / migration notes: None. HTTP, SDK, capability, persistence, and session-execution contracts are unchanged; newly classified spans and non-streaming request metrics are an expected observability baseline change.

Linked Issues

Refs #6378

Follow-up to #6833

中文说明

本 PR 的改动

本 PR 为全部 48 条显式 legacy /session/sessions/permission 路由新增声明式 telemetry catalog,提供 canonical route 匹配,并固定经过审计的 41 条 handler-resolved / 7 条 primary-bound 归因分类。

handler-resolved 请求现在会把 qwen-code.workspace.hash 晚绑定到 session creation、restore、transcript 或 live-owner 路由实际选择的 runtime。middleware 不再重复执行 live-owner 查询,无法解析或存在歧义的请求会省略 workspace hash,而不是回退到 primary workspace。

request span 与 metric 的结算现在会区分长连接 session event stream 和有界的 request-scoped generation SSE,同时保留 heartbeat 与 daemon status 现有的 dashboard 排除语义。设计文档和 daemon observability 文档记录了覆盖边界、失败语义以及刻意保持 daemon 全局聚合的 metrics。

为什么需要

PR #6833 为更多 legacy session 操作完成了 owner routing,但有意把 telemetry 留作 #6378 的后续工作。此改动之前,许多显式 legacy 路由没有 canonical request span,owner-routed 路由可能被归因到 primary workspace,并且 rewind/shell 请求可能重复解析 live owner。

补齐 catalog 和 workspace 晚绑定归因后,多 workspace trace 将更可靠,同时不改变 HTTP 合约、SDK 行为、session 执行或 metric cardinality。

Reviewer 测试计划

验证方式

  • 运行 4 个 telemetry 与 resolver 专项测试文件,确认 59 个测试全部通过,覆盖双向 Express catalog 漂移守卫、canonical matcher 场景、晚绑定归因、失败路径、resolver seam 和 SSE metric 行为。
  • 使用 primary 和 secondary workspace 以及 telemetry.outfile 启动 daemon;确认 owner-routed secondary 请求记录 secondary hash,primary-bound 例外记录 primary hash,无法解析的多 workspace 请求不包含 qwen-code.workspace.hash
  • 检查 telemetry JSONL,确认 request span 只包含 workspace hash,绝不包含原始 workspace path。
  • 运行 build、typecheck、lint 和 npm run verify:pr,所有确定性检查都应通过。本地 verify:pr 已通过依赖安装、build、audit、lockfile、isolation、lint、formatting、i18n、schema、typecheck、fast-path bundle 以及完整的 739 项 serve 回归,随后在超长的全量 unit 步骤中遇到两个无关 flaky:multi-workspace archived-listing 文件立即完整重跑为 88/88,通过;core user-memory 超时用例立即单独重跑为 1/1,通过。远端 CI 应确认干净 runner 的结果。

证据(改动前后)

N/A — 仅 observability 改动,不涉及 UI。

测试平台

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

macOS、Node.js 22.22.3、npm 10.9.8、本地双 workspace daemon、JSONL telemetry outfile、禁用 sandbox。

风险与范围

  • 主要风险或取舍:route catalog 或 publication seam 漂移可能导致 span 缺失或归因错误;双向路由枚举、精确 attribution 集合断言、matcher 测试和 resolver seam 测试共同守护这些边界。
  • 未验证 / 范围外:per-workspace HTTP metric label、access-log workspace 字段、rate-limit path normalization、隐式 HEAD/OPTIONS、parser 之前的拒绝 span、workspace session-group 与 workspace-qualified organization 缺口、ACP/WebSocket telemetry,以及开放 secondary branch/fork/cd 执行能力均不在范围内。Windows 和 Linux 未在本地测试。
  • 破坏性变更 / 迁移说明:无。HTTP、SDK、capability、persistence 和 session execution 合约不变;新增分类的 span 和非流式请求 metric 属于预期的 observability 基线变化。

关联 Issue

Refs #6378

Follow-up to #6833

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

Copy link
Copy Markdown
Collaborator Author

Daemon legacy session workspace telemetry E2E

Goal

Verify that legacy session request spans use the workspace selected by route
handling in a daemon with primary and secondary workspaces.

Baseline dry run

  1. Start the globally installed qwen serve with telemetry JSONL output
    enabled.
  2. Call a legacy route that is absent from the old telemetry classifier, such
    as GET /session/missing/status.
  3. Stop the daemon gracefully and inspect only qwen-code.daemon.request
    records to confirm the missing-span baseline.

Implementation verification

Repeat the scenario with the locally built CLI:

  1. A secondary owner-routed request contains the secondary workspace hash.
  2. A primary-bound legacy request contains the primary workspace hash even when
    its session id names a secondary session.
  3. A missing or ambiguous dynamic request omits
    qwen-code.workspace.hash.
  4. A successful SSE connection emits a span on close without adding ordinary
    HTTP request metrics; a failed handshake is counted normally.
  5. Graceful shutdown flushes the JSONL output.
  6. No span attribute contains a raw workspace path.

Result

Passed on 2026-07-16 (Asia/Shanghai).

  • Baseline: global qwen 0.19.10 with telemetry outfile returned 404 for
    GET /session/missing/status and emitted no
    qwen-code.daemon.request span for that route.
  • Local build: a daemon started with canonical primary
    /private/tmp/qwen-telemetry-e2e.uv2HkP/primary and secondary
    /private/tmp/qwen-telemetry-e2e.uv2HkP/secondary workspaces.
  • POST /session targeting the secondary workspace and failing later
    validation with 400 emitted canonical route POST /session and secondary
    hash 2b508937d6a1e479.
  • Primary-bound GET /session/secondary-synthetic/export returned 404 and
    emitted primary hash 138c14a5e7719a30.
  • Dynamic GET /session/missing/status returned 404 and emitted a request span
    without qwen-code.workspace.hash.
  • SIGINT graceful shutdown flushed all three request spans to JSONL.
  • The inspected request-span attributes contained only workspace hashes; no
    raw workspace cwd appeared in span attributes.
  • Successful and failed SSE metric settlement is covered by the middleware
    tests because the outfile exporter records spans, not the in-process metric
    sink calls needed to distinguish those cases.

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

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

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

Qwen Code · serve A/B

@doudouOUC doudouOUC self-assigned this Jul 16, 2026
@doudouOUC
doudouOUC marked this pull request as ready for review July 16, 2026 06:43
@doudouOUC
doudouOUC enabled auto-merge July 16, 2026 06:43
@doudouOUC
doudouOUC requested a review from wenshao July 16, 2026 06:43
@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR! (Re-triage at @wenshao's request, post maintainer verification.)

Template looks good ✓

Problem: Real observability gap, not theoretical. Legacy session routes had incomplete or incorrect workspace attribution — owner-routed requests fell back to the primary workspace hash, and rewind/shell resolved the live owner twice. Documented follow-up to #6833 and #6378, reproducible in any multi-workspace daemon.

Direction: Aligned. Completing the telemetry catalog and fixing workspace attribution is a direct continuation of prior merged work in the same domain. No CHANGELOG reference, but the area is clearly within the project's observability goals.

Size: ~612 production lines (4 files), ~968 test lines (4 files), ~143 docs lines (3 files). All within packages/cli/src/serve/ — single-package, not cross-package. The feat type means no hard block; the 500+ production line advisory was flagged in the prior run and the maintainer has now verified and signed off.

Approach: The declarative catalog is the right structure — 48 routes with a 41/7 attribution split, auditable at a glance, and guarded by a bidirectional drift test. Deferred attribution via setDaemonTelemetryWorkspace() is minimal and correct (first-selection-wins, Symbol-keyed, cleaned up on settlement). The SSE vs request-scoped metrics distinction is well-designed. No simpler path exists that covers the same ground. The review-feedback commits (8167bf5, cb7e92c, 1eefbed) tightened test coverage to 65 assertions including multi-runtime creation, live-owner transcript, ambiguity, and encoded session IDs.

Moving on to code review. 🔍

中文说明

感谢贡献!(应 @wenshao 要求,在维护者验证后重新 triage。)

模板完整 ✓

问题: 真实的可观测性缺口,非理论性关注。Legacy session 路由的 workspace 归因不完整或不正确——owner-routed 请求回退到 primary workspace hash,rewind/shell 重复解析 live owner。是 #6833#6378 的已记录后续工作,在任何多 workspace daemon 中可复现。

方向: 对齐。补齐 telemetry catalog 并修复 workspace 归因是同领域已合并工作的直接延续。无 CHANGELOG 参考,但该领域明确属于项目的可观测性目标。

规模: 约 612 行生产代码(4 个文件),约 968 行测试(4 个文件),约 143 行文档(3 个文件)。全部在 packages/cli/src/serve/ 内——单包,非跨包。feat 类型无硬性阻塞;500+ 生产行关注提示在上次运行中标记,维护者已验证并签字。

方案: 声明式 catalog 是正确的结构——48 条路由,41/7 归因分类,一目了然,由双向漂移测试守护。通过 setDaemonTelemetryWorkspace() 的延迟归因最小且正确(首次选择优先、Symbol 键控、结算后清理)。SSE 与 request-scoped 指标区分设计合理。不存在能以更小复杂度覆盖同样范围的替代路径。评审反馈提交(8167bf5cb7e92c1eefbed)将测试覆盖收紧至 65 个断言,包括多 runtime 创建、live-owner transcript、歧义和编码 session ID。

进入代码审查 🔍

Qwen Code · qwen3.7-max

Reviewed at 1eefbedbf89034dd10b40b3dabd5d80d72b5595c · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Code Review

Independent proposal: Before reading the diff, I'd solve this with: (1) a data-driven route catalog replacing the regex chain in resolveDaemonTelemetryRoute, (2) a deferred attribution hook so handlers publish the resolved workspace after runtime selection, (3) a drift guard test comparing the catalog against Express registrations, and (4) SSE lifetime exclusion from HTTP request metrics. The PR matches this approach exactly and goes further — first-selection-wins semantics, Symbol-keyed response context to avoid namespace collisions, decodePathSegment applied to workspace-scoped routes that previously returned raw URL-encoded session IDs, and proper getDeferredRuntimeRequestTiming integration from the merge-base.

Findings: No critical blockers, no AGENTS.md violations.

  • The catalog-based matcher in matchLegacySessionTelemetryRoute is a clear upgrade over the scattered regex chain — 48 entries with a 41/7 split inspectable at a glance, and the bidirectional drift guard test prevents silent divergence from Express registrations in either direction.
  • Deferred attribution via Symbol-keyed daemonTelemetryResponseContext on the response object is minimal and correct — first-selection-wins (idempotent on same value, ignores later different value), cleaned up after settlement to prevent leaks.
  • The four publication seams (requireSessionRuntime, session creation, session load/resume, transcript resolver) are each correctly placed at the moment of runtime selection, before downstream trust/conflict/validation checks — so failures after unique selection retain the correct attribution.
  • SSE metrics distinction is well-designed: GET /session/:id/events with headersSent=true and status 200 is excluded from HTTP request metrics (connection lifetime ≠ request latency), handshake failures counted normally, and POST /session/:id/generate remains in ordinary metrics since it's bounded request-scoped SSE.
  • Middleware cleanup in server.ts — removing the resolveSessionWorkspaceCwd callback parameter and the inline owner-lookup — simplifies the middleware call site by ~15 lines while making attribution more reliable (no more double-lookup).
  • All telemetry paths are wrapped in try/catch so failures never affect request handling — appropriate defensive coding for an observability layer.

Reuse check: The catalog matcher is purpose-built for Express route matching with attribution metadata. No existing utility or dependency covers this. No reuse concern.

Real-Scenario Testing

This is an observability-only change with no user-visible behavior surface — no TUI output, no API contract changes, no session execution changes. tmux testing cannot meaningfully exercise internal telemetry middleware attribution. Verification relies on:

  • CI tests passed (31m48s on ubuntu-latest): all unit tests green, including the 4 new/updated test files with 65 assertions covering catalog drift guard, canonical matcher cases, deferred attribution, first-selection-wins, SSE metrics exclusion, heartbeat metrics ring exclusion, settlement after runtime removal, pre-resolved resolver failure resilience, malformed encoding retention, multi-runtime creation, live-owner transcript, and dual-runtime ambiguity.
  • Serve A/B passed (14m43s): the dedicated serve regression suite confirmed no behavioral regressions.
  • Web Shell E2E Smoke passed (5m37s): end-to-end smoke test confirmed no breakage.
  • Maintainer live daemon E2E (@wenshao, at 1eefbed): 65/65 targeted tests pass, tamper proofs confirm the drift guard and publication seams are load-bearing (deleting a catalog entry or a setDaemonTelemetryWorkspace call site flips the expected tests red), and a real two-workspace daemon A/B drove 9 scenarios with correct attribution on the PR head vs 2 wrong/lossy spans on the merge-base. Wire parity confirmed — identical HTTP status codes on both builds, no raw workspace paths in span attributes.

The unit tests and the maintainer's live E2E comprehensively cover the critical paths. This testing approach matches the change's nature.

中文说明

代码审查

独立方案: 在阅读 diff 之前,我会用以下方式解决:(1) 数据驱动的路由 catalog 替代 resolveDaemonTelemetryRoute 中的正则链,(2) 延迟归因钩子让 handler 在 runtime 选择后发布解析的 workspace,(3) drift guard 测试比较 catalog 与 Express 注册,(4) SSE 生命周期从 HTTP 请求指标中排除。PR 完全匹配此方案并更进一步——首次选择优先语义、Symbol 键控的 response context 避免命名空间冲突、decodePathSegment 应用于之前返回原始 URL 编码 session ID 的 workspace-scoped 路由、以及从 merge-base 正确集成 getDeferredRuntimeRequestTiming

发现: 无关键阻塞项,无 AGENTS.md 违规。

  • matchLegacySessionTelemetryRoute 中基于 catalog 的匹配器明显优于散落的正则链——48 条目,41/7 分类一目了然,双向 drift guard 测试防止在任一方向与 Express 注册静默偏离。
  • 通过 Symbol 键控 daemonTelemetryResponseContext 的延迟归因最小且正确——首次选择优先(相同值幂等,忽略后续不同值),结算后清理以防泄漏。
  • 四个发布点(requireSessionRuntime、session 创建、session 加载/恢复、transcript resolver)都正确放置在 runtime 选择时刻,在后续 trust/conflict/validation 检查之前——因此唯一选择后的失败保留正确归因。
  • SSE 指标区分设计合理:GET /session/:id/eventsheadersSent=true 且状态 200 时从 HTTP 请求指标中排除(连接生命周期 ≠ 请求延迟),握手失败正常计数,POST /session/:id/generate 保持在普通指标中因为是有界的 request-scoped SSE。
  • server.ts 中的 middleware 清理——移除 resolveSessionWorkspaceCwd 回调参数和内联 owner 查找——简化 middleware 调用点约 15 行,同时使归因更可靠(不再重复查找)。
  • 所有 telemetry 路径都用 try/catch 包裹,确保故障不影响请求处理——对可观测性层来说是合适的防御。

复用检查: catalog 匹配器是为带归因元数据的 Express 路由匹配专门构建的。没有现有工具或依赖覆盖此功能。无复用问题。

真实场景测试

这是仅涉及可观测性的改动,无用户可见行为面——无 TUI 输出、无 API 合约变更、无 session 执行变更。tmux 测试无法有意义地验证内部 telemetry middleware 归因。验证依赖:

  • CI 测试通过(ubuntu-latest 31m48s):所有单元测试通过,包括 4 个新增/更新的测试文件,65 个断言覆盖 catalog drift guard、canonical matcher 场景、延迟归因、首次选择优先、SSE 指标排除、heartbeat 指标环排除、runtime 移除后的结算、pre-resolved resolver 失败弹性、畸形编码保留、多 runtime 创建、live-owner transcript 和双 runtime 歧义。
  • Serve A/B 通过(14m43s):专项 serve 回归套件确认无行为回归。
  • Web Shell E2E Smoke 通过(5m37s):端到端冒烟测试确认无破损。
  • 维护者在线 daemon E2E@wenshao,在 1eefbed):65/65 专项测试通过,破坏性实验确认 drift guard 和发布点是承重的(删除 catalog 条目或 setDaemonTelemetryWorkspace 调用点会精确翻红预期测试),真实双 workspace daemon A/B 驱动 9 个场景在 PR head 上归因正确,而 merge-base 只有 2 个错误或丢失的 span。Wire 一致性确认——两个构建上 HTTP 状态码完全相同,span 属性中无原始 workspace path。

单元测试和维护者的在线 E2E 全面覆盖了关键路径。此测试方式匹配改动的性质。

Qwen Code · qwen3.7-max

Reviewed at 1eefbedbf89034dd10b40b3dabd5d80d72b5595c · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — clean review across all stages, maintainer has independently verified with live daemon E2E and signed off LGTM, all review feedback addressed, CI green at the reviewed commit.

This is a well-executed observability PR that has matured through multiple review rounds into a solid, thoroughly-verified change. The declarative catalog replaces a fragile regex chain with an auditable, drift-guarded data structure. The deferred attribution mechanism is minimal and correct — first-selection-wins, Symbol-keyed context, proper cleanup on settlement. Tests grew to 65 assertions covering every critical path. The maintainer's live E2E with tamper proofs is the strongest verification this kind of change can get.

The concerns that prompted the previous Stage 3 defer (telemetry infrastructure scope, 612 production lines, new handler contract) have been fully addressed by the maintainer's independent verification — @wenshao confirmed every publication seam is load-bearing, the fail-safe direction is correct (omit hash, never misattribute), and wire parity holds across all driven scenarios.

Approving. ✅

中文说明

置信度:5/5 — 各阶段审查均通过,维护者已独立通过在线 daemon E2E 验证并签字 LGTM,所有评审反馈已处理,CI 在所审查的 commit 上全绿。

这是一个执行良好的可观测性 PR,经过多轮评审迭代已成熟为扎实、充分验证的改动。声明式 catalog 用可审计、有 drift guard 的数据结构替代了脆弱的正则链。延迟归因机制最小且正确——首次选择优先、Symbol 键控 context、结算时正确清理。测试增长至 65 个断言,覆盖每个关键路径。维护者的在线 E2E 及破坏性实验是此类改动能获得的最强验证。

之前 Stage 3 延迟的顾虑(telemetry 基础设施范围、612 行生产代码、新 handler 合约)已被维护者的独立验证完全解决——@wenshao 确认每个发布点是承重的,fail-safe 方向正确(省略 hash,绝不误归因),wire 一致性在所有驱动场景中成立。

批准。 ✅

Qwen Code · qwen3.7-max

Reviewed at 1eefbedbf89034dd10b40b3dabd5d80d72b5595c · re-run with @qwen-code /triage

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — no blockers. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

Comment thread packages/cli/src/serve/routes/session.ts
Comment thread packages/cli/src/serve/routes/session.ts
Comment thread packages/cli/src/serve/routes/session.ts
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

doudouOUC commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

[codex] Review feedback addressed in 8167bf5bdc.

Feedback Action
Multi-runtime secondary session creation publication Added a two-runtime creation test asserting exactly one publication for the secondary runtime.
Multi-runtime live-owner transcript publication Added a test for a live session owned by the secondary runtime.
Sole-active-storage transcript publication Added a test that scans two runtimes and attributes the only active transcript to the secondary runtime.

Validation: targeted telemetry/resolver suite 62/62, full workspace typecheck, ESLint, and Prettier passed. verify:pr was skipped per the PR author's explicit direction.

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

Comment thread packages/cli/src/serve/routes/session.ts
Comment thread packages/cli/src/serve/server/telemetry.ts
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

[codex] Review feedback addressed in cb7e92ce46.

Feedback Action
Ambiguous transcript storage attribution Added a dual-runtime ambiguity test asserting a 500 response and no workspace publication.
Encoded workspace transcript session ID Updated the workspace transcript test to use session%2F1 and assert session/1.

Validation: targeted tests 55/55, full workspace typecheck, ESLint, and Prettier passed. verify:pr was skipped per the explicit direction from the PR author.

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI failing: route. Reviewed.

— qwen3.7-max via Qwen Code /review

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

[codex] Thanks — no code change is needed for this review result. The cancelled route job belongs to a Qwen Autofix run triggered by the PR author review reply; the current PR checks are 11/11 passing, and the watcher found no failed PR check to rerun.

@wenshao

wenshao commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /resolve

@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

Qwen Code resolved the merge conflicts and pushed the branch update.

Merge Conflict Resolution Summary — PR #7003

Base branch: main
PR branch: agent/legacy-session-workspace-telemetry
Commit: chore(serve): resolve merge conflicts between legacy session telemetry and deferred runtime

Conflicted Files

1. packages/cli/src/serve/server/telemetry.ts (2 conflicts)

Conflict 1 — startMs initialization + response context setup:

  • HEAD (PR): Introduced telemetryRes variable (cast of res) and set up daemonTelemetryResponseContext for handler_resolved routes, with a simple Date.now() for startMs.
  • origin/main: Introduced deferredRuntime = getDeferredRuntimeRequestTiming(req) and computed startMs from deferredRuntime?.startedAt.getTime() ?? Date.now().
  • Resolution: Kept both changes — deferredRuntime timing from main, followed by the telemetryRes + handler-resolved context setup from the PR. These are independent concerns that compose cleanly.

Conflict 2 — recordDaemonHttpRequest call:

  • HEAD (PR): Added SSE exclusion logic — successful SSE connections (GET /session/:id/events, status 200, headers sent) are not recorded as HTTP requests to avoid counting long-lived SSE as latency. Called recordDaemonHttpRequest(durationMs, route.route, res.statusCode) with 3 args.
  • origin/main: Added a 4th argument (deferredRuntime?.path) to recordDaemonHttpRequest to track deferred runtime path in metrics. Called it unconditionally (no SSE exclusion).
  • Resolution: Kept the SSE exclusion guard from the PR, and passed deferredRuntime?.path as the 4th argument from main. The successfulSse variable is still referenced downstream for the recordRequest metrics-ring exclusion.

2. packages/cli/src/serve/server/telemetry.test.ts (1 conflict)

Conflict — import block:

  • HEAD (PR): Imported daemonTelemetryMiddleware, legacySessionTelemetryRoutes, resolveDaemonTelemetryRoute, setDaemonTelemetryWorkspace from ./telemetry.js and `M

wenshao
wenshao previously approved these changes Jul 16, 2026

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not reviewed: verification — the review posts findings, but no verifier ran (Step 4 builds its prompt with agent-prompt --role verify; none was recorded, so the findings were not verified).

— qwen3.7-max via Qwen Code /review

Comment thread packages/cli/src/serve/server/telemetry.test.ts
Comment thread packages/cli/src/serve/server/telemetry.test.ts
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author
Thread Action Result
recordDaemonHttpRequest exact-arity assertions Fixed Added the explicit fourth undefined argument in commit 1eefbedbf8; all 48 targeted tests pass.
Telemetry-disabled workspace hashing Rejected JavaScript optional chaining skips argument evaluation when span is undefined; the existing regression test passes.

Verification: targeted Vitest 48/48, Prettier, npm run build, and npm run typecheck all passed. Per maintainer instruction, npm run verify:pr was not run.

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Downgraded from Approve to Comment: CI failing: route. Reviewed.

— qwen3.7-max via Qwen Code /review

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

[codex] Thanks — won’t take this one. The cancelled route run 29549036324 was superseded by successful route runs 29549039046 and 29551511646; all current checks are green.

@doudouOUC
doudouOUC requested a review from wenshao July 17, 2026 06:18
@wenshao

wenshao commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

Maintainer verification — local real-build & live-daemon E2E at 1eefbed

Verdict: LGTM. Everything this PR claims was reproduced locally against real builds: the catalog and its drift guard are load-bearing, workspace attribution binds to the runtime the handler actually selected, unresolved requests omit the hash instead of falling back to primary, the documented primary-bound exceptions behave as documented, SSE settlement works, and the HTTP wire is byte-compatible with the merge-base on every driven scenario.

Setup

  • Isolated detached worktree at head 1eefbed, full npm ci + bundle; a second worktree at the merge-base 10a39d15d built identically for A/B.
  • Bundle identity check: the PR-unique symbol daemonTelemetryResponseContext is present in the PR bundle's server chunk and absent from the base bundle, so the A/B really compares PR vs base code.
  • macOS, Node v22.23.1, npm 10.9.8.

1. Targeted suites, typecheck, lint — plus two tamper proofs

vitest run on the four telemetry/resolver files: 65/65 pass (48 + 7 + 1 + 9; the count grew from the PR body's 59 via the review-feedback commits). Workspace typecheck, eslint, and prettier on the touched files all pass.

To confirm the guards are load-bearing rather than decorative, I broke the code both ways and watched the right tests go red:

  • Tamper A — deleting one catalog entry (GET /session/:id/status) flips the drift guard: expected [ …(47) ] to deeply equal [ …(48) ]. The catalog cannot silently diverge from the Express registrations in either direction.
  • Tamper B — removing the live-owner setDaemonTelemetryWorkspace() publication in requireSessionRuntime fails 4 tests that name that exact seam.

Both tampers were reverted; worktree clean afterwards.

targeted suites + tamper proofs

2. Static audit of the publication seams

  • All 41 handler_resolved routes flow through one of the four publishing resolvers (requireSessionRuntime, session-creation, session-restore, transcript resolver) — spot-checked the inline-registered ones (status, pending-prompts, DELETE /session/:id) individually.
  • Failure direction is fail-safe: a resolver that bails before selection only omits the hash; nothing can misattribute, because publication is first-selection-wins and happens at the moment of runtime selection.
  • The drift guard's four registrars are the complete set of legacy-route registration sites — the only other textual app.post('/session'…) hit in serve/ is a doc comment in auth.ts.

3. Live two-workspace daemon A/B (real bundles, identical driver)

node dist/cli.js serve --port 0 --hostname 127.0.0.1 --workspace <primary> --workspace <secondary> --token … with QWEN_TELEMETRY_ENABLED=true QWEN_TELEMETRY_TARGET=local QWEN_TELEMETRY_OUTFILE=… OTEL_BSP_SCHEDULE_DELAY=300, isolated HOME, dummy OpenAI env. A real live session was created in the secondary workspace (POST /session {cwd: secondary} → 200), then 10 scenarios were driven against each build and the qwen-code.daemon.request spans read off the outfile.

PR head — 9/9 driven scenarios span, all attributions correct:

Scenario Span result
POST /session creating in secondary 200 · secondary hash (late-bound creation attribution)
GET /session/:id/status on the live secondary session 200 · secondary hash (live-owner routing)
GET /session/foo%2Fbar/status 404 · canonical route, session.id decoded to foo/bar, no hash
GET /session/missing/status, POST /session/missing/prompt 404 · span present, no hash (no primary fallback)
GET /session/:id/export (secondary session id) 404 · primary hash — documented primary-bound exception
POST /sessions/archive 200 · primary hash (pre-resolved)
GET /session/:id/events SSE held 1400 ms span settles on close, duration 1405 ms, secondary hash
GET /session/missing/events failed handshake 404 · counted normally

Merge-base, same driver — 2 spans total, and both are wrong or lossy: POST /session carries the primary hash even though the session was created in the secondary workspace; POST /session/missing/prompt falls back to the primary hash; status/export/archive/events produced no span at all.

live A/B span tables

4. Wire parity & span hygiene

  • Every one of the 10 driven requests returned the identical HTTP status on both builds — consistent with the observability-only claim (and with the serve A/B bot's result).
  • Raw-path leak scan: 0 of 9 PR request spans contain a workspace cwd; only 16-hex sha256 hashes appear.
  • SIGINT graceful shutdown flushed all 9 request spans to the outfile.
  • Cross-process spans (qwen-code.daemon.bridge, qwen-code.daemon.session_start) present and unchanged on both builds; the session.id resource attribute on parameterless routes (daemon:<pid>) is pre-existing and identical on both.

wire parity + hygiene

Not covered locally

  • The in-process HTTP metric sink isn't observable through the outfile exporter, so successful-SSE exclusion vs counted failed-handshake at the metrics level rests on the 48 middleware unit tests (which assert exactly that seam).
  • Windows/Linux not exercised locally; remote CI is green at the same head (unit 31m48s, Serve A/B 14m43s, web-shell smoke 5m37s).
中文版本(Chinese version)

维护者验证 — 在 1eefbed 上的本地真实构建与在线 daemon E2E

结论:LGTM。 本 PR 的全部声明都在本地真实构建上得到复现:catalog 与 drift guard 确实承担守护职责;workspace 归因绑定到 handler 实际选择的 runtime;无法解析的请求省略 hash 而不再回退 primary;文档声明的 primary-bound 例外行为与文档一致;SSE 结算正确;所有驱动场景下 HTTP wire 与 merge-base 完全一致。

环境

  • 在 head 1eefbed 建立隔离 detached worktree,完整 npm ci + bundle;另在 merge-base 10a39d15d 以相同方式构建第二个 worktree 用于 A/B。
  • 产物身份核验:PR 独有符号 daemonTelemetryResponseContext 存在于 PR bundle 的 server chunk、不存在于基线 bundle,确保 A/B 比较的确实是 PR 与基线代码。
  • macOS、Node v22.23.1、npm 10.9.8。

1. 专项测试、typecheck、lint,以及两个破坏性实验

对 4 个 telemetry/resolver 测试文件运行 vitest run:65/65 通过(48 + 7 + 1 + 9;比 PR 描述中的 59 多,来自评审反馈提交新增的用例)。workspace typecheckeslintprettier(改动文件)全部通过。

为确认这些守卫是承重的而非装饰性的,我从两个方向破坏代码并观察对应测试翻红:

  • 实验 A — 删除一条 catalog 条目(GET /session/:id/status)后 drift guard 立即失败:expected [ …(47) ] to deeply equal [ …(48) ]。catalog 无法在任一方向与 Express 注册静默漂移。
  • 实验 B — 移除 requireSessionRuntime 中 live-owner 的 setDaemonTelemetryWorkspace() 发布点,4 个测试失败并精确点名该 seam。

两个实验均已还原,worktree 干净。

专项测试与破坏性实验

2. 发布点静态审计

  • 全部 41 条 handler_resolved 路由都经过 4 个发布 resolver 之一(requireSessionRuntime、session 创建、session 恢复、transcript resolver)——内联注册的 statuspending-promptsDELETE /session/:id 逐一抽查。
  • 失败方向是 fail-safe:resolver 在选择前退出只会省略 hash;由于发布是首次选择生效、且发生在 runtime 选择时刻,不可能产生误归因。
  • drift guard 的 4 个注册器就是 legacy 路由注册点的全集——serve/ 下唯一的其他 app.post('/session'…) 文本命中是 auth.ts 中的文档注释。

3. 双 workspace 在线 daemon A/B(真实产物,同一驱动脚本)

node dist/cli.js serve --port 0 --hostname 127.0.0.1 --workspace <primary> --workspace <secondary> --token …,附 QWEN_TELEMETRY_ENABLED=true QWEN_TELEMETRY_TARGET=local QWEN_TELEMETRY_OUTFILE=… OTEL_BSP_SCHEDULE_DELAY=300、隔离 HOME、dummy OpenAI 环境变量。先在 secondary workspace 创建真实 live session(POST /session {cwd: secondary} → 200),然后对两个构建驱动 10 个场景,从 outfile 读取 qwen-code.daemon.request span。

PR head — 9/9 场景产生 span,归因全部正确:

场景 Span 结果
POST /session 在 secondary 创建 200 · secondary hash(创建时晚绑定归因)
对 live secondary session 的 GET /session/:id/status 200 · secondary hash(live-owner 路由)
GET /session/foo%2Fbar/status 404 · canonical route,session.id 解码为 foo/bar,无 hash
GET /session/missing/statusPOST /session/missing/prompt 404 · span 存在,无 hash(不再回退 primary)
GET /session/:id/export(secondary session id) 404 · primary hash——文档声明的 primary-bound 例外
POST /sessions/archive 200 · primary hash(pre-resolved)
GET /session/:id/events SSE 持流 1400 ms span 在连接关闭时结算,时长 1405 ms,secondary hash
GET /session/missing/events 握手失败 404 · 正常计数

merge-base,同一驱动 — 总共只有 2 个 span,且均错误或有损:POST /session 虽在 secondary 创建却带 primary hash;POST /session/missing/prompt 回退 primary hash;status/export/archive/events 完全没有 span

在线 A/B span 对比

4. Wire 一致性与 span 卫生

  • 10 个驱动请求在两个构建上返回完全相同的 HTTP 状态——与"仅可观测性改动"的声明一致(也与 serve A/B bot 的结果一致)。
  • 原始路径泄漏扫描:9 个 PR request span 中 0 个包含 workspace cwd;只出现 16 位 sha256 hash。
  • SIGINT 优雅关停将全部 9 个 request span 刷入 outfile。
  • 跨进程 span(qwen-code.daemon.bridgeqwen-code.daemon.session_start)在两个构建上均存在且不变;无参数路由上的 session.id 资源属性(daemon:<pid>)为既有行为,两侧一致。

Wire 一致性与卫生检查

本地未覆盖

  • 进程内 HTTP metric sink 无法通过 outfile exporter 观测,因此 SSE 成功流排除计数 / 握手失败正常计数这一层依赖 48 个 middleware 单元测试(正是断言该 seam)。
  • Windows/Linux 未在本地验证;远端 CI 在同一 head 全绿(unit 31m48s、Serve A/B 14m43s、web-shell smoke 5m37s)。

@wenshao

wenshao commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@doudouOUC
doudouOUC added this pull request to the merge queue Jul 17, 2026
Merged via the queue into QwenLM:main with commit e74c0cd Jul 17, 2026
94 of 96 checks passed
@doudouOUC
doudouOUC deleted the agent/legacy-session-workspace-telemetry branch July 20, 2026 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants