Skip to content

feat(cli): add selective offline Codex memory import - #14149

Closed
callmeYe wants to merge 11 commits into
mainfrom
codex/codex-memory-import
Closed

callmeYe wants to merge 11 commits into
mainfrom
codex/codex-memory-import

Conversation

@callmeYe

Copy link
Copy Markdown
Collaborator

What this PR does

Adds an offline qwen memory import codex workflow 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-run always prevents memory writes, and --json and 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

  • Use a temporary synthetic Codex summary/handbook with user preferences, project entries, duplicates, and an ambiguous project scope. Confirm project grouping, main-checkout normalization, visible global fallback reasons, and front-loaded selection counts for long paths.
  • In interactive review, confirm eligible entries start selected, group and individual toggles preserve selection, and conflicts, blocked entries, and previously handled entries cannot be selected. Confirm Import writes only the selected eligible entries to private temporary destinations and reports concise counts.
  • Confirm dry-run, cancellation, JSON preview, and non-interactive invocation without IDs leave native memory unchanged. Confirm source-note review is explicit and changing the source before confirmation rejects the import before creating destination directories or locks.
  • Repeat a synthetic import and confirm duplicates are not written twice, existing memories are not overwritten, and previously handled entries stay skipped.

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

OS Status
🍏 macOS ✅ Unit, build/typecheck, and synthetic terminal tests
🪟 Windows ⚠️ Not tested locally
🐧 Linux ⚠️ Not tested locally

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

  • Main risk or tradeoff: global fallback broadens a selected entry's scope and is explicitly identified in review and confirmation. Duplicate/conflict checks compare normalized text and identity; they do not infer semantic contradictions.
  • Not validated / out of scope: continuous synchronization, external memory providers, model-assisted consolidation, skill migration, and changing Qwen's normal per-worktree recall.
  • Breaking changes / migration notes: additive command only. Existing topic documents are not overwritten; the private import ledger preserves handled/forgotten state. Only code, synthetic unit tests, and documentation are committed; no personal memory, preview output, local test artifacts, or generated import records are included.

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 摘要。

评审验证

  • 使用临时合成摘要和手册,包含用户偏好、项目条目、重复内容和模糊项目范围。确认项目分组、主 checkout 归一、全局回退原因,以及长路径前可见的选择数量。
  • 确认交互界面默认选中可导入条目,整组和单条切换保留选择状态;冲突、被阻止和已处理条目不能选中。确认导入只向临时私有目标写入选中的可导入条目,并输出简短数量结果。
  • 确认 dry-run、取消、JSON 预览和未指定 ID 的非交互调用不修改原生记忆。确认来源说明需要明确审阅,确认前来源变化会在创建目标目录和锁之前拒绝导入。
  • 重复执行合成导入,确认不重复写入、不覆盖已有记忆,已处理条目继续跳过。

前后对比与验证证据

改动前没有离线选择性导入 Codex 原生记忆的命令或按范围审阅流程。改动后,合成终端测试覆盖分组审阅、默认选择、明确确认、去重、来源变化拒绝和只读预览。长项目标签被截断时仍显示选择数量。不附带私有预览截图或本地运行测试夹具。

构建、类型检查、打包、定向 lint 及七个相关 CLI 测试文件的 778 项测试通过。合成 PTY 和非交互场景验证导入流程及长标签显示;禁网计数未记录网络尝试。另附独立 E2E 汇总评论。

本地测试平台为 macOS,覆盖单测、构建、类型检查和合成终端测试;Windows 和 Linux 未在本地测试。环境为 Node.js 22+、隔离的合成来源与运行目录及真实 PTY;只读场景同时禁用了 Node 文件系统写权限。

风险与范围

  • 全局回退扩大被选中条目的生效范围,因此在审阅及确认时明确标注。去重和冲突检查基于规范化文本及身份,不判断语义矛盾。
  • 不涵盖持续同步、外挂记忆提供方、模型辅助整合、技能迁移,也不改变 Qwen 日常按 worktree 召回记忆的行为。
  • 这是新增命令,不覆盖已有主题文档;私有导入台账保留已处理和已遗忘状态。提交仅包含代码、合成单元测试及文档,不含个人记忆、预览输出、本地测试产物或生成的导入记录。

设计文档:英文 · 中文,两版设计决策、约束和验收标准保持一致。无关联 Issue。

@callmeYe

Copy link
Copy Markdown
Collaborator Author

E2E test report

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

  • Interactive review: default eligible selection, select all/clear all, group and individual toggles, ineligible entries, details, confirmation, Back, and cancellation passed.
  • Synthetic application: selected entries reached the expected user/project destinations; duplicates were handled without extra topic files, and native indexes and import records matched the selected set. Repeat import skipped already handled entries.
  • Preview safety: dry-run, cancellation, JSON, and non-TTY preview paths made no memory changes. Filesystem write-denied preview checks passed; network instrumentation recorded no network attempts.
  • Review consistency: changing source content or adding, editing, or removing supplemental notes after preview rejected the import before destination directories or locks were created.
  • Terminal rendering: confirmation remained usable at 32×12; at 80×24, a long project path retained its selection count at the beginning of the row before and after toggling.
  • Full-text review: synthetic snapshot access, restrictive permissions, cleanup, and recoverable editor failures were covered in the earlier preview verification.

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.

@qwen-code-review-bot

qwen-code-review-bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

✅ Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-review-bot

qwen-code-review-bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Thanks for the PR — this is a substantial, well-documented piece of work.

Template looks good ✓ — all required sections present, Tested on filled in, and both design docs are linked. I checked the EN/ZH pair rather than assuming: identical heading structure at identical line positions across all 11 sections, so they are genuinely synchronized and not a stub translation.

Problem: this is a feat, not a fix, so a before/after reproduction isn't the right bar. The gap itself is real and concrete — Codex's cross-project handbook can't be dropped into Qwen's scoped memory without losing project ownership or dragging in unrelated history. What's thin is the demand signal: there's no linked issue and no user request behind it. Worth saying plainly, since that's a product-direction question rather than a code question.

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 — packages/cli/src/config/config.ts matches packages/*/src/config/**. Breakdown of the +4,687 / −0:

Bucket Lines Files
Production logic 1,920 7
Tests 2,514 7
Docs 253 3
Generated/schema 0 0

The core touch itself is only 3 lines (command registration, mirroring mcpCommand exactly), but the rule is path-based and production lines are well over 500, so flagging for maintainer awareness. 1,920 production lines also crosses the 1,000-line large-PR advisory — informational only, not a block.

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 — StandaloneMemoryImportPreview is a thin Ink host around MemoryImportPreview, not a duplicated component tree, which was my first suspicion. The honest 80/20 question is the interactive layer: MemoryImportPreview.tsx + its host are 603 production lines and 743 test lines, roughly a third of the PR, on top of a --dry-run / --json / --select path that already scripts the whole flow. Would preview-then---select have covered the migration use case for a first version, with the TUI as a follow-up? Not a blocker — just the thing I'd want a maintainer to weigh before this lands as one unit.

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, Lint & Static (ubuntu-latest, Node 22.x) was red, failing in check-lint-gate-freshness.mjs because the branch hadn't incorporated main's lint-gate changes. That has since been addressed — the branch was rebased onto main mid-review (82 commits, now at c716e8a8), and I confirmed the PR's own diff is byte-for-byte unchanged by it, so this gate assessment still applies as written. Lint and the test matrix are now running for the first time against this code; results are tracked in the Stage 2 comment. Separately, qwen memory … is routed through resolveBootstrapRoute via firstPositional, which is broader than the firstArg === 'serve' / 'mcp' checks it sits next to and affects every invocation — I looked hard at that in Stage 2 and the added tests do pin the escape cases.

Moving on to code review. 🔍

中文说明

感谢贡献——这是一份体量很大、文档也很扎实的工作。

模板完整 ✓——所有必填章节齐全,Tested on 已填写,两份设计文档也都附了链接。我没有想当然,而是实际比对了中英文两版:11 个章节的标题结构和行位置完全一致,确实是同步的,不是占位翻译。

**问题:**这是 feat 而非 fix,所以 before/after 复现不是合适的门槛。缺口本身是真实且具体的——Codex 的跨项目手册无法直接放进 Qwen 的按范围记忆,否则会丢失项目归属或带入无关历史。比较薄弱的是需求信号:没有关联 issue,也没有用户诉求支撑。这一点值得直说,因为它属于产品方向问题,而不是代码问题。

**方向:**记忆在这里是一等子系统(原生主题文档、索引、遗忘/整合、密钥扫描),本 PR 复用了这套机制而不是另建一套并行存储——所以与架构是契合的。导入另一个 agent 的记忆格式是合理但相对小众的能力。CHANGELOG:没有跨 agent 记忆导入的直接先例;该领域相关,但这不是在跟随已确立的行业惯例。

**规模:**确实触及核心路径——packages/cli/src/config/config.ts 命中 packages/*/src/config/**。+4,687 / −0 的构成如下:生产逻辑 1,920 行(7 个文件)、测试 2,514 行(7 个文件)、文档 253 行(3 个文件)、生成/schema 0 行。

核心改动本身只有 3 行(命令注册,与 mcpCommand 完全一致),但规则是按路径判定的,且生产行数远超 500,因此提请维护者关注。1,920 行生产代码也越过了 1,000 行大 PR 提示线——仅供参考,不构成阻塞。

**方案:**在看 diff 之前,我自己的基线大约是 3 个文件(解析器、导入子命令、至多一个 UI 组件)。最终是 7 个生产文件,而这个拆分大体是值得的——StandaloneMemoryImportPreview 是围绕 MemoryImportPreview 的一层很薄的 Ink 宿主,并不是重复的组件树,而这正是我最初的疑虑。诚实的 80/20 问题在交互层:MemoryImportPreview.tsx 加宿主共 603 行生产代码、743 行测试,约占整个 PR 的三分之一,而 --dry-run / --json / --select 这条路径其实已经能脚本化地走完整流程。对第一版来说,"预览 + --select" 是否已经能覆盖迁移场景,而把 TUI 留作后续?这不是阻塞项——只是在整体合入之前,我希望维护者先权衡的一点。

Diff 卫生良好:零删除、没有顺手重构、没有格式化噪声、没有无关改动。测试与生产代码比例优于 1:1。

**风险:**无升级风险信号——未命中与回滚相关的高风险路径。关于 CI:在我最初审查的那个 commit 上,Lint & Static (ubuntu-latest, Node 22.x) 是红的,失败在 check-lint-gate-freshness.mjs,原因是分支尚未合入 main 上的 lint gate 变更。这一点已经解决——审查过程中分支被 rebase 到了 main(82 个 commit,现在是 c716e8a8),而我确认了 PR 自身的 diff 逐字节未变,因此本阶段的评估依然成立。lint 和测试矩阵现在才第一次针对这份代码运行;结果记录在 Stage 2 评论中。另外,qwen memory … 是通过 resolveBootstrapRoute 里的 firstPositional 路由的,这比它旁边的 firstArg === 'serve' / 'mcp' 判断更宽,且影响每一次调用——我在 Stage 2 重点看了这块,新增测试确实钉住了那些逃逸情形。

进入代码审查 🔍

— Qwen Code · qwen3.8-max-2026-09-02

Reviewed at c716e8a85443586fc8f95db242564c139b14d51b · re-run with @qwen-code /triage

@qwen-code-review-bot

qwen-code-review-bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Re-anchored to c716e8a8. The head moved while this review was running — the branch picked up 82 commits from main. I checked whether that invalidated the review rather than assuming: the PR's own diff is byte-for-byte identical to what I reviewed at 888269d4 (same 17 files, same +4,687/−0, same per-file counts). The only change anywhere in the diff is two hunk offsets in config.ts (@@ -873 → @@ -883, @@ -930 → @@ -940), because main grew by 10 lines above them. So the findings below stand unchanged; only the CI evidence and the commit footer moved.

Code review

I 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 preview.dryRun, then gates the actual write on !args.dryRun — which looks unreachable. It isn't, because plan() sets dryRun: Boolean(options.dryRun || !options.selectedIds?.length). Running with no --select gives preview.dryRun === true while args.dryRun === false, so the confirm-and-write path is live and --dry-run still forces it off. Correct, but the reachability rests on a coupling between two files that nothing states. This is the case AGENTS.md means by "the why cannot be conveyed through naming" — a one-line comment would save the next reader the ten minutes I spent on it.

The broader firstPositional route is actually pinned. resolveBootstrapRoute matches memory at any argv position, unlike the firstArg === 'serve' / 'mcp' checks beside it, and it sits on the startup path of every invocation. The added tests cover exactly the escape surfaces I went looking for: ['--prompt','memory'] → default, ['--','memory'] → default, ['explain','memory'] → default, and ['--model','test-model','memory','--help'] → memory. The version intercept still fires first. Good.

I also verified every core symbol this imports actually exists on main with a matching signature — the PR adds zero lines to packages/core, so all of it had to already be there: getAutoMemoryRoot, getUserAutoMemoryRoot, scanAllAutoMemoryTopicDocuments, scanAllUserAutoMemoryTopicDocuments, ScannedAutoMemoryDocument, buildManagedAutoMemoryIndex, scanForSecrets, atomicWriteFile, openNoFollow, realpathNearestExisting, and the three Storage runtime-dir methods. All present. scanAllAutoMemoryTopicDocuments(project, undefined, false) correctly passes trustedProject = false. The subpath imports (…-core/memory/paths.js etc.) follow the house rule of importing from the defining module rather than the package root, matching existing precedent in summaryCommand.ts and MemoryDialog.tsx.

The security posture is genuinely careful rather than decorative: openNoFollow on every source and ledger read, symlink/containment checks, destinations rejected inside the source, project dirs, or any Git checkout, 0700/0600 modes, exclusive wx create, an advisory lock, ledger reserve-before-publish so an interruption can't resurrect forgotten content, secret redaction before display, and control-character escaping on all untrusted text reaching stdout.

Suggestions (non-blocking)

  • The path-containment predicate is implemented twice. within() in codex-import.ts and the inline protectedRoots.some(...) relative-path check in StandaloneMemoryImportPreview.tsx are the same non-trivial logic in two changed files. This one is worth extracting, because it is a security boundary — it is what keeps o-action review snapshots out of the source tree, native memory roots, and Git checkouts. Two copies of a containment check drift, and drift here leaks private memory text into a repository.
  • PREVIEW_SETTINGS defeats the type checker. { merged: { ui: {} } } as unknown as LoadedSettings is a double cast. I agree with not calling loadSettings here, but if LoadedSettings ever gains a field this component reads, it resolves to undefined at runtime with no compile error. A narrower typed stub would keep the guarantee the design doc is asking for.
  • User docs say "browser" for a terminal UI. docs/users/features/memory.md: "previews open a browser with three views" and "These modes do not open the browser." This is an Ink TUI — the wording will confuse exactly the users the page is for.
  • resolveProject is the maintenance hotspot. ~120 lines of dense regex heuristics parsing natural-language scope annotations (cwd=/work/atlas or atlas worktree on release/example and friends). I want to be fair here: the complexity is inherent to reading someone else's prose, it fails safe in the right direction (unresolvable → labelled global fallback with a reason, never a silent write to the wrong project), and it carries 483 lines of tests. So not a violation — but it is the file a future maintainer will curse, and it is the part I'd most want a second pair of eyes on.

Reuse is good throughout: the UI leans on existing RadioButtonSelect, MarkdownDisplay, useKeypress, useTerminalSize, and useLaunchEditor instead of building new primitives, and StandaloneMemoryImportPreview is a thin host rather than a parallel component tree.

I checked whether the tests assert anything. They do — previews by default and with selected IDs without creating destination files asserts the runtime dir is empty and re-reads the source file to confirm it is byte-identical, and refuses symlinked source and ledger files actually creates the symlinks and expects rejection rather than mocking. The 23 importer cases name real invariants (index-cap-spanning duplicates, no resurrection after forget, concurrent-lock refusal, credential blocking without echo).

Files changed (17)
File What changed
docs/design/codex-memory-import.md English design doc: behavior, ownership rules, dedup/ledger semantics, acceptance criteria
docs/design/codex-memory-import.zh-CN.md Chinese design doc — verified structurally identical to the English one, not a stub
docs/users/features/memory.md New user-facing section: command usage, key bindings, status table, project resolution, ledger warnings
packages/cli/src/cli.ts Adds the memory bootstrap route and fast-path dispatch, plus a help entry
packages/cli/src/cli.test.ts Pins route resolution edge cases and the offline dispatch that avoids loading the full CLI graph
packages/cli/src/commands/memory.ts New yargs command tree, flag definitions, plaintext renderer, output sanitizing, TTY handoff
packages/cli/src/commands/memory.test.ts Command-level tests including exit-code propagation
packages/cli/src/commands/memory/codex-import.ts Planner and writer: destination resolution, Git main-checkout normalization, dedup/conflict, ledger, locking
packages/cli/src/commands/memory/codex-import.test.ts 23 cases over the invariants above, using real temporary Git repositories
packages/cli/src/commands/memory/codex-parser.ts Markdown section/entry extraction, scope annotation parsing, ID and date derivation
packages/cli/src/commands/memory/codex-parser.test.ts Parser cases including adversarial and ambiguous scope forms
packages/cli/src/config/config.ts Three lines registering memoryCommand and its exit-list entry, mirroring mcpCommand
packages/cli/src/config/config.test.ts Exit-status propagation test for memory commands
packages/cli/src/ui/components/MemoryImportPreview.tsx Presentational three-view review component with parent-owned selection state
packages/cli/src/ui/components/MemoryImportPreview.test.tsx Component tests for navigation, selection, and confirmation
packages/cli/src/ui/components/StandaloneMemoryImportPreview.tsx Ink render host: providers, temp review-file creation, raw-mode and cleanup
packages/cli/src/ui/components/StandaloneMemoryImportPreview.test.tsx Host tests for artifact permissions, location rejection, and cleanup

Testing evidence

This 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 c716e8a85443586fc8f95db242564c139b14d51b. I am not re-running anything the author claimed; their "778 tests passed" line is the author's claim, not evidence I verified.

The lint failure I reported against the previous head is gone. At 888269d4, Lint & Static (ubuntu-latest, Node 22.x) was red — it failed in check-lint-gate-freshness.mjs before ESLint ran, because the branch hadn't incorporated main's changes to eslint.legacy-filenames.mjs and .github/workflows/ci.yml:

The lint gate changed on 'main' after this branch last incorporated it:

  - eslint.legacy-filenames.mjs: 2eee9b7186be fix(cli): recognize markdown fences only on matching lines (#13331) (2026-10-13)
  - .github/workflows/ci.yml: df5470e43095 feat(web-shell): Add workspace file browser and previews (#13878) (2026-10-13)

This lane checks out the branch head alone, so its green proves the branch
passes the gate AS THE BRANCH DEFINES IT — with the files above, that gate
is stale. Merge or rebase 'main' into this branch and push to
re-validate under the current gate.
##[error]Process completed with exit code 1.

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 c716e8a8 addressed it — that check is now in_progress on the new head instead of failed, so ESLint is finally executing against these 1,920 production lines for the first time.

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: Lint & Static and Test (ubuntu-latest, Node 22.x) are both in progress, Integration Tests (no-AK, No Sandbox) is in progress, and Test (macos-latest) / Test (windows-latest) are skipped pending the ubuntu leg. 4 PR workflow runs are incomplete. I did not poll or sleep-wait on any of them — per the gate's rules a poll loop burns runner minutes and still gives up before a ~30-minute suite finishes, so I fetched once and am reporting what exists.

  • Not verified: lint. In progress at the time of writing; it has never completed against this code on either head.
  • Not verified: unit tests, typecheck, build, integration tests. Not yet reported on this commit.
  • My Stage 2 house-style observations (filenames comply — kebab-case.ts for new .ts, PascalCase.tsx for components; consistent-type and subpath-scoped imports; no any in what I read; collocated tests) are my own reading of the diff, not a tool result.

The checks that have reported on this head are green or intentionally skipped: 4 successes and 3 skips out of 18 checks so far. verify and tmux-testing have not been triggered by anyone.

Final CI results for c716e8a (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Hosted process fault gates / MySQL 8.4 / Java 21 🚫 cancelled
Runtime Broker and Managed Agent MariaDB / Java 21 🚫 cancelled
Test (ubuntu-latest, Node 22.x) ❌ failure
web-shell E2E Smoke (ubuntu-latest, Node 22.x) 🚫 cancelled
arm64 ✅ success
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
EventTransport broker gates / RocketMQ 5.5.1 / Java 21 ✅ success
Flyway migration version uniqueness ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Lint & Static (ubuntu-latest, Node 22.x) ✅ success
macos-latest / Java 21 ✅ success
Native Linux CLI boundary ✅ success
Native Windows settings and streams ✅ success
OpenTUI no-flicker gate ✅ success
Real daemon E2E / Java 11 ✅ success
TUI parity snapshots (ink vs opentui) ✅ success
ubuntu-latest / Java 11 ✅ success
ubuntu-latest / Java 17 ✅ success
ubuntu-latest / Java 21 ✅ success
windows-latest / Java 21 ✅ success
x64 ✅ success

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: @qwen-code /tmux — that the interactive flow really writes only the selected eligible entries, that --dry-run leaves no directory, ledger, or lock on disk in a real PTY, and that selection counts stay visible when a long project path truncates, which is a visual claim no unit test can pin. @qwen-code /verify — that the destination-safety and no-resurrection invariants are load-bearing, i.e. that the suite actually fails when the containment check or the reserve-before-publish ledger order is removed, rather than passing either way. The unit tests are well-named and read as substantive, but a green suite proves the tests pass, not that they would catch a regression in these specific guards.

中文说明

已重新锚定到 c716e8a8。 审查过程中 head 发生了移动——分支合入了 main 的 82 个 commit。我没有想当然,而是核实了这是否会让审查失效:PR 自身的 diff 与我在 888269d4 审查的内容逐字节相同(同样 17 个文件、同样 +4,687/−0、每个文件的增删数完全一致)。整个 diff 中唯一的变化是 config.ts 里两处 hunk 偏移(@@ -873 → @@ -883、@@ -930 → @@ -940),原因是 main 在其上方增加了 10 行。因此以下结论保持不变;只有 CI 证据和 commit 页脚发生了变化。

代码审查

我完整读了 diff。没有关键阻塞项——没有我能证实的正确性缺陷、安全漏洞或回归。以下是非阻塞建议和一点复杂度观察。

有两处我本以为有问题、结果是对的,值得记录下来,因为略读的审查者会在这两处绊住:

交互式导入分支看起来像死代码,其实不是。 handler 用 preview.dryRun 作为进入 TUI 的条件,又用 !args.dryRun 作为真正写入的条件——看着不可达。实际可达,因为 plan() 里是 dryRun: Boolean(options.dryRun || !options.selectedIds?.length)。不带 --select 运行时 preview.dryRun === true 而 args.dryRun === false,所以"确认并写入"这条路径是活的,而 --dry-run 仍然能强制关闭它。逻辑正确,但这个可达性依赖两个文件之间一处没人说明的耦合。这正是 AGENTS.md 所说的"why 无法通过命名表达"的情形——加一行注释能给下一位读者省下我花掉的十分钟。

更宽的 firstPositional 路由其实已被测试钉住。 resolveBootstrapRoute 在 argv 任意位置匹配 memory,与旁边的 firstArg === 'serve' / 'mcp' 不同,而它位于每次调用的启动路径上。新增测试恰好覆盖了我主动去找的逃逸面:['--prompt','memory'] → default、['--','memory'] → default、['explain','memory'] → default,以及 ['--model','test-model','memory','--help'] → memory。版本拦截仍然先触发。没问题。

我还核实了这个 PR 导入的每一个 core 符号在 main 上确实存在且签名匹配——本 PR 对 packages/core 是零改动,所以这些必须本来就在:getAutoMemoryRoot、getUserAutoMemoryRoot、scanAllAutoMemoryTopicDocuments、scanAllUserAutoMemoryTopicDocuments、ScannedAutoMemoryDocument、buildManagedAutoMemoryIndex、scanForSecrets、atomicWriteFile、openNoFollow、realpathNearestExisting,以及三个 Storage 运行时目录方法。全部存在。scanAllAutoMemoryTopicDocuments(project, undefined, false) 正确传入 trustedProject = false。子路径导入(…-core/memory/paths.js 等)遵循了"从定义模块导入而非包根"的项目规则,与 summaryCommand.ts、MemoryDialog.tsx 的既有写法一致。

安全处理是真正细致的,不是装饰性的:每次读取源文件和台账都走 openNoFollow、符号链接与包含关系检查、目标目录若落在源目录/项目目录/任何 Git checkout 内一律拒绝、0700/0600 权限、wx 独占创建、advisory lock、台账"先预留后发布"(因此中断不会复活已遗忘内容)、展示前做密钥脱敏,以及所有进入 stdout 的不可信文本都做了控制字符转义。

建议(非阻塞)

  • 路径包含判断实现了两遍。 codex-import.ts 里的 within() 和 StandaloneMemoryImportPreview.tsx 里内联的 protectedRoots.some(...) 相对路径检查,是同一段非平凡逻辑出现在两个改动文件中。这一条值得抽取,因为它是安全边界——正是它保证 o 操作生成的审阅快照不会落进源码树、原生记忆目录和 Git checkout。包含检查的两份副本会漂移,而这里漂移的后果是把私有记忆文本泄进仓库。
  • PREVIEW_SETTINGS 绕过了类型检查。 { merged: { ui: {} } } as unknown as LoadedSettings 是双重断言。我同意这里不该调用 loadSettings,但如果 LoadedSettings 将来新增一个本组件会读取的字段,运行时它会静默变成 undefined,编译期没有任何报错。更窄的类型化 stub 才能保住设计文档想要的那个保证。
  • 用户文档把终端 UI 写成了"浏览器"。 docs/users/features/memory.md 里的 "previews open a browser with three views" 和 "These modes do not open the browser"。这是 Ink 终端界面——这个措辞恰好会误导这页文档的目标读者。
  • resolveProject 是维护热点。 约 120 行密集的正则启发式,用来解析自然语言作用域标注(cwd=/work/atlas or atlas worktree on release/example 之类)。这里我想说得公道些:复杂度源自要读别人写的散文,它的失败方向是安全的(无法解析 → 带原因标注的全局回退,绝不会静默写到错误项目),并且配了 483 行测试。所以不算违规——但它确实是未来维护者会抱怨的那个文件,也是我最希望有第二双眼睛看一遍的部分。

整体复用做得不错:UI 依赖既有的 RadioButtonSelect、MarkdownDisplay、useKeypress、useTerminalSize、useLaunchEditor,没有另造原语;StandaloneMemoryImportPreview 是一层薄宿主,而不是平行的组件树。

我检查了测试是否真的有断言。确实有——previews by default and with selected IDs without creating destination files 既断言运行时目录为空,又重新读取源文件确认逐字节一致;refuses symlinked source and ledger files 真的创建了符号链接并期待抛错,而不是 mock。23 个导入器用例都指向真实不变量(跨越索引上限的去重、遗忘后不复活、并发锁拒绝、密钥拦截且不回显)。

测试证据

这是无人值守的 CI 运行,所以我没有构建或执行任何 PR 代码——下面的证据来自 PR 自身的 CI,通过 API 读取当前 head c716e8a85443586fc8f95db242564c139b14d51b 的结果。我没有复跑作者声称的任何内容;他们的"778 项测试通过"是作者的说法,不是我核实的证据。

我在上一个 head 上报的 lint 失败已经消失。 在 888269d4 上,Lint & Static (ubuntu-latest, Node 22.x) 是红的——它失败在 check-lint-gate-freshness.mjs、ESLint 运行之前,原因是分支尚未合入 main 对 eslint.legacy-filenames.mjs 和 .github/workflows/ci.yml 的改动。我把它判定为分支陈旧而非 PR 引入的失败,依据是 diff 和检查本身的身份,而不是日志里的任何声称:失败的步骤正是新鲜度门禁自己,而它点名的两个文件本 PR 都没有改动。rebase 到 c716e8a8 解决了这个问题——该检查在新 head 上是 in_progress 而非失败,所以 ESLint 终于第一次真正对这 1,920 行生产代码执行。

这个 commit 上还没有任何东西是绿的,因此我没有测试结果可以引用。 新 head 落地后 CI 从头重跑:Lint & Static 和 Test (ubuntu-latest, Node 22.x) 都在进行中,Integration Tests (no-AK, No Sandbox) 在进行中,而 Test (macos-latest) / Test (windows-latest) 在 ubuntu 环节完成前是 skipped。还有 4 个 PR workflow run 未完成。我没有轮询或 sleep 等待任何一项——按门禁规则,轮询循环会烧掉 runner 时间,而且在一个约 30 分钟的测试套件结束前就会放弃,所以我只取了一次并如实报告现有状态。

  • 未验证:lint。 撰写时仍在进行中;在两个 head 上都还没有对这份代码成功跑完过。
  • 未验证:单元测试、类型检查、构建、集成测试。 在这个 commit 上尚未出结果。
  • 我在 Stage 2 中关于项目风格的观察(文件名合规——新 .ts 用 kebab-case.ts、组件用 PascalCase.tsx;consistent-type 且按子路径导入;我读到的部分没有 any;测试与源码同目录)是我自己对 diff 的阅读,不是工具结果。

该 head 上已出结果的检查都是绿色或有意跳过:目前 18 个检查中 4 个成功、3 个跳过。verify 和 tmux-testing 还没有人触发。

沙箱化验证可以解决静态审查解决不了的部分,而且作者有写权限,两条通道都可用:@qwen-code /tmux——验证交互流程是否真的只写入被选中的合格条目、--dry-run 是否在真实 PTY 中不留任何目录/台账/锁,以及长项目路径被截断时选择数量是否仍然可见(这是视觉声明,单元测试钉不住)。@qwen-code /verify——验证目标目录安全性与"不复活"这两条不变量是否真的承重,也就是说,移除包含检查或去掉"先预留后发布"的台账顺序后测试是否真的会失败,而不是两种情况都通过。这些单元测试命名良好、读起来是实质的,但绿色的测试套件只能证明测试通过,不能证明它们能拦住这几个具体防护的回归。

— Qwen Code · qwen3.8-max-2026-09-02

Reviewed at c716e8a85443586fc8f95db242564c139b14d51b · re-run with @qwen-code /triage

@qwen-code-review-bot

Copy link
Copy Markdown
Collaborator

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 c716e8a8 while I was reviewing — the branch was rebased onto main, picking up 82 commits. I verified rather than assumed that this left my review intact: the PR's own diff is byte-for-byte identical to what I reviewed (same 17 files, same +4,687/−0), with only two hunk offsets in config.ts shifting because main grew above them. The rebase also cleared the lint-gate staleness that had Lint & Static failing on the previous head, which was the one concrete thing I'd have asked for. So that ask is already satisfied.

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 --dry-run / --json / --select path that already scripts the entire flow end to end. Having read it, I don't think that's waste: the migration is inherently a human-judgement task (deciding whether an old Codex preference still applies, and whether a global fallback is acceptable), and a scripted ID list is a poor surface for that judgement. So I've moved off my initial read. But it does mean this lands as one ~4,700-line unit rather than a reviewable sequence, and that's a real cost the maintainer should weigh knowingly.

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. packages/cli/src/config/config.ts matches packages/*/src/config/**, so Stage 0 applies. The core change itself is 3 lines of command registration that mirror mcpCommand exactly, and I'd have no hesitation about those 3 lines on their own. But the rule is path-based, and at 1,920 production lines this crosses both the 500-line maintainer-awareness threshold and the 1,000-line large-PR advisory. Under the gate's rules that escalation caps this at 3/5 and rules out an automatic approval no matter how clean the rest of the review is — so I'm deferring rather than approving, and there is no approve-on-green marker on this comment.

I also want to be explicit that I cannot currently attest to a green build. CI restarted from scratch on the new head: Lint & Static and Test (ubuntu-latest, Node 22.x) are in progress, the macOS and Windows legs are skipped pending ubuntu, and 4 PR workflow runs are incomplete. I did not poll for them. ESLint has never completed against these 1,920 lines on either head. Nothing suggests the code is broken — the earlier failure was mechanical staleness, now rebased away — but "tests pass" is not something I can claim, and the author's "778 tests passed" is their own claim, which I did not and could not re-run. Per the gate's rules I never executed PR-derived code in this unattended run; all evidence above is the PR's own CI read through the API plus my static reading.

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. within() in codex-import.ts and the inline equivalent in StandaloneMemoryImportPreview.tsx are two copies of a check whose entire job is keeping private memory text out of Git checkouts, the source tree, and native memory roots. Two copies drift, and drift here is a privacy leak rather than a cosmetic bug. The as unknown as LoadedSettings cast, the "browser" wording in the user docs, and a one-line comment on the preview.dryRun / !args.dryRun coupling are all cheap follow-ups that need not block.

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 移动到了 c716e8a8——分支被 rebase 到 main,合入了 82 个 commit。我没有想当然,而是核实了这是否会让我的审查失效:PR 自身的 diff 与我审查过的内容逐字节相同(同样 17 个文件、同样 +4,687/−0),只有 config.ts 里两处 hunk 偏移发生变化,因为 main 在其上方增加了内容。这次 rebase 也清除了上一个 head 上导致 Lint & Static 失败的 lint gate 陈旧问题,而那正是我唯一会明确提出的要求。所以那一项已经满足了。

从逐行审查中退一步看:这是作者认真思考过各种失败模式后的细致工作。零删除、没有顺手重构、没有格式化噪声,测试代码量超过生产代码且确实断言了它所声称的不变量,安全处理远超功能本身的最低要求——符号链接安全读取、对源和目标两侧都做包含关系检查、独占创建写入、台账"先预留后发布"(因此导入中断不会复活用户已刻意遗忘的内容)。设计文档中英双语且真正同步,不是应付性的翻译。如果六个月后接手这份代码,我大致会感谢作者。

回到我自己的独立方案:我当初设想约 3 个文件,而这里是 7 个。差距几乎全在交互层——三视图 Ink 审阅界面用了 603 行生产代码加 743 行测试,而 --dry-run / --json / --select 这条路径其实已经能端到端脚本化整个流程。读完之后我不认为这是浪费:迁移本质上是一项需要人判断的工作(判断一条陈旧的 Codex 偏好是否仍然适用、全局回退是否可接受),而脚本化的 ID 列表不适合承载这种判断。所以我改变了最初的看法。但这确实意味着它以约 4,700 行的整体落地,而不是一个可分步审查的序列——这个代价值得维护者知情后权衡。

有两件事我希望由人来决定,都不是我能从 diff 里得出结论的:

这是不是被需要的? 没有关联 issue,背后也没有用户诉求。缺口是真实的——Codex 的跨项目手册确实无法在不丢失归属的情况下直接放进 Qwen 的按范围记忆——但"缺口真实"和"我们应该长期维护这个能力"是两个不同的问题。这是一条从某个特定其他 agent 记忆格式迁移的通道,而这是一份比 PR 本身更长久的维护承诺。这属于产品方向判断,不是代码质量判断。作者有写权限,而且目前还没有独立的人工审查,所以除作者之外还没有人对"我们是否要拥有这个能力"表过态。

核心路径改动加上体量。 packages/cli/src/config/config.ts 命中 packages/*/src/config/**,因此 Stage 0 适用。核心改动本身只有 3 行命令注册,与 mcpCommand 完全一致,单看这 3 行我毫无犹豫。但规则是按路径判定的,而在 1,920 行生产代码下,本 PR 同时越过了 500 行的维护者关注阈值和 1,000 行的大 PR 提示线。按门禁规则,这个升级会把置信度封顶在 3/5,并排除自动批准——无论其余审查多么干净。所以我选择移交,而不是批准,本条评论也不带 approve-on-green 标记。

我还要明确一点:我目前无法背书一个绿色的构建。 新 head 上 CI 从头重跑:Lint & Static 和 Test (ubuntu-latest, Node 22.x) 都在进行中,macOS 与 Windows 环节在 ubuntu 完成前是跳过的,还有 4 个 PR workflow run 未完成。我没有轮询等待。ESLint 在两个 head 上都还没有对这 1,920 行成功跑完过。这些都不表明代码是坏的——早先的失败是机械性的陈旧问题,现已通过 rebase 消除——但"测试通过"是我无法声称的,而作者的"778 项测试通过"是其本人声明,我没有也无法复跑。按门禁规则,在这次无人值守运行中我从未执行任何 PR 带来的代码;以上全部证据来自通过 API 读取的 PR 自身 CI,以及我的静态阅读。

合入前我唯一会要求修改的、也是我唯一认为不只是表面问题的建议:去掉重复的路径包含判断。 codex-import.ts 里的 within() 和 StandaloneMemoryImportPreview.tsx 里的内联等价实现,是同一份检查的两个副本,而这份检查的全部职责就是把私有记忆文本挡在 Git checkout、源码树和原生记忆目录之外。两份副本会漂移,而这里漂移的后果是隐私泄露,而不是外观问题。as unknown as LoadedSettings 断言、用户文档里的"浏览器"措辞,以及为 preview.dryRun / !args.dryRun 耦合补一行注释,都是成本很低的后续项,不必阻塞。

我没有请求变更——我没有发现阻塞项,审查结果确实是正面的。我选择移交,原因是:升级属于策略规定、需求信号需要人来判断,而且 CI 尚未产出一个可供批准的绿色结果。一旦维护者权衡过方向问题、且 CI 在这个 head 上变绿,我认为代码层面没有任何东西会成为阻碍。

— Qwen Code · qwen3.8-max-2026-09-02

Reviewed at c716e8a85443586fc8f95db242564c139b14d51b · re-run with @qwen-code /triage

@qwen-code-review-bot

Copy link
Copy Markdown
Collaborator

⏸️ 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: QWEN_MAINTAINER_HANDLE is unset, this PR carries no labels (so no area matched), and there is no prior human review to fall back to. Whoever picks this up, the reasons I'm deferring are:

  1. Stage 0 escalated it for maintainer awareness. The feat touches a core path (packages/cli/src/config/config.ts matches packages/*/src/config/**) at 1,920 production lines, crossing both the 500-line awareness threshold and the 1,000-line large-PR advisory. Under the gate's rules that caps this at 3/5 and rules out an automatic approval regardless of review quality — so there is deliberately no approve-on-green marker on my Stage 3 comment, and the deferred approval will not fire when CI goes green.
  2. The product-direction question is a human judgement call. There's no linked issue and no user request behind this. The gap is real, but whether we want to own and maintain a migration path from one specific other agent's memory format is not something I can settle from a diff. The author has write access and no independent human review has landed yet.
  3. I can't attest to a green build. CI restarted on the new head c716e8a8: Lint & Static and Test (ubuntu-latest) were still in progress, macOS/Windows legs skipped pending ubuntu, 4 workflow runs incomplete. ESLint has never completed against these lines on either head.

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 main landed mid-review and cleared the lint-gate staleness that had failed the previous head (I confirmed the PR's own diff is byte-for-byte unchanged by it), and the one suggestion I'd treat as more than cosmetic is de-duplicating the path-containment predicate shared by codex-import.ts and StandaloneMemoryImportPreview.tsx — two drifting copies of the check that keeps private memory text out of Git checkouts is how that text ends up in one.

中文说明

⏸️ 移交维护者处理——不批准,也不请求变更。 这件事需要人来判断。

我无法确定性地解析出负责人,因此不猜测登录名:QWEN_MAINTAINER_HANDLE 未设置,本 PR 没有标签(所以没有匹配到任何领域),也没有在先的人工审查可供回退。无论由谁接手,我移交的原因如下:

  1. Stage 0 已将其提请维护者关注。 这个 feat 触及核心路径(packages/cli/src/config/config.ts 命中 packages/*/src/config/**),生产代码 1,920 行,同时越过 500 行关注阈值和 1,000 行大 PR 提示线。按门禁规则,这会把置信度封顶在 3/5,并排除自动批准——与审查质量无关。因此我的 Stage 3 评论刻意不带 approve-on-green 标记,CI 变绿时也不会触发延迟批准。
  2. 产品方向问题需要人来判断。 没有关联 issue,背后也没有用户诉求。缺口是真实的,但"我们是否愿意拥有并维护一条从某个特定其他 agent 记忆格式迁移的通道",不是我能从 diff 里得出结论的。作者有写权限,且目前还没有独立的人工审查。
  3. 我无法背书绿色构建。 CI 在新 head c716e8a8 上重跑:Lint & Static 和 Test (ubuntu-latest) 仍在进行中,macOS/Windows 环节在 ubuntu 完成前被跳过,还有 4 个 workflow run 未完成。ESLint 在两个 head 上都还没有对这些代码成功跑完过。

代码审查本身是正面的——没有发现阻塞项、安全处理扎实、测试断言了真实不变量、双语设计文档确实同步。完整细节见上方的 Stage 2 与 Stage 3 评论。

同时有两点值得作者关注,均不阻塞:rebase 到 main 已在审查过程中落地,清除了导致上一个 head 失败的 lint gate 陈旧问题(我确认了 PR 自身的 diff 未因此发生任何逐字节变化);而我唯一认为不只是表面问题的建议,是去掉 codex-import.ts 与 StandaloneMemoryImportPreview.tsx 共用的路径包含判断的重复实现——一份"防止私有记忆文本进入 Git checkout"的检查若存在两份会漂移的副本,正是这些文本最终进入其中的途径。

— Qwen Code · qwen3.8-max-2026-09-02

Reviewed at c716e8a85443586fc8f95db242564c139b14d51b · re-run with @qwen-code /triage

@callmeYe
callmeYe requested a review from wenshao October 14, 2026 08:35
@callmeYe

callmeYe commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator Author

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.

@qwen-code-review-bot

qwen-code-review-bot commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head f64733f, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

health-deep-with-session

field PR base (before) this PR (after)
activeWorkStaleMs 5 8

— Qwen Code · serve A/B

@qwen-code-review-bot qwen-code-review-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.

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)

Comment thread packages/cli/src/commands/memory/codex-import.ts
Comment thread packages/cli/src/commands/memory/codex-import.ts
Comment thread packages/cli/src/commands/memory/codex-import.ts
Comment thread packages/cli/src/commands/memory/codex-import.ts
Comment thread packages/cli/src/commands/memory/codex-import.ts Outdated
Comment thread packages/cli/src/commands/memory/codex-parser.ts
Comment thread packages/cli/src/commands/memory/codex-parser.ts
Comment thread packages/cli/src/ui/components/MemoryImportPreview.tsx
Comment thread packages/cli/src/ui/components/MemoryImportPreview.tsx
Comment thread packages/cli/src/ui/components/StandaloneMemoryImportPreview.tsx
@qwen-code-review-bot

Copy link
Copy Markdown
Collaborator

⏳ Historical-head review — head moved to 23ba93a82b6a9805b69b99f86b33957129b8cc9a while this review was in flight (past the salvage threshold), so the run finished and posted against the head it reviewed: 755d7fb50d3e0718180d3b3ed96b49c054357b2a (#10110). The next automatic review covers the delta from that anchor. Full log in the workflow run.

中文说明

⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 23ba93a82b6a9805b69b99f86b33957129b8cc9a(已过 salvage 阈值),因此评审跑完并针对其实际评审的 head 755d7fb50d3e0718180d3b3ed96b49c054357b2a 发布(#10110)。下一次自动评审将从该锚点起评审增量。完整日志见 workflow 运行。

@callmeYe
callmeYe dismissed qwen-code-review-bot’s stale review October 15, 2026 03:15

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.

@callmeYe

Copy link
Copy Markdown
Collaborator Author

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.

@callmeYe callmeYe closed this Oct 15, 2026
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.

2 participants