Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
b06954f
fix(telemetry): align session lifecycle with OTel
zjunothing Aug 6, 2026
e774bb6
Merge branch 'main' into fix/issue-8589-otel-session-lifecycle
wenshao Aug 6, 2026
5b9c8c6
feat(telemetry): complete session lifecycle coverage
zjunothing Aug 6, 2026
716b243
Merge branch 'main' into fix/issue-8589-otel-session-lifecycle
wenshao Aug 6, 2026
babe9db
fix(telemetry): deduplicate deferred session starts
zjunothing Aug 7, 2026
0588bf9
test(telemetry): cover duplicate session starts
zjunothing Aug 7, 2026
aba197f
fix(serve): emit daemon session starts
zjunothing Aug 7, 2026
8cbb1ed
Merge branch 'main' into fix/issue-8589-otel-session-lifecycle
qwen-code-dev-bot Aug 7, 2026
8274e0c
fix(telemetry): repair session lifecycle test wiring and record attri…
qwen-code-dev-bot Aug 7, 2026
cdfaea1
Merge branch 'main' into fix/issue-8589-otel-session-lifecycle
qwen-code-dev-bot Aug 8, 2026
05859b8
fix(telemetry): skip session lifecycle transition on same-id resume (…
qwen-code-dev-bot Aug 8, 2026
946b27a
Merge branch 'main' into fix/issue-8589-otel-session-lifecycle
qwen-code-dev-bot Aug 8, 2026
95e2b18
test(telemetry): pin session lifecycle behaviors per review (#8616)
qwen-code-dev-bot Aug 8, 2026
7e30622
Merge branch 'main' into fix/issue-8589-otel-session-lifecycle
qwen-code-dev-bot Aug 8, 2026
2ac0d47
fix(telemetry): emit session start catch-up on every init path (#8616)
qwen-code-dev-bot Aug 8, 2026
2b160e0
Merge branch 'main' into fix/issue-8589-otel-session-lifecycle
qwen-code-dev-bot Aug 8, 2026
33916e2
Merge remote-tracking branch 'origin/main' into fix/issue-8589-otel-s…
qwen-code-dev-bot Aug 9, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions docs/design/otel-session-lifecycle-design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# OpenTelemetry Session Lifecycle

## Status

Implemented in issue #8589.

## Scope

Qwen Code already records the application session ID as `session.id` and maps
it to `gen_ai.conversation.id` on GenAI LLM and agent spans. This design adds
the OpenTelemetry General Session lifecycle events without removing the
existing Qwen-specific telemetry fields or event names.

The implementation follows the Development-status General Session semantic
conventions at:

<https://opentelemetry.io/docs/specs/semconv/general/session/>

The GenAI conversation mapping follows:

<https://github.com/open-telemetry/semantic-conventions-genai/blob/main/docs/gen-ai/gen-ai-agent-spans.md>

## Event representation

The standard lifecycle events are emitted as OpenTelemetry LogRecords with
the required `event.name` attribute:

| Event | Required attributes | Emission point |
| --------------- | ------------------- | -------------------------------------------------------- |
| `session.start` | `session.id` | Initial `Config` initialization and every session switch |
| `session.end` | `session.id` | Session switch and telemetry shutdown |

The existing `qwen-code.config` / `cli_config` and RUM `session_start` events
remain unchanged for backward compatibility. The standard records are
additive and are emitted through the configured OpenTelemetry logs pipeline.

## Session continuation

`Config.startNewSession()` is used for both replacing the current conversation
(`/clear`, `/new`) and resuming a persisted conversation. A persisted
`sessionData` argument identifies the latter continuation case. On a
continuation, the new `session.start` record includes
`session.previous_id`; replacement sessions do not claim continuation.

The outgoing session is ended before the new session starts. Resuming the
session the user is already in (same `session.id`) records no lifecycle
transition at all. Telemetry shutdown ends the currently active session
before shutting down the SDK.

## Session id reuse on `/resume`

Qwen Code's session model predates this design: `/resume` restores a
persisted conversation under its original session id instead of minting a new
one. Two consequences follow for the lifecycle stream:

- A resumed id can carry more than one disjoint
`session.start`/`session.end` window within a single process (for example:
start `A`, `/clear` to `B`, then `/resume` back to `A`).
- `session.previous_id` points from the resumed id to the session that was
active at resume time. That session may have been created _after_ the
resumed id, so lineage edges can point backwards in time and can form
cycles.

This is the reverse of the OTel General Session convention's id-rotation
model, in which a freshly minted id points back at the retired one. Backends
counting sessions or computing durations should key on
(`session.id`, `session.start` timestamp) windows rather than `session.id`
alone. Whether `/resume` should mint a new id instead is a session-model
decision outside this design.

## Known limitations (daemon / ACP)

Daemon-spawned ACP sessions build a fresh `Config` per session
(`loadCliConfig()`) and never flow through `Config.startNewSession()`, so in
that path today:

- a conversation session receives `session.start` from its `Config`
initialization but no `session.end` when the session is later switched or
disposed, and
- process shutdown ends the session id last recorded in the telemetry session
context — in an ACP child that is the boot-time session, not the
conversation session.

A single ACP child can also host several concurrent sessions, which the
single process-level "current session" tracked by the context cannot
represent. Closing this gap requires lifecycle design for multi-session
processes and is deferred to a follow-up.
Comment on lines +86 to +87

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.

[Suggestion] The PR closes #8589 but leaves two of its acceptance criteria unmet on the daemon/ACP path, and the follow-up this sentence defers to does not exist as a tracked issue (two GitHub searches found only #8589 itself). Verified in code: every ACP session teardown passes shutdownTelemetry: false (acpAgent.ts), and the only production logSessionEnd call site is Config.startNewSession (config.ts), which these sessions never flow through — so a daemon-served conversation session emits session.start from Config.initialize() and is never ended when disposed. — Failure scenario: issue #8589's criteria require "a closed session produces an OTel session.end event with the same session.id" and tests covering "close, multi-session attribution"; backends keying session windows on (session.id, session.start) — the pairing this design doc tells consumers to use — keep every daemon-served session open indefinitely, and merging "Closes #8589" drops this half with no tracked remainder. Fix: file a follow-up issue covering session.end on ACP/daemon per-session disposal and reference it here and in the PR description before merge, or obtain explicit maintainer sign-off on the reduced scope in the PR thread.

中文说明

本 PR 声称关闭 #8589,但 daemon/ACP 路径上该 issue 的两条验收标准仍未满足,而本句所延后的后续工作并不存在对应的跟踪 issue(两次 GitHub 搜索只找到 #8589 本身)。已在代码中核实:ACP 每个会话的 teardown 都传 shutdownTelemetry: false(acpAgent.ts),而 logSessionEnd 唯一的生产调用点是 Config.startNewSession(config.ts),这些会话从不经过该路径——因此 daemon 服务的会话会在 Config.initialize() 发出 session.start,被销毁时却永远不发 end。触发场景:#8589 的验收标准要求“关闭的会话以相同 session.id 产生 OTel session.end 事件”、测试需覆盖“close、多会话归属”;按本设计文档告知消费者的 (session.id, session.start) 配对方式划定会话窗口的后端,会看到 daemon 服务的每个会话永远不关闭;合并“Closes #8589”将使这一半验收标准在无跟踪残留的情况下被丢弃。修复:在合并前创建一个覆盖 ACP/daemon 每会话销毁时补发 session.end 的后续 issue,并在此处与 PR 描述中引用;或在 PR 讨论中获得维护者对缩减范围的明确认可。

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


## Compatibility and safety

- `session.id` remains on existing spans and logs.
- `gen_ai.conversation.id` remains the session correlation field for GenAI
spans.
- `session.previous_id` is emitted only when the application has an explicit
persisted continuation, and it is never equal to the new `session.id`.
- Cold-start resumptions (`--resume`, `--continue`, `--fork-session`) do not
carry `session.previous_id`; startup lineage, including the fork source, is
left to a follow-up.
- Session event emission is best-effort through the existing OTel logger and
does not block session switching or shutdown.
20 changes: 20 additions & 0 deletions docs/developers/development/telemetry.md
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,12 @@ The following events are logged:
- `qwen-code.config`: Emitted once at startup with CLI configuration.
- **Attributes**: `model`, `sandbox_enabled`, `core_tools_enabled`, `approval_mode`, `file_filtering_respect_git_ignore`, `debug_mode`, `truncate_tool_output_threshold`, `truncate_tool_output_lines`, `hooks` (comma-separated, omitted if disabled), `ide_enabled`, `interactive_shell_enabled`, `mcp_servers`, `mcp_servers_count`, `mcp_tools`, `mcp_tools_count`, `output_format`, `skills`, `subagents`

- `session.start`: A session begins. Emitted after telemetry initialization at startup and again on every session switch; lifecycle semantics are described in the Spans section.
- **Attributes**: `session.id` (string), `session.previous_id` (string, present only when this start continues a persisted conversation under a new session id)
Comment on lines +565 to +566

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.

[Suggestion] This attribute description uses the OTel id-rotation model ("continues a persisted conversation under a new session id"), but the implementation's primary continuation flow does the reverse: /resume continues under the ORIGINAL persisted id, and session.previous_id points from it to the session that was active at resume time — which can be NEWER (start A → /clear to B → /resume back to A emits two disjoint windows keyed on session.id=A plus an A→B edge where B is newer). The Spans section this line delegates to covers only previous_id presence, and the design doc's id-reuse warnings appear nowhere in this file (verified: reuse/disjoint/cycle/backwards are absent from telemetry.md). — Failure scenario: a backend built from this doc keys lineage/duration on session.id alone, merging the two disjoint A windows into one wrong duration and orienting A→B as if A were minted after B — exactly the miscounting the design doc says backends must guard against by keying on (session.id, session.start) windows. Fix: reword to the implemented semantics and add the design doc's caveats (disjoint windows per id, backwards/cyclic edges, (session.id, session.start) keying), linking to the design doc.

Suggested change
- `session.start`: A session begins. Emitted after telemetry initialization at startup and again on every session switch; lifecycle semantics are described in the Spans section.
- **Attributes**: `session.id` (string), `session.previous_id` (string, present only when this start continues a persisted conversation under a new session id)
- `session.start`: A session begins. Emitted after telemetry initialization at startup and again on every session switch; lifecycle semantics are described in the Spans section.
- **Attributes**: `session.id` (string), `session.previous_id` (string, present only when this start resumes a persisted conversation; it carries the session id that was active at resume time, which can be newer than the resumed id because `/resume` continues under the original persisted session id)
中文说明

该属性描述套用了 OTel 的 id 轮换模型(“在新的 session id 下延续持久化会话”),但实现中的主要续接流程恰好相反:/resume持久化 id 下延续,session.previous_id 从它指向恢复时刻正在使用的会话——而那个会话可能更新(启动 A → /clear 到 B → /resume 回 A,会导出两条以 session.id=A 为键的不相交窗口,外加一条 B 比 A 新的 A→B 边)。本行指向的 Spans 章节只说明了 previous_id 的出现条件;设计文档中关于 id 复用的警告在本文件中完全缺失(已验证:telemetry.md 中不含 reuse/disjoint/cycle/backwards)。触发场景:按本文档实现的后端只以 session.id 为键计算谱系/时长,会把两条不相交的 A 窗口合并成一个错误时长,并把 A→B 边的方向误判为 A 晚于 B 产生——正是设计文档明确要求后端通过以 (session.id, session.start) 窗口为键来规避的错误统计。修复:按实际语义改写,并补上设计文档中的注意事项(同一 id 多段不相交窗口、时间倒置/成环的边、以 (session.id, session.start) 为键),同时链接设计文档。

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


- `session.end`: A session ends. Emitted before a session switch replaces the current session, and at telemetry shutdown.
- **Attributes**: `session.id` (string)

- `qwen-code.user_prompt`: User submits a prompt.
- **Attributes**: `prompt_length` (int), `prompt_id` (string), `prompt` (string, excluded if `log_prompts_enabled` is false), `auth_type` (string)

Expand Down Expand Up @@ -858,6 +864,20 @@ The daemon process (long-running HTTP server mode) exposes its own metrics.

Distributed tracing spans form a tree rooted at `qwen-code.interaction`. Each interaction is a trace root with its own `traceId`; cross-prompt correlation uses the `session.id` attribute.

Session lifecycle is also exported through the OpenTelemetry General Session
semantic conventions. When the OTel logs pipeline is enabled, Qwen Code emits
`session.start` and `session.end` log events with the required `session.id`
Comment on lines +867 to +869

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.

[Suggestion] The new LogRecord events are documented only as prose here in the Spans section, while every sibling log event has a bullet entry in the Logs catalog ("The following events are logged:" → "Core Session Events", ~line 556, where qwen-code.config etc. are listed) — Concrete cost: a consumer consulting that catalog to build a dashboard or query finds no session.start/session.end entries and concludes they don't exist. Suggested fix: add bullet entries there (with session.id and optional session.previous_id), keeping only a cross-reference in this section.

中文说明

新的 LogRecord 事件仅以散文形式记录在 Spans 章节,而 Logs 目录("The following events are logged:" → "Core Session Events",约 556 行)中每个同级日志事件都有条目。代价:按该目录构建仪表板的用户找不到 session.start/session.end,会以为它们不存在。建议:在目录中补充条目,此处仅保留交叉引用。

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

attribute (cataloged under Core Session Events above). A resumed persisted
conversation includes `session.previous_id` on its `session.start` event only
when the resumed session id differs from the current one; cold-start
resumptions (`--resume`, `--continue`, `--fork-session`) do not carry it.
`/clear` and other replacement flows intentionally do not claim continuation
because they discard the previous conversation.

The existing Qwen-specific `qwen-code.config`/`cli_config` and RUM
`session_start` records remain available for compatibility. GenAI request
spans continue to use `gen_ai.conversation.id` for the same owning session ID.

- `qwen-code.interaction`: Root span for each user prompt turn.
- **Attributes**: `session.id`, optional ARMS extension `gen_ai.user.id`, `qwen-code.prompt_id`, `qwen-code.message_type`, `qwen-code.model`, `qwen-code.approval_mode`, `interaction.sequence`, `interaction.duration_ms`, `qwen-code.turn_status` ("ok"/"error"/"cancelled")

Expand Down
1 change: 1 addition & 0 deletions packages/core/src/config/config-session-env.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ vi.mock('../telemetry/index.js', () => ({
isTelemetrySdkInitialized: vi.fn().mockReturnValue(false),
shutdownTelemetry: vi.fn().mockResolvedValue(undefined),
refreshSessionContext: vi.fn(),
logSessionEnd: vi.fn(),
}));
vi.mock('../core/contentGenerator.js', () => ({
resolveContentGeneratorConfigWithSources: vi.fn().mockReturnValue({
Expand Down
88 changes: 88 additions & 0 deletions packages/core/src/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ import {
isTelemetrySdkInitialized,
shutdownTelemetry,
refreshSessionContext,
logStartSession,
logSessionEnd,
} from '../telemetry/index.js';
import type {
ContentGenerator,
Expand Down Expand Up @@ -326,6 +328,8 @@ vi.mock('../telemetry/loggers.js', async (importOriginal) => {
return {
...actual,
logRipgrepFallback: vi.fn(),
logStartSession: vi.fn(actual.logStartSession),
logSessionEnd: vi.fn(actual.logSessionEnd),
};
});

Expand Down Expand Up @@ -2232,6 +2236,84 @@ describe('Server Config (config.ts)', () => {
});

describe('startNewSession', () => {
it('records no lifecycle transition when resuming the current session id', async () => {
const sessionId = 'same-session-id';
const config = new Config({ ...baseParams, sessionId });
await config.initialize({
skipGeminiInitialization: true,
skipHooks: true,
skipMcpDiscovery: true,
skipSkillManager: true,
skipFileCheckpointing: true,
});
vi.mocked(logSessionEnd).mockClear();
vi.mocked(logStartSession).mockClear();

config.startNewSession(sessionId, {
conversation: { messages: [] },
} as unknown as ResumedSessionData);

expect(logSessionEnd).not.toHaveBeenCalled();
expect(logStartSession).toHaveBeenCalledWith(
config,
expect.anything(),
undefined,
);
});

it('ends the outgoing session before starting a replacement without continuation', async () => {
const config = new Config({ ...baseParams });
await config.initialize({
skipGeminiInitialization: true,
skipHooks: true,
skipMcpDiscovery: true,
skipSkillManager: true,
skipFileCheckpointing: true,
});
const outgoingSessionId = config.getSessionId();
const endedSessionIds: string[] = [];
vi.mocked(logSessionEnd).mockClear();
vi.mocked(logStartSession).mockClear();
vi.mocked(logSessionEnd).mockImplementationOnce((cfg: Config) => {
endedSessionIds.push(cfg.getSessionId());
});

config.startNewSession('replacement-session');

expect(endedSessionIds).toEqual([outgoingSessionId]);
expect(logStartSession).toHaveBeenCalledWith(
config,
expect.anything(),
undefined,
);
expect(vi.mocked(logSessionEnd).mock.invocationCallOrder[0]).toBeLessThan(
vi.mocked(logStartSession).mock.invocationCallOrder[0],
);
});

it('carries the outgoing session id when resuming a different persisted session', async () => {
const config = new Config({ ...baseParams });
await config.initialize({
skipGeminiInitialization: true,
skipHooks: true,
skipMcpDiscovery: true,
skipSkillManager: true,
skipFileCheckpointing: true,
});
const outgoingSessionId = config.getSessionId();
vi.mocked(logStartSession).mockClear();

config.startNewSession('resumed-session-id', {
conversation: { messages: [] },
} as unknown as ResumedSessionData);
Comment on lines +2306 to +2308

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.

[Suggestion] R6-2: The resume-path test asserts only the logStartSession lineage argument and never asserts that logSessionEnd is called for the outgoing session, leaving the resume transition's session.end emission unguarded — probe-verified: gating logSessionEnd on isSessionTransition && !sessionData (dropping the end on every persisted resume) leaves all tests in this file green, and adding the missing assertion flips red against that mutant and green against the original. — Failure scenario: a refactor making that one-line change would ship silently: every /resume of a different persisted session would stop closing the outgoing session's window, leaving an unbalanced session.start/session.end stream — the unterminated-window condition the design doc warns backends about. Fix: mirror the sibling replacement test —

      const endedSessionIds: string[] = [];
      vi.mocked(logSessionEnd).mockClear();
      vi.mocked(logStartSession).mockClear();
      vi.mocked(logSessionEnd).mockImplementationOnce((cfg: Config) => {
        endedSessionIds.push(cfg.getSessionId());
      });

      config.startNewSession('resumed-session-id', {
        conversation: { messages: [] },
      } as unknown as ResumedSessionData);

      expect(endedSessionIds).toEqual([outgoingSessionId]);
      expect(vi.mocked(logSessionEnd).mock.invocationCallOrder[0]).toBeLessThan(
        vi.mocked(logStartSession).mock.invocationCallOrder[0],
      );
中文说明

该 resume 路径测试只断言了 logStartSession 的谱系参数,从未断言对离任会话调用了 logSessionEnd,resume 切换时的 session.end 发射因此没有任何测试防护——已用探针验证:把生产代码门控改为 isSessionTransition && !sessionData(即恢复持久化会话时丢弃离任会话的 end)后,本文件全部测试仍为绿;补上缺失断言后,该变异红、原始代码绿(双向翻转)。触发场景:一次单行重构即可悄悄改变行为——此后每次 /resume 到不同的持久化会话都不再关闭离任会话的窗口,产生不配对的 session.start/session.end 流——正是设计文档提醒后端防范的"未终结窗口"。修复:对照同组的 replacement 测试补齐——切换前 mockClear() logSessionEnd,用 mockImplementationOnce 捕获 cfg.getSessionId(),断言 expect(endedSessionIds).toEqual([outgoingSessionId]),并加 end 先于 start 的 invocationCallOrder 检查。

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


expect(logStartSession).toHaveBeenCalledWith(
config,
expect.anything(),
outgoingSessionId,
Comment on lines +2311 to +2313

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.

[Suggestion] This resume-path test asserts only the logStartSession lineage argument and never asserts that logSessionEnd is called for the outgoing session, leaving the resume transition's session.end emission unguarded — probe-verified: gating logSessionEnd on isSessionTransition && !sessionData (dropping the end on every persisted resume) leaves all 503 tests in this file green, and adding the missing assertion flips red against that mutant and green against the original. — Failure scenario: a refactor making that one-line change would ship silently: every /resume of a different persisted session would stop closing the outgoing session's window, leaving an unbalanced session.start/session.end stream — the unterminated-window condition the design doc warns backends about. Fix: mirror the replacement test — mockClear() logSessionEnd before the switch, capture cfg.getSessionId() in a mockImplementationOnce, and assert expect(endedSessionIds).toEqual([outgoingSessionId]) plus the end-before-start invocationCallOrder check.

中文说明

该 resume 路径测试只断言了 logStartSession 的谱系参数,从未断言对离任会话调用了 logSessionEnd,resume 切换时的 session.end 发射因此没有任何测试防护——已用探针验证:把生产代码门控改为 isSessionTransition && !sessionData(即恢复持久化会话时丢弃离任会话的 end)后,本文件全部 503 个测试仍为绿;补上缺失断言后,该变异红、原始代码绿(双向翻转)。触发场景:一次单行重构即可悄悄改变行为——此后每次 /resume 到不同的持久化会话都不再关闭离任会话的窗口,产生不配对的 session.start/session.end 流——正是设计文档提醒后端防范的“未终结窗口”。修复:对照替换(replacement)测试的写法——切换前对 logSessionEnd 执行 mockClear(),用 mockImplementationOnce 捕获 cfg.getSessionId(),断言 expect(endedSessionIds).toEqual([outgoingSessionId]),并加 end 先于 start 的 invocationCallOrder 检查。

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

);
});

it('rejects a session switch while the current recorder owns the writer lease', () => {
const config = new Config({ ...baseParams, chatRecording: true });
const originalSessionId = config.getSessionId();
Expand All @@ -2248,6 +2330,9 @@ describe('Server Config (config.ts)', () => {
}
).chatRecordingService = recorder;

vi.mocked(logSessionEnd).mockClear();
vi.mocked(logStartSession).mockClear();

expect(() => config.startNewSession('replacement-session')).toThrow(
expect.objectContaining({
name: 'SessionWriterUnavailableError',
Expand All @@ -2258,6 +2343,9 @@ describe('Server Config (config.ts)', () => {
expect(config.getChatRecordingService()).toBe(recorder);
expect(finalize).not.toHaveBeenCalled();
expect(flush).not.toHaveBeenCalled();
// A rejected switch must leave the live session's lifecycle untouched.
expect(logSessionEnd).not.toHaveBeenCalled();
expect(logStartSession).not.toHaveBeenCalled();
});

const resumedGoalSession = (
Expand Down
19 changes: 16 additions & 3 deletions packages/core/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ import {
shutdownTelemetry,
refreshSessionContext,
logStartSession,
logSessionEnd,
logRipgrepFallback,
Comment on lines 131 to 133

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.

[Suggestion] Untested-wiring pattern (3/6 — this import): the efficacy probe reverted this hunk on its own and every test stayed green; no test in this diff exercises the Config-side wiring to logSessionEnd, so a later refactor pruning this list would compile-pass CI while the session.end emission dies silently — same fix as the sibling locations: a startNewSession test observing the emission gates this wiring too.

中文说明

未测试的接线(模式 3/6——此导入):探针单独回退该代码块后所有测试仍然通过;本 diff 没有任何测试覆盖 Config 侧到 logSessionEnd 的接线,后续重构若删掉该导入,CI 编译通过但 session.end 发射会悄悄失效。修复同上:通过观测发射行为的 startNewSession 测试一并覆盖。

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

RipgrepFallbackEvent,
StartSessionEvent,
Expand Down Expand Up @@ -3797,7 +3798,15 @@ export class Config {
});

const previousSessionId = this.sessionId;
this.sessionId = sessionId ?? randomUUID();
const nextSessionId = sessionId ?? randomUUID();
// Resuming the session the user is already in keeps the same id. That is
// not a lifecycle transition: ending it here would record session.end for
// a live session and pair it with a duplicate session.start.
const isSessionTransition = nextSessionId !== previousSessionId;
if (isSessionTransition) {
logSessionEnd(this);
}
this.sessionId = nextSessionId;
// Unconditional: startNewSession is only called on the canonical Config
// instance (the one that already claimed via sessionEnvClaimed), so this
// correctly updates the env var to reflect the new active session.
Expand Down Expand Up @@ -3843,7 +3852,11 @@ export class Config {
// one, and the "N-shotted" PR label would span sessions.
CommitAttributionService.resetInstance();
if (this.initialized) {
logStartSession(this, new StartSessionEvent(this));
logStartSession(
this,
new StartSessionEvent(this),
sessionData && isSessionTransition ? previousSessionId : undefined,
);
Comment on lines +3855 to +3859

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.

[Critical] Resuming the currently-active session emits session.start with session.previous_id === session.id, violating the invariant this PR's own design doc states ("it is never equal to the new session.id"). The gate keys only on sessionData presence, never on id change. The /resume picker does not exclude the current session (DialogManager renders SessionPicker without disabledIds, unlike the delete dialog right below it), the active session is persisted live and sorts first by mtime, and handleResume has no same-id guard — Failure scenario: /resume + Enter on the first row (the session the user is already in) → session.start with session.previous_id === session.id, a self-loop for any consumer chaining sessions by previous_id.

Suggested change
logStartSession(
this,
new StartSessionEvent(this),
sessionData ? previousSessionId : undefined,
);
logStartSession(
this,
new StartSessionEvent(this),
sessionData && previousSessionId !== this.sessionId
? previousSessionId
: undefined,
);
中文说明

恢复当前正在使用的 session 时,会发出 session.previous_id === session.idsession.start,违反本 PR 设计文档中"previous_id 永不等于新 session.id"的约定。续接判断仅依据 sessionData 是否存在,未校验 id 是否变化。/resume 选择器没有排除当前 session(DialogManager 渲染 SessionPicker 时未传 disabledIds,而紧随其后的删除对话框传了),当前 session 被实时持久化且按 mtime 排第一,handleResume 也没有同 id 防护——触发路径:/resume 后直接回车选中第一行(即用户当前所在 session)→ 产生自环路的续接记录。

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

Comment on lines +3855 to +3859

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.

[Suggestion] Untested-wiring pattern (2/6 — the continuation gate): the efficacy probe reverted this hunk on its own and every test stayed green — nothing verifies that previousSessionId is forwarded only when sessionData is provided. The only new test (session-events.test.ts) calls the emitters directly with explicit arguments, so it cannot see this decision — Failure scenario: inverting the ternary makes /clear claim continuation of the conversation it just discarded and resumed sessions lose session.previous_id; ships green. Fix: drive Config.startNewSession with and without sessionData and assert the third argument of logStartSession.

中文说明

未测试的接线(模式 2/6——续接门控):探针单独回退该代码块后所有测试仍然通过——没有任何测试验证 previousSessionId 仅在提供 sessionData 时传入。新增的唯一测试直接以显式参数调用 emitter,无法覆盖这个决策。修复:以有/无 sessionData 驱动 Config.startNewSession,断言 logStartSession 的第三个参数。

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

}

// Refresh the runtime.json sidecar so external observers (terminal
Expand All @@ -3859,7 +3872,7 @@ export class Config {
// sidecar that happens to share the outgoing session id
// mirrors the kimi-cli "write only when a session is
// established for this process" rule.
if (this.runtimeStatusEnabled && previousSessionId !== this.sessionId) {
if (this.runtimeStatusEnabled && isSessionTransition) {
const oldPath = this.storage.getRuntimeStatusPath(previousSessionId);
const newPath = this.storage.getRuntimeStatusPath(this.sessionId);
const cliVersion = this.cliVersion ?? null;
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/telemetry/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ export const EVENT_API_ERROR = 'qwen-code.api_error';
export const EVENT_API_CANCEL = 'qwen-code.api_cancel';
export const EVENT_API_RESPONSE = 'qwen-code.api_response';
export const EVENT_CLI_CONFIG = 'qwen-code.config';
export const EVENT_SESSION_START = 'session.start';
export const EVENT_SESSION_END = 'session.end';
export const EVENT_EXTENSION_DISABLE = 'qwen-code.extension_disable';
export const EVENT_EXTENSION_ENABLE = 'qwen-code.extension_enable';
export const EVENT_EXTENSION_INSTALL = 'qwen-code.extension_install';
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export {
} from './config.js';
export {
logStartSession,
logSessionEnd,
logUserPrompt,
Comment on lines 34 to 36

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.

[Suggestion] Untested-wiring pattern (4/6 — this barrel export): the efficacy probe reverted this hunk on its own and every test stayed green; no test exercises the public telemetry surface for logSessionEnd — a dropped/renamed export would surface only as a runtime import failure at the consumer. Covered by the same suggested startNewSession/shutdown tests as the sibling locations (the broken config-session-env mock fix confirms the export exists but does not assert behavior).

中文说明

未测试的接线(模式 4/6——此 barrel 导出):探针单独回退该代码块后所有测试仍然通过;没有测试覆盖 logSessionEnd 的公开遥测接口——导出被删除/改名只会在消费方运行时才暴露。修复同兄弟位置:由 startNewSession/shutdown 测试一并覆盖。

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

logUserRetry,
logToolCall,
Expand Down
Loading
Loading