fix(core): dedupe hierarchical memory files by canonical identity - #9600
Conversation
A workspace-level QWEN.md that symlinks to an ancestor QWEN.md was discovered through two lexical paths, and the Set<string> dedup in discovery only compared path strings. Both aliases were then read and attached to the system prompt (double content, inflated fileCount, duplicated relative @imports). Dedupe discovered candidates by fs.realpath identity, keeping the first-discovered lexical path so ordering, display, and import resolution are unchanged. Fixes #9597
|
Re-run on the current head. The fix commit itself hasn't changed since the first pass — the two commits added since are merges of Template looks good ✓ — all sections filled in, including the bilingual summary. Problem: observed bug with solid evidence. #9597 ships a step-by-step reproduction (symlink layout, Direction: clearly aligned — a correctness fix in core memory discovery: the same physical file injected twice doubles token usage and duplicates Size: core path touched ( Approach: scope feels right — one Risk: no elevated risk signals — the changed files don't match any revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明在当前 head 上重跑。自首次审查以来修复提交本身未变——之后新增的两个提交均为合并 模板完整 ✓ —— 各部分均已填写，包含中文摘要。 问题：已观测到的 bug，证据充分。#9597 提供了完整复现步骤（符号链接布局、 方向：明确对齐 —— 核心记忆发现的正确性修复：同一物理文件被注入两次会导致 token 用量翻倍、每次 prompt 中 规模：触及核心路径（ 方案：范围合理 —— 一个 风险：无升级风险信号 —— 改动文件未命中任何与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewRe-reviewed at the current head ( Things I verified while reading:
No correctness blockers, no regressions, nothing beyond the minimal change set. The four new tests do real work: two cannot pass without the fix ( On the earlier review rounds: the open inline threads are all Suggestions, and I don't consider any of them blocking. The two substantive ones are documented scope consequences rather than defects — imports that only resolve beside a dropped alias now vanish (direct result of the issue-sanctioned "keep the first-discovered lexical path"; resolving imports against the file's real directory is arguably the right semantics), and hardlink aliases are not collapsed ( Testing evidence (this PR's own CI — per triage policy I do not run PR code)The full unit suite is green on the reviewed commit. The macOS/Windows One historical note, for the record: the previous head Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed lane, for the record: Not verified: Windows symlink/junction semantics — no CI lane or sandboxed lane exercises Windows for this PR. Everything else is covered by the suite plus the maintainer's live A/B (real CLI + daemon, 13 layouts). 中文说明代码审查在当前 head（ 阅读中核实的关键点：
无正确性阻塞项、无回归、改动集合最小。四个新测试起到实际作用：两条没有修复就不可能通过（ 关于更早的 review 轮次：遗留的行内线程均为 Suggestion，我认为没有一条构成阻塞。两条实质性的属于已记录的范围后果而非缺陷——只能从被丢弃别名一侧解析的 import 会消失（这是 issue 明确允许的"保留最先发现词法路径"的直接结果；按文件真实目录解析 import 反而是更合理的语义），硬链接别名不会合并（ 测试证据（本 PR 自己的 CI —— 按 triage 策略不运行 PR 代码）主单测套件在受审 commit 上全绿。macOS/Windows 的 一点历史记录：上一个 head 沙箱通道，备录在案： 未验证：Windows 符号链接/junction 语义——本 PR 没有任何 CI 通道或沙箱通道在 Windows 上运行。其余一切由套件与维护者实机 A/B（真实 CLI + daemon，13 种布局）覆盖。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 5/5 — clean across every stage on the current head; CI fully green, and the behavioural question that deferred the first pass is now settled by more than the suite. The previous pass approved the idea but withheld approval pending CI on What settled the behavioural gap since the last pass: the unit suite is green on this head; the two repro tests provably fail without the helper; and @wenshao's independent verification below drove the real bundled CLI and The one thing nobody has verified is Windows symlink/junction semantics — no CI or sandboxed lane runs Windows for this PR (#9481 has the lane outage). I'm not holding the PR for it: the change leans only on portable All stages clean, no pending checks, approval guardrail clear (same-repo 中文说明置信度：5/5 —— 当前 head 上每个阶段都干净；CI 全绿，且令首轮暂缓的行为性问题如今已有超出套件本身的证据支撑。 上一轮认可了方案本身，但因 自上轮以来补齐行为证据的是：该 head 上单测套件全绿；两条复现测试在没有该辅助函数时必然失败；@wenshao 下文的独立验证在 macOS 上用真实打包 CLI 与 唯一无人验证过的是 Windows 符号链接/junction 语义——本 PR 没有任何 CI 或沙箱通道在 Windows 上运行（通道故障见 #9481）。不因此卡住这个 PR：改动只依赖可移植的 各阶段全部干净、无待决检查、批准护栏通过（同仓库 — Qwen Code · qwen3.8-max Reviewed at |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
Not explored to full depth (tool budget reached): "You are review agent 6a — Agent 6a: Undirected audit —…": None — I completed all checks within the tool budget..
Not reviewed: verification and reverse audit — neither the verifier nor the reverse auditor was launched with a prompt this skill builds — the posted findings were ruled on, and the misses the rest of the review left were hunted, if at all, without the briefs this skill certifies against.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
未探索到全部深度（达到工具调用预算）："You are review agent 6a — Agent 6a: Undirected audit —…"：None — I completed all checks within the tool budget.。
未审查：验证与反向审计——验证 agent 与反向审计 agent 都没有用本 skill 构建的 prompt 启动——发布的发现即便被裁定过、评审其余部分遗漏的问题即便被搜寻过，也都缺失了本 skill 用以认证的 brief。
— qwen-code-review via Qwen Code /review (v0.21.10)
| identity = await fs.realpath(filePath); | ||
| } catch { | ||
| // Keep the lexical identity so unreadable/unresolvable entries are | ||
| // still handled exactly as before downstream. |
There was a problem hiding this comment.
[Suggestion] The catch block in dedupeByCanonicalIdentity (lines 237-239) that handles fs.realpath failures (broken symlinks, permission errors, etc.) is not exercised by any of the four new tests. All four tests create working symlinks that resolve successfully. A future refactor that accidentally removes or changes this catch behaviour would ship without test feedback.
[Suggestion] dedupeByCanonicalIdentity 中的 catch 块（第 237-239 行）处理 fs.realpath 失败（断链符号链接、权限错误等），但四个新测试均未覆盖此路径。所有测试都创建了成功解析的有效符号链接。未来重构若意外移除或更改此 catch 行为，将在没有测试反馈的情况下上线。
Suggested fix: Add a test that creates a broken symlink (e.g., fsPromises.symlink("nonexistent-target", ...)) and calls loadServerHierarchicalMemory, verifying that the function does not throw and that the broken symlink's entry is handled gracefully.
— qwen-code-review via Qwen Code /review (v0.21.10)
| await createTestFile( | ||
| path.join(testRootDir, DEFAULT_CONTEXT_FILENAME), | ||
| 'outside scan boundary marker', | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The comment at this line describes the pre-fix behavior in present tense ("the parent file is loaded twice... its @import content is attached twice as well"), but the test asserts the opposite (expect(occurrences).toBe(1)). A reader unfamiliar with the PR would expect toBe(2) from the comment, then find toBe(1) and be confused.
[Suggestion] 该行注释以现在时描述了修复前的行为，但测试断言了相反的结果。不熟悉此 PR 的读者会从注释预期 toBe(2)，然后看到 toBe(1) 感到困惑。
Suggested fix: Replace the comment with text that describes the test's actual purpose, e.g.:
// Both directories contain shared.md, so without dedup the import would
// resolve from each parent-file path and attach the same content twice.
// With dedup by canonical identity, the parent is loaded once and the
// import is attached once.
— qwen-code-review via Qwen Code /review (v0.21.10)
| new FileDiscoveryService(projectRoot), | ||
| [], | ||
| DEFAULT_FOLDER_TRUST, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The test loads a context file once when a workspace-level file is a symlink to an ancestor file asserts result.contextFilePaths.toHaveLength(1) but does not assert which path is in the list. The dedup keeps the first-discovered lexical path; if a future refactor changes the discovery order, the wrong path could be retained while the test still passes.
[Suggestion] 测试断言了 result.contextFilePaths.toHaveLength(1)，但未断言列表中具体是哪个路径。如果未来重构改变了发现顺序，错误的路径可能被保留而测试仍然通过。
Suggested fix: Add an assertion like:
expect(result.contextFilePaths[0]).toContain(path.basename(DEFAULT_CONTEXT_FILENAME));
— qwen-code-review via Qwen Code /review (v0.21.10)
| async function dedupeByCanonicalIdentity( | ||
| filePaths: string[], | ||
| ): Promise<string[]> { | ||
| const seenIdentities = new Set<string>(); |
There was a problem hiding this comment.
[Suggestion] The catch block in dedupeByCanonicalIdentity is unconditional — it swallows every error from fs.realpath, including EACCES (permission denied), EMFILE/ENFILE (file descriptor exhaustion), and EIO (disk I/O error). For non-ENOENT errors, the failure is a symptom worth surfacing, yet the dedup silently degrades to lexical identity with no diagnostic.
[Suggestion] dedupeByCanonicalIdentity 中的 catch 块是无条件的——它吞掉了 fs.realpath 的所有错误，包括 EACCES、EMFILE/ENFILE 和 EIO。对于非 ENOENT 的错误，去重逻辑静默退化为词法路径标识，没有任何诊断信息。
Suggested fix: Narrow the catch to ENOENT specifically, or add a logger.warn for non-ENOENT errors:
} catch (err: unknown) {
if (err instanceof Error && (err as NodeJS.ErrnoException).code !== 'ENOENT') {
logger.warn(`realpath failed for ${filePath}: ${err.message}; falling back to lexical identity`);
}
}— qwen-code-review via Qwen Code /review (v0.21.10)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-3 which-alias-survives is pinned by no test — already reported by concurrent reviewer (comment 3823843944 at memoryDiscovery.test.ts:1373)
- R1-4 realpath-failure fallback branch untested — already reported by concurrent reviewer (comment 3823843930 at memoryDiscovery.ts:237)
- R1-5 present-tense test comment contradicting the toBe(1) assertion — already reported by concurrent reviewer (comment 3823843935 at memoryDiscovery.test.ts:1418)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 5": executing the new vitest suite to confirm it passes — worktree has no node_modules/dist and a full npm install + build exceeds the tool budget; pass/fail was ….
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 5"：executing the new vitest suite to confirm it passes — worktree has no node_modules/dist and a full npm install + build exceeds the tool budget; pass/fail was …。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| * announced only once (#9597). The first-discovered lexical path is kept | ||
| * so ordering, display, and relative @import resolution are unchanged. |
There was a problem hiding this comment.
[Suggestion] Dedupe keeps the first-discovered lexical alias — the upward scan unshifts ancestors, so the ancestor wins over a workspace symlink alias — and readGeminiMdFiles resolves relative @imports against path.dirname of the kept path. When an import target exists only beside the dropped alias, its content silently leaves the system prompt (tree mode, the default, keeps the bare @token in the attached content; flat mode skips silently). Confirmed by a probe in a scratch tree at the reviewed commit:
PR: memoryContent = "--- Context from: ../QWEN.md ---\nAlways answer F1A_PARENT. @team-notes.md\n..."
(bare @token, imported content gone)
dedupe off: memoryContent = "...<!-- Imported from: team-notes.md -->\nF1A_IMPORT_MARKER team instructions..."
(import attached)
The concrete trigger: repo/QWEN.md contains @team-notes.md, repo/team-a/QWEN.md is a symlink to it (the #9597 layout), team-notes.md exists only in repo/team-a/, cwd = repo/team-a — after this change the team's instructions are no longer attached. Issue #9597 itself prescribed retaining the selected lexical path for import resolution, so this may be an accepted tradeoff — but the docstring's unqualified "relative @import resolution are unchanged" claim is false for the dropped alias, and the fourth new test masks the corner by creating shared.md in both directories. If the corner-case content loss is judged blocking instead, keep the alias whose lexical path is the canonical path (preserving its position), or resolve import bases through fs.realpath, and add a test where the import target exists only beside the dropped alias.
| * announced only once (#9597). The first-discovered lexical path is kept | |
| * so ordering, display, and relative @import resolution are unchanged. | |
| * announced only once (#9597). The first-discovered lexical path is kept | |
| * so ordering and display are unchanged; relative @imports resolve from | |
| * the kept path's directory, so a target that exists only beside a | |
| * dropped alias is no longer attached. |
中文说明
去重保留最先发现的词法路径——向上扫描通过 unshift 先加入祖先目录，因此祖先路径会胜过工作区的符号链接别名——而 readGeminiMdFiles 解析相对 @import 时以保留路径的 path.dirname 为基准目录。当某个 import 目标只存在于被丢弃别名旁边的目录时，其内容会静默地从系统提示词中消失（默认的 tree 模式会在附加内容中留下裸的 @token；flat 模式则静默跳过）。已在评审提交的临时目录中通过探针确认：
PR: memoryContent = "--- Context from: ../QWEN.md ---\nAlways answer F1A_PARENT. @team-notes.md\n..."
（只剩裸 @token，import 内容丢失）
关闭去重: memoryContent = "...<!-- Imported from: team-notes.md -->\nF1A_IMPORT_MARKER team instructions..."
（import 正常附加）
具体触发场景：repo/QWEN.md 含 @team-notes.md，repo/team-a/QWEN.md 是指向它的符号链接（即 #9597 的布局），team-notes.md 只存在于 repo/team-a/，cwd = repo/team-a——此变更后团队的指令将不再被附加。Issue #9597 本身指定保留所选词法路径用于 import 解析，因此这可能是可接受的取舍——但 docstring 中「relative @import resolution are unchanged」的无条件表述对被丢弃的别名并不成立，且第四个新测试通过在两个目录都创建 shared.md 掩盖了这个角落场景。如果认为该角落场景的内容丢失应视为阻塞性问题，可改为保留词法路径即规范路径的那个别名（保持其数组位置），或通过 fs.realpath 解析 import 基准目录，并补充一个 import 目标只存在于被丢弃别名旁边的测试。
— qwen3.8-max via Qwen Code /review (v0.21.14)
| for (const filePath of filePaths) { | ||
| let identity = path.resolve(filePath); | ||
| try { | ||
| identity = await fs.realpath(filePath); |
There was a problem hiding this comment.
[Suggestion] Dedupe keys on fs.realpath, which resolves only symlinks. Hardlink aliases of the same physical file produce distinct realpaths, so both survive and the identical content is attached twice — contradicting the function's own invariant that one physical file reached through multiple lexical paths is "read, attached, and announced only once". Confirmed by a probe in a scratch tree at the reviewed commit:
PR (fs.link pair, same inode): fileCount: 2, occurrences: 2, contextFilePaths: ["../QWEN.md","QWEN.md"]
dev:ino identity instead: fileCount: 1, occurrences: 1 (all 4 shipped symlink tests still green)
The trigger: a user who keeps one physical context file and exposes it into the workspace as a hardlink (ln projectRoot/QWEN.md cwd/QWEN.md — common in backup/sync workflows that avoid symlinks) gets the exact #9597 duplication. Issue #9597's own suggested fix allows "fs.realpath, or an equivalent file identity", so a device+inode key is in scope; fs.stat follows symlinks, so symlink aliases still merge.
| identity = await fs.realpath(filePath); | |
| const { dev, ino } = await fs.stat(filePath); | |
| identity = `${dev}:${ino}`; |
中文说明
去重以 fs.realpath 作为身份键，但它只解析符号链接。同一物理文件的硬链接别名会产生不同的 realpath，因此两者都会保留、相同内容被附加两次——与函数自身「一个物理文件经由多个词法路径到达时只被读取、附加、通知一次」的不变量相矛盾。已在评审提交的临时目录中通过探针确认：
PR（fs.link 对，同 inode）: fileCount: 2, occurrences: 2, contextFilePaths: ["../QWEN.md","QWEN.md"]
改用 dev:ino 身份: fileCount: 1, occurrences: 1（四个已随 PR 提供的符号链接测试仍全部通过）
触发场景：用户把同一份物理上下文文件以硬链接方式暴露进工作区（ln projectRoot/QWEN.md cwd/QWEN.md——在避免符号链接的备份/同步工作流中很常见），就会得到与 #9597 完全相同的重复。Issue #9597 自己建议的修复方式允许「fs.realpath 或等价的文件身份」，因此以 device+inode 为键在范围之内；fs.stat 会跟随符号链接，所以符号链接别名仍会合并。
— qwen3.8-max via Qwen Code /review (v0.21.14)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- present-tense test comment contradicting the toBe(1) assertion — already reported by @doudouOUC (comment 3823843935 at memoryDiscovery.test.ts:1418)
- no test pins which alias survives dedupe — already reported by @doudouOUC (comment 3823843944 at memoryDiscovery.test.ts:1373)
- realpath-failure fallback branch untested — already reported by @doudouOUC (comment 3823843930 at memoryDiscovery.ts:237)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent 5": executing memoryDiscovery.test.ts to confirm the new tests pass — the review worktree has no node_modules / dist and a monorepo npm ci + build was beyond ….
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/core/src/utils/memoryDiscovery.ts:239 — [review] canonical-duplicate drop is silent; every other discovery event in this file logspackages/core/src/utils/memoryDiscovery.ts:559 — [review] extension-path route through the dedupe is documented but untested
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent 5"：executing memoryDiscovery.test.ts to confirm the new tests pass — the review worktree has no node_modules / dist and a monorepo npm ci + build was beyond …。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| * announced only once (#9597). The first-discovered lexical path is kept | ||
| * so ordering, display, and relative @import resolution are unchanged. |
There was a problem hiding this comment.
[Suggestion] R1-1: Still stands — the dedupe keeps the first-discovered lexical alias, and readGeminiMdFiles resolves relative @imports against path.dirname of the kept path, so an import target that exists only beside a dropped alias still silently leaves the system prompt, and the docstring's unqualified "relative @import resolution are unchanged" claim remains false for the dropped alias. The anchored code is unchanged since round 1 and the round-1 probe (bare @token left behind, imported content gone) still reproduces.
New this round — a second entrance, probe-confirmed at e3f47df99 (both arms): discovery appends the global/home path before the upward scan, so a ~/.qwen/QWEN.md that is a symlink to the project QWEN.md wins the dedupe. PR arm: fileCount: 1, contextFilePaths: ["~/.qwen/QWEN.md"], a project-relative @docs/style.md is left as literal unexpanded text (occurrences: 0) and the file classifies as user memory; with the dedupe disabled the import attaches (occurrences: 1) and classifies as project.
Issue #9597 itself allows retaining the selected lexical path, so this may remain an accepted tradeoff — in that case make the docstring honest about it:
| * announced only once (#9597). The first-discovered lexical path is kept | |
| * so ordering, display, and relative @import resolution are unchanged. | |
| * announced only once (#9597). The first-discovered lexical path is kept | |
| * so ordering and display are unchanged; relative @imports resolve from | |
| * the kept path's directory, so a target that exists only beside a | |
| * dropped alias is no longer attached. |
中文说明
仍然成立——去重保留最先发现的词法路径，而 readGeminiMdFiles 解析相对 @import 时以保留路径的 path.dirname 为基准，因此只存在于被丢弃别名旁边的 import 目标仍会静默地从系统提示词中消失，docstring 中「relative @import resolution are unchanged」的无条件表述对被丢弃别名依旧不成立。锚定代码自上一轮以来未变，上一轮的探针（只剩裸 @token、import 内容丢失）仍可复现。
本轮新增——第二个触发入口，已在 e3f47df99 通过探针确认（双臂对照）：发现流程在上行扫描之前先加入全局/主目录路径，因此指向项目 QWEN.md 的符号链接 ~/.qwen/QWEN.md 会赢下去重。PR 臂：fileCount: 1、contextFilePaths: ["~/.qwen/QWEN.md"]，项目相对的 @docs/style.md 以裸文本留存（出现次数 0），且文件被归类为 user 记忆；关闭去重后 import 正常附加（出现次数 1）并归类为 project。
Issue #9597 本身允许保留所选词法路径，因此这可能仍是可接受的取舍——若是如此，请让 docstring 如实描述该行为。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| for (const filePath of filePaths) { | ||
| let identity = path.resolve(filePath); | ||
| try { | ||
| identity = await fs.realpath(filePath); |
There was a problem hiding this comment.
[Suggestion] R1-2: Still stands — dedupe keys on fs.realpath, which resolves only symlinks. Hardlink aliases of the same physical file produce distinct realpaths, so both survive and the identical content is attached twice — contradicting the function's own invariant that one physical file reached through multiple lexical paths is "read, attached, and announced only once". The anchored code is unchanged since round 1; the round-1 probe still reproduces: an fs.link pair (same inode) gives fileCount: 2, occurrences 2, while a dev:ino identity gives fileCount: 1, occurrences 1 with all four shipped symlink tests still green. The trigger: a user who keeps one physical context file and exposes it into the workspace as a hardlink (ln projectRoot/QWEN.md cwd/QWEN.md — common in backup/sync workflows that avoid symlinks) gets the exact #9597 duplication. Issue #9597's own suggested fix allows "fs.realpath, or an equivalent file identity", so a device+inode key is in scope; fs.stat follows symlinks, so symlink aliases still merge.
| identity = await fs.realpath(filePath); | |
| const { dev, ino } = await fs.stat(filePath); | |
| identity = `${dev}:${ino}`; |
中文说明
仍然成立——去重以 fs.realpath 作为身份键，但它只解析符号链接。同一物理文件的硬链接别名会产生不同的 realpath，因此两者都会保留、相同内容被附加两次——与函数自身「一个物理文件经由多个词法路径到达时只被读取、附加、通知一次」的不变量相矛盾。锚定代码自上一轮以来未变，上一轮探针仍可复现：fs.link 对（同 inode）得到 fileCount: 2、出现 2 次，而改用 dev:ino 身份得到 fileCount: 1、出现 1 次，且四个已随 PR 提供的符号链接测试仍全部通过。触发场景：用户把同一份物理上下文文件以硬链接方式暴露进工作区（ln projectRoot/QWEN.md cwd/QWEN.md——在避免符号链接的备份/同步工作流中很常见），就会得到与 #9597 完全相同的重复。Issue #9597 自己建议的修复方式允许「fs.realpath 或等价的文件身份」，因此以 device+inode 为键在范围之内；fs.stat 会跟随符号链接，符号链接别名仍会合并。
— qwen3.8-max via Qwen Code /review (v0.22.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
5 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- present-tense test comment contradicting the toBe(1) assertion — already reported (comment 3823843935 at memoryDiscovery.test.ts:1418)
- no test pins which lexical alias survives dedupe — already reported (comment 3823843944 at memoryDiscovery.test.ts:1373)
- realpath-failure fallback branch untested — already reported (comment 3823843930 at memoryDiscovery.ts:237)
- canonical-duplicate drop is silent while every other discovery event logs — already disclosed in round 2's deferral list (review 5000624656)
- extension-path route through the dedupe documented but untested — already disclosed in round 2's deferral list (review 5000624656)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Test Plan (not a blocker): packages/core/src/utils/memoryDiscovery.test.ts — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
本轮确认的 5 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
Test Plan（非阻断）：packages/core/src/utils/memoryDiscovery.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| * physical file reached through multiple lexical paths (e.g. a workspace | ||
| * QWEN.md that symlinks to an ancestor QWEN.md) is read, attached, and | ||
| * announced only once (#9597). The first-discovered lexical path is kept | ||
| * so ordering, display, and relative @import resolution are unchanged. |
There was a problem hiding this comment.
[Suggestion] R1-1: Still stands — the dedupe keeps the first-discovered lexical alias (the upward scan unshifts ancestors, so the ancestor wins over a workspace symlink alias), and readGeminiMdFiles resolves relative @imports against path.dirname of the kept path. When an import target exists only beside the dropped alias, its content silently leaves the system prompt (tree mode, the default, keeps the bare @token in the attached content; flat mode skips silently). Re-confirmed at this commit: processImports(content, path.dirname(filePath), ...) resolves imports from the kept alias only. Concrete trigger: repo/QWEN.md contains @team-notes.md, repo/team-a/QWEN.md is a symlink to it, team-notes.md exists only in repo/team-a/, cwd = repo/team-a — after this change the team's instructions are no longer attached. Issue #9597 itself prescribed retaining the selected lexical path for import resolution, so this may be an accepted tradeoff — but the docstring's unqualified "relative @import resolution are unchanged" claim is false for the dropped alias, and no test covers the target-exists-only-beside-the-dropped-alias corner. If it is accepted behavior, say so in the docstring (suggestion below); otherwise keep the alias whose lexical path is the canonical path, or resolve import bases through fs.realpath, and add a test for that corner.
| * so ordering, display, and relative @import resolution are unchanged. | |
| * so ordering and display are unchanged; relative @imports resolve from | |
| * the kept path's directory, so a target that exists only beside a | |
| * dropped alias is no longer attached. |
中文说明
R1-1：仍然成立 —— 去重保留最先发现的词法路径（向上扫描通过 unshift 先加入祖先目录，因此祖先路径会胜过工作区的符号链接别名），而 readGeminiMdFiles 解析相对 @import 时以保留路径的 path.dirname 为基准目录。当某个 import 目标只存在于被丢弃别名旁边的目录时，其内容会静默地从系统提示词中消失（默认的 tree 模式会在附加内容中留下裸的 @token；flat 模式则静默跳过）。已在本提交上再次确认：processImports(content, path.dirname(filePath), ...) 只从保留的别名解析 import。具体触发场景：repo/QWEN.md 含 @team-notes.md，repo/team-a/QWEN.md 是指向它的符号链接，team-notes.md 只存在于 repo/team-a/，cwd = repo/team-a —— 此变更后团队的指令将不再被附加。Issue #9597 本身指定保留所选词法路径用于 import 解析，因此这可能是可接受的取舍 —— 但 docstring 中「relative @import resolution are unchanged」的无条件表述对被丢弃的别名并不成立，且没有测试覆盖 import 目标只存在于被丢弃别名旁边的角落场景。如果这是可接受的行为，请在 docstring 中如实说明（见下方建议）；否则可改为保留词法路径即规范路径的那个别名，或通过 fs.realpath 解析 import 基准目录，并补充相应测试。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| for (const filePath of filePaths) { | ||
| let identity = path.resolve(filePath); | ||
| try { | ||
| identity = await fs.realpath(filePath); |
There was a problem hiding this comment.
[Suggestion] R1-2: Still stands — dedupe keys on fs.realpath, which resolves only symlinks. Hardlink aliases of the same physical file produce distinct realpaths, so both survive and the identical content is attached twice — contradicting the function's own docstring ("one physical file reached through multiple lexical paths ... is read, attached, and announced only once"). Concrete trigger: a workspace keeps one canonical QWEN.md and hard-links it into an ancestor or sibling discovery directory (ln projectRoot/QWEN.md cwd/QWEN.md — hard links, not symlinks; common in dotfile setups that avoid symlinking); both paths pass discovery, realpath returns each hard-linked name unchanged, and the identical content is injected twice — the exact #9597 symptom via a different aliasing mechanism. Test 2 does not cover this case, since hard-linked names are one inode, not two physical files. If hard-link aliases are in scope, key identity on fs.stat's dev+ino (with the lexical-path fallback on stat failure); if they are deliberately out of scope for #9597, narrow the docstring to canonical path identity (symlinks only) so it stops overselling the behavior.
中文说明
R1-2：仍然成立 —— 去重以 fs.realpath 为键，而它只解析符号链接。同一物理文件的硬链接别名会产生不同的 realpath，因此两者都会保留，相同内容被附加两次 —— 与函数自身 docstring（「一个物理文件经由多个词法路径被访问时……只被读取、附加和通知一次」）相矛盾。具体触发场景：工作区保留一份规范的 QWEN.md，并将其硬链接到祖先或同级发现目录（ln projectRoot/QWEN.md cwd/QWEN.md —— 是硬链接而非符号链接，在避免符号链接的 dotfile 配置中很常见）；两个路径都能通过发现流程，realpath 对每个硬链接名称原样返回，相同内容被注入两次 —— 正是 #9597 的症状经由另一种别名机制复现。测试 2 覆盖不到这种情况，因为硬链接名称是同一个 inode，而不是两个物理文件。如果硬链接别名在范围内，请以 fs.stat 的 dev+ino 作为身份键（stat 失败时回退到词法路径）；如果它们有意不在 #9597 的范围内，请把 docstring 收窄为规范路径身份（仅符号链接），以免过度承诺行为。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| }); | ||
| }); | ||
|
|
||
| describe('symlink aliases of the same physical file (#9597)', () => { |
There was a problem hiding this comment.
[Suggestion] R3-1: No test covers the cross-filename alias case — one memory filename aliasing another's physical file (e.g. AGENTS.md symlinked to QWEN.md). The default scan list is ['QWEN.md', 'AGENTS.md'], and dedupeByCanonicalIdentity runs on the candidate list flattened across filenames, so that shape is deduplicated today — verified by a probe at this commit: the PR arm returns fileCount === 1, the marker once, kept path ../QWEN.md; the dedupe-removed mutant arm flips to AssertionError: expected 2 to be 1. But all four tests in this block alias QWEN.md onto itself, so a future refactor keying identity per filename (or moving the dedupe into the per-filename branch) would silently restore duplicate injection for the ln -s QWEN.md AGENTS.md layout with no red test — user-visible in /memory fileCount and display paths. Add one test here: create projectRoot/QWEN.md with marker content, symlink projectRoot/AGENTS.md → QWEN.md, then assert fileCount === 1 and the marker occurs once. Note: the suite's afterEach resets the filename list to QWEN.md only, so the new test must call setGeminiMdFilename([DEFAULT_CONTEXT_FILENAME, AGENT_CONTEXT_FILENAME]) itself — otherwise AGENTS.md is never scanned and the test passes vacuously.
中文说明
R3-1：没有测试覆盖跨文件名别名的情况 —— 即某个记忆文件名是另一个物理文件的别名（例如 AGENTS.md 符号链接到 QWEN.md）。默认扫描列表是 ['QWEN.md', 'AGENTS.md']，而 dedupeByCanonicalIdentity 运行在跨文件名扁平化后的候选列表上，因此这种形态目前确实会被去重 —— 已在本提交上通过探针验证：PR 侧返回 fileCount === 1、标记出现一次、保留路径为 ../QWEN.md；移除去重的突变体侧翻转为 AssertionError: expected 2 to be 1。但本块四个测试都只把 QWEN.md 别名到它自身，因此未来若把身份键改为按文件名划分（或把去重移入按文件名分支），会在所有现有测试不飘红的情况下，悄悄让 ln -s QWEN.md AGENTS.md 布局重新出现重复注入 —— 且用户可在 /memory 的 fileCount 与展示路径中看到。请在此补充一个测试：创建带标记内容的 projectRoot/QWEN.md，将 projectRoot/AGENTS.md 符号链接到 QWEN.md，然后断言 fileCount === 1 且标记只出现一次。注意：套件 afterEach 会把文件名列表重置为仅 QWEN.md，所以新测试必须自行调用 setGeminiMdFilename([DEFAULT_CONTEXT_FILENAME, AGENT_CONTEXT_FILENAME]) —— 否则 AGENTS.md 根本不会被扫描，测试会空转通过。
— qwen3.8-max via Qwen Code /review (v0.22.0)
Independent local verification — real CLI + real daemon, A/B against merge-baseI rebuilt this PR locally and verified it end-to-end through the bundled CLI and through Environment
1. Red/green control on the unit tests
2. End-to-end through the real bundled CLIEach scenario is the issue's own directory shape (
Before — base arm, S1 (the user-visible bug: two rows in
After — PR arm, S1:
Relative
Regression guards on the PR arm — identical content in two real files stays two blocks (S3), and the symlink-only workspace still loads (S2):
3. Daemon surface from the issue (
|
| # | Layout | Base → PR (fileCount, retained blocks) |
|---|---|---|
| P1 | issue layout | 2 → 1 (../../QWEN.md) |
| P2 | workspace .git boundary, symlink only |
1 → 1 |
| P3 | distinct files, identical content | 2 → 2 |
| P4 | @shared.md resolvable from both directories |
marker 2× → 1× |
| P5 | @shared.md exists only next to the symlink alias |
marker 1× → 0× (see note a) |
| P6 | broken symlink at agent/QWEN.md |
1 → 1 (real file once; unreadable alias skipped exactly as before) |
| P7 | hard link alias | 2 → 2 (see note b) |
| P8 | ~/.qwen/QWEN.md -> <root>/QWEN.md |
2 → 1, retained as ~/.qwen/QWEN.md (i.e. classified as user memory) |
| P9 | extensionContextFilePaths=[root/w/../QWEN.md] aliasing a scanned file |
1 → 1 (already collapsed by path.resolve on base) |
| P10 | ancestor root/QWEN.md is the symlink, real file in the workspace |
2 → 1, retained ../../QWEN.md (first discovered) |
| P11 | contextFileName: ["QWEN.md","qwen.md"] on case-insensitive APFS |
2 → 1 — bonus: fs/promises.realpath is the native call and canonicalises case on macOS |
| P12 | <root>/.qwen/QWEN.local.md -> ../QWEN.md |
2 → 1 (dedupe runs after the local slot is appended, as the review noted) |
| P13 | includeDirectories=[root/agent-link] where agent-link -> w/agent, cwd = w/agent |
2 → 1, retained ../../agent-link/QWEN.md (see note c) |
Notes (none blocking)
a. Imports that only resolve from the alias directory now vanish (P5). On base the symlink alias was read too, so a @shared.md that exists only next to the symlink was attached once (the failed ancestor-side import only surfaces in debug logging). On the PR arm only the ancestor path survives, its directory has no shared.md, and the import silently disappears (import failures go to the debug logger only). This is the direct consequence of "keep the first-discovered lexical path", which the issue explicitly allows, and resolving imports relative to the file's real directory is arguably the right semantics — recording it so the behaviour change is on the record.
b. Hard links are not collapsed (P7). fs.realpath gives distinct paths for two links to one inode, so both are still loaded. Consistent with the PR's stated scope ("only paths resolving to the same physical file"); dev/ino identity would be needed to cover this and the issue does not ask for it.
c. Display-path precedence with --include-directories (P13). When an include directory is a symlinked alias of the cwd, the include path is discovered first and wins, so /context shows ../../agent-link/QWEN.md rather than QWEN.md. Purely presentational — the content is attached once.
Cosmetic: the PR body still refers to packages/core/src/utils/memoryDiscovery.ts; after the merge with main the file lives at packages/core/src/memory/memoryDiscovery.ts, which is where the diff correctly applies.
Verdict: verified — no blocker from my side. ✅
中文说明
本地独立验证 —— 真实 CLI + 真实 daemon，与 merge-base 双臂对比
我在本地重新构建了这个 PR，并通过打包后的 CLI 和 qwen serve 做了端到端验证，而不只是单测层面。结论：修复行为与描述完全一致，四条回归守护全部成立，没有发现阻塞合并的问题。 文末三条非阻塞观察。
环境
| PR 臂 | 624602d70d7a3bb6f212cb452eb8566a0d5253e0（PR head） |
| Base 臂 | 8be07151f69641520e9cf692c8190bf094d25588（与 main 的 merge-base）—— 用 PR head 把两个改动文件还原到 merge-base 版本构建，其余字节完全一致 |
| 构建 | 两臂都完整构建出 dist/cli.js（npm run build → npm run bundle） |
| 双臂证明 | PR 独有字面量 dedupeByCanonicalIdentity 只出现在 PR bundle 的一个 chunk 中，base 的 chunk 里没有 |
| 模型端 | 仓库自带的 integration-tests/fake-openai-server.ts；假模型的回复会回显 system prompt 里带了几个 --- Context from: … --- 块、fixture 标记出现了几次，因此每张截图自证 |
| TUI 驱动 | integration-tests/terminal-capture（node-pty + xterm.js），CLI 以 --auth-type openai --openai-base-url <fake> --model dummy 启动，隔离 HOME |
| 文件系统 | macOS APFS（大小写不敏感）—— 作者只在 Linux 测过，这里补上 |
1. 单测红/绿对照
| 运行 | 结果 |
|---|---|
PR 的 memoryDiscovery.test.ts 打到 base 源码（负对照） |
2 失败 / 50 通过 —— 失败的正好是两条复现用例（expected 2 to be 1：父文件+符号链接的 fileCount，以及重复的 @import）；两条回归守护在 base 上也通过，守护本该如此 |
| 同一文件打到 PR 源码 | 52 通过 |
memoryDiscovery + memoryImportProcessor + rulesDiscovery，PR 源码 |
125 / 125 通过（与 PR 描述一致） |
eslint --max-warnings 0、prettier --check、core tsc --build |
干净 |
2. 真实打包 CLI 端到端
每个场景都是 issue 原始目录形态（root/QWEN.md，root/w/agent/QWEN.md -> ../../QWEN.md，会话 cwd = root/w/agent），配场景专属标记。每臂读三个面：/context detail 的 Memory files、首条 prompt 的 Read context files: 公告、以及假模型实际收到的 system prompt 里的 --- Context from: 块。
| 场景 | Base | PR |
|---|---|---|
| S1 issue #9597 布局 | 三个面都列出 ../../QWEN.md、QWEN.md 两条 · system prompt 中标记 2× |
只有 ../../QWEN.md · 标记 1× |
S2 工作区自带 .git（祖先在扫描边界外，只有符号链接可发现） |
QWEN.md · 1× |
QWEN.md · 1× —— 仍能加载 |
| S3 内容相同的两个不同物理文件 | 2 行 · 2× | 2 行 · 2× —— 未被合并 |
S4 root/QWEN.md = @shared.md，两个目录旁都有 shared.md |
2 行 · 导入标记 2× | 1 行 · 1× |
截图见上文英文部分：S1 base/PR 前后对比、S4 base/PR 导入对比、PR 臂上 S3/S2 两条守护。
3. issue 提到的 daemon 面（GET /session/:id/context-usage?detail=true）
每臂从符号链接工作区启动 qwen serve，POST /session 后查询 context-usage。Base 报出两个词法路径，PR 臂只报一次（截图见上）。
4. 通过构建后的 core 跑布局矩阵（packages/core/dist 的 loadServerHierarchicalMemory，隔离 HOME）
| # | 布局 | Base → PR（fileCount，保留的块） |
|---|---|---|
| P1 | issue 布局 | 2 → 1（../../QWEN.md） |
| P2 | 工作区 .git 边界，仅符号链接 |
1 → 1 |
| P3 | 不同文件、相同内容 | 2 → 2 |
| P4 | @shared.md 两个目录都能解析 |
标记 2× → 1× |
| P5 | @shared.md 仅存在于符号链接旁 |
标记 1× → 0×（见备注 a） |
| P6 | agent/QWEN.md 为断链 |
1 → 1（真实文件一次；不可读别名与以前一样被跳过） |
| P7 | 硬链接别名 | 2 → 2（见备注 b） |
| P8 | ~/.qwen/QWEN.md -> <root>/QWEN.md |
2 → 1，保留为 ~/.qwen/QWEN.md（即归类为用户记忆） |
| P9 | extensionContextFilePaths=[root/w/../QWEN.md] 别名已扫描文件 |
1 → 1（base 上 path.resolve 已能合并） |
| P10 | 祖先 root/QWEN.md 是符号链接，真实文件在工作区 |
2 → 1，保留 ../../QWEN.md（先发现者） |
| P11 | 大小写不敏感 APFS 上 contextFileName: ["QWEN.md","qwen.md"] |
2 → 1 —— 额外收益：fs/promises.realpath 是原生调用，macOS 上会规范化大小写 |
| P12 | <root>/.qwen/QWEN.local.md -> ../QWEN.md |
2 → 1（去重在 local 槽位追加之后运行，与评审所述一致） |
| P13 | includeDirectories=[root/agent-link]，agent-link -> w/agent，cwd = w/agent |
2 → 1，保留 ../../agent-link/QWEN.md（见备注 c） |
备注（均不阻塞）
a. 只能从别名目录解析的导入会消失（P5）。 base 上符号链接别名也会被读取，所以只存在于符号链接旁的 @shared.md 会被附加一次（祖先那一侧的导入失败只出现在 debug 日志里）。PR 臂只保留祖先路径，其目录下没有 shared.md，导入就静默消失（导入失败只写 debug 日志）。这是「保留最先发现的词法路径」的直接后果，issue 明确允许，而且按文件真实目录解析导入本来也更合理 —— 记录在此只是让行为变化有据可查。
b. 硬链接不会合并（P7）。 同一 inode 的两个链接 fs.realpath 结果不同，仍会都加载。与 PR 声明的范围一致（「只合并解析到同一物理文件的路径」）；要覆盖这种情况需要 dev/ino 身份，issue 也没有要求。
c. --include-directories 下的展示路径优先级（P13）。 include 目录是 cwd 的符号链接别名时，include 路径先被发现而胜出，/context 显示 ../../agent-link/QWEN.md 而非 QWEN.md。纯展示问题，内容只附加一次。
小瑕疵：PR 描述仍写 packages/core/src/utils/memoryDiscovery.ts；合并 main 后文件位于 packages/core/src/memory/memoryDiscovery.ts，diff 落点正确。
结论：验证通过 —— 我这边没有阻塞项。 ✅
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 21957 passed · 0 failed · 21957 total Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：21957 通过 · 0 失败 · 21957 总计 抖动门：✅ 1 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #9600 Deep Verification — fix(core): dedupe hierarchical memory files by canonical identityVerdict: 中文摘要
ScopeCentral claim: one physical context file reached through multiple lexical paths (workspace Secondary claims: (1) regression guards hold — distinct physical files with identical content stay two blocks; a symlink whose target lies outside the scan boundary still loads; unresolvable paths fall back to lexical identity with pre-existing behavior; (2) the first-discovered lexical path is kept, so ordering/display/import resolution are unchanged. A/B load-bearing proofMock-free harness (
18/18 assertions passed on every arm (base, head, surgical control). The flip cells (A, B, E, H) prove the dedupe is load-bearing; cells C/D/F prove no over-dedup or behavior change; cell A additionally confirms the ordering claim — the ancestor path Base rebuild note: reusing the root Corrections to the PR description
FindingsS1 — Reviewer Test Plan command silently runs 37 of 125 tests (Suggestion)vitest positional args are regex filters; filters matching nothing are silently skipped as long as at least one other filter matches. The command verbatim from the PR body: npx vitest run src/utils/memoryDiscovery.test.ts src/utils/memoryImportProcessor.test.ts src/utils/rulesDiscovery.test.ts --root packages/coreran only cd packages/core && npx vitest run src/memory/memoryDiscovery.test.ts src/utils/memoryImportProcessor.test.ts src/config/rulesDiscovery.test.tsS2 — "first-discovered lexical path is kept" is unpinned by tests (Suggestion, coverage gap)Mutation M2 (keep-last path per identity instead of keep-first; S3 — catch fallback branch is load-bearing but unpinned (Suggestion, coverage gap)Mutation M3 (catch rethrows instead of lexical fallback) also leaves all 4 tests green — no unit test feeds an unresolvable path into dedup (upward discovery S4 — test suite does not isolate
|
| Gate | Result |
|---|---|
packages/core vitest, the 3 affected files (src/memory/memoryDiscovery.test.ts 52, src/utils/memoryImportProcessor.test.ts 37, src/config/rulesDiscovery.test.ts 36) |
125/125 pass (03-vitest-gate-clean-env.png, logs/vitest-head-gate-clean.log) |
packages/core full suite |
21,723 passed, 10 skipped, exit 0 (logs/vitest-core-full.log) |
npm run typecheck (core) |
exit 0; liveness proven — planted const x: number = "s" produced TS2322/TS6133, restored |
npx eslint on both changed files |
exit 0; liveness proven — planted unused var reported no-unused-vars, restored |
Vacuity and mutation matrix (source-level, on the head tree)
| Mutant | Change | New #9597 tests | Classification |
|---|---|---|---|
| M1 | dedupe call → identity passthrough (= un-fixed code) | KILLED — exactly the 2 repro tests fail expected 2 to be 1; both regression guards stay green. Witness 04-vacuity-m1-unfixed-source.png, logs/mutation-M1.log |
positive control + non-vacuity proof |
| M2 | keep-last instead of keep-first | SURVIVED (4/4 green) | coverage gap (S2) |
| M3 | catch rethrows instead of fallback | SURVIVED (4/4 green); dist variant rejects on cell F | coverage gap, guard load-bearing (S3) |
Per-commit attribution (depth-2 shallow checkout)
The merge history is grafted (HEAD^2^1 unavailable), but the fix commit object a5921127 and its parent 3e99b716 are present locally. git diff 3e99b716..a5921127 with the src/utils/ → src/memory/ path rename applied is byte-identical to the aggregate git diff HEAD^1..HEAD (only index blob hashes differ). The entire substantive change is therefore the single fix commit; the two merge commits contribute only the main merge that renamed the file's directory.
Not covered
- Windows / macOS behavior — the PR marks both untested and the Windows CI lane is down (ci: the Windows lane is red — 59 tests across 10 files, unobserved since the merge queue stopped running #9481).
fs.realpathsemantics are platform-standard, but no cell ran there. - Surfaces above the loader —
/memory show,/context, context-usage reporting consumefileCount/contextFilePathsfrom this function (the sole server-side choke point: onlypackages/core/src/config/config.tscalls it, CLI re-exports that), so they inherit the fix; none was driven separately through the TUI. - The PR author's standalone live-repro script was not run verbatim; harness cells A/B cover the same wire shape (function-level reproduction of the issue's layout, not a model-call E2E).
- Performance — the PR's "one extra
fs.realpathper discovered file" cost was not measured (session-start path, small N). - Base-side
npm ci— not run; base reused the rootnode_modules(PR touches no dependency manifest). Resulting type drift was confined to files outside the tested closure (see A/B note) and neutralized by the surgical control. - Mutation of the
dedupedFilePaths.length === 0debug-log condition — cosmetic-only consumer, not exercised.
Methodology
Environment: the CI verify container (node:22-bookworm), merge-ref checkout at depth 2; npm ci + npm run build pre-run at HEAD. The A/B harness (harness/ab-symlink-dedupe.mjs) imports the compiled dist/src/memory/memoryDiscovery.js and dist/src/services/fileDiscoveryService.js directly (no stubs anywhere on the tested path), builds real directory/symlink layouts under os.tmpdir(), and isolates discovery with scratch HOME/QWEN_HOME; the base side ran from a git worktree at HEAD^1 rebuilt with node ../../scripts/build_package.js, and import-closure realpath checks confirmed no @qwen-code/* workspace symlink contamination (the closure imports none). Mutations were applied to source (vitest) and to compiled dist (node patch), each restored afterward (git status clean after every step); every gate's liveness was proven by planting and detecting a violation before citing its green. Assertion totals in assertions.json: A/B harness 18×3 arms + 1 M3-variant crash check; vitest 125 (targeted) + 21,723 (full core) + 37 (verbatim-plan run) + 4+4+4 (M1/M2/M3 scoped runs) + 1 stale-path detection check; typecheck/lint gates and their 2 liveness probes = 4. Raw per-cell logs in logs/, harnesses in harness/, images in evidence/ (01-ab-base-vs-head.png, 02-reviewer-plan-verbatim-stale-paths.png, 03-vitest-gate-clean-env.png, 04-vacuity-m1-unfixed-source.png, 05-mutation-matrix-m2-m3.png).
Flakiness gate log
rounds=5 files=1 skipped=0
file packages/core/src/memory/memoryDiscovery.test.ts: (cd packages/core) npx --no-install vitest run ./src/memory/memoryDiscovery.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/core/src/memory/memoryDiscovery.test.ts: PPPPP
verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/core/src/memory/memoryDiscovery.test.ts: P (exit 0)
round 2 · packages/core/src/memory/memoryDiscovery.test.ts: P (exit 0)
round 3 · packages/core/src/memory/memoryDiscovery.test.ts: P (exit 0)
round 4 · packages/core/src/memory/memoryDiscovery.test.ts: P (exit 0)
round 5 · packages/core/src/memory/memoryDiscovery.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.22.2. |
4 participants
What this PR does
Hierarchical memory discovery can reach the same physical context file through multiple lexical paths — most notably when a workspace-level
QWEN.mdis a symlink aliasing an ancestorQWEN.md. The existing dedup in discovery compares path strings only (Set<string>), so both aliases survive and the file is read, attached to the system prompt, and announced twice — and any relative@importthat resolves from both directories is attached twice as well. This PR dedupes the discovered candidates by canonical file identity (fs.realpath) before reading, keeping the first-discovered lexical path so ordering, display, and import resolution are unchanged. Paths whose identity cannot be resolved (broken symlink, unreadable file) fall back to their lexical path, preserving today's behavior for those entries.Why it's needed
Fixes #9597. Users who share one
QWEN.mdacross workspaces via a symlink (e.g. multi-workspace daemon setups where the ancestor scan stops at the workspace.gitboundary, so a symlink keeps the shared instructions available) currently get the same instructions injected twice: doubled token usage, doubled entries in/memory show//context/ context-usage reporting, and duplicated relative imports.Reviewer Test Plan
How to verify
packages/core/src/utils/memoryDiscovery.test.ts(symlink aliases of the same physical file (#9597)): on unfixed code the two repro tests fail (fileCount2 instead of 1 for parent file + child symlink;@importmarker attached twice), and all four pass with the fix. The other two tests are regression guards: distinct physical files with identical content must remain two blocks, and a symlink whose target lies outside the project-root scan boundary must still load.loadServerHierarchicalMemoryfrom the child cwd against the built package): before the fix —fileCount: 3,contextFilePathslists both../../QWEN.mdandQWEN.md, marker content attached twice; after the fix — single block, content attached once.npx vitest run src/utils/memoryDiscovery.test.ts src/utils/memoryImportProcessor.test.ts src/utils/rulesDiscovery.test.ts --root packages/core→ 125/125 pass. Coretypecheckandeslinton changed files clean; prettier applied.Evidence (Before & After)
N/A (not user-visible TUI; function-level before/after captured in "How to verify")
Tested on
Environment (optional)
Unit tests (vitest) plus a node script against the built
@qwen-code/qwen-code-coredist for the live repro. No real model / live TUI involved.Risk & Scope
fs.realpathper discovered context file (small N, session-start path). Dedup keeps the first-discovered lexical path — when both the ancestor and the symlink alias are discovered, the ancestor path is retained for display; the issue explicitly allows retaining the selected lexical path.memoryImportProcessorinternals (imports still dedupe within one file's import tree as before). The new tests create symlinks viafs.symlinklike existing core suites; the Windows CI lane is not currently running (see ci: the Windows lane is red — 59 tests across 10 files, unobserved since the merge queue stopped running #9481).Linked Issues
Fixes #9597
中文说明
这个 PR 做了什么
分层记忆（hierarchical memory）发现可能通过多个词法路径到达同一个物理上下文文件——最典型的情况是工作区级的
QWEN.md是指向祖先目录QWEN.md的符号链接。现有的去重只比较路径字符串（Set<string>），因此两个别名都会保留下来，该文件会被读取两次、附加到系统提示词两次、并在通知/展示中出现两次——任何能从两个目录解析到的相对@import也会被附加两次。本 PR 在读取前按规范文件身份（fs.realpath）对发现的候选去重，保留最先发现的词法路径，排序、展示和 import 解析均不变。无法解析身份的路径（断链符号链接、不可读文件）回退到其词法路径，保持这些条目与现状一致的行为。为什么需要
修复 #9597。通过符号链接在多个工作区共享同一份
QWEN.md的用户（例如多工作区 daemon 场景：祖先扫描在工作区.git边界处停止，需要靠符号链接保住共享指令）目前会收到两份相同的指令：token 用量翻倍、/memory show//context/ context-usage 报告中出现两条重复条目、相对 import 也被重复附加。Reviewer 测试计划
如何验证
packages/core/src/utils/memoryDiscovery.test.ts中新增单测（symlink aliases of the same physical file (#9597)）：在未修复代码上两条复现测试失败（父文件 + 子符号链接时fileCount为 2 而不是 1；@import标记被附加两次），修复后四条全部通过。另外两条是回归守护：内容相同的两个不同物理文件必须仍是两个块；目标位于项目根扫描边界之外的符号链接仍要能加载。loadServerHierarchicalMemory）：修复前fileCount: 3、contextFilePaths同时列出../../QWEN.md和QWEN.md、标记内容附加两次；修复后只有一个块、内容只附加一次。npx vitest run src/utils/memoryDiscovery.test.ts src/utils/memoryImportProcessor.test.ts src/utils/rulesDiscovery.test.ts --root packages/core→ 125/125 通过。coretypecheck、改动文件eslint干净，已跑 prettier。证据（修复前后）
N/A（非用户可见 TUI；函数级前后对比见「如何验证」）
测试环境
环境（可选）
vitest 单测 + 对构建后的
@qwen-code/qwen-code-coredist 跑 node 脚本做端到端复现。不涉及真实模型 / 实际 TUI。风险与范围
fs.realpath（数量很小，且只在会话启动路径）。去重保留最先发现的词法路径——当祖先路径和符号链接别名同时被发现时，展示保留祖先路径；issue 中已明确允许保留所选词法路径。memoryImportProcessor内部逻辑不变（import 仍在单个文件的导入树内去重）。新测试通过fs.symlink创建符号链接，与 core 现有测试一致；Windows CI 通道目前未运行（见 ci: the Windows lane is red — 59 tests across 10 files, unobserved since the merge queue stopped running #9481）。关联 Issue
Fixes #9597