fix(cli): map rewind turns after compression

[Jerry2003826]

Fixes #4046

Summary

• Detect the compressed-history summary pair in the rewind UI/API history mapper.
• Align the remaining uncompressed API tail against the end of the UI turn list, so /rewind can target recent turns that survived compression.
• Keep turns absorbed into the compression summary unreachable instead of mapping them onto later tail turns.
• Ignore the synthetic compression continuation bridge when counting tail user turns.

Root Cause

After compression, the API history starts with a synthetic summary user message and fixed model acknowledgement, followed by the preserved tail. The previous computeApiTruncationIndex() counted UI user turns from the beginning and then tried to find the same count in API history. Because several UI turns are collapsed into the single summary pair, even a target turn in the preserved tail could resolve to -1.

Validation

• npm run test --workspace=packages/cli -- src/ui/utils/historyMapping.test.ts -t "compression"
• npm run test --workspace=packages/cli -- src/ui/utils/historyMapping.test.ts
• npm run test --workspace=packages/cli -- src/ui/AppContainer.test.tsx -t "rewind"
• npm run lint --workspace=packages/cli -- src/ui/utils/historyMapping.ts src/ui/utils/historyMapping.test.ts
• npx prettier --check packages/cli/src/ui/utils/historyMapping.ts packages/cli/src/ui/utils/historyMapping.test.ts
• npm run build --workspace=packages/core
• npm run typecheck --workspace=packages/cli

[wenshao]

Fix correctly diagnoses #4046 and the refactor is clean. Two blocking concerns and a few nits inline.

Out-of-band concern (not inline because it's in a different file): packages/cli/src/acp-integration/session/Session.ts:344 (#computeApiTruncationIndexForUserTurn) is a near-duplicate of the pre-fix logic and still has the same compression bug for the ACP rewind path. Consider consolidating Session.ts onto computeApiTruncationIndex (preferred — removes the duplication), applying the same fix there, or filing a follow-up so #4046 doesn't half-close.

Test gaps: the three new tests cover the happy path but miss (a) compression + startup-context-pair together (startIndex = 2 then summary at 2/3), (b) targetOrdinal === 1 when the first turn was compressed away (should return -1; behavior is correct but no regression net), and (c) a tail containing tool-call entries (functionCall + functionResponse) between the bridge and the next user prompt.

[wenshao]

Magic-string coupling between cli and core (blocking). These two literals are also hardcoded in packages/core/src/services/chatCompressionService.ts:420,432. If either side edits the text (typo fix, i18n, model-prompt tuning), this mapper silently regresses to the original bug — and CI will pass because the unit tests use the same literals.

Compare to STARTUP_CONTEXT_MODEL_ACK, which is exported from core and imported on line 9. Please export COMPRESSION_SUMMARY_MODEL_ACK and COMPRESSION_CONTINUATION_BRIDGE from chatCompressionService.ts (or a shared constants module) and import them here. Bonus: a core-side test that asserts the emitted Content uses those exact constants would make future renames break loudly.

[wenshao]

Bridge detection by exact text match is over-broad. If a user happens to type exactly Continue with the prior task using the context above., that genuine prompt is silently dropped from the tail mapping and shifts every subsequent ordinal — so rewinding to any post-bridge turn lands one slot off.

Unlikely in practice but easy to harden: mark the synthetic bridge with something that can never collide with user input — either a sentinel flag on the Content (e.g., a private extension property), or a zero-width character / unique marker in the text — and match on that instead of the human-readable string.

[wenshao]

nit: the apiHistory[startIndex + 1]?.role === 'model' check is redundant — hasTextPart on the next line already short-circuits to false if the Content is missing/has no parts/has no matching text part. The role check only guards against a user-role Content with a text part equal to the ack literal, which is a separate (and unlikely) coupling concern. Consider dropping it, or roll the role check into a hasModelTextPart helper for symmetry with isUserTextContent.

[wenshao]

nit: the ?? -1 is unreachable by construction. targetOrdinal ≤ totalRealUserTurns (the ordinal can only be set while iterating real user turns), and apiTailUserIndices.length = totalRealUserTurns - compressedTurnCount, so targetOrdinal - compressedTurnCount - 1 is always in [0, apiTailUserIndices.length - 1] once we've passed the targetOrdinal <= compressedTurnCount guard on line 176.

Not worth blocking on — defensive ?? -1 is fine — but if you prefer it to read as intentional, a one-liner comment ("defensive: shouldn't fire given the guard above") would help future readers.

[wenshao]

Suggest adding a sibling test for the targetOrdinal === 1 + compression case (rewind to the very first UI turn after it was absorbed into the summary). The current code returns -1 correctly because the compression branch on line 161 runs before the targetOrdinal === 1 early-return on line 183, but that ordering is load-bearing and easy to flip during a future refactor without any test catching it.

[wenshao]

[Suggestion] ACP 压缩路径使用 this.turn 推导 totalUserTurns，但 this.turn 可能偏离实际的 user-text entry 数量——slash-command prompt 会递增 turn 但不产生 user-text 条目，cron prompt 会产生条目但不递增 turn。相比之下 historyMapping.ts 直接遍历 UI history 计数更为可靠。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

/review 补充发现

PR 正确诊断了 #4046，核心思路无误。所有测试通过（155 tests），无类型错误。

以下是在 @wenshao 已有 5 条 inline comment 之外的补充发现：

Suggestion: Session.ts 使用 this.turn 计算 compressedTurnCount，可能偏离实际值

Session.#computeApiTruncationIndexForUserTurn 用 Math.max(this.turn, targetTurnIndex + 1) 作为总轮次数。但 this.turn 在 slash-command prompt（递增 turn 但不产生 user-text 条目）和 cron prompt（产生条目但不递增 turn）场景下会偏离实际值。UI 路径 computeApiTruncationIndex 直接遍历 UI history 计数，更为可靠。建议从 API history 直接统计 user-text entry 总数。

Suggestion: 四个辅助函数在 Session.ts 和 historyMapping.ts 中重复实现

hasTextPart、hasModelTextPart、hasCompressionSummaryPair、getApiUserTextIndices 在两处逻辑完全一致。建议提取到共享模块，避免压缩格式变更时遗漏同步。

Suggestion: 错误信息缺少诊断上下文

rewindToTurn 抛出的 "Cannot rewind to the requested turn. It may have been compressed or does not exist." 不含 targetTurnIndex 或 compressedTurnCount，调试不便。

Suggestion: 整个 rewind 流水线零日志

rewindToTurn、computeApiTruncationIndex 等关键函数中无任何日志，缺少操作审计踪迹。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

补充发现（在 @wenshao 已有 6 条 inline comment 之外）：

[Suggestion] 代码重复：Session.ts 与 historyMapping.ts — hasTextPart、hasModelTextPart、hasCompressionSummaryPair、getApiUserTextIndices 等辅助函数在两处近乎相同的实现。未来压缩格式变更需双文件同步维护。建议提取到公共工具模块。

[Suggestion] 缺少测试覆盖：targetOrdinal < 0 guard 分支 — getUiTurnOrdinals 新增的提前返回分支（historyMapping.test.ts）无测试用例覆盖。

[Suggestion] #recordModelFacingUserTurn 仅通过 mock 测试 — 计数器递增从未通过真实的 send 路径验证（Session.test.ts），如果调用点被意外移除无法检测。

---

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] modelFacingUserTurnCount 在 rewind 后不回退，导致 rewind→send→rewind 循环后可达轮次变为不可达。

#computeApiTruncationIndexForUserTurn 用 Math.max(this.modelFacingUserTurnCount, targetTurnIndex + 1) 计算 compressedTurnCount，但 modelFacingUserTurnCount 只增不减。rewind 到早期轮次后发送新消息会使计数器膨胀，后续 rewind 时被错误地判定为「unreachable turn」。

Suggested change

	this.modelFacingUserTurnCount,
	// 截断后重置计数器为实际保留的轮次数
	const truncatedHistory = this.#chat.getHistory();
	this.modelFacingUserTurnCount = this.#getApiUserTextIndices(
	truncatedHistory,
	this.#hasStartupContext(truncatedHistory) ? 2 : 0,
	false,
	).length;
	const totalUserTurns = Math.max(
	this.modelFacingUserTurnCount,
	targetTurnIndex + 1,
	);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] isModelFacingUserPromptRecord 未排除 ? 前缀命令，导致 modelFacingUserTurnCount 与 UI 轮次计数背离。

isRealUserTurn（historyMapping.ts）排除 ?-prefixed 命令，但此处不排除。当用户输入 ?help 时 ACP 路径的计数器会额外 +1，压缩轮次计算出现偏差。

Suggested change

	return true;
	if (record.subtype === 'cron') {
	return true;
	}
	const fullText = textParts.join(' ');
	if (fullText.startsWith('?')) {
	return false;
	}
	return !isSlashCommand(fullText);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] restoreHistory() restores only the API Content[], but this rewind path also mutates modelFacingUserTurnCount. ACP's session/restoreSessionHistory returns the saved history after an edit/cancel flow, but the counter remains at the rewound value. After compression, later rewind mapping uses that stale counter to derive compressedTurnCount, so a restored session can incorrectly make absorbed turns reachable or reject valid tail turns.

Please snapshot and restore the counter together with the chat history, e.g. return an object like { history, modelFacingUserTurnCount } from captureHistorySnapshot() and have restoreHistory() restore both fields. Recomputing from Content[] after compression is not enough because the absorbed turn count is not recoverable from the summary pair.

— gpt-5.5 via Qwen Code /review

[wenshao]

[Critical] This increments modelFacingUserTurnCount before the prompt is actually accepted by #sendMessageStreamWithAutoCompression(). If the send is skipped by the session token limit (max_tokens), #preserveUnsentMessageHistory(..., false) drops the non-functionResponse user text from API history, but the counter has already advanced.

After compression, ACP rewind derives compressedTurnCount from this counter, so a skipped/unsent prompt can silently inflate the total turn count and later make valid tail turns unreachable or map them to the wrong API entry. Move the counter increment until after sendResult.responseStream is present, or roll it back on every skipped-send path where the user text is not preserved as model-facing API history.

— gpt-5.5 via Qwen Code /review

[wenshao]

[Suggestion] mid_turn_user_message records are counted as independent model-facing user turns when initializing modelFacingUserTurnCount from resumed history. However, resume/API reconstruction merges these records into the previous user Content rather than treating them as standalone rewind turn boundaries.

That inflates the resumed counter relative to the API user-text entries used by compressed-tail mapping, which can make ACP rewind reject reachable tail turns after a resume. Please exclude record.subtype === 'mid_turn_user_message' here, matching the boundary logic used when rebuilding turn boundaries.

— gpt-5.5 via Qwen Code /review

[wenshao]

Requested changes based on the inline comments. — gpt-5.5 via Qwen Code /review

[wenshao]

[Critical] isHistorySnapshot guard accepts NaN, Infinity, -Infinity, negative, and non-integer values for modelFacingUserTurnCount. typeof NaN === 'number' is true in JS. A malformed ACP client can send {history: [...], modelFacingUserTurnCount: NaN} which permanently poisons the session — Math.max(NaN, ...) propagates NaN through all downstream arithmetic in #computeApiTruncationIndexForUserTurn, making every future rewindToTurn fail silently.

Suggested change

	const isHistorySnapshot =
	const isHistorySnapshot =
	!!history &&
	typeof history === 'object' &&
	!Array.isArray(history) &&
	Array.isArray((history as { history?: unknown }).history) &&
	Number.isInteger(
	(history as { modelFacingUserTurnCount?: unknown })
	.modelFacingUserTurnCount,
	) &&
	((history as { modelFacingUserTurnCount?: unknown })
	.modelFacingUserTurnCount as number) >= 0;

Number.isInteger() rejects NaN, Infinity, floats, and the >= 0 rejects negatives — matching the existing validation pattern for targetTurnIndex at line 1004.

— mimo-v2.5-pro via Qwen Code /review

[wenshao]

[Suggestion] Redundant structuredClone. GeminiChat.getHistory() already returns structuredClone(history) (see packages/core/src/core/geminiChat.ts:1225). The outer structuredClone here deep-clones an already-deep-cloned result — a redundant O(n) allocation over the entire conversation.

Suggested change

	history: structuredClone(
	history: this.config.getGeminiClient()!.getChat().getHistory(),

— mimo-v2.5-pro via Qwen Code /review

[wenshao]

[Suggestion] When restoreHistory receives the backward-compatible Content[] format, modelFacingUserTurnCount is left at its previous value — potentially desynchronized from the restored history. While the current producer always sends HistorySnapshot, older ACP clients or direct API callers could hit this path. A stale counter causes #computeApiTruncationIndexForUserTurn to compute wrong compressedTurnCount, producing incorrect rewind results with an unhelpful error message.

Consider computing a fallback counter from the restored history in the legacy branch:

if (Array.isArray(snapshot)) {
  // Legacy path: derive count from restored history
  this.modelFacingUserTurnCount = history.filter(c => c.role === 'user').length;
} else {
  this.modelFacingUserTurnCount = snapshot.modelFacingUserTurnCount;
}

— mimo-v2.5-pro via Qwen Code /review

[wenshao]

[Suggestion] Missing test coverage for the new HistorySnapshot behavior. Consider adding:

1. Verify modelFacingUserTurnCount is actually restored — this test checks the snapshot shape and setHistory call, but doesn't assert the internal counter changed (if the if (!Array.isArray(snapshot)) branch were removed, the test would still pass).

2. Legacy Content[] happy path — no test calls restoreHistory with a plain Content[] and verifies it works (the only Content[] tests are error-path guards that throw before reaching the history-setting code).

3. Deep clone verification — no test confirms that mutating the returned snapshot doesn't affect the session's internal history (the structuredClone addition).

4. NaN/Infinity rejection — add a test for extMethod('restoreSessionHistory', { history: { history: [], modelFacingUserTurnCount: NaN } }) to verify it throws after the guard fix.

— mimo-v2.5-pro via Qwen Code /review

[wenshao]

[Suggestion] computeVisibleModelFacingUserTurnCount counts the synthetic compression summary user entry as a real user turn.

When the legacy Content[] path receives a history with an active compression summary pair, this function counts the summary user entry (at startIndex) because getApiUserTextIndices does not skip it. This inflates modelFacingUserTurnCount by 1, which feeds into #computeApiTruncationIndexForUserTurn's compressedTurnCount calculation and can make one tail turn unreachable during rewind.

Both historyMapping.ts:86 and #computeApiTruncationIndexForUserTurn (line 472) detect hasCompressionSummaryPair and shift to startIndex + 2. This helper should do the same:

Suggested change

	function computeVisibleModelFacingUserTurnCount(apiHistory: Content[]): number {
	function computeVisibleModelFacingUserTurnCount(apiHistory: Content[]): number {
	const startIndex = hasStartupContext(apiHistory) ? 2 : 0;
	if (hasCompressionSummaryPair(apiHistory, startIndex)) {
	return getApiUserTextIndices(apiHistory, startIndex + 2, true).length;
	}
	return getApiUserTextIndices(apiHistory, startIndex, true).length;
	}

Adding a test with a compression summary pair in the legacy array derivation test would also catch this.

— mimo-v2.5-pro via Qwen Code /review_

[wenshao]

The core logic is correct — compression-aware tail alignment works as intended. All 217 tests pass, build succeeds, no type errors. The refactor cleanly extracts shared utilities.

New findings (not covered in prior reviews)

1. [Suggestion] Stop hook continuation missing #recordModelFacingUserTurn
(pre-existing code, not in diff). The stop hook continuation loop sends messages via #sendMessageStreamWithAutoCompression but never calls #recordModelFacingUserTurn, unlike the main send loop (line 734) and cron path (line 1474). After a stop hook continuation, modelFacingUserTurnCount is deflated relative to the API history's actual user-text entries. This inflates compressedTurnCount, potentially making reachable tail turns appear unreachable.

2. [Suggestion] No dedicated tests for apiHistoryUtils.ts
The new shared utility module exports 6 functions with no dedicated test file. Key edge-case branches (empty parts, mixed text + functionResponse, boundary conditions) are tested only indirectly.

3. [Suggestion] No test for #rollbackModelFacingUserTurn path
The rollback method (send fails → counter decremented) has zero test coverage.

Low-confidence finding (needs human review)

[Critical — low confidence] Prompt injection via restoreSessionHistory
acpAgent.ts:1024-1060 accepts any Content[] and passes it to chat.setHistory(). A malicious ACP client could replace the startup context pair with a poisoned version matching the model ack text but with adversarial user entry. Practical risk is low (ACP runs on localhost).

— mimo-v2.5-pro via Qwen Code /review

[wenshao]

本审查在 @wenshao 已有 16 条 inline comments + 4 轮 review 之外，聚焦遗漏的 gap：

Critical

1. rewind 失败路径缺少诊断日志 — #computeApiTruncationIndexForUserTurn 返回 -1 时不记录 compressedTurnCount、apiTailUserIndices.length 等关键状态，导致生产环境无法定位根因。
2. 空历史快照被接受 — isHistorySnapshot 接受 { history: [], modelFacingUserTurnCount: 0 }，可静默清空整个会话。
3. modelFacingUserTurnCount 被赋值为索引而非剩余计数 — this.modelFacingUserTurnCount = targetTurnIndex（基于 0 的索引）在语义上不等价于剩余回合数，多次 rewind 循环后累积误差。

Suggestion（非文件级）

• historyMapping.ts 与 Session.ts 的压缩感知倒回实现应提取到 apiHistoryUtils.ts 统一维护
• apiHistoryUtils.ts（6 个导出函数）和 chatCompressionConstants.ts（常量值验证）缺少专用测试文件

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] rewind 失败路径缺少诊断日志，生产环境无法定位根因。

#computeApiTruncationIndexForUserTurn 在压缩感知路径中返回 -1 时仅抛出通用错误 'Cannot rewind to the requested turn'，不记录 modelFacingUserTurnCount、compressedTurnCount、apiTailUserIndices.length、targetTurnIndex 等关键中间值。凌晨 3 点收到 rewind 不工作的上报时，无法从日志中判断是计数器偏移、压缩回合数计算错误还是 tail 索引越界。

Suggested change

	0,
	// 在返回 -1 前添加：
	this.debugLogger?.warn('rewind failed', {
	targetTurnIndex,
	modelFacingUserTurnCount: this.modelFacingUserTurnCount,
	compressedTurnCount,
	apiTailUserIndicesLength: apiTailUserIndices.length,
	hasCompressionSummaryPair: true,
	totalUserTurns,
	});

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] 空历史快照被接受，可静默清空整个会话。

isHistorySnapshot 接受 { history: [], modelFacingUserTurnCount: 0 }。该快照会通过 session.restoreHistory 调用 chat.setHistory([]) 静默清空整个会话历史。当前校验没有对 history 数组设最小长度限制。

Suggested change

	(history as { modelFacingUserTurnCount?: unknown })
	// 在 restoreHistory 或 isHistorySnapshot 后添加：
	if (Array.isArray(history) && history.length === 0 && modelFacingUserTurnCount > 0) {
	throw new Error('Inconsistent snapshot: empty history with non-zero turn count');
	}

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] modelFacingUserTurnCount 被赋值为索引（0-based）而非剩余回合计数。

rewindToTurn 中 this.modelFacingUserTurnCount = targetTurnIndex 将基于 0 的索引赋给表示剩余回合数的计数器。例如：3 个回合（索引 0,1,2），rewindToTurn(2) 截断后仅剩 1 个回合，但计数器被设为 2。当前被 Math.max 防御掩盖，但多次 rewind+resend 循环后累积误差可能导致有效回合被错误拒绝。

Suggested change

	// targetTurnIndex is zero-based; after truncating before that turn,
	// 从截断后的历史重新计算：
	this.modelFacingUserTurnCount = computeVisibleModelFacingUserTurnCount(
	chat.getHistory(),
	);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] computeVisibleModelFacingUserTurnCount 在压缩活跃时低估计数器，导致 compressedTurnCount = 0 退化。

旧版数组恢复路径下，该函数仅统计尾部可见回合数，不包含被压缩摘要代表的压缩回合。当后续 compressedTurnCount = Math.max(0, totalUserTurns - apiTailUserIndices.length) 计算时，totalUserTurns 偏低，使得 compressedTurnCount = 0，让本应不可达的已压缩回合变为可达。

Suggested change

	function computeVisibleModelFacingUserTurnCount(apiHistory: Content[]): number {
	// 至少为压缩摘要保留 1 个回合：
	if (hasCompressionSummaryPair(apiHistory, startIndex)) {
	const tailCount = getApiUserTextIndices(apiHistory, startIndex + 2, true).length;
	return tailCount + 1; // 摘要至少代表 1 个压缩回合
	}

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] Math.max(this.modelFacingUserTurnCount, targetTurnIndex + 1) 静默掩盖计数器不一致。

当 modelFacingUserTurnCount 偏低时，Math.max 使用 targetTurnIndex + 1 作为下限，让计算跑通，但压缩回合数被低估。建议在此分支发出 warning 日志标记可能的状态不一致，而非静默修正。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] License 头错误：Copyright 2025 Google LLC 应改为 Copyright 2025 Qwen Code。

同 PR 新增的 apiHistoryUtils.ts 使用 Copyright 2025 Qwen Code，此文件为 copy-paste 遗留。

Suggested change

	* Copyright 2025 Google LLC
	* Copyright 2025 Qwen Code

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] COMPRESSION_CONTINUATION_BRIDGE 的不可见 Unicode 前缀可能被中间件剥离。

零宽字符（\u200B\u200C\u200D\u2060）在某些日志系统、序列化层或 hook 链中可能被规范化去除，导致 bridge 检测静默失败，用户 prompt 被误认为桥接条目。建议在检测路径添加 debug 级别日志。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] #rollbackModelFacingUserTurn 的非取消停止原因路径缺少测试覆盖。

当 sendResult.stopReason !== 'cancelled'（如 max_tokens、session_token_limit_exceeded）时，回滚逻辑无测试验证。建议添加测试使 mock 返回 { responseStream: undefined, stopReason: 'max_tokens' } 并验证计数器正确回滚。

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

Qwen Code Review — PR #4242

Verdict: REQUEST CHANGES

Summary

This PR fixes #4046 by adding a modelFacingUserTurnCount field and compression-aware mapping in Session.#resolveApiIndex. The approach is sound in principle, but there are three high-confidence issues with the counter lifecycle and validation that can cause incorrect rewind targeting.

Critical Issues (3 high-confidence)

1. Stop hook continuation skips #recordModelFacingUserTurn

#recordModelFacingUserTurn is called in the main send path (line 734) and ACP send path (line 1474), but NOT in the stop hook continuation path (lines 980-1001). When a stop hook creates a synthetic user message and continues the conversation, the counter is never incremented. After N stop hook continuations, modelFacingUserTurnCount undercounts by N, causing Math.max in #resolveApiIndex to mask the drift until the error becomes large enough to produce wrong mappings.

→ See inline comment on Session.ts:1129.

2. Math.max safety net amplifies counter drift

totalUserTurns = Math.max(this.modelFacingUserTurnCount, targetTurnIndex + 1) ensures the value never drops below targetTurnIndex + 1, but this masks a too-low counter. With N missing turns from Finding 1, compressedTurnCount becomes max(counter, targetTurnIndex+1) - tailLen, which can be too small, causing the mapping to target an earlier turn than intended.

→ See inline comment on Session.ts:481.

3. No upper bound validation for modelFacingUserTurnCount

The isHistorySnapshot check in acpAgent.ts:1052 validates modelFacingUserTurnCount >= 0 but not <= actualTurnCount. An external caller can pass an arbitrarily large value, which would inflate compressedTurnCount and make all rewind targets resolve to -1 (blocked).

→ See inline comment on acpAgent.ts:1052.

Needs Human Review

4. Dual compression-aware mapping implementations (Critical/low-confidence)

Session.#resolveApiIndex (lines 472-507) and historyMapping.ts:computeApiTruncationIndex (lines 73-126) both handle compression-aware turn mapping but use different data sources (API-level counter vs UI-level ordinals). The Session version uses modelFacingUserTurnCount and targetTurnIndex; the historyMapping version uses totalRealUserTurns from UI history and targetOrdinal. This duplication creates a risk of future divergence.

Info

• Deterministic analysis: 102 TS4111 errors (all pre-existing from root tsconfig), 0 eslint errors
• Build: OK
• Tests: 217/217 pass
• Autofix: skipped (Critical issues are architectural decisions)

---

Reviewed by mimo-v2.5-pro

[wenshao]

Critical: Stop hook continuation path skips #recordModelFacingUserTurn

This method increments modelFacingUserTurnCount and is correctly called in the main send path (~line 734) and ACP send path (~line 1474). However, the stop hook continuation path (lines 980-1001) creates a synthetic user message and sends it via #sendMessageStreamWithAutoCompression without calling #recordModelFacingUserTurn.

After N stop hook continuations, modelFacingUserTurnCount undercounts by N. This causes Math.max in #resolveApiIndex to silently mask the drift, eventually producing wrong rewind mappings.

Fix: Add this.#recordModelFacingUserTurn(nextMessage) before the #sendMessageStreamWithAutoCompression call at ~line 995, matching the pattern in the main send path (line 734).

---

Reviewed by mimo-v2.5-pro

[wenshao]

Critical: Math.max safety net amplifies counter drift

totalUserTurns = Math.max(this.modelFacingUserTurnCount, targetTurnIndex + 1) ensures the value never drops below targetTurnIndex + 1, but this masks a too-low counter from the stop hook bug (see comment on #recordModelFacingUserTurn).

With N missing turns from stop hook continuations:

• compressedTurnCount = max(counter, targetTurnIndex+1) - tailLen
• When targetTurnIndex + 1 > counter, Math.max uses targetTurnIndex + 1 instead of the true count
• This can make compressedTurnCount too small, causing the mapping to target an earlier (wrong) turn

Fix: Remove the Math.max safety net and instead ensure the counter is always correct (fix Finding 1). If a fallback is needed, prefer returning -1 (blocked) rather than guessing.

---

Reviewed by mimo-v2.5-pro

[wenshao]

Critical: No upper bound validation for modelFacingUserTurnCount

The validation checks Number.isInteger(modelFacingUserTurnCount) && >= 0 but does NOT check an upper bound. An external caller (e.g., IDE companion extension) can pass an arbitrarily large value.

If modelFacingUserTurnCount > actualTurnCount, then:

• compressedTurnCount = modelFacingUserTurnCount - apiTailUserIndices.length becomes too large
• All rewind targets resolve to -1 (blocked), making rewind completely non-functional

Fix: Add an upper bound check, e.g.:

&& ((history as { modelFacingUserTurnCount?: unknown })
  .modelFacingUserTurnCount as number) <= MAX_REASONABLE_TURN_COUNT

Or validate against history.history.length at restore time.

---

Reviewed by mimo-v2.5-pro

[wenshao]

Critical: Stop hook continuation path skips #recordModelFacingUserTurn

This method increments modelFacingUserTurnCount and is correctly called in the main send path (~line 734) and ACP send path (~line 1474). However, the stop hook continuation path (lines 980-1001) creates a synthetic user message and sends it via #sendMessageStreamWithAutoCompression without calling #recordModelFacingUserTurn.

After N stop hook continuations, modelFacingUserTurnCount undercounts by N. This causes Math.max in #resolveApiIndex to silently mask the drift, eventually producing wrong rewind mappings.

Fix: Add this.#recordModelFacingUserTurn(nextMessage) before the #sendMessageStreamWithAutoCompression call at ~line 995, matching the pattern in the main send path (line 734).

---

Reviewed by mimo-v2.5-pro

[wenshao]

Critical: Math.max safety net amplifies counter drift

totalUserTurns = Math.max(this.modelFacingUserTurnCount, targetTurnIndex + 1) ensures the value never drops below targetTurnIndex + 1, but this masks a too-low counter from the stop hook bug (see comment on #recordModelFacingUserTurn).

With N missing turns from stop hook continuations:

• compressedTurnCount = max(counter, targetTurnIndex+1) - tailLen
• When targetTurnIndex + 1 > counter, Math.max uses targetTurnIndex + 1 instead of the true count
• This can make compressedTurnCount too small, causing the mapping to target an earlier (wrong) turn

Fix: Remove the Math.max safety net and instead ensure the counter is always correct (fix Finding 1). If a fallback is needed, prefer returning -1 (blocked) rather than guessing.

---

Reviewed by mimo-v2.5-pro

[wenshao]

Critical: No upper bound validation for modelFacingUserTurnCount

The validation checks Number.isInteger(modelFacingUserTurnCount) && >= 0 but does NOT check an upper bound. An external caller (e.g., IDE companion extension) can pass an arbitrarily large value.

If modelFacingUserTurnCount > actualTurnCount, then:

• compressedTurnCount = modelFacingUserTurnCount - apiTailUserIndices.length becomes too large
• All rewind targets resolve to -1 (blocked), making rewind completely non-functional

Fix: Add an upper bound check, e.g.:

&& ((history as { modelFacingUserTurnCount?: unknown })
  .modelFacingUserTurnCount as number) <= MAX_REASONABLE_TURN_COUNT

Or validate against history.history.length at restore time.

---

Reviewed by mimo-v2.5-pro

[wenshao]

[Suggestion] Catch blocks do not rollback modelFacingUserTurnCount after #recordModelFacingUserTurn increments it.

Three send paths increment the counter at the top of try but never call #rollbackModelFacingUserTurn in catch:

• Main send (line 738 → catch ~795): exception re-thrown, counter dirty
• Stop hook (line 1002 → catch ~1058): same pattern
• Cron ACP (line 1483 → catch ~1563): error swallowed, permanent drift

The try-internal early-return paths correctly rollback. The exception paths were missed.

Suggested change

	this.#recordModelFacingUserTurn(nextMessage);
	} catch (error) {
	+ this.#rollbackModelFacingUserTurn(recordedModelFacingTurn);

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

No new review findings beyond the 14 existing inline comments from prior review rounds. Downgraded from Approve to Comment: CI failing (Test windows-latest, Node 22.x).

Deterministic checks: Build passes. All 275 PR-affected tests pass (6 test files). 102 pre-existing TS4111 type errors in vscode-ide-companion (not from this PR — diff only added a type alias).

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Critical] recordedModelFacingTurn is scoped to the whole cron execution instead of each send iteration. After the initial cron prompt is recorded, a later tool-response iteration can leave this flag true even though #recordModelFacingUserTurn(nextMessage) returns false. If that later tool-response send throws, the catch block rolls back the previously successful cron user turn, drifting modelFacingUserTurnCount too low and breaking compression-aware rewind mapping.

Reset the flag inside the while (nextMessage !== null) loop so each iteration only rolls back its own attempted send.

Suggested change

	let recordedModelFacingTurn = false;
	while (nextMessage !== null) {
	if (ac.signal.aborted) return;
	const functionCalls: FunctionCall[] = [];
	let usageMetadata: GenerateContentResponseUsageMetadata | null =
	null;
	const streamStartTime = Date.now();
	let recordedModelFacingTurn = false;

— gpt-5.5 via Qwen Code /review

[wenshao]

No new findings beyond the 15 existing inline comments from prior review rounds. All issues are already captured.

Deterministic checks: Build passes. tsc finds 102 TS4111 errors in qwenAgentManager.ts and acpConnection.ts, but these are pre-existing (the PR only changes type annotations in these files — the index-signature property access errors affect unrelated lines not touched by this diff).

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[wenshao]

[Suggestion] apiHistoryUtils.ts is not Prettier-formatted (createDebugLogger is mis-indented and missing the trailing comma), so npx prettier --check packages/cli/src/utils/apiHistoryUtils.ts packages/cli/src/utils/apiHistoryUtils.test.ts fails.

Run Prettier, or format the import block as:

import {
  COMPRESSION_CONTINUATION_BRIDGE_MARKER,
  COMPRESSION_SUMMARY_MODEL_ACK,
  STARTUP_CONTEXT_MODEL_ACK,
  createDebugLogger,
} from '@qwen-code/qwen-code-core';

— gpt-5.5 via Qwen Code /review

[wenshao]

New findings (beyond the 22 existing inline comments):

[Suggestion] SessionMessageHandler.test.ts — 5 stale mocks still use old Content[] format

Lines 241, 448, 591, 663, 786 return bare Content[] as historyBeforeRewind instead of the new { history: Content[], modelFacingUserTurnCount: number } format. Line 527 was correctly updated but the other 5 were missed. Tests pass only because vi.fn().mockResolvedValue bypasses TypeScript's type checker.

[Nice to have] apiHistoryUtils.ts:46 — hasTextPart missing typeof part.text === 'string'

Sibling function isCompressionContinuationBridge (same file) explicitly checks typeof part.text === 'string', but hasTextPart omits it. Runtime-safe (=== against a string always returns false for non-strings), but inconsistent within the same file.

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

[wenshao]

[Critical] The new empty-history guard has no dedicated test.

All existing restoreHistory([]) calls in Session.test.ts (lines 775, 784, 795) set up a pending prompt first, so they hit the earlier "prompt is running" guard and never reach this check. No test sends a valid-but-empty snapshot when no prompt is active.

A regression removing this guard would silently allow empty snapshots to wipe session history.

Suggested change

	const history = Array.isArray(snapshot) ? snapshot : snapshot.history;
	// In Session.test.ts, add:
	it('rejects restoring an empty history array', () => {
	expect(() => session.restoreHistory([])).toThrow(
	'Cannot restore an empty history snapshot',
	);
	expect(mockChat.setHistory).not.toHaveBeenCalled();
	});
	it('rejects restoring an empty HistorySnapshot', () => {
	expect(() =>
	session.restoreHistory({ history: [], modelFacingUserTurnCount: 0 }),
	).toThrow('Cannot restore an empty history snapshot');
	});

— qwen-latest-series-invite-beta-v28 via Qwen Code /review

[wenshao]

[Suggestion] structuredClone was removed from the restore direction. The old code was setHistory(structuredClone(history)).

The existing review comment about "redundant structuredClone" concerns the capture direction (getHistory() already returns a clone). The restore direction is a separate defense — it prevents callers who retain a reference to the passed array from mutating the session's internal state.

GeminiChat.setHistory stores by reference (this.history = history), so any post-restore mutation of the input array silently corrupts the chat.

Suggested change

	}
	this.config.getGeminiClient()!.getChat().setHistory(structuredClone(history));

— qwen-latest-series-invite-beta-v28 via Qwen Code /review