Skip to content

fix(core): preserve managed memory during microcompaction - #6714

Merged
wenshao merged 4 commits into
QwenLM:mainfrom
yiliang114:codex/6487-preserve-memory-read
Jul 11, 2026
Merged

fix(core): preserve managed memory during microcompaction#6714
wenshao merged 4 commits into
QwenLM:mainfrom
yiliang114:codex/6487-preserve-memory-read

Conversation

@yiliang114

Copy link
Copy Markdown
Collaborator

What this PR does

This PR keeps successful managed-memory read_file results available across idle, size-based, /compress-fast, and memory-pressure microcompaction. It recognizes project, user, and team memory through realpath-aware containment checks, while ordinary tool results continue to be compacted normally.

Why it's needed

Managed-memory topic files are loaded lazily through read_file. Before this change, their content was replaced with [Old tool result content cleared] after leaving the recent-result window. The model could then silently lose durable guidance it had already loaded.

Reviewer Test Plan

How to verify

Read a managed-memory topic file, trigger microcompaction with enough later tool calls, and then ask the model to use a unique value from that memory without reading the file again. Ordinary old tool results should be cleared, while the managed-memory content should remain available.

Evidence (Before & After)

Before: the memory marker disappeared from the final API request, and the model reported that it needed to read the file again.

After: ordinary old tool results are still cleared, but the complete managed-memory Markdown and marker remain in the request. The model outputs ORIOLE-6487-VERIFY-SURVIVES without re-reading the file.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

Local node dist/cli.js bundle using API Config, with toolResultsTotalCharsThreshold: 1 and toolResultsNumToKeep: 1 to force the size-based microcompaction path.

Risk & Scope

  • Main risk or tradeoff: repeated managed-memory reads remain in history until full compaction and can increase token usage.
  • Not validated / out of scope: full compaction remains lossy; the stale index issue after /remember is handled separately by fix(memory): refresh instructions after remember #6497.
  • Breaking changes / migration notes: None.

Linked Issues

Fixes #6713
Refs #6487

中文说明

这个 PR 做了什么

这个 PR 会在 idle、size-based、/compress-fast 和 memory-pressure microcompaction 中保留成功读取的 managed-memory 内容。路径识别覆盖 project、user 和 team memory,并使用 realpath containment check;普通工具结果仍会正常压缩。

为什么需要

managed-memory topic 通过 read_file 按需加载。修改前,内容移出 recent-result 窗口后会被替换为 [Old tool result content cleared],导致模型静默丢失已经加载的持久指导。

Reviewer Test Plan

读取 managed-memory topic,执行足够多的后续工具调用触发 microcompaction,然后要求模型在不重新读取文件的情况下使用其中的唯一值。预期普通旧工具结果被清除,但 managed-memory 内容保持可用。

Before:memory marker 从最终 API 请求中消失,模型表示需要重新读取文件。

After:普通旧工具结果仍被清除,但 managed-memory 的完整 Markdown 和 marker 保留,模型无需重新读取即可输出 ORIOLE-6487-VERIFY-SURVIVES

风险和范围

  • 主要取舍:重复读取的 managed-memory 内容会保留到 full compaction,可能增加 token 占用。
  • 不在范围内:full compaction 仍为有损摘要;/remember 后索引过期由 fix(memory): refresh instructions after remember #6497 处理。
  • Breaking changes:无。

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — all required sections present, bilingual description included.

Problem: Observed bug with clear reproduction in #6713. Managed-memory topic files loaded via read_file are silently cleared during microcompaction, causing the model to lose durable guidance. This is a real, reproducible issue — not theoretical hardening.

Direction: Aligned. Memory content that the model has already loaded should survive microcompaction — it's durable guidance, not ephemeral tool output. Full compaction remaining lossy is the right boundary. No direct CHANGELOG reference, but this falls squarely within the scope/memory and category/core domain.

Size: Core paths touched — 139 production logic lines (client.ts: 9, geminiChat.ts: 9, paths.ts: 27, memoryPressureMonitor.ts: 25, microcompact.ts: 69). Well under the 500-line threshold. 301 test lines + 99 docs lines.

Approach: The callback-predicate design (preserveReadFileResult) is clean and minimal. Injected at the collectCompactablePartRefs and planSizeBasedClearing layers without restructuring the existing compaction logic. All three production callers (idle/size-based, /compress-fast, memory-pressure) wired consistently. Symlink safety via realpathNearestExisting follows the established isTeamAutoMemPath pattern. Scope feels right — nothing extraneous.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ — 所有必要章节齐全,包含双语描述。

问题: #6713 中有明确复现的已观测 bug。通过 read_file 加载的 managed-memory 主题文件在微压缩时被静默清除,导致模型丢失已加载的持久指导。这是真实可复现的问题,不是理论性加固。

方向: 对齐。模型已加载的记忆内容应该在微压缩中存活——这是持久指导,不是临时工具输出。完整压缩仍然有损是正确的边界。CHANGELOG 无直接参考,但属于 scope/memorycategory/core 领域。

规模: 触及核心路径 — 139 行生产逻辑(client.ts: 9, geminiChat.ts: 9, paths.ts: 27, memoryPressureMonitor.ts: 25, microcompact.ts: 69)。远低于 500 行阈值。301 行测试 + 99 行文档。

方案: 回调谓词设计(preserveReadFileResult)简洁且最小化。注入到 collectCompactablePartRefsplanSizeBasedClearing 层,无需重构现有压缩逻辑。三个生产调用者(idle/size-based、/compress-fast、memory-pressure)一致接入。通过 realpathNearestExisting 的符号链接安全性遵循已有的 isTeamAutoMemPath 模式。范围合理——无多余内容。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@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.

No review findings. Downgraded from Approve to Comment: CI checks still pending.

— qwen3.7-max via Qwen Code /review

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

2a. Code Review

Independent proposal (before reading the diff):
I'd add a predicate/callback to MicrocompactOptions that identifies read_file results targeting managed-memory paths. Before computing clear plans, filter those results out of the compactable set. The predicate needs to check project, user, and team memory roots with symlink-safe resolution. All three production callers (client pre-send, geminiChat /compress-fast, memoryPressureMonitor) need the same predicate.

Comparison with PR:
The PR's approach matches this proposal exactly. The implementation is clean and follows existing patterns:

  • isManagedMemoryPath in paths.ts correctly checks all three managed roots (project, user, team) with realpathNearestExisting for symlink safety — consistent with isTeamAutoMemPath.
  • buildPreservedReadRefs correctly skips error responses and requires all paths for an ambiguous call ID to be protected before preserving.
  • planSizeBasedClearing correctly filters preserved refs before computing keepRecent budgets and char totals — protected memory doesn't consume the recent-result budget.
  • All three callers pass identical predicates using isManagedMemoryPath(filePath, projectRoot, targetDir).

Reuse check: isManagedMemoryPath composes existing primitives (getAutoMemoryRoot, getUserAutoMemoryRoot, getTeamAutoMemoryRoot, realpathNearestExisting) — no duplication. The new PreserveReadFileResult type is exported for caller use, which is appropriate.

No critical issues found. The code is focused, follows project conventions (ESM, no any, consistent type imports), and handles edge cases well (ambiguous IDs, error responses, symlink escapes).

2b. Real-Scenario Testing

Ran all 3 related test suites in tmux:

 RUN  v3.2.4 /home/github-runner/actions-runner-17/_work/qwen-code/qwen-code/.qwen/worktrees/triage/packages/core
      Coverage enabled with v8

 ✓ src/memory/team-paths.test.ts (10 tests) 10ms
 ✓ src/services/microcompaction/microcompact.test.ts (63 tests) 17ms
 ✓ src/services/memoryPressureMonitor.test.ts (72 tests) 65ms

 Test Files  3 passed (3)
      Tests  145 passed (145)
   Start at  23:28:54
   Duration  2.92s (transform 285ms, setup 46ms, collect 442ms, tests 92ms, environment 1ms, prepare 233ms)

TypeScript typecheck (tsc --noEmit -p packages/core/tsconfig.json): clean, no errors.

New tests cover: idle/forced/size-based preservation, mixed protected+compactable results, ambiguous call IDs, error responses, symlink escapes, size overage with only protected memory, and protected memory not counting against the recent-result budget.

中文说明

2a. 代码审查

独立方案(读 diff 前):MicrocompactOptions 中添加谓词/回调来识别目标为 managed-memory 路径的 read_file 结果。在计算清除计划前将这些结果从可压缩集中过滤掉。谓词需要使用符号链接安全解析来检查 project、user 和 team 记忆根目录。三个生产调用者需传入相同谓词。

与 PR 对比: PR 方案完全匹配。实现干净且遵循现有模式:

  • isManagedMemoryPath 正确使用 realpathNearestExisting 检查三个 managed 根目录
  • buildPreservedReadRefs 正确跳过错误响应并要求模糊 call ID 的所有路径都受保护
  • planSizeBasedClearing 在计算 keepRecent 预算前正确过滤保留的 refs
  • 三个调用者一致传入相同谓词

无关键问题。 代码聚焦,遵循项目约定,边界情况处理到位。

2b. 实际场景测试

在 tmux 中运行了全部 3 个相关测试套件:145 个测试全部通过。TypeScript 类型检查通过,无错误。

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

This PR solves a real, well-documented bug (#6713) with a focused, minimal change. The callback-predicate approach is the right level of abstraction — it extends the existing microcompaction pipeline without restructuring it, and all three production callers are wired consistently.

The test suite is thorough: 7 new test cases covering the preservation semantics across all trigger paths (idle, forced, size-based), edge cases (ambiguous call IDs, error responses, symlink escapes), and the interaction with the recent-result budget. All 145 tests across the 3 affected files pass, and typecheck is clean.

The tradeoff — managed-memory reads consuming tokens until full compaction — is acknowledged in the PR description and is the right call: durable guidance matters more than reclaiming those tokens when full compaction remains available as the hard cap.

No concerns. Approving.

中文说明

这个 PR 用聚焦且最小的改动解决了一个真实的、有详细记录的 bug(#6713)。回调谓词方案是正确的抽象层次——扩展了现有微压缩管道而没有重构,三个生产调用者一致接入。

测试套件全面:7 个新测试用例覆盖所有触发路径(idle、forced、size-based)下的保留语义、边界情况(模糊 call ID、错误响应、符号链接逃逸)以及与最近结果预算的交互。3 个受影响文件的 145 个测试全部通过,类型检查干净。

取舍——managed-memory 读取在完整压缩前占用 token——在 PR 描述中有说明且是正确选择:持久指导比回收这些 token 更重要,而完整压缩仍可作为硬上限。

无顾虑。通过。

Qwen Code · qwen3.7-max

@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. ✅

@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.

Comment thread packages/core/src/memory/paths.ts
Comment thread packages/core/src/core/client.ts
Comment thread packages/core/src/services/microcompaction/microcompact.ts
@yiliang114

Copy link
Copy Markdown
Collaborator Author

本轮只处理 PR 目标内的 bug/CI/回归问题。这 3 条 review 建议分别涉及缓存 managed memory root realpath、抽取 preserve predicate factory、复用 microcompaction callId map,都是性能/抽象方向的改进建议;当前 CI 已通过且没有证据表明它们是本 PR 引入的 correctness 问题。按自动化的最小修复边界,本轮不扩大 PR 实现,作为后续优化方向处理。

@wenshao wenshao 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.

Comment thread packages/core/src/services/microcompaction/microcompact.ts
Comment thread packages/core/src/services/microcompaction/microcompact.test.ts
@yiliang114

Copy link
Copy Markdown
Collaborator Author

Local Tmux verification

I ran a real interactive verification on codex/6487-preserve-memory-read at commit f7122c0e9 using the built node dist/cli.js.

Scenario

  • Started Qwen Code in a 200×50 Tmux session.
  • Used a deterministic local OpenAI-compatible mock endpoint, so no external model/API was involved.
  • Created a project managed-memory topic containing the unique marker MEMORY-6487-SURVIVES.
  • Set toolResultsTotalCharsThreshold=1 and toolResultsNumToKeep=1 to force size-based microcompaction.
  • Sent a real user prompt through the TUI.
  • The mock model drove the normal tool loop:
    1. read_file on the managed-memory topic.
    2. Two ordinary run_shell_command calls.
    3. A final response based on the exact request JSON received by the model.

Observed result

Qwen's debug log recorded:

[TOOL-RESULT MC] tool result chars 528 > 1, cleared 1 tool result(s) (~52 tokens)

The request sent after microcompaction contained:

  • The managed-memory read_file result, including MEMORY-6487-SURVIVES.
  • [Old tool result content cleared] for the older ordinary shell result.
  • The newer ordinary shell result kept as the recent result.

The final TUI output was:

E2E RESULT
managed memory marker visible: true
ordinary old result cleared: true
marker: MEMORY-6487-SURVIVES

Tmux final result

Exact request JSON after microcompaction

Coverage

  • ✅ Real TUI interaction and tool-call loop
  • ✅ Managed-memory content survives size-based microcompaction
  • ✅ Ordinary stale tool output is still compacted
  • ✅ Exact model-facing JSON confirms the preserved/cleared content
  • ✅ Existing automated tests also cover idle, forced, and memory-pressure callers

Full compaction and ordinary historical tool-result retention remain unchanged by design and are outside this PR's scope.

@wenshao

wenshao commented Jul 11, 2026

Copy link
Copy Markdown
Collaborator

✅ Maintainer local verification — recommended for merge

I built this PR locally from a clean worktree and verified it end-to-end. The change does exactly what it claims: a read_file result for a managed-memory file survives microcompaction, while ordinary tool results are still cleared. No regressions in the touched areas.

Environment: local build · macOS 24.6 · Node v22.23.1 · @qwen-code/qwen-code-core@0.19.9 · PR head 06862d078 (merge of main) · npm ci from scratch.


1. Behavioral proof (the important part)

Instead of the unit tests' synthetic startsWith('/memory/') predicate, I drove the real production wiringisManagedMemoryPath()microcompactHistory() — against a real on-disk memory file (QWEN_CODE_MEMORY_LOCAL=1, memory root <project>/.qwen/memory/), then contrasted it with the pre-PR baseline (microcompaction with no preserve predicate).

  • The real predicate classifies the memory file => true and the ordinary source file => false.
  • BEFORE (main behavior): the memory read is replaced with [Old tool result content cleared] — the marker is lost.
  • AFTER (this PR): the memory read is preserved (marker ORIOLE-6487-VERIFY-SURVIVES still present), the ordinary read is still cleared, and only the ordinary path shows up in evictedReadPaths.

I also ran a before/after differential on the PR's own suite: reverting only the production microcompact.ts back to main (keeping the PR's test file) makes the three preservation tests fail with [Old tool result content cleared]; restoring the PR's version makes them pass again. This confirms the tests actually exercise the new behavior.

Behavioral proof

2. Test & quality evidence

  • 144 tests pass across the three changed test files (microcompact.test.ts, team-paths.test.ts, memoryPressureMonitor.test.ts).
  • tsc --noEmit and eslint are clean on all changed files.
  • Broader regression sweep of the touched directories (microcompaction/, memory/, memoryPressureMonitor, geminiChat, client) — 952 tests green.

Test & quality evidence

3. Notes for reviewers

  • Coverage is complete across entry points. The same isManagedMemoryPath predicate is wired into every production microcompaction call site — pre-send idle + size-based (client.ts), /compress-fast force (geminiChat.ts), and memory-pressure compaction (memoryPressureMonitor.ts) — so the preservation is consistent regardless of what triggers compaction.
  • Correct edge-case handling, verified: symlinks escaping a memory root are not protected (realpath-aware containment); a read_file whose reused call-id maps to mixed paths is not preserved; project / user / team roots are all recognized.
  • Known tradeoff (acknowledged in the PR, worth a nod): protected memory reads are not charged against the recent-result budget and remain until full compaction, so repeated reads of the same memory file can retain multiple copies and grow token usage. The size-based path handles the degenerate "only protected memory left" case by reporting the overage rather than clearing memory — full compaction stays the hard cap. This is a reasonable "durable guidance > reclaimed tokens" choice, but it's the one behavior to keep in mind.

Verdict: functionally correct, well-tested, no regressions observed locally. 👍

🇨🇳 中文说明(点击展开)

✅ 维护者本地验证 —— 建议合并

我在干净的 worktree 中从头构建了本 PR 并做了端到端验证。改动完全符合描述:managed-memory 文件的 read_file 结果能在 microcompaction 中保留,而普通工具结果仍会被清除。 在受影响范围内未发现回归。

环境: 本地构建 · macOS 24.6 · Node v22.23.1 · @qwen-code/qwen-code-core@0.19.9 · PR head 06862d078(已合并 main)· 全新 npm ci

1. 行为验证(重点)

我没有用单测里合成的 startsWith('/memory/') predicate,而是驱动真实的生产链路 —— isManagedMemoryPath()microcompactHistory() —— 针对真实落盘的 memory 文件QWEN_CODE_MEMORY_LOCAL=1,memory root 为 <project>/.qwen/memory/),并与改动前的基线(不带 preserve predicate 的 microcompaction)对比。

  • 真实 predicate 将 memory 文件判定为 => true,普通源码文件判定为 => false
  • 改动前(main 行为): memory 读取结果被替换为 [Old tool result content cleared],marker 丢失。
  • 改动后(本 PR): memory 读取被保留(marker ORIOLE-6487-VERIFY-SURVIVES 仍在),普通读取仍被清除,evictedReadPaths 里只出现普通文件路径。

我还对 PR 自带的测试做了前后差分:只把生产代码 microcompact.ts 回退到 main(保留 PR 的测试文件),三个 preservation 测试会因 [Old tool result content cleared] 而失败;恢复 PR 版本后又全部通过。这证明这些测试确实覆盖了新行为,而非空跑。

2. 测试与质量证据

  • 三个改动测试文件(microcompact.test.tsteam-paths.test.tsmemoryPressureMonitor.test.ts)共 144 个测试全部通过。
  • 所有改动文件的 tsc --noEmiteslint 均无报错。
  • 对受影响目录(microcompaction/memory/memoryPressureMonitorgeminiChatclient)的更大范围回归 —— 952 个测试全绿。

3. 给 reviewer 的说明

  • 入口覆盖完整。 同一个 isManagedMemoryPath predicate 接入了每一个生产 microcompaction 调用点 —— 发送前 idle + size-based(client.ts)、/compress-fast 强制压缩(geminiChat.ts)、内存压力压缩(memoryPressureMonitor.ts)—— 因此无论由哪种触发方式,保留行为都一致。
  • 边界处理正确,已验证: 逃逸出 memory root 的 symlink 不会被保护(realpath 容器判断);复用 call-id 且映射到混合路径的 read_file 不会被保留;project / user / team 三种 root 都能识别。
  • 已知取舍(PR 已说明,提示一下): 被保护的 memory 读取不计入 recent-result 预算,会保留到完整 compaction,因此对同一 memory 文件的重复读取可能保留多份、增加 token 占用。size-based 路径对「只剩受保护 memory」这种极端情况会上报 overage 而非清除 memory —— 完整 compaction 仍是硬上限。这是「持久指导 > 回收 token」的合理取舍,但这一点值得留意。

结论: 功能正确、测试充分、本地未见回归。👍

Verification screenshots are real terminal output from the local run, hosted on branch pr-assets/pr-6714-verify.

@yiliang114

Copy link
Copy Markdown
Collaborator Author

本轮按 review comments 做了最小 closeout:

  • 补充 managed-memory read_file error response 的回归测试:即使传入 preserveReadFileResult,error response 也不会被当作可保留的成功读取结果,且 error response 保持原样。
  • 未在 buildPreservedReadRefs 增加 debug log:microcompact.ts 当前没有局部 logger 注入;为一条诊断建议引入 logger 会扩大这个 PR 的行为面,适合后续单独处理。

验证:cd packages/core && ../../node_modules/.bin/vitest run src/services/microcompaction/microcompact.test.ts(63 passed),prettier --check src/services/microcompaction/microcompact.test.tsgit diff --check

@yiliang114
yiliang114 requested a review from wenshao July 11, 2026 14:03

@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.

Comment thread packages/core/src/core/client.ts
Comment thread packages/core/src/core/client.ts
Comment thread packages/core/src/core/geminiChat.ts
Comment thread packages/core/src/services/memoryPressureMonitor.ts
@wenshao

wenshao commented Jul 11, 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. ✅

@yiliang114

Copy link
Copy Markdown
Collaborator Author

Reviewed the remaining suggestion-only threads. I am not changing code for these in this PR because they are cleanup/future-proofing suggestions rather than bugs, CI failures, or behavior gaps in the managed-memory preservation fix:

  • removing unnecessary getTargetDir() optional chaining/fallbacks
  • preserving a hypothetical future caller override of preserveReadFileResult

@wenshao
wenshao added this pull request to the merge queue Jul 11, 2026
Merged via the queue into QwenLM:main with commit 263dbba Jul 11, 2026
90 of 91 checks passed
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.

Managed memory content is cleared by microcompaction

3 participants