Repository navigation
Conversation
E2E test reportVerified on macOS using synthetic data in isolated temporary source and native-memory directories. No private memories, preview output, captures, generated ledgers, or local test datasets are attached to this PR.
Build, typecheck, bundle, focused lint, and 778 tests across seven affected CLI test files passed. Windows and Linux were not tested locally. No real-user memory import was performed during verification. |
|
Thanks for the PR — this is a substantial, well-documented piece of work. Template looks good ✓ — all required sections present, Problem: this is a Direction: memory is a first-class subsystem here (native topic docs, indexes, forget/consolidation, secret scanning), and this reuses that machinery instead of inventing a parallel store — so it fits the architecture. Importing another agent's memory format is a plausible but fairly niche surface. CHANGELOG: no direct reference to cross-agent memory import; the area is relevant but this isn't following an established industry pattern. Size: core paths are touched —
The core touch itself is only 3 lines (command registration, mirroring Approach: my independent baseline before reading the diff was ~3 files (parser, import subcommand, at most one UI component). You landed 7 production files, and the split mostly earns its keep — Diff hygiene is clean: zero deletions, no drive-by refactors, no formatting churn, no unrelated edits. Test-to-production ratio is better than 1:1. Risk: no elevated risk signals — no match against the high-risk revert-correlated paths. On CI: at the commit I first reviewed, Moving on to code review. 🔍 中文说明感谢贡献——这是一份体量很大、文档也很扎实的工作。 模板完整 ✓——所有必填章节齐全, **问题:**这是 **方向:**记忆在这里是一等子系统(原生主题文档、索引、遗忘/整合、密钥扫描),本 PR 复用了这套机制而不是另建一套并行存储——所以与架构是契合的。导入另一个 agent 的记忆格式是合理但相对小众的能力。CHANGELOG:没有跨 agent 记忆导入的直接先例;该领域相关,但这不是在跟随已确立的行业惯例。 **规模:**确实触及核心路径—— 核心改动本身只有 3 行(命令注册,与 **方案:**在看 diff 之前,我自己的基线大约是 3 个文件(解析器、导入子命令、至多一个 UI 组件)。最终是 7 个生产文件,而这个拆分大体是值得的—— Diff 卫生良好:零删除、没有顺手重构、没有格式化噪声、没有无关改动。测试与生产代码比例优于 1:1。 **风险:**无升级风险信号——未命中与回滚相关的高风险路径。关于 CI:在我最初审查的那个 commit 上, 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewI read the whole diff. No critical blockers — no correctness bug, security hole, or regression that I can substantiate. What follows are non-blocking suggestions and one complexity observation. Two things I expected to be wrong and turned out to be right, worth recording because a skimming reviewer will trip on both: The interactive-import branch reads like dead code but isn't. The handler gates the TUI on The broader I also verified every core symbol this imports actually exists on The security posture is genuinely careful rather than decorative: Suggestions (non-blocking)
Reuse is good throughout: the UI leans on existing I checked whether the tests assert anything. They do — Files changed (17)
Testing evidenceThis is an unattended CI run, so I did not build or execute any PR code — the evidence below is the PR's own CI, read through the API for the current head The lint failure I reported against the previous head is gone. At I classified that as branch staleness rather than a PR-caused failure from the diff and the check identity, not from anything the log claimed: the failing step was the freshness gate itself, and neither file it named is touched by this PR. The rebase to Nothing is green yet on this commit, so there is no test result for me to cite. CI restarted from scratch when the new head landed:
The checks that have reported on this head are green or intentionally skipped: 4 successes and 3 skips out of 18 checks so far. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Sandboxed verification would settle the parts static review cannot, and the author has write access so both lanes are available: 中文说明
代码审查我完整读了 diff。没有关键阻塞项——没有我能证实的正确性缺陷、安全漏洞或回归。以下是非阻塞建议和一点复杂度观察。 有两处我本以为有问题、结果是对的,值得记录下来,因为略读的审查者会在这两处绊住: 交互式导入分支看起来像死代码,其实不是。 handler 用 更宽的 我还核实了这个 PR 导入的每一个 core 符号在 安全处理是真正细致的,不是装饰性的:每次读取源文件和台账都走 建议(非阻塞)
整体复用做得不错:UI 依赖既有的 我检查了测试是否真的有断言。确实有—— 测试证据这是无人值守的 CI 运行,所以我没有构建或执行任何 PR 代码——下面的证据来自 PR 自身的 CI,通过 API 读取当前 head 我在上一个 head 上报的 lint 失败已经消失。 在 这个 commit 上还没有任何东西是绿的,因此我没有测试结果可以引用。 新 head 落地后 CI 从头重跑:
该 head 上已出结果的检查都是绿色或有意跳过:目前 18 个检查中 4 个成功、3 个跳过。 沙箱化验证可以解决静态审查解决不了的部分,而且作者有写权限,两条通道都可用: — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 3/5 — the code review came back clean and I'd be comfortable with the engineering, but Stage 0 escalated this for maintainer awareness, so the decision isn't mine to make. This cap is policy on an otherwise-clean review, not doubt about the code. One process note first: the head moved to Stepping back from the line-by-line: this is careful work by someone who thought hard about failure modes. Zero deletions, no drive-by refactors, no formatting churn, a test suite that outsizes the production code and actually asserts the invariants it names, and a security posture that goes well past what the feature strictly needs — symlink-safe reads, containment checks on both the source and the destination, exclusive-create writes, a ledger that reserves before publishing so an interrupted import can't resurrect content the user deliberately forgot. The design docs are bilingual and genuinely synchronized, not a token translation. If I picked this up in six months I'd mostly thank the author. Going back to my independent proposal: I'd sketched ~3 files, and this is 7. The gap is almost entirely the interactive layer — 603 production lines plus 743 test lines for a three-view Ink review UI, on top of a The two things I'd want a human to decide, neither of which I can settle from the diff: Is this wanted? There's no linked issue and no user request behind it. The gap is real — Codex's cross-project handbook genuinely can't be dropped into Qwen's scoped memory without losing ownership — but "real gap" and "we should maintain this surface" are different questions. This is a migration path from one specific other agent's memory format, which is a support commitment that outlives the PR. That's a product-direction call, not a code-quality one. The author has write access and no independent human review has landed yet, so nobody outside the author has weighed in on whether this is a surface we want to own. The core-path touch plus the size. I also want to be explicit that I cannot currently attest to a green build. CI restarted from scratch on the new head: The one change I'd ask for before merge, and the only suggestion I'd treat as more than cosmetic: de-duplicate the path-containment predicate. I'm not requesting changes — I found no blocker, and the review is genuinely positive. I'm deferring because the escalation is policy, the demand signal is a human judgement call, and CI has not yet produced a green result to approve against. Once a maintainer has weighed the direction question and CI lands green on this head, I see nothing in the code standing in the way. 中文说明置信度:3/5 —— 代码审查结果是干净的,工程质量我认可,但 Stage 0 已将本 PR 提请维护者关注,所以这个决定不该由我来做。这个封顶是策略性的、针对一份其余部分都干净的审查,而不是对代码本身有疑虑。 先说一个流程情况:审查过程中 head 移动到了 从逐行审查中退一步看:这是作者认真思考过各种失败模式后的细致工作。零删除、没有顺手重构、没有格式化噪声,测试代码量超过生产代码且确实断言了它所声称的不变量,安全处理远超功能本身的最低要求——符号链接安全读取、对源和目标两侧都做包含关系检查、独占创建写入、台账"先预留后发布"(因此导入中断不会复活用户已刻意遗忘的内容)。设计文档中英双语且真正同步,不是应付性的翻译。如果六个月后接手这份代码,我大致会感谢作者。 回到我自己的独立方案:我当初设想约 3 个文件,而这里是 7 个。差距几乎全在交互层——三视图 Ink 审阅界面用了 603 行生产代码加 743 行测试,而 有两件事我希望由人来决定,都不是我能从 diff 里得出结论的: 这是不是被需要的? 没有关联 issue,背后也没有用户诉求。缺口是真实的——Codex 的跨项目手册确实无法在不丢失归属的情况下直接放进 Qwen 的按范围记忆——但"缺口真实"和"我们应该长期维护这个能力"是两个不同的问题。这是一条从某个特定其他 agent 记忆格式迁移的通道,而这是一份比 PR 本身更长久的维护承诺。这属于产品方向判断,不是代码质量判断。作者有写权限,而且目前还没有独立的人工审查,所以除作者之外还没有人对"我们是否要拥有这个能力"表过态。 核心路径改动加上体量。 我还要明确一点:我目前无法背书一个绿色的构建。 新 head 上 CI 从头重跑: 合入前我唯一会要求修改的、也是我唯一认为不只是表面问题的建议:去掉重复的路径包含判断。 我没有请求变更——我没有发现阻塞项,审查结果确实是正面的。我选择移交,原因是:升级属于策略规定、需求信号需要人来判断,而且 CI 尚未产出一个可供批准的绿色结果。一旦维护者权衡过方向问题、且 CI 在这个 head 上变绿,我认为代码层面没有任何东西会成为阻碍。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
⏸️ Deferring to a maintainer — not approving, not requesting changes. Needs a human call on this one. I could not resolve an accountable owner deterministically, so I'm not guessing a login:
The code review itself is positive — no blockers found, strong security posture, tests that assert real invariants, bilingual design docs genuinely synchronized. Full detail in the Stage 2 and Stage 3 comments above. Two things worth the author's attention meanwhile, neither blocking: the rebase onto 中文说明⏸️ 移交维护者处理——不批准,也不请求变更。 这件事需要人来判断。 我无法确定性地解析出负责人,因此不猜测登录名:
代码审查本身是正面的——没有发现阻塞项、安全处理扎实、测试断言了真实不变量、双语设计文档确实同步。完整细节见上方的 Stage 2 与 Stage 3 评论。 同时有两点值得作者关注,均不阻塞:rebase 到 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Update (2026-10-15): published 0b3e2bd without force, preserving the concurrent c8ae3c9 commit as its parent. The PR remains 19 code/documentation/synthetic-test files. No actual memories, previews, local fixtures, personal paths or raw reports are published. The Ubuntu failure on 23ba93a was two replay tests timing out during initial prompt settlement, before any replay assertion. Both failed with a synthetic 1500 ms model delay under the default 1 s preparation wait. Only those waits now use the existing 10 s convention; whole-test timeouts and all business assertions are unchanged. Full Hosted Harness: 461/461 pass, no unhandled errors. Wrong-watermark and wrong-recovery-code production mutations still fail. Earlier email admission and fresh-Session reconciliation fixes remain intact; neither removes its whole-test timeout. Blocking-review repairs cover native structured metadata, interrupted index recovery, actionable lock recovery and complete cleanup, conflicting project metadata, residual scope text, and Markdown structure. They include synthetic red/green probes, live-lock and actual SIGINT checks, and native validator/corpus checks. Privacy destination checks and source timestamps remain intentional; real uncapped recall disproved the claim that an omitted index entry can never be recalled. Each Critical thread has a specific disposition and evidence. After integrating the concurrent commit, all seven focused CLI files pass: 820/820. Full workspace build, bundle and full typecheck passed; final CLI build/bundle/typecheck, ESLint and Prettier passed after integration. Final bundled bare/--debug/--safe-mode dry-runs produced pure JSON with zero network attempts and no memory destination creation. Two clean final self-audits and a local medium quick review found no remaining actionable issue; this is not formal approval. New-head remote CI still requires confirmation. The user explicitly requires selective offline migration, main-project ownership, visible global fallback, default selection, JSON and a write-free dry-run. Native recall inside linked worktrees remains unchanged and is documented. Java/Desktop follow the repository daily/manual policy; their absence from the PR checks is not a passing result. The existing maintainer request remains in place; no duplicate review dispatch or ping was added. Remaining non-blocking Suggestions are deliberately deferred under the repository late-review scope rule, including CLI wrapper/help/EPIPE behavior, extra display sanitization, mapping diagnostics, source/ledger size and performance limits, additional mutation coverage, title collisions, preview event batching, truncation notice, and cleanup diagnostics. Their exact discussion records are retained below for follow-up. The two duplicate scope Suggestions are closed by the Critical fixes. This deferral does not assert these suggestions are implemented. Deferred discussions: comment 4277813890, comment 4277813893, comment 4277813898, comment 4277813904, comment 4277813913, comment 4277813918, comment 4277813924, comment 4277813928, comment 4277813933, comment 4277813940, comment 4277813947, comment 4277813953, comment 4277813958, comment 4277813965, comment 4277813967, comment 4277813971, comment 4277813979, comment 4277813983, comment 4277813987, comment 4277813993, comment 4277813997, comment 4277814003, comment 4277814007, comment 4277814012, comment 4277814015, comment 4277814019, comment 4277814023, comment 4277814026, comment 4277814029, comment 4277814034, comment 4277814038, comment 4277814044, comment 4277814061, comment 4277814064, comment 4277814067, comment 4277814072, comment 4277814077, comment 4277814084. |
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
activeWorkStaleMs |
5 |
8 |
— Qwen Code · serve A/B
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- docs/users/features/memory.md:285 user-doc wording for the in-terminal review UI — already reported (comment 6162939809)
- packages/cli/src/ui/components/StandaloneMemoryImportPreview.tsx:84 duplicated path-containment predicate shared with codex-import.ts — already reported (comment 6162939809)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; this path- and git-metadata-heavy code was exercised on Linux only.
Not explored to full depth (tool budget reached): chunk 8: could not execute the reproduction for finding 1 — packages/core/dist is absent in this review worktree, so neither the built dist nor vitest can resolve @…; "agent reverse-audit (round 1)": did not open MarkdownDisplay / mermaidVisualRenderer to confirm the doc's "Fenced content, including Mermaid, stays inert text; preview rendering must not inv…; "agent reverse-audit (round 2)": could not verify ink 7.0.3's render Instance type — node_modules is absent from both the PR worktree and the main checkout, so instance?.cleanup() at Sta…; "agent reverse-audit (round 2)": could not attribute the failing "Lint & Static (ubuntu-latest, Node 22.x)" lane beyond establishing that it is .github/scripts/check-lint-gate-freshness.mjs r…; "agent reverse-audit (round 2)": user-doc claim "Backtick-quote paths containing scope delimiters such as commas or parentheses" not traced into codex-parser.ts 's annotation handling., and 2 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查(原文为英文):build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未审查(原文为英文):build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI; this path- and git-metadata-heavy code was exercised on Linux only.
未探索到全部深度(达到工具调用预算):chunk 8:could not execute the reproduction for finding 1 — packages/core/dist is absent in this review worktree, so neither the built dist nor vitest can resolve @…;"agent reverse-audit (round 1)":did not open MarkdownDisplay / mermaidVisualRenderer to confirm the doc's "Fenced content, including Mermaid, stays inert text; preview rendering must not inv…;"agent reverse-audit (round 2)":could not verify ink 7.0.3's render Instance type — node_modules is absent from both the PR worktree and the main checkout, so instance?.cleanup() at Sta…;"agent reverse-audit (round 2)":could not attribute the failing "Lint & Static (ubuntu-latest, Node 22.x)" lane beyond establishing that it is .github/scripts/check-lint-gate-freshness.mjs r…;"agent reverse-audit (round 2)":user-doc claim "Backtick-quote paths containing scope delimiters such as commas or parentheses" not traced into codex-parser.ts 's annotation handling.,另有 2 条。
未审查:反向审计——评审时间预算不足,未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.25.1)
|
⏳ Historical-head review — head moved to 中文说明⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 |
Superseded at 0b3e2bd: all 9 Critical findings have per-thread fixes or evidence-backed dispositions; all threads are resolved. Native metadata and index/lock recovery pass real synthetic red/green probes; Markdown/scope repairs pass 820 focused CLI tests. Checkout privacy and native timestamp/index policy are intentionally retained; uncapped recall was directly verified. Remaining Suggestions are explicitly deferred in comment 6164307830 under the late-review scope rule. No maintainer rejection is being dismissed; normal maintainer approval and CI remain required.
|
Superseded by #14281 at the author’s request. The replacement implements the redesigned shared CLI/HTTP memory import service: explicit preview and selection, scoped background consolidation, durable recovery and native recall. It starts from current main without carrying unrelated CI fixes from this PR. Further review and CI follow-up will continue on #14281. |
What this PR does
Adds an offline
qwen memory import codexworkflow that converts reviewed Codex handbook and summary entries into Qwen's private native memory. Interactive review groups entries by destination, provides list/detail and full-text views, selects eligible entries by default, and supports all/none and group selection. Selection counts appear before project paths so long labels cannot hide them. Import requires a separate confirmation;--dry-runalways prevents memory writes, and--jsonand explicit ID selection remain available for scripts.Project entries resolve to the verified main checkout, including when their source refers to a linked worktree. Exact project mappings take precedence. Entries without a reliable project destination become clearly labeled global fallback candidates. The importer checks exact duplicates and conflicts, preserves existing documents, records handled entries to avoid restoring forgotten memories, and requires renewed review if the source changes before confirmation.
Why it's needed
Codex's cross-project handbook cannot be copied directly into Qwen's scoped native memory without losing ownership and importing unrelated history. This provides a selective migration path with readable previews and explicit destinations, while retaining offline operation and the existing native memory format. Transcripts, skills, and raw rollout summaries are excluded.
Reviewer Test Plan
How to verify
Evidence (Before & After)
Before: there is no offline selective Codex-to-native-memory import command or scoped review flow. After: synthetic terminal tests exercised grouped review, default selection, explicit confirmation, duplicate handling, source-change rejection, and read-only preview paths. Long project labels retain their selection counts when truncated. No private preview captures or local runtime fixtures are attached.
Validation: build, typecheck, bundle, focused lint, and 778 tests across the seven affected CLI test files passed. Synthetic PTY and headless scenarios verified the import workflow and long-label rendering; network-denial instrumentation recorded no network attempts. A separate comment summarizes the E2E results.
Tested on
Environment (optional)
Node.js 22+, isolated synthetic source/runtime directories, and real PTYs. Read-only scenarios also ran with Node filesystem write permission disabled.
Risk & Scope
Design: English · 简体中文. Both versions cover the same decisions, constraints, and acceptance criteria.
Linked Issues
None.
中文说明
本 PR 的改动
新增离线
qwen memory import codex,将审阅后的 Codex 手册和摘要条目转换为 Qwen 私有原生记忆。交互审阅按目标分组,支持列表、详情和全文查看,默认选中可导入条目,并支持全选、清空和整组选取。选择数量显示在项目路径之前,避免被长路径截断。导入需要单独确认;--dry-run始终禁止写入记忆,保留--json和显式 ID 选择供脚本使用。项目条目归到验证后的主 checkout,包括来源指向 linked worktree 的情况;精确项目映射优先。无法可靠确定项目目标的条目成为明确标注的全局回退候选。导入器检查精确重复和冲突、保留已有文档,并记录已处理条目,避免恢复已经遗忘的记忆;确认前来源变化则要求重新审阅。
为什么需要
直接把 Codex 跨项目手册复制到 Qwen 原生记忆,会丢失项目归属并引入无关历史。本功能提供有清晰预览和明确目标的选择性迁移,保持离线运行并复用现有原生记忆格式。不导入会话、技能或原始 rollout 摘要。
评审验证
前后对比与验证证据
改动前没有离线选择性导入 Codex 原生记忆的命令或按范围审阅流程。改动后,合成终端测试覆盖分组审阅、默认选择、明确确认、去重、来源变化拒绝和只读预览。长项目标签被截断时仍显示选择数量。不附带私有预览截图或本地运行测试夹具。
构建、类型检查、打包、定向 lint 及七个相关 CLI 测试文件的 778 项测试通过。合成 PTY 和非交互场景验证导入流程及长标签显示;禁网计数未记录网络尝试。另附独立 E2E 汇总评论。
本地测试平台为 macOS,覆盖单测、构建、类型检查和合成终端测试;Windows 和 Linux 未在本地测试。环境为 Node.js 22+、隔离的合成来源与运行目录及真实 PTY;只读场景同时禁用了 Node 文件系统写权限。
风险与范围
设计文档:英文 · 中文,两版设计决策、约束和验收标准保持一致。无关联 Issue。