Repository navigation
Conversation
2026-10-20: preserve native user-memory capacity; repair Java CheckstylePublished memory fix 46b716e. Capped user scans and index generation prioritize native entries over the reserved imported/ subtree, including long native links under the character budget; imports use remaining catalogue space. Existing caps and project/team ordering remain unchanged. Exact-parent regressions were red; real-service synthetic probes now preserve the original catalogue with (a) 1 native + 201 imports, (b) 200 native + 1 import, and (c) a nearly full character budget containing a long native link + 20 imports. Omitted imported entries remain searchable and explicitly fetchable. R2-3 is addressed. Preserved and independently verified bot commit 29fb438 for R1-37: the old service fails the mature-store regression, while the bounded index reaches one model call. A synthetic 55,581-byte metadata index is reduced to 32,541 bytes; relevant full evidence is admitted first, oversized relevant evidence rejects with zero model calls/budget, and an omitted exact-body duplicate remains protected by the full snapshot. Reservation equals the actual serialized model input. This verifies deterministic admission/exact-body protection, not semantic completeness of the existing relevance heuristic or a production billing limit. A separate real CI failure occurred in Java fast job118520265720 (run39390337018), actual merge900c71b of head29fb and main9345c4: 5000 tests / 0 failures / 21 skipped, followed by six Checkstyle violations in an upstream test source. Current main had the identical blob. Normal main merge cf9f04c preserves the functional patch exactly (stable patch ID58ad1e5d8d6e1960fcd4fc442ff86e985cf17042); final fix3db4352af6448f38b1b01cd387420e9771f2421f removes only one unused import and seven surplus blank lines. No Java executable logic/assertions or CI gates changed. The same Checkstyle9.3 reproduces six violations before and zero after; both configured source groups pass (608 managed + 110 broker files). This local checker verification is not a full Maven/database integration rerun. Final merged-tree verification: core214/214, CLI87/87, full workspace build, bundle, sequential typecheck, focused lint/format and normal hooks pass. Final real bundled CLI and daemon→ACP HTTP each pass preview (zero model/native writes), selected1/2, added1, one loopback model call, and idempotent repeat with no extra call. Two clean memory-delta self-audits and a local medium quick review found no additional issue; the trivial Java delta also received a clean audit/quick pass. These are not formal approval. Only synthetic isolated data was used; no real memories or production providers were accessed. Final PR scope is46 code/docs/synthetic-test files (the native-capacity repair adds four core paths to the earlier41, and the Java style fix adds one). New-head CI is starting, not confirmed green. All 67 review threads are now closed after evidence-backed fixes or the explicit dispositions below. The six obsolete bot blocking reviews were dismissed through the normal GitHub action; live reviewDecision is REVIEW_REQUIRED, with no APPROVED review. Maintainer direction/approval and current-head CI remain outstanding. This is not a merge or approval. Explicit non-blocking dispositions after more than five review roundsPer the repository's Critical-only rule, the following remaining Suggestion threads are recorded individually here and closed as addressed/deferred rather than silently expanded into this PR. Deferred means future work, not implemented behavior; the existing scope, preview-estimate, relevance, diagnostics, cleanup and coverage limitations remain visible. No human request or approval is being dismissed.
2026-10-20: invalidate stale repository identities (03663c6)R3-6 is repaired without requiring a daemon restart or a cache clear from the import process. Cached recall now validates the recorded Git/path dependencies; changed topology is resolved again and invalid registrations fail closed. The original pointer syntax, linked-worktree checks, runtime-root selection, ordinary-memory isolation and 256-entry cache cap remain unchanged. The read-race check rejects changed dependencies before a resolved identity is cached; this is not an atomic filesystem snapshot. Purely synthetic red/green evidence: new tracked regression tests failed 3/6 on exact bbb32f4 and pass 6/6 after the fix. Seven independent topology mutations reproduced stale native search results before the fix; all seven stop old-repository recall in both the fixed source and final built core. Core scan/search/fetch/recall/forget tests pass 236/236; CLI import service/source/HTTP tests pass 86/86. Actual bundled CLI and daemon→ACP→loopback HTTP each preview without model/native-memory writes, import one explicitly selected source with one model call, and reuse the same task on repeat with no extra model call. Pointer/commondir read-race probes reject the raced identity and recover on the next lookup. Unchanged cache hits use dependency stats rather than repeating discovery, realpath or metadata reads (23 lstat calls per hit for the linked-worktree fixture). No imported runtime directory still means no identity lookup. Full workspace build using the current upstream 6144 MiB script, bundle, sequential typecheck, focused lint/format and normal commit hooks passed. This commit changes four files only: implementation, synthetic regression tests and synchronized design docs; no real memories or production model calls were used. The preceding bbb32f4 CI now confirms the upstream heap repair across Ubuntu, lint/static, no-key integration, Java fast/real daemon, Serve A/B, TUI and native helpers. Its Web Shell job checked out exact bbb32f4 and finished with 306 passed plus 2 flaky retries (approval-mode settings and browser notifications), not 308 first-pass passes. These are previous-head results; new 03663c6 checks must run independently. Reference publishing and full database lanes that were skipped are not claimed as passed. The two Critical findings concerning the complete metadata-index ceiling (R1-37) and user-memory index capacity (R2-3) remain open. Formal approval is still absent and this PR remains blocked. Earlier entries below are historical evidence, not current-head CI results. 2026-10-20: adopt the upstream build-heap repair (bbb32f4)The latest failing head a0c430 and its tested merge a6189a5 hit the fixed 4096 MiB V8 heap cap while compiling the CLI. All 13 failed checks were inspected: 12 show that compilation OOM and the Managed Agent fast-tier check aggregates it. These logs do not show a separate test assertion failure. The Serve A/B failure occurred in the head installation, not proof that its base failed. After reproducing this with the normal frozen-lockfile installation, an initial one-line 8 GiB repair was published as 2712f46. Main was then found to already contain #14822 (eeec911), which sets the cap to 6144 MiB. The final commit merges exact main ca06036 and takes its package.json unchanged, resolving the conflicting cap. The final build cap is 6 GiB, not 8 GiB. All intervening bot merges are preserved. The PR remains 41 files, with no package.json, lockfile, or workflow diff against that main; the feature patch's stable patch ID is identical before and after this integration. Final merged-tree validation used the normal frozen-lockfile installation with prepare enabled: full workspace build and bundle passed at 6 GiB. Full typecheck and the targeted ACP import-channel test passed. The 86 synthetic CLI import tests first returned 85 passes and one 5-second HTTP task-poll timeout; the unchanged exact case and then the unchanged full 86-test combination passed. That first timeout's cause remains undetermined, so this is not claimed as a clean first run. Normal commit hooks and the eight-file gate-freshness check passed. Local macOS/Node 22.17 verification does not replace the new Linux/Windows/Node 22.23.2 CI run. The three Critical concerns about large metadata context, native user-index capacity, and stale repository-identity cache remain open. No real memories or production model calls were used. Remote CI for the final merge still needs verification. 2026-10-19: reject unsupported symlink roots before paid consolidationPublished 3151dc7 on 58b8194 without rewriting history. R1-6 now uses its stated fail-closed acceptance alternative: a symlinked publication root produces an actionable per-item failure before scanning existing content or reserving model budget. This is not support for publishing through a symlink or across filesystems; that capability is explicitly deferred in both design translations. Ordinary native scanning, private-state symlink guards, budget retention and existing-memory protection remain unchanged. The delta is four files: four production lines, one synthetic regression and synchronized design notes. Exact-parent reproduction consumed one model call before failing generically; the fixed service/source/HTTP suite passes 86/86. Actual bundled CLI and daemon→ACP→HTTP both reject the symlink layout on first attempt and retry with zero wire calls / zero reserved units and an unchanged external synthetic target. Changing the synthetic layout to a regular directory allows the same task/key to complete on retry with one added document and exactly one loopback call. Full workspace build, final bundle, sequential typecheck, focused lint/format and normal hooks pass. Two clean self-audit passes and a local medium quick review found no additional issue in this delta; neither is formal approval. No real memories or production models were accessed. Detailed R1-6 evidence and explicit capability limit is replied and resolved. Three existing Criticals remain (large metadata context admission, user index capacity and long-lived Git identity caching); blocking reviews remain valid. New-head CI is starting/queued, not yet confirmed green. The prior 58b8194 Ubuntu job 118031599389 did succeed with verified exact checkout: server 1383/1383, CLI 43193 passed / 104 skipped, core 37469 passed / 19 skipped. That closes the earlier four fixture failures but is not new-head CI evidence. The PR remains unapproved and unmerged, with 41 code/documentation/synthetic-test files and no private artifacts. 2026-10-19: repair the four Ubuntu CI runtime-fixture failuresPublished 58b8194 on exact parent 2f94 without rewriting history. Ubuntu job 118005434962 (run 39236011054) checked out 2f94 and failed four injected-registry tests because their synthetic runtime omitted its required sessionRuntimeBaseDir. Startup memory-import recovery exposed that incomplete fixture through path.resolve(undefined). This repair adds only the suite-private temporary runtime directory: one test file, four lines. Production runtime ownership, error handling, assertions, timeouts and CI gates are unchanged. The exact parent reproduces the same four failures (4 failed / 10 passed); the repaired group passes 14/14. HTTP import regression passes 21/21. Full workspace build, sequential typecheck, focused ESLint/Prettier and normal commit hooks pass. The broader server-file run was 1379 passed / 4 different failures (two status-code mismatches and two timeouts); those groups' eight cases pass in focused runs on both the repaired file and exact parent. Their broader-run causes remain unclassified, so this is not an all-green full-file claim. No Unhandled Errors occurred. All verification used synthetic isolated data, with no real memories or production models. The published SHA is verified remotely and the worktree is clean. New-head workflows are starting/queued; the old failed run is superseded, not rerun. The PR remains blocked by existing reviews, with four Criticals still open and no approval. This CI repair does not resolve or dismiss those findings. The PR diff remains 41 code/documentation/synthetic-test files with no private artifacts. 2026-10-19: preserve durable HTTP receipts and distinguish server failuresPublished 2f94b5a while preserving the concurrent main merge 1bf32ae. R3-5 now distinguishes input errors (400), explicitly missing batches (404), unavailable runtime generations (503 with Retry-After), and unexpected server/storage failures (500). Successful durable submit/retry admissions retain their 202 task ID if the runtime closes afterward. Publication guards and post-read access checks remain; a 202 is admission, not completion. Eight changed files contain only the shared import errors/service, route, synthetic tests and synchronized English/Chinese design/API documentation. The eight new regressions produce seven failures and one control pass against exact parent 1bf; the fixed six-file suite passes 98/98, with the final HTTP revision independently passing 21/21. Real router/service timing probes cover pre-admission drain, durable receipts, and no source-content disclosure after closure or trust revocation. With the final bundled daemon, a synthetic task file made unreadable by real filesystem permissions returns 500, and restoring permissions returns 200/completed; invalid requests remain 400, missing batches 404, and missing bearer authentication 401. Bundled CLI/HTTP import, same-key idempotency, no-write/no-model preview, and transient recovery also pass. Full workspace build, bundle, typecheck, focused lint/format and normal hooks pass. Local audit and medium quick review found no additional issue in this delta; neither is formal approval. Lifecycle timing injection is not a real process-crash test. One earlier mixed test run had an unclassified old drain assertion failure; final serial suites passed. No real memories or production models were used. Detailed R3-5 evidence is replied and resolved. Four Criticals remain (large metadata context admission, user index capacity, native symlink-root handling, and long-lived Git identity caching); blocking reviews remain valid. New-head CI run 39236011054 and related workflows are running or queued, so previous green heads are not presented as this head's result. The PR is not approved or merged. Current PR diff remains 41 code/documentation/synthetic-test files, with no private artifacts. 2026-10-19: retry failed HTTP startup recoveryPublished 5e40886 on 0957945 without rewriting history. R2-17 now retains the service and worker map while tracking recovery separately: task requests await a shared recovery attempt, failures are logged and return 503 memory_import_recovery_unavailable with Retry-After: 1, and the next request retries. Successful recovery remains cached; preview does not trigger recovery. Six changed files contain only the route, synthetic tests and synchronized English/Chinese design/API documentation. All three new regressions fail against the exact prior router; the final HTTP suite passes 13/13 and command/local/environment coverage adds 13/13. Actual-service probes verify queued work resumes after transient startup failure, concurrent callers share one attempt, and partial recovery does not duplicate model work. The final bundled daemon was tested with a real filesystem obstruction in an isolated synthetic runtime: GET returned 503 while preview remained 200 with zero model/native-memory writes; restoring the directory allowed the next poll to complete the queued task with one loopback model call, retaining the previously charged usage. Basic bundled CLI/HTTP import and idempotency also pass. Full workspace build, bundle, typecheck, lint/format, normal hooks and two clean local audit passes completed. No real memory or production model was used. Recovery evidence closes R2-17 only. Five Criticals remain, including R3-5: general runtime/storage error classification and durable-response handling were separately reproduced but are not fixed by this change. Blocking reviews remain valid and there is no approval or merge. New-head CI has started and is not yet verified. For the preceding 095 head, actual Ubuntu/Web Shell checkout logs confirm 43,123 CLI tests, 37,445 core tests and 298 Web Shell E2E passes; those results are not claimed for this new commit. 2026-10-19: cross-group replay and unpublished-intent recoveryPublished 0957945 on top of the concurrent bot merge 4cd33e9 without rewriting history. This change is limited to the import service, synthetic regressions and the linked English/Chinese design documents (four files). R1-35 now recognizes committed outputs across groups of the same task and locked scope while preserving recorded baseline hashes. R3-3 durably rolls back a changed-memory intent only before native publication; a fresh confirmed task can reconsolidate against current memory, while published edits and forgotten outputs remain protected. On the exact prior implementation, six regression cases produced four failures and two passing controls. Final related coverage is 87/87; ten recovery/protection probes and two extra cleanup/baseline controls passed. A cleanup failure after ledger rollback leaves an orphan stage rather than a poisoned source ID. A three-group retry preserves edits already included in a later baseline. Final bundled CLI and real daemon→ACP→loopback HTTP each completed preview, selected import and idempotent repetition: preview made no model/native-memory writes and repetition made no extra model call. Full workspace build, final bundle, full typecheck, focused lint/format and normal hooks passed. All data and targets were synthetic and isolated; no real memories or production model were used. Fault injection is not a claim of real SIGKILL testing or atomic filesystem fencing. Cross-group evidence and unpublished-intent evidence close those two threads. Six Critical findings remain: large-context admission, native index capacity, root-recovery retry, symlink native roots, HTTP lifecycle/error semantics and stale Git-identity cache. Existing blocking reviews remain valid; no approval or merge is claimed. New-head CI has started and still requires remote verification; prior-head success does not establish this head's result. 2026-10-19: native metadata, retry recovery and pipe fixes; browser CI follow-upPublished d202ca0 (memory repair deda75c) on d75eab0 without rewriting history. R1-5 now builds discovery metadata from the native parser's recognized fields instead of a second frontmatter regex. R1-40 validates scope before retaining proposals; retries can discard only the invalid item from older journals, preserving valid siblings and charged usage. R3-2 uses the existing broken-pipe handler for asynchronous stdout errors. R1-1 and R1-2 were already fixed and are now closed with current-code recovery/race evidence. Five new red regressions failed on the prior implementation; final import coverage is 82/82. Synthetic mixed legacy journals verified that retry requests only the invalid item, preserves edited siblings, does not restore forgotten siblings and does not reset usage. Final bundled CLI and daemon→ACP→loopback HTTP each completed a selected import; repeating added no model calls. Offline preview had no model/native-memory writes. Real OS pipes closed before output or after one byte now exit 0 without EPIPE, while full readers still receive complete versioned JSON. Full workspace build, final bundle, full typecheck, focused lint/format and normal commit hooks passed. All inputs/targets were synthetic and isolated; no real memories or production model were used. Observed a new failure on d75's Ubuntu Web Shell E2E job 117877119954 (run 39192410625): the remote-computer return URL still contained its one-shot Connections marker, or was still in the verification handoff. A real Chromium probe delaying the returned App module by 12.5 seconds reproduced the retained-marker failure under the original 10-second wait; the marker then cleared normally. The narrow test-only fix waits up to 30 seconds for the same exact clean URL, retaining the 60-second whole-test deadline and all category/catalog assertions. Delayed boot now passes; permanently retaining the marker still fails after 30 seconds. The actual failed case and two previously flaky cases passed 3/3 without retries locally. This macOS evidence does not establish the cause of every CI attempt; the new Ubuntu run still must pass. No navigation production code or CI gate was changed. This closes five threads, not the whole review. Cross-batch journal recovery, large-context indexing, native index capacity, recovery-root retry, symlink native roots, poisoned prepublication intents, HTTP lifecycle/error semantics and stale identity caches remain under review. No approval or merge is claimed. 2026-10-19: lock ownership and preview admission fixesPublished d75eab0 on top of the latest concurrent bot merge e26a391, preserving its upstream changes. This eight-file change addresses R1-3, R2-2, R5-1, R5-2 and R5-3: unknown retries no longer allocate directories, lock loss is isolated to dependent work, detected old holders cannot replace task/proposal/ledger records or publish staged documents, and Markdown parser/expanded-preview admission is bounded. The existing global budgets and ordinary user cancellation remain intact. Reproduced each failure before fixing it. Final targeted CLI coverage: 78/78; complete workspace build, final bundle, full typecheck, focused lint/format and normal commit hooks passed. Actual bundled CLI and daemon/ACP HTTP with isolated synthetic sources and a loopback model each imported one document with one call; repeating the submission added no model calls. Ordinary preview performed zero model calls, native writes or task writes. Three excessive-input shapes were rejected by CLI and HTTP while the daemon remained healthy. Real file-lock probes injected compromise notifications during model work, scope work and temporary task-state writes: replacement state/usage and ledger bytes remained identical, unrelated work completed, explicit cancellation still persisted cancelled, and scope loss published no project document. This is notification-injection evidence, not a real 120-second stale-lease experiment or an atomic filesystem fencing-token guarantee. Synthetic 64 KiB delimiter-heavy input changed from ~5.85 seconds parsing to ~0.95 ms rejection; the setext case changed from ~2.23 seconds / ~1.38 GB heap growth to ~2.05 ms / <0.7 MB. These local measurements are not universal parser guarantees. Small controls retain their ancestor context. HTTP rejection remains generic Only code, bilingual docs and synthetic regression tests were committed. No user memory or production model was accessed. Local reports, probes and raw outputs remain untracked/ignored. New-head CI is still pending verification. Other blocking review findings remain open: this fixes five findings and does not clear the formal CHANGES_REQUESTED reviews or imply maintainer approval. Review follow-up and CI — updated 2026-10-16 06:18 UTCCurrent head remains Response to doudouOUC's COMMENTED review: the review body discusses The main synchronization request was handled by normal merge Non-blocking items are tracked explicitly here:
No new code or data was published in this follow-up. No approval has been submitted; normal maintainer approval is still needed. No review rerun or repeated reviewer request was dispatched. Cost and traversal update — 840442bImplemented the requested cost controls after reproducing the concerns. Confirmed sources are grouped by original project and selected scope, with at most eight sections / 64 KiB per model request. The model receives the complete bounded metadata index plus relevant full evidence instead of every existing body. Index-only uncertainty is explicitly a conflict; oversized metadata or relevant evidence fails before a paid call. Offline previews expose aggregate and per-group call/token estimates. Unassigned groups remain provisional and estimates cover all selectable sections, not an unsubmitted selection. Tasks permit at most 32 logical calls and 512,000 reserved token units, with an 8,192-token output allowance per request. Reservations are persisted before invocation and are not refunded on failure, cancellation, restart or retry. Import disables unattended outer retries. These are logical-call/conservative-reservation limits, not guaranteed provider-billed-token or currency caps; provider-internal retries and tokenizer accounting can differ. Native semantic quality still requires production-model evaluation. Complete validated batch responses and baseline hashes are retained before the first publication. Recovery reuses them while preserving per-source ledgers, native validation, selection isolation and edit/forget checks. A reproduced interleaving bug was fixed by completing a project batch before moving to another group. Git identity lookups now have a bounded cache tied to the existing root-cache reset; scans with no imported layer skip the walk entirely. Writes refresh identity. Ordinary memory isolation and workspace authorization are unchanged. Synthetic measurements and final validation:
Only code, bilingual contracts and synthetic unit tests are committed. No real memory, local preview, test-run output, reports, personal paths or external-product changes are included. The normal main merge updates existing gate definitions; this PR does not modify CI gates relative to main. Fresh remote CI is pending verification. Existing maintainer review requests remain; the previously manually cancelled automatic review was not restarted. Synthetic verification reportValidated commit
Limits: local end-to-end model behavior used a deterministic loopback provider, not a production model; this verifies the integration contract rather than production model quality. Local validation was on macOS. Remote CI and normal maintainer review are still required. The full build used an 8 GiB Node heap after the default 4 GiB invocation exhausted memory. This replaces #14149 with the shared CLI/HTTP service design. It contains no unrelated fixes carried from that PR and no DataAgent changes. CI follow-up — d2acc05The first Ubuntu run failed six contract assertions, and the no-AK run failed the baseline capability assertion. Reproduced all seven locally. The fix registers the existing memory command/capability in the exact expected lists and documents the two narrowly scoped environment reads without weakening the process-environment guard. The environment audit also reproduced a real ownership issue: an existing runtime snapshot with an absent project-scope key inherited the process scope. Import now honors the complete selected-runtime snapshot; an overlay runtime without a snapshot does not inherit process scope either. Only parent-process runtimes lacking a snapshot retain compatibility fallback. Legacy process-global memory path overrides remain rejected. Both absence cases failed under the original expression with wrong-scope existing memory in the model input and pass after the fix. Validation: the original seven CI failures now pass, scope/environment coverage passes 13/13, the actual bundled no-AK capability check passes, and synthetic HTTP preview/import/idempotence passes with no production model. Full workspace build, final CLI build/bundle, full typecheck, focused lint/format and normal commit hooks passed. Only six source/test files changed; no private data or unrelated CI changes. Expanded-suite limitation: one four-file run passed 1470 tests but had transient HTTP parsing/transcript failures; an entire server-file rerun passed 1371 but timed out in a different attachment-upload test. Each affected case passed focused comparison with both original and fixed expressions. Their root cause remains undetermined; this is not a claim that the complete server suite passed in one run. The new remote CI must still be checked. Historical consolidation-cost assessment — d2acc05 (superseded by the update above)Rechecked the current implementation against upstream openai/codex commit Codex external import preserves selected project resources and scope metadata, then enqueues global consolidation rather than extracting every imported section separately (import implementation). The pipeline skips model work when the synchronized workspace has no changes and its artifacts validate, otherwise starts a consolidation agent; its incremental instructions prioritize changed evidence and retain detailed sources behind references (pipeline, instructions). A consolidation agent may make multiple model turns: one queued job is not one model request. Large first imports can still be expensive. Native rollout extraction is a separate pipeline; its rollout-count defaults must not be represented as external-import file limits. The useful direction is bounded batching by confirmed target scope, compact/relevant existing-memory context with progressive evidence access, and admission-time estimates plus explicit aggregate limits and actual usage reporting. This must retain selected-source isolation, native validation, durable retries, and commit-time edit/forget protection. Codex's global consolidation ownership must not be copied across Qwen workspace authorization boundaries. These are assessment recommendations, not claims of implemented behavior or measured production savings. No production model or user memory was used for this assessment. Remote CI is now successful on d2acc05, including Ubuntu, lint/static, no-AK, Serve A/B, TUI, native, Web Shell E2E and reference distribution build/verification. Web Shell checkout was verified as the exact head; its browser summary is 254 passed and one mobile-WebKit focus test marked flaky after retry, not 255 first-attempt passes. Reference publication was skipped, not published. That automatic review was subsequently cancelled manually by a maintainer; no formal approval has been submitted. Normal maintainer review is still required. For triage accuracy, #14149 is already closed and replaced by this PR. The CLI/HTTP redesign was explicitly requested by the end user; that is user demand, not maintainer approval of the product direction. |
|
Thanks — this is a serious piece of work, and a much better shape than #14149. Template looks good ✓ — every required section present, Problem: this is a Direction: memory is a first-class subsystem here and this reuses it instead of standing up a parallel store — imported documents land in the native scan/search/fetch/recall/forget path, consolidation goes through the existing Two process notes I would not want buried:
Size: core paths are touched —
Of the production lines, 228 are in You are a listed core owner with write access, so the external two-tier core gate does not apply to you and nothing here is blocked on size. Two things are still worth flagging, both informational: 1,826 production lines crosses the 1,000-line large-PR advisory, and the Approach: my independent baseline before reading the diff was much smaller — a parser plus a Diff hygiene is clean: 5 deletions total, no drive-by refactors, no formatting churn, no unrelated edits. Test-to-production ratio is roughly 1:1.6 and the tests assert the invariants they name rather than restating the implementation. Risk: Stage 1e matched one high-risk path — Moving on to code review. 🔍 中文说明感谢贡献——这是一份很扎实的工作，形态也比 #14149 好得多。 模板完整 ✓——所有必填章节齐全， **问题：**这是 方向：记忆在这里是一等子系统，本 PR 复用它而不是另建并行存储——导入的文档进入原生扫描/搜索/获取/召回/遗忘链路，整理走既有的 有两个流程问题我不希望被埋没：
**规模：**确实触及核心路径—— 其中 228 行生产代码位于 你是有写权限的既定 core owner，因此外部两级核心门禁不适用于你，这里没有任何基于体量的阻塞。但有两点仍值得提示，均为信息性：1,826 行生产代码越过了 1,000 行大 PR 提示线； 方案：在读 diff 之前，我自己的基线要小得多——一个解析器加一个 Diff 卫生良好：总共只有 5 行删除，没有顺手重构、没有格式化噪声、没有无关改动。测试与生产代码比例约 1:1.6，测试断言的是它声称的不变量，而不是复述实现。 **风险：**Stage 1e 命中一个高风险路径—— 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Independent proposal first (written before reading the diff, from the title and "Why it's needed" only): a Codex Markdown parser, a The PR exceeds my baseline on durability and I have moved off it there: a half-published import that resurrects something the user deliberately forgot is a real failure mode, and the intent→commit ledger keyed on content digest is a sound answer. It exceeds my baseline on routing too — refusing unknown/untrusted/draining/SSH/legacy-override runtimes rather than falling back to primary is the correct call and matches how the rest of the workspace routes behave. Findings1.
const imported = getImportedProjectMemoryRoot(projectRoot);
if (fs.existsSync(imported)) roots.push(imported);The This lands on a hot path, and it is worse than one call: A module-level 2. The imported layer silently ignores The consequence is concrete in a monorepo: import while working in 3. 4. Up to 2,048 serial paid model calls are accepted with a 202 and no cost signal. 5. Consolidation input is capped at 512 KiB including all existing same-scope memory, so the feature degrades to unusable as a user's memory grows. 6. 7. Minor, non-blocking. What I checked and found soundI verified every cross-package symbol this PR imports actually exists on The security posture is genuinely strong and better than #14149's: I looked hard for the class of bug that sank #14149 (the Sequence: confirmed import through the daemonsequenceDiagram
participant P1 as Host client
participant P2 as Daemon route
participant P3 as MemoryImportService
participant P4 as ACP bridge
participant P5 as Agent child
participant P6 as Model
participant P7 as Imported memory root
P1->>P2: POST preview with source envelope
P2-->>P1: sections, groups, warnings (pure parse, no writes)
P1->>P2: POST batches with selection and idempotency key
P2->>P3: submit
P3->>P3: validate selection, persist source and task
P3-->>P1: 202 task queued
P3->>P3: worker acquires per-scope lock
P3->>P3: snapshot existing same-scope memory
P3->>P4: invokeWorkspaceCommand import plan
P4->>P5: control ext method
P5->>P6: runSideQuery, no tools, maxAttempts 1
P6-->>P5: decision plus proposed documents
P5->>P5: validate schema and native document format
P5-->>P3: validated proposal
P3->>P3: recheck memory version, stage files, write ledger intent
P3->>P7: rename staged directory into imported root
P3->>P7: rebuild index, mark ledger committed
P3-->>P1: status reports added refs
Files changed (29 of 29)
Testing evidenceThis is an unattended CI run ( CI had not settled when I fetched it, and I did not poll. Four Not verified, and why: the author's "559 CLI cases, 31 core cases", the full-workspace build/typecheck, and the SIGKILL recovery probes are the author's own claims on macOS only — I did not and could not re-run them here. Nothing in this run independently confirms them. CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle most of what CI cannot: 中文说明先说独立方案（在读 diff 之前，仅依据标题和"为什么需要"写下）：一个 Codex Markdown 解析器、一个先预览再通过既有记忆 API 写入显式选择的 在持久性上本 PR 超出了我的基线，我也因此改变了看法：半发布的导入复活用户刻意遗忘的内容是真实的失败模式，以内容摘要为键的 intent→commit 台账是恰当的解法。在路由上也超出我的基线——对未知/不可信/退出中/SSH/旧式覆盖配置的运行时一律拒绝而不回退主运行时，这是正确的做法，与其余工作区路由的行为一致。 发现 1（唯一我希望合入前修掉的）： **发现 2：**导入层完全忽略 发现 3： **发现 4：**最多 2,048 次串行付费模型调用被一个 202 接受，且没有任何成本提示。 **发现 5：**整理输入上限 512 KiB，且包含同范围全部已有记忆，因此用户记忆越多、该功能越不可用。 发现 6： 发现 7（次要，不阻塞）： **已核实且认为可靠的部分：**我逐个核对了本 PR 跨包导入的符号在 安全处理确实扎实，且优于 #14149：每次私有读取都带 我重点排查了拖垮 #14149 的那类缺陷（那里的 测试证据：这是无人值守的 CI 运行（ **我抓取时 CI 尚未结束，且我没有轮询。**有 4 个 未验证及其原因：作者的"559 项 CLI、31 项 core 用例"、全工作区构建/类型检查、以及 SIGKILL 恢复探测，都是作者本人在 macOS 上的声明——我在本次运行中没有也无法重跑。没有任何东西独立确认了这些结果。 沙箱化验证可以解决大部分 CI 无法解决的问题： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "memory_import_v1" |
— Qwen Code · serve A/B
|
Confidence: 3/5 — the engineering is good and I found no merge-blocker, but there are two things I would want fixed, a direction question that has now gone unanswered across two PRs, and CI that has not landed. Deferring, not approving. Going back to my independent proposal: I sketched a parser, a subcommand, and one endpoint. This is 17 production files. Having read all of it, I have genuinely moved — the ledger and the fail-closed workspace routing are not decoration, they are the difference between "import that mostly works" and "import that cannot resurrect something you deliberately forgot." That is the right instinct and it is executed carefully. If I picked this up in six months I would thank the author for the durability model. But I would curse them for finding 1. Adding an uncached, synchronous, recursive-up-to-the-filesystem-root walk to On the honest 80/20 question, my answer is different from #14149's and I want to be explicit that I am not just repeating the prior triage. There, the questionable third was an interactive TUI; here it is the model-backed consolidation, and I think that one deserves a harder look. It converts a deterministic transformation into one paid model call per selected section — up to 2,048 of them, serially, accepted with a 202 and no cost signal — and it introduces the entire validation surface that findings 4 and 5 are about. The stated justification is that direct per-bullet conversion "loses historical context and does not reconcile old knowledge with existing native memory." That may well be true, but it is asserted, and it is the load-bearing assumption of the whole design. A before/after on one real Codex memory set — deterministic conversion vs. consolidated, side by side — would settle whether the model layer earns its cost and its complexity. Nobody has shown that, on either PR. The pattern question, since Stage 3 asks it of me directly: this is the same author's second large PR for the same feature in two days, both self-reviewed, neither with a linked issue or an independent human review, the first carrying nine Critical findings from Two concrete process asks, neither of which needs me:
Why I am deferring rather than approving, concretely: CI had not settled when I fetched it (four And it needs a human for a reason the size policy does not capture. You have write access and are a listed core owner, so the external two-tier core gate is exempt and nothing in this review is capped by policy — my 3/5 reflects real reservations, not a rule. But you are not the ⏸️ Deferring to @wenshao. Four things need your call, and none of them are resolvable from the diff, the tests, or the description:
Findings 3, 4, 6 and 7 in Stage 2 are smaller and can be follow-ups. I found no security defect: the containment, symlink, exclusive-create, allowlist and untrusted-data handling are all sound, and I verified every cross-package symbol this PR imports actually exists on 中文说明置信度：3/5 —— 工程质量不错，我没有发现合入阻塞项，但有两处我希望修掉、一个已经跨两个 PR 无人回答的方向问题，以及尚未结束的 CI。因此是转交，不是批准。 回到我自己的独立方案：我当初设想的是一个解析器、一个子命令、一个接口，而这里是 17 个生产文件。全部读完之后我确实改变了看法——台账和 fail-closed 的工作区路由不是装饰，它们是"基本能用的导入"与"不可能复活用户刻意遗忘内容的导入"之间的差别。这个直觉是对的，实现也很细致。六个月后接手这份代码，我会感谢作者设计了这套持久性模型。 但发现 1 会让我骂人。在 关于诚实的 80/20 问题，我的答案与 #14149 那次不同，我想明确说明我不是在重复上一轮 triage。那次可疑的三分之一是交互式 TUI；这次是模型整理，而我认为这一项值得更严格地审视。它把一个确定性转换换成了每个选中章节一次付费模型调用——最多 2,048 次、串行、被一个 202 接受且没有成本提示——并引入了发现 4 和发现 5 所针对的整个校验面。其陈述的理由是逐条直接转换"丢失历史上下文、无法与已有原生记忆协调"。这很可能成立，但它是断言，而且是整个设计的承重假设。在一份真实的 Codex 记忆集上做一次 before/after——确定性转换与整理后结果并排对比——就能判断模型层是否值回它的成本和复杂度。两个 PR 都没有人展示过这个。 Stage 3 直接问我的模式问题：这是同一作者两天内为同一功能提交的第二个大 PR，两个都是自审，都没有关联 issue、都没有独立人工审查，第一个带着 两个具体的流程请求，都不需要我来做：
为什么是转交而不是批准，具体地说：我抓取时 CI 尚未结束（4 个 而且它需要人的理由，是体量策略捕捉不到的。你有写权限、是既定 core owner，因此外部两级核心门禁豁免，本次审查没有任何策略性封顶——我的 3/5 反映的是真实的保留意见，不是规则。但你不是 ⏸️ **转交给 @wenshao。**有四件事需要你的判断，都无法从 diff、测试或描述中得出结论：
Stage 2 中的发现 3、4、6、7 较小，可以作为后续跟进。我没有发现安全缺陷：包含检查、符号链接、独占创建、白名单和不可信数据处理都是可靠的，而且我逐个核实了本 PR 跨包导入的符号在 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
There was a problem hiding this comment.
Re-verified the current head (d2acc053f4) against the triage findings. The shape of this is right and I could not find a security defect: destinations are server-derived, the symlink/exclusive-create/containment checks hold, the consolidation input is framed as untrusted data with no tools, and every route is workspace-qualified with no fallback to the primary runtime. The durability model (ledger, intent→commit, edit/forget conflict protection, lease recovery) is the part that makes this more than a nice-to-have, and it is executed carefully.
One item I would want fixed before merge:
getProjectAutoMemoryRootsnow resolves the imported root through an uncached, synchronous walk that goes from the project root up to the filesystem root (existsSyncper ancestor, thenlstat/realpathon.git, then a sha256) — and it runs for every user, including those who never import, on a path every project-scope memory scan already takes at least twice (structured-scan.ts645/705/791,metadata-migration.ts:226). Because the service snapshots existing memory two to three times per section, an N-section import pays this roughly 6N times. The sibling root resolver is memoized in the same file for exactly this reason; a module-level memo for the git identity (cleared next to the existing cache) removes essentially all of it.
Smaller items, none of which need to hold the merge, and the first two are also worth a line of documentation rather than code:
- The imported layer is always repository-keyed while ordinary project memory follows
QWEN_CODE_MEMORY_PROJECT_SCOPE. Besides the monorepo case, the published layer is therefore visible to every trusted workspace of the same repository, not just linked worktrees. Either honour the setting or state the rule explicitly in the design and API docs. - Published documents carry no provenance marker and are indistinguishable from native memory in recall. I checked whether that is a new trust boundary and concluded it is not (the mutation gate applies, and an authenticated client can already author memory through the remember route), but the design doc's "source paths are provenance" wording reads as if the published artifact carried provenance — please clarify that it does not.
- A runtime directory inside a Git checkout is rejected permanently, with a message that does not name the cause and is not listed among the v1 limitations in the API doc. Fail-closed is right; make it actionable.
- Up to 2,048 serial paid calls are accepted with a 202 and no cost signal, and the 512 KiB consolidation input (which includes all existing same-scope memory) is only enforced per section. A preview-level call estimate and one up-front batch check would make both host-visible instead of surfacing as a generic failed item.
- Please either justify or drop
memoryin the launcher's in-process fast path, and note the legacy-path-override rejection the daemon performs but the CLI entry point does not (withQWEN_CODE_MEMORY_BASE_DIRset, a user-scope CLI import fails per section with a retry that can never succeed).
Also: main has moved since this head, and its last two merges touch the same capability/expected-feature contract files this PR edits. The textual merge is clean, but please rebase and re-run the affected suites before landing.
中文说明：
我按当前 head（d2acc053f4）复核了此前各阶段的结论。整体形态是对的，我没有发现安全缺陷：写入目标由服务端推导，符号链接、独占创建、包含检查都成立；整理输入的模型调用被框定为不可信数据且无工具；所有路由都限定工作区、不回退主运行时。持久化模型（台账、intent→commit、编辑/遗忘冲突保护、租约恢复）是让它不止于"锦上添花"的部分，实现也很细致。
有一项我希望在合入前修掉：
getProjectAutoMemoryRoots现在通过一次无缓存、同步、逐级向上直到文件系统根的遍历来解析导入根目录（每级一次existsSync，再对.git做lstat/realpath，最后算 sha256）。它对所有用户都会执行——包括从不使用该功能的人——而这条路径每次项目范围记忆扫描至少走两次（structured-scan.ts645/705/791、metadata-migration.ts:226）；服务每个章节会做两到三次已有记忆快照，因此 N 个章节的导入大约要付 6N 次。同文件里的另一个根目录解析器正是出于这个原因做了 memo，给 git 身份加一个模块级缓存（与现有缓存一并清理）即可基本消除。
其余较小的项，都不需要阻塞合入，前两项其实更适合用一行文档说明而非改代码：
- 导入层永远以仓库为键，而普通项目记忆遵循
QWEN_CODE_MEMORY_PROJECT_SCOPE。除了 monorepo 场景，已发布层因此对同一仓库的所有可信工作区可见，而不只是 linked worktree。要么遵循该设置，要么在设计文档和接口文档中明确写出这条规则。 - 已发布的文档不带来源标记，在召回时与原生记忆无法区分。我核实过这是否构成新的信任边界，结论是不构成（变更门禁仍然适用，且已认证客户端本就能通过 remember 路由写记忆），但设计文档中"来源路径即来源信息"的措辞容易被读成发布产物带有来源标记——请澄清它其实没有。
- 位于 Git 检出内的运行时目录会被永久拒绝，错误信息没有指出真正原因，接口文档的 v1 限制列表里也没有这一条。fail-closed 方向是对的，但信息应当可操作。
- 最多 2,048 次串行付费调用被一个 202 接受且没有任何成本提示，而 512 KiB 的整理输入（含同范围全部已有记忆）只在单章节层面校验。在预览里给出调用数估算、并在提交时做一次批次级检查，可以让宿主提前看到而不是最终以一条泛化的失败项呈现。
- 请要么解释、要么去掉启动脚本进程内快速路径里的
memory；另外请说明守护进程会做、而 CLI 入口没做的旧式路径覆盖拒绝（设置了QWEN_CODE_MEMORY_BASE_DIR时，CLI 的用户范围导入会按章节失败，且重试永远不会成功）。
另外：该 head 之后 main 已有推进，且最近两次合入触及了本 PR 修改的同一些能力/期望特性契约文件。文本合并是干净的，但请在合入前 rebase 并重跑受影响的用例。
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped after the round 1/2 convergence pair; rounds 3-5 were not run, so the two-consecutive-dry-rounds convergence criterion was never met (the orchestrator ran out of context, not the review time budget — the pair REPORTED findings, it did not go dry).
Not reviewed: build-and-test (test half) — the build phase consumed the whole 600s Agent 7 budget: all three changed workspaces compiled green (19/19 per-workspace builds exit 0), but no unit suite executed and the test-efficacy probe was inconclusive (harnessValidated null — its disposable tree lacks built dist/).
Not reviewed: verifier mutation probing — review scratch-tree refused for every shard (repo-local git includeIf chain resolving to a missing credentials file), so guard-removal mutations could not run in a sanctioned tree; verifiers substituted read-only probes against built dist/ artifacts and the product own injection points, and several findings carry a witness: not run line.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none — I also verified and dismissed, rather than leaving unchecked: rebuildAutoMemoryIndexAtRoot 's if (!existsSync(root)) return '' early-out (so forget.ts…; "agent reverse-audit (round 2)": did not verify by execution that a repo-local-root forget leaves <projectRoot>/.qwen/memory/MEMORY.md stale (no test run; Finding 1 rests on reading getProje…; "agent reverse-audit (round 2)": did not open packages/cli/src/serve/memory-import.test.ts to check whether an existing preview test already pins a route-level size bound (Finding 2's witness…; "agent reverse-audit (round 1)": none — but two checks were left unfinished for scope, not budget: I did not walk run() / prepareProposal / importSection as their own territory (they sit past….
[Critical] R1-39: A repeated --source is collected by yargs into an array, fails the typeof === 'string' test, and is silently discarded in favour of the default directory Failure scenario: qwen memory import codex --apply --source /work/proj-a-memories --source /work/proj-b-memories --selection s.json --idempotency-key k - or, far more likely, a shell wrapper or host that appends its own --source after one the user already supplied. The typeof check fails, the default is used instead, and the confirmed import reads, consolidates and publishes from /.codex/memories while the operator believes they imported /work/proj-b-memories. Paid model calls are made and native memory is written from a directory the user never named; nothing in the output identifies which directory was actually read. The same silent-default shape applies to any non-string value yargs can produce for the option. Witness: [probe] Three arms reading which directory was actually opened: arm 1 (single --source) /.codex/memories fallback - so the absent-flag case must keep working; only the supplied-but-not-a-single-string case changes. Acceptance criterion — the test that must go red if this fix is removed: packages/cli/src/commands/memory.test.ts - assert that parsing ['memory','import','codex','--dry-run','--source','a','--source','b'] rejects with a message naming --source, and that a single --source is still honoured. Red today (the array is discarded and the default used).READ dirA=false dirB=true — the control, honours the flag; arm 2 (repeated --source) READ dirA=false dirB=false codexHomeDefault=true — identical to arm 3 (no flag at all). The explicitly named directory is silently replaced by the default with no diagnostic. Suggested fix: Reject rather than default when the flag was supplied but is not a single string: if (args['source'] !== undefined && typeof args['source'] !== 'string') throw new Error('--source must be given exactly once, as a single directory path.'); and only fall back to the CODEX_HOME/default resolution when the flag is absent. Alternatively declare the option non-repeating so yargs itself errors. The fix must not violate this existing fact: The default resolution the else-branch falls through to is the documented one - docs/design/external-memory-import.md names memory import codex --source <directory> and the CODEX_HOME/
[Critical] R1-6: privateDirectory's blanket symlink rejection is applied to the user memory destination root, which core deliberately supports as a symlink (dotfiles layout) Failure scenario: A dotfiles user with ln -s ~/dotfiles/qwen-memories ~/.qwen/memories. Recall, snapshot('user') and the scan all work (they follow the root symlink). Import proceeds: the model call is made, task.usage.modelCalls/reservedTokens are incremented and persisted, the stage directory is written and the ledger records phase 'intent'. Then the walk reaches ~/.qwen/memories, lstat reports a symlink, and it throws. run()'s catch discards that message (only ImportBudgetError/ImportContextError reasons survive) and stores the generic 'Import could not finish. Existing memories were preserved; retry after resolving runtime, model or storage availability.' retry() re-enters the previous.phase === 'intent' branch, calls the same privateDirectory and throws again - so user-scope import is permanently broken for that user, with a paid model call already spent and an error that never names the symlink. Nothing in docs/design/external-memory-import-api.md states the destination root must not be a symlink (it excludes symlinked SOURCES, L20). Witness: [probe] Control/symlink pair with <memory base>/memories a symlink (the dotfiles layout core supports): control | {coreScanComplete:true,proposeCalls:1,modelCallsCharged:1,firstStatus:"added",ledgerPhase:"committed"} vs symlinked | {coreScanComplete:true,proposeCalls:1,modelCallsCharged:1,firstStatus:"failed",retryStatus:"failed",ledgerPhase:"intent"}. Deterministic split; the model call is charged before the throw. Suggested fix: Do not anchor the destination-root check on the runtime-state invariant. Verify the publication target through the mechanism core already uses for that root - getMemoryRootTrustedAnchor(root) / resolveTrustedMemoryFile - and confine privateDirectory's symlink walk to paths under this.base (runtime state, stage, admission). If a symlinked destination root must be refused, refuse it at admission, before prepareProposal spends budget, and surface that specific reason instead of the generic one. The fix must not violate this existing fact: Core grants symlink tolerance to the USER root only: { followRootSymlink: scope === 'user', onUnreadableDir } with the comment 'Project and team roots can live INSIDE the repository ... where a committed symlink would redirect the scan' (packages/core/src/memory/structured-scan.ts:441-447). A fix must not extend symlink tolerance to project or team roots. Acceptance criterion — the test that must go red if this fix is removed: packages/cli/src/memory-import/service.test.ts - a user-scope case ({ ...selection()[0], scope: 'user' }, as at L125/L395) that creates <runtime>/memories as a symlink to a sibling real directory and asserts the section ends status: 'added' with the document published under the symlinked root. Goes red against the current walk. If the author instead chooses up-front refusal, the witness is that submit()/run() rejects before propose is called (assert the propose mock was not invoked).
中文说明
仅完成部分审查，审查缺口已披露。
未审查（原文为英文）：reverse audit — stopped after the round 1/2 convergence pair; rounds 3-5 were not run, so the two-consecutive-dry-rounds convergence criterion was never met (the orchestrator ran out of context, not the review time budget — the pair REPORTED findings, it did not go dry).
未审查（原文为英文）：build-and-test (test half) — the build phase consumed the whole 600s Agent 7 budget: all three changed workspaces compiled green (19/19 per-workspace builds exit 0), but no unit suite executed and the test-efficacy probe was inconclusive (harnessValidated null — its disposable tree lacks built dist/).
未审查（原文为英文）：verifier mutation probing — review scratch-tree refused for every shard (repo-local git includeIf chain resolving to a missing credentials file), so guard-removal mutations could not run in a sanctioned tree; verifiers substituted read-only probes against built dist/ artifacts and the product own injection points, and several findings carry a witness: not run line.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：none — I also verified and dismissed, rather than leaving unchecked: rebuildAutoMemoryIndexAtRoot 's if (!existsSync(root)) return '' early-out (so forget.ts…；"agent reverse-audit (round 2)"：did not verify by execution that a repo-local-root forget leaves <projectRoot>/.qwen/memory/MEMORY.md stale (no test run; Finding 1 rests on reading getProje…；"agent reverse-audit (round 2)"：did not open packages/cli/src/serve/memory-import.test.ts to check whether an existing preview test already pins a route-level size bound (Finding 2's witness…；"agent reverse-audit (round 1)"：none — but two checks were left unfinished for scope, not budget: I did not walk run() / prepareProposal / importSection as their own territory (they sit past…。
[Critical] R1-39: A repeated --source is collected by yargs into an array, fails the typeof === 'string' test, and is silently discarded in favour of the default directory Failure scenario: qwen memory import codex --apply --source /work/proj-a-memories --source /work/proj-b-memories --selection s.json --idempotency-key k - or, far more likely, a shell wrapper or host that appends its own --source after one the user already supplied. The typeof check fails, the default is used instead, and the confirmed import reads, consolidates and publishes from /.codex/memories while the operator believes they imported /work/proj-b-memories. Paid model calls are made and native memory is written from a directory the user never named; nothing in the output identifies which directory was actually read. The same silent-default shape applies to any non-string value yargs can produce for the option. Witness: [probe] Three arms reading which directory was actually opened: arm 1 (single --source) /.codex/memories fallback - so the absent-flag case must keep working; only the supplied-but-not-a-single-string case changes. Acceptance criterion — the test that must go red if this fix is removed: packages/cli/src/commands/memory.test.ts - assert that parsing ['memory','import','codex','--dry-run','--source','a','--source','b'] rejects with a message naming --source, and that a single --source is still honoured. Red today (the array is discarded and the default used).READ dirA=false dirB=true — the control, honours the flag; arm 2 (repeated --source) READ dirA=false dirB=false codexHomeDefault=true — identical to arm 3 (no flag at all). The explicitly named directory is silently replaced by the default with no diagnostic. Suggested fix: Reject rather than default when the flag was supplied but is not a single string: if (args['source'] !== undefined && typeof args['source'] !== 'string') throw new Error('--source must be given exactly once, as a single directory path.'); and only fall back to the CODEX_HOME/default resolution when the flag is absent. Alternatively declare the option non-repeating so yargs itself errors. The fix must not violate this existing fact: The default resolution the else-branch falls through to is the documented one - docs/design/external-memory-import.md names memory import codex --source <directory> and the CODEX_HOME/
[Critical] R1-6: privateDirectory's blanket symlink rejection is applied to the user memory destination root, which core deliberately supports as a symlink (dotfiles layout) Failure scenario: A dotfiles user with ln -s ~/dotfiles/qwen-memories ~/.qwen/memories. Recall, snapshot('user') and the scan all work (they follow the root symlink). Import proceeds: the model call is made, task.usage.modelCalls/reservedTokens are incremented and persisted, the stage directory is written and the ledger records phase 'intent'. Then the walk reaches ~/.qwen/memories, lstat reports a symlink, and it throws. run()'s catch discards that message (only ImportBudgetError/ImportContextError reasons survive) and stores the generic 'Import could not finish. Existing memories were preserved; retry after resolving runtime, model or storage availability.' retry() re-enters the previous.phase === 'intent' branch, calls the same privateDirectory and throws again - so user-scope import is permanently broken for that user, with a paid model call already spent and an error that never names the symlink. Nothing in docs/design/external-memory-import-api.md states the destination root must not be a symlink (it excludes symlinked SOURCES, L20). Witness: [probe] Control/symlink pair with <memory base>/memories a symlink (the dotfiles layout core supports): control | {coreScanComplete:true,proposeCalls:1,modelCallsCharged:1,firstStatus:"added",ledgerPhase:"committed"} vs symlinked | {coreScanComplete:true,proposeCalls:1,modelCallsCharged:1,firstStatus:"failed",retryStatus:"failed",ledgerPhase:"intent"}. Deterministic split; the model call is charged before the throw. Suggested fix: Do not anchor the destination-root check on the runtime-state invariant. Verify the publication target through the mechanism core already uses for that root - getMemoryRootTrustedAnchor(root) / resolveTrustedMemoryFile - and confine privateDirectory's symlink walk to paths under this.base (runtime state, stage, admission). If a symlinked destination root must be refused, refuse it at admission, before prepareProposal spends budget, and surface that specific reason instead of the generic one. The fix must not violate this existing fact: Core grants symlink tolerance to the USER root only: { followRootSymlink: scope === 'user', onUnreadableDir } with the comment 'Project and team roots can live INSIDE the repository ... where a committed symlink would redirect the scan' (packages/core/src/memory/structured-scan.ts:441-447). A fix must not extend symlink tolerance to project or team roots. Acceptance criterion — the test that must go red if this fix is removed: packages/cli/src/memory-import/service.test.ts - a user-scope case ({ ...selection()[0], scope: 'user' }, as at L125/L395) that creates <runtime>/memories as a symlink to a sibling real directory and asserts the section ends status: 'added' with the document published under the symlinked root. Goes red against the current walk. If the author instead chooses up-front refusal, the witness is that submit()/run() rejects before propose is called (assert the propose mock was not invoked).
— qwen3.8-max via Qwen Code /review (v0.25.2)
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — stopped after the round 1/2 convergence pair; rounds 3-5 were not run, so the two-consecutive-dry-rounds convergence criterion was never met (the orchestrator ran out of context, not the review time budget — the pair REPORTED findings, it did not go dry).
Not reviewed: build-and-test (test half) — the build phase consumed the whole 600s Agent 7 budget: all three changed workspaces compiled green (19/19 per-workspace builds exit 0), but no unit suite executed and the test-efficacy probe was inconclusive (harnessValidated null — its disposable tree lacks built dist/).
Not reviewed: verifier mutation probing — review scratch-tree refused for every shard (repo-local git includeIf chain resolving to a missing credentials file), so guard-removal mutations could not run in a sanctioned tree; verifiers substituted read-only probes against built dist/ artifacts and the product own injection points, and several findings carry a witness: not run line.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": none — I also verified and dismissed, rather than leaving unchecked: rebuildAutoMemoryIndexAtRoot 's if (!existsSync(root)) return '' early-out (so forget.ts…; "agent reverse-audit (round 2)": did not verify by execution that a repo-local-root forget leaves <projectRoot>/.qwen/memory/MEMORY.md stale (no test run; Finding 1 rests on reading getProje…; "agent reverse-audit (round 2)": did not open packages/cli/src/serve/memory-import.test.ts to check whether an existing preview test already pins a route-level size bound (Finding 2's witness…; "agent reverse-audit (round 1)": none — but two checks were left unfinished for scope, not budget: I did not walk run() / prepareProposal / importSection as their own territory (they sit past….
[Critical] R1-39: A repeated --source is collected by yargs into an array, fails the typeof === 'string' test, and is silently discarded in favour of the default directory Failure scenario: qwen memory import codex --apply --source /work/proj-a-memories --source /work/proj-b-memories --selection s.json --idempotency-key k - or, far more likely, a shell wrapper or host that appends its own --source after one the user already supplied. The typeof check fails, the default is used instead, and the confirmed import reads, consolidates and publishes from /.codex/memories while the operator believes they imported /work/proj-b-memories. Paid model calls are made and native memory is written from a directory the user never named; nothing in the output identifies which directory was actually read. The same silent-default shape applies to any non-string value yargs can produce for the option. Witness: [probe] Three arms reading which directory was actually opened: arm 1 (single --source) /.codex/memories fallback - so the absent-flag case must keep working; only the supplied-but-not-a-single-string case changes. Acceptance criterion — the test that must go red if this fix is removed: packages/cli/src/commands/memory.test.ts - assert that parsing ['memory','import','codex','--dry-run','--source','a','--source','b'] rejects with a message naming --source, and that a single --source is still honoured. Red today (the array is discarded and the default used).READ dirA=false dirB=true — the control, honours the flag; arm 2 (repeated --source) READ dirA=false dirB=false codexHomeDefault=true — identical to arm 3 (no flag at all). The explicitly named directory is silently replaced by the default with no diagnostic. Suggested fix: Reject rather than default when the flag was supplied but is not a single string: if (args['source'] !== undefined && typeof args['source'] !== 'string') throw new Error('--source must be given exactly once, as a single directory path.'); and only fall back to the CODEX_HOME/default resolution when the flag is absent. Alternatively declare the option non-repeating so yargs itself errors. The fix must not violate this existing fact: The default resolution the else-branch falls through to is the documented one - docs/design/external-memory-import.md names memory import codex --source <directory> and the CODEX_HOME/
[Critical] R1-6: privateDirectory's blanket symlink rejection is applied to the user memory destination root, which core deliberately supports as a symlink (dotfiles layout) Failure scenario: A dotfiles user with ln -s ~/dotfiles/qwen-memories ~/.qwen/memories. Recall, snapshot('user') and the scan all work (they follow the root symlink). Import proceeds: the model call is made, task.usage.modelCalls/reservedTokens are incremented and persisted, the stage directory is written and the ledger records phase 'intent'. Then the walk reaches ~/.qwen/memories, lstat reports a symlink, and it throws. run()'s catch discards that message (only ImportBudgetError/ImportContextError reasons survive) and stores the generic 'Import could not finish. Existing memories were preserved; retry after resolving runtime, model or storage availability.' retry() re-enters the previous.phase === 'intent' branch, calls the same privateDirectory and throws again - so user-scope import is permanently broken for that user, with a paid model call already spent and an error that never names the symlink. Nothing in docs/design/external-memory-import-api.md states the destination root must not be a symlink (it excludes symlinked SOURCES, L20). Witness: [probe] Control/symlink pair with <memory base>/memories a symlink (the dotfiles layout core supports): control | {coreScanComplete:true,proposeCalls:1,modelCallsCharged:1,firstStatus:"added",ledgerPhase:"committed"} vs symlinked | {coreScanComplete:true,proposeCalls:1,modelCallsCharged:1,firstStatus:"failed",retryStatus:"failed",ledgerPhase:"intent"}. Deterministic split; the model call is charged before the throw. Suggested fix: Do not anchor the destination-root check on the runtime-state invariant. Verify the publication target through the mechanism core already uses for that root - getMemoryRootTrustedAnchor(root) / resolveTrustedMemoryFile - and confine privateDirectory's symlink walk to paths under this.base (runtime state, stage, admission). If a symlinked destination root must be refused, refuse it at admission, before prepareProposal spends budget, and surface that specific reason instead of the generic one. The fix must not violate this existing fact: Core grants symlink tolerance to the USER root only: { followRootSymlink: scope === 'user', onUnreadableDir } with the comment 'Project and team roots can live INSIDE the repository ... where a committed symlink would redirect the scan' (packages/core/src/memory/structured-scan.ts:441-447). A fix must not extend symlink tolerance to project or team roots. Acceptance criterion — the test that must go red if this fix is removed: packages/cli/src/memory-import/service.test.ts - a user-scope case ({ ...selection()[0], scope: 'user' }, as at L125/L395) that creates <runtime>/memories as a symlink to a sibling real directory and asserts the section ends status: 'added' with the document published under the symlinked root. Goes red against the current walk. If the author instead chooses up-front refusal, the witness is that submit()/run() rejects before propose is called (assert the propose mock was not invoked).
中文说明
仅完成部分审查，审查缺口已披露。
未审查（原文为英文）：reverse audit — stopped after the round 1/2 convergence pair; rounds 3-5 were not run, so the two-consecutive-dry-rounds convergence criterion was never met (the orchestrator ran out of context, not the review time budget — the pair REPORTED findings, it did not go dry).
未审查（原文为英文）：build-and-test (test half) — the build phase consumed the whole 600s Agent 7 budget: all three changed workspaces compiled green (19/19 per-workspace builds exit 0), but no unit suite executed and the test-efficacy probe was inconclusive (harnessValidated null — its disposable tree lacks built dist/).
未审查（原文为英文）：verifier mutation probing — review scratch-tree refused for every shard (repo-local git includeIf chain resolving to a missing credentials file), so guard-removal mutations could not run in a sanctioned tree; verifiers substituted read-only probes against built dist/ artifacts and the product own injection points, and several findings carry a witness: not run line.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：none — I also verified and dismissed, rather than leaving unchecked: rebuildAutoMemoryIndexAtRoot 's if (!existsSync(root)) return '' early-out (so forget.ts…；"agent reverse-audit (round 2)"：did not verify by execution that a repo-local-root forget leaves <projectRoot>/.qwen/memory/MEMORY.md stale (no test run; Finding 1 rests on reading getProje…；"agent reverse-audit (round 2)"：did not open packages/cli/src/serve/memory-import.test.ts to check whether an existing preview test already pins a route-level size bound (Finding 2's witness…；"agent reverse-audit (round 1)"：none — but two checks were left unfinished for scope, not budget: I did not walk run() / prepareProposal / importSection as their own territory (they sit past…。
[Critical] R1-39: A repeated --source is collected by yargs into an array, fails the typeof === 'string' test, and is silently discarded in favour of the default directory Failure scenario: qwen memory import codex --apply --source /work/proj-a-memories --source /work/proj-b-memories --selection s.json --idempotency-key k - or, far more likely, a shell wrapper or host that appends its own --source after one the user already supplied. The typeof check fails, the default is used instead, and the confirmed import reads, consolidates and publishes from /.codex/memories while the operator believes they imported /work/proj-b-memories. Paid model calls are made and native memory is written from a directory the user never named; nothing in the output identifies which directory was actually read. The same silent-default shape applies to any non-string value yargs can produce for the option. Witness: [probe] Three arms reading which directory was actually opened: arm 1 (single --source) /.codex/memories fallback - so the absent-flag case must keep working; only the supplied-but-not-a-single-string case changes. Acceptance criterion — the test that must go red if this fix is removed: packages/cli/src/commands/memory.test.ts - assert that parsing ['memory','import','codex','--dry-run','--source','a','--source','b'] rejects with a message naming --source, and that a single --source is still honoured. Red today (the array is discarded and the default used).READ dirA=false dirB=true — the control, honours the flag; arm 2 (repeated --source) READ dirA=false dirB=false codexHomeDefault=true — identical to arm 3 (no flag at all). The explicitly named directory is silently replaced by the default with no diagnostic. Suggested fix: Reject rather than default when the flag was supplied but is not a single string: if (args['source'] !== undefined && typeof args['source'] !== 'string') throw new Error('--source must be given exactly once, as a single directory path.'); and only fall back to the CODEX_HOME/default resolution when the flag is absent. Alternatively declare the option non-repeating so yargs itself errors. The fix must not violate this existing fact: The default resolution the else-branch falls through to is the documented one - docs/design/external-memory-import.md names memory import codex --source <directory> and the CODEX_HOME/
[Critical] R1-6: privateDirectory's blanket symlink rejection is applied to the user memory destination root, which core deliberately supports as a symlink (dotfiles layout) Failure scenario: A dotfiles user with ln -s ~/dotfiles/qwen-memories ~/.qwen/memories. Recall, snapshot('user') and the scan all work (they follow the root symlink). Import proceeds: the model call is made, task.usage.modelCalls/reservedTokens are incremented and persisted, the stage directory is written and the ledger records phase 'intent'. Then the walk reaches ~/.qwen/memories, lstat reports a symlink, and it throws. run()'s catch discards that message (only ImportBudgetError/ImportContextError reasons survive) and stores the generic 'Import could not finish. Existing memories were preserved; retry after resolving runtime, model or storage availability.' retry() re-enters the previous.phase === 'intent' branch, calls the same privateDirectory and throws again - so user-scope import is permanently broken for that user, with a paid model call already spent and an error that never names the symlink. Nothing in docs/design/external-memory-import-api.md states the destination root must not be a symlink (it excludes symlinked SOURCES, L20). Witness: [probe] Control/symlink pair with <memory base>/memories a symlink (the dotfiles layout core supports): control | {coreScanComplete:true,proposeCalls:1,modelCallsCharged:1,firstStatus:"added",ledgerPhase:"committed"} vs symlinked | {coreScanComplete:true,proposeCalls:1,modelCallsCharged:1,firstStatus:"failed",retryStatus:"failed",ledgerPhase:"intent"}. Deterministic split; the model call is charged before the throw. Suggested fix: Do not anchor the destination-root check on the runtime-state invariant. Verify the publication target through the mechanism core already uses for that root - getMemoryRootTrustedAnchor(root) / resolveTrustedMemoryFile - and confine privateDirectory's symlink walk to paths under this.base (runtime state, stage, admission). If a symlinked destination root must be refused, refuse it at admission, before prepareProposal spends budget, and surface that specific reason instead of the generic one. The fix must not violate this existing fact: Core grants symlink tolerance to the USER root only: { followRootSymlink: scope === 'user', onUnreadableDir } with the comment 'Project and team roots can live INSIDE the repository ... where a committed symlink would redirect the scan' (packages/core/src/memory/structured-scan.ts:441-447). A fix must not extend symlink tolerance to project or team roots. Acceptance criterion — the test that must go red if this fix is removed: packages/cli/src/memory-import/service.test.ts - a user-scope case ({ ...selection()[0], scope: 'user' }, as at L125/L395) that creates <runtime>/memories as a symlink to a sibling real directory and asserts the section ends status: 'added' with the document published under the symlinked root. Goes red against the current walk. If the author instead chooses up-front refusal, the witness is that submit()/run() rejects before propose is called (assert the propose mock was not invoked).
— qwen3.8-max via Qwen Code /review (v0.25.2)
|
@qwen-code /resolve |
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
🤖 Could not produce a passing fix for this feedback (round 1/100). This item now needs a human; the loop stays engaged and still picks up new feedback and base conflicts, but will not retry this item on its own. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 未能为该反馈产生可通过验证的修复（第 1/100 轮）。此项现在需要人工处理；循环保持在线，仍会拾取新反馈与 base 冲突，但不会自行重试此项。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/39003076111 🧠 Handled by Qwen Code · model/模型 |
Resolve scripts/cli-entry.js: keep both the 'memory' in-process fast path (this PR) and the managed-runtime-bootstrap fast paths (main). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
AppContainer mount effects from earlier tests can still be in flight (the beforeEach leaves the real config.initialize() un-awaited), and an unscoped getInteractiveModSession mockReturnValue lets those stale effects call the current test's initialize() mock, intermittently inflating the exact-once assertion to 51+ calls. Scoping the mock by Config identity makes the stale calls a no-op, matching the real function's behavior for unregistered configs.
|
🔀 Base updated: red check(s) [Serve A/B (ubuntu-latest, Node 22.x), TUI parity snapshots (ink vs opentui), SDK Java (fast) / ubuntu-latest / Java 21, Test (ubuntu-latest, Node 22.x), OpenTUI no-flicker gate, Lint & Static (ubuntu-latest, Node 22.x), Integration Tests (no-AK, No Sandbox), web-shell E2E Smoke (ubuntu-latest, Node 22.x), SDK Java (fast) / Real daemon E2E / Java 11, Managed Agent fast tier] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Serve A/B (ubuntu-latest, Node 22.x), TUI parity snapshots (ink vs opentui), SDK Java (fast) / ubuntu-latest / Java 21, Test (ubuntu-latest, Node 22.x), OpenTUI no-flicker gate, Lint & Static (ubuntu-latest, Node 22.x), Integration Tests (no-AK, No Sandbox), web-shell E2E Smoke (ubuntu-latest, Node 22.x), SDK Java (fast) / Real daemon E2E / Java 11, Managed Agent fast tier] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
|
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind What I found before stopping: Autofix round blocked: required
|
|
🤖 AutoFix deferred a stale-base refresh — the fix did not pass verification and this PR is behind Autofix blocked: the deterministic build rejection is a repository heap ceiling, not this round's changePR #14281, branch BlockerThe deterministic gate rejects the round with: The build script runs the CLI compiler with Evidence this is not caused by the round
中文说明🤖 AutoFix 暂缓了一次过期 base 刷新 —— 修复未通过验证且本 PR 落后于 Autofix 受阻：确定性构建拒绝来自仓库的堆上限，而非本轮改动PR #14281，分支 阻塞项确定性验证门以如下错误拒绝了本轮： 构建脚本以 证据：这不是本轮改动造成的
已尝试的操作
为何未应用本范围内修复唯一有效的杠杆是构建配置，而不是功能代码：
两者都是本 PR 足迹之外的验证与构建机制改动。根构建脚本就是仓库自己的验证门命令；修改它属于维护者决策，并会影响所有包。从构建中排除测试会在无提示的情况下放弃当前可编译文件的类型检查。若无维护者输入，应用任一方案都会违反本轮的范围规则。 请求维护者采取的行动请选择其一：
Run log: https://github.com/QwenLM/qwen-code/actions/runs/39359034712 🧠 Handled by Qwen Code · model/模型 |
|
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 15/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 15/100 轮）。改动内容与我反驳保留之处如下： Address review — PR #14281Feedback triagedReview DispositionsR1-37 — reproduced and fixed this round
Fix: Test: a new Docs: the English and Chinese design documents now both state that the index is bounded to the context budget and that only non-fitting index-only entries are omitted. Verified fixed at HEAD (no code change this round)
Changes
ConflictsNone. Budget noteThe feedback carried a budget warning for the previous round. This round reproduced and fixed only the one Critical that still stood (R1-37), re-verified the other seven against HEAD, and kept the change to a 30-line production change, one focused regression test, and a 2-line documentation sync. Verification
中文说明处理评审 — PR #14281已分类的反馈评审 处置R1-37 — 本轮复现并修复
修复： 测试： 文档：英文和中文设计文档现在都说明索引按上下文预算有界，只省略放不下的仅有索引条目。 在 HEAD 上验证为已修复（本轮无代码改动）
变更
冲突无。 预算说明反馈包含上一轮的超时预算警告。本轮只复现并修复仍然存在的唯一 Critical（R1-37），对其余七条在 HEAD 上重新验证；改动为 30 行生产代码、一个针对性回归测试以及 2 行文档同步。 验证
Critical-only mode is active: 5 change-producing rounds are complete. The workflow excluded the non-Critical feedback below from this round's actionable sections; the items remain open for human follow-up. Maintainer feedback is deferred only after its author has used 2 regular feedback batches in this window's Critical-only tail; authors at that budget, if any, are named below. ( 中文说明已进入仅处理 Critical 的模式：已完成 5 个产生改动的轮次。本轮可执行区域已排除下方非 Critical 反馈；这些条目保持开放，留待人工跟进。维护者反馈仅在其本人于本窗口 Critical-only 阶段已使用 2 批常规反馈预算后才会延后；达到预算的作者（如有）在下方点名。（评论 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 8 selected review thread(s). · 已关闭全部选中的 8 条评审线程。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
📊 Takeover milestone — round 15/100, in the current window. Census: 4 pushed fix(es), 0 no-change review(s), 1 timeout(s), 2 rejected attempt(s), 0 deliberate stop(s) under instruction (deferred to a human), 8 other round(s) (crash / model error / gate error / infra), 6 base update(s). This many rounds deserves a human look. Options: keep going (fine — nothing changes), split or reduce the PR if rounds keep accumulating, or release takeover (remove the 中文说明📊 接管里程碑 —— 第 15/100 轮（当前窗口）。统计：推送修复 4 次、审阅无需改动 0 次、超时 1 次、验证拒绝 2 次、按指示有意停止（移交人工）0 次、其他轮次（崩溃/模型错误/门错误/infra）8 次、base 更新 6 次。 轮次到这个量值得人工看一眼。可选：继续（无需操作）；若轮次持续累积，考虑拆分或缩减 PR；或释放接管（移除 |
Stale at 3db4352. Criticals in the review bodies and inline ledgers have been fixed and rechecked, including R1-37 bounded context (29fb, independent red/green) and final R2-3 native capacity (46b, three actual-service cases). Earlier ownership, recovery, auth, parser, publication and Git-cache fixes remain intact after the normal main merge. All 67 threads are resolved. Remaining Suggestion dispositions are individually linked as fixed/deferred under the repository Critical-only-after-five-rounds rule in comment6188530836. No live human CHANGES_REQUESTED or approval exists. This removes obsolete bot blocking state; it does not approve the PR or bypass current CI/maintainer approval.
✅ Local maintainer verification (round 2): merge-ready — 222/222 aggregate checksVerified head 中文摘要结论：merge-ready（222/222）。 上一轮（head 上一轮 2 项非阻塞观察的复测： ① 不受信任工作区的拒绝已变成单行错误（不再打印 yargs 帮助和堆栈）——已修复；② CLI 预览仍不读取未被引用的 rollout summary（HTTP 预览会给出 warning）——仍然成立，属有意的本地读取限制。 本轮新证据： 真实 daemon 只在 head 上宣告 head 之后的新提交： 原生索引容量优先（ 未覆盖： 成熟记忆库的 32 KiB existing-context 端到端探针（本轮 rig 中扫描器未把造出的原生文档送入请求，故只由 PR 自带单测覆盖）；“失败项重试”无法稳定注入（500 被 workspace worker 的有界重试吸收）；typecheck/lint/全仓套件、Windows/Linux、TUI、生产计费。 Previous-finding status (round 1 → this head)Round 1 (comment of 2026-10-18, head
Round-1 “Not covered” items that this round now covers: HTTP routes, cancellation, SIGKILL lease recovery, linked-worktree sharing, independent-clone isolation. Central claim and A/BClaim re-measured at the new head: the build exposes Both trees were installed with the pinned pnpm and fully built (
Screenshots: New production commits since the previously verified head
Gates
FindingsNo blocking finding introduced by this PR. Non-blocking observations only:
Not covered
MethodologyTwo detached worktrees at the verified head and the merge-base, each installed with the pinned pnpm and fully built (exit 0 both). Verified locally by @wenshao with Qwen Code. Artifacts: Evidence captures
Merge state at posting: |
|
@qwen-code /resolve |
qwen-code-review-bot
left a comment
qwen-code-review-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": I did not read docs/design/external-memory-import-api.md 's failure-mapping table, so I could not confirm whether "managed memory unavailable" is a declared te…; "agent reverse-audit (round 1)": I did not measure how many runtimes a hosted daemon has registered at createServeApp time, nor how many batches typically survive a restart, so finding 3's N×…; "agent reverse-audit (round 1)": my assigned range ends mid-file at diff line 5498, inside packages/core/src/core/baseLlmClient.test.ts ; any further hunks of that file lie in the next chunk a…; chunk 16: could not execute the suite to confirm the six tests pass — the review worktree has no node_modules , so verification is static-only (notably the untested-at-r…; chunk 5: did not execute packages/cli vitest to confirm the two open questions above empirically (would require npm run build for the workspace dist/ prerequisites…, and 1 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/memory-import/service.ts:827 — [review] Critical [fails-closed] [new-surface] The new import snapshot gate treats the scan's *informational* ref_collision status — emitted whenever two project memory roots contain the sam…docs/design/external-memory-import-api.md:34 — [review] The API contract enumerates every source limit but omits the enforced per-submission selection cap of 2,048 items, while affirmatively telling hosts they may select *every* selectable …packages/cli/src/acp-integration/acpAgent.ts:10819 — [review] The new agent-side ext-method handler — including its fail-closed availability/trust gate — has no test anywhere in the tree.packages/cli/src/cli.ts:431 — [review] The bootstrap memory help hardcodes the subcommand name and its describe text that actually live in packages/cli/src/commands/memory.ts , and — unlike the mcp equivalent — nothing pins them together…packages/cli/src/cli.ts:506 — [review] runMemoryFastPath is a near-verbatim copy of runMcpFastPath — the parser construction, the fail handler, .exitProcess(false) and the hasFlag(argv.slice(2), '--help', '-h') help intercept are …packages/managed-agent-server/src/test/java/com/alibaba/qwen/code/managedagent/HostedMcpResultDeliveryIT.java:39 (+2 locations) — [review] The PR’s own scope statement is falsified by three unrelated changes in the diff, and the head commit…packages/cli/src/commands/memory.ts:151 — [review] The API doc shipped in this diff promises retry-by-idempotency-key, but the CLI reaches service.retry() only after a fresh submit() of the *current* local source, so any drift in the Co…packages/cli/src/memory-import/service.ts:359 — [review] The documented crash-recovery guarantee rests entirely on one untested constant: no committed test exercises stale-lease takeover, so the only evidence for it is a manual SIGKILL prob…packages/core/src/memory/import-paths.ts:134 — [review] New exported core symbol memoryImportRepositoryIdentity has zero consumers anywhere in the tracked tree — it is a dead public API on the core package.packages/cli/src/serve/memory-import.test.ts:86 — [review] The runtime double never sets routeFileSystemFactory.sshWorkspace , so the SSH trust-boundary refusal inside active() has no test anywhere in the repository.packages/core/src/memory/import-budget.ts:66 — [review] The estimate charges the *ceiling* prompt and context allowance per batch (74,752 fixed), so exceedsBudget trips at 7 batches regardless of content, while the same object reports li…packages/core/src/memory/import-paths.ts:12 — [review] readMetadata hand-rolls a git-metadata read whose flag expression silently collapses to a plain symlink-following O_RDONLY open on Windows, instead of reusing the hardened readers t…packages/core/src/memory/structured-scan.ts:466 — [review] The new cap-priority tie-break demotes imported documents only in user scope, but this same PR is what first puts imported documents into the *project*-scope scan, where the predi…packages/core/src/memory/paths.ts:222 — [review] The warn-once latch is a process-global boolean guarding a **per-repository** failure, so the first broken repository consumes the diagnostic for every other repository in the same process.packages/cli/src/memory-import/local.ts:20 — [review] A locally generated sessionId is passed to loadCliConfig without the sessionIdGenerated: true flag that marks an id as freshly generated, so it takes the caller-supplied-id occupan…packages/cli/src/commands/memory.ts:60 — [review] The option's own describe — the text qwen memory import codex --help prints — says the value is a JSON array, while the handler treats it as a file path, contradicting the closing hint i…packages/cli/src/memory-import/service.test.ts:508 — [review] The test is named "cancels before publication **and supports explicit retry**", but its body never calls retry() — it asserts only status === 'cancelled' and that nothing was…packages/cli/src/memory-import/service.test.ts:1030 — [review] The budget-exhaustion test passes vacuously when zero model calls are made, so it cannot distinguish "stopped at the limit" from "never started".packages/cli/src/memory-import/service.test.ts:948 — [review] The too-large submit's result is discarded, so the first half of the comment's claim — a processed source skips the model and its context — is asserted nowhere.packages/cli/src/memory-import/service.ts:206 — [review] selectImportContext 's relevance test tokenizes on runs of ≥4 \p{L}\p{N}_- characters, which for CJK text yields whole punctuation-delimited runs instead of words, so a Chinese mem…- …and 58 more (see the run report)
中文说明
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：I did not read docs/design/external-memory-import-api.md 's failure-mapping table, so I could not confirm whether "managed memory unavailable" is a declared te…；"agent reverse-audit (round 1)"：I did not measure how many runtimes a hosted daemon has registered at createServeApp time, nor how many batches typically survive a restart, so finding 3's N×…；"agent reverse-audit (round 1)"：my assigned range ends mid-file at diff line 5498, inside packages/core/src/core/baseLlmClient.test.ts ; any further hunks of that file lie in the next chunk a…；chunk 16：could not execute the suite to confirm the six tests pass — the review worktree has no node_modules , so verification is static-only (notably the untested-at-r…；chunk 5：did not execute packages/cli vitest to confirm the two open questions above empirically (would require npm run build for the workspace dist/ prerequisites…，另有 1 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 78 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.25.3)
|
⏳ Historical-head review — head moved to 中文说明⏳ 历史 head 评审 —— 本次评审进行中 head 移动到了 |
|
Qwen Code did not run conflict resolution for this request. PR #14281 does not currently have merge conflicts with main. |
This branch has not been deployed
5 participants
What this PR does
Adds a Qwen-owned external memory import service for CLI and cloud integrations, starting with Codex. Clients submit relative Markdown paths and contents, inspect a deterministic preview, explicitly select sections and destination scope, then start durable background consolidation using the selected workspace's configured model. Results enter native memory search, fetch, recall and forget. Main checkouts and registered linked worktrees share the imported project layer while ordinary memory isolation remains unchanged.
The service exposes
memory_import_v1, workspace-qualified HTTP preview/submit/status/source/cancel/retry endpoints, and offlinememory import codex --dry-run --json. Unknown source scope stays unassigned. The server owns destination paths and validates active workspace trust; it never falls back to the primary runtime. Only selected source snapshots are retained. Journaled publication, idempotency, cancellation and retry protect existing, edited and forgotten memories.Consolidation uses bounded batches per source project and confirmed scope, a compact same-scope metadata index with relevant evidence, and durable aggregate call/token reservations. Preview exposes usage estimates without reading native memories or invoking a model. Failure, cancellation and retry do not replenish the budget. Saved batch proposals preserve per-source recovery and edit/forget protection; imported repository identity lookups are cached and skipped entirely when no imported layer exists.
Why it's needed
A cloud runtime cannot read a user's laptop filesystem. Direct per-bullet conversion also loses historical context and does not reconcile old knowledge with existing native memory. This replaces #14149 with shared backend capabilities and model-backed consolidation. No external product UI, independent collector, dependency installation requirement or unrelated CI repair is included.
Reviewer Test Plan
How to verify
Submit a synthetic Codex source set containing two projects and a referenced distilled Markdown resource. Preview should return grouped sections, references and assignment reasons without model calls or memory/task writes. An ambiguous section should stay unassigned.
Select only one project and submit to a trusted non-primary workspace. Observe queued/running/completed results, then fetch its retained source and search the native result. The other project's text must never reach the model or retained snapshot; unknown/untrusted/draining destinations must not fall back to primary.
Repeat the same request/key and then try another key with the same selected content. Native documents should not multiply. Edit or forget an imported document; another import must return conflict rather than restore it.
Cancel during consolidation, retry a failed item, and restart after a forced process kill before/after publication. Recovery should wait out a dead worker's lease, preserve accurate counts/refs and publish each document only once.
Verify the imported project result through a linked worktree, and verify an independent clone remains isolated. Ordinary per-worktree memory must retain its existing behavior.
Verify a 12-section selected project is processed in two bounded calls, while excluded groups never reach the model. Preview should show group and aggregate usage estimates; retrying a budget-exhausted task must not reset the allowance. Interrupt between publications and verify saved proposals resume without another paid call or restoring edited/forgotten knowledge.
Evidence (Before & After)
Before: the installed baseline CLI rejected
memory import codex, and the base daemon did not advertise the capability. After: synthetic shipped-CLI dry-runs (bare, leading debug and safe mode) returned versioned pure JSON with zero network attempts and zero filesystem changes. Confirmed CLI import and the real bundled daemon → real ACP child → loopback OpenAI-compatible model produced native searchable memory without unselected input. Bare/safe confirmed import rejected before source access or authentication.Initial implementation tests passed: 559 CLI cases, 31 core cases, and the focused cold-workspace ACP bridge case. Full workspace build and typecheck passed; affected packages were rebuilt after final edits, with a final bundle and CLI typecheck. ESLint and formatting passed. Real SIGKILL probes left stale lock directories and recovered automatically after approximately 120 seconds, without manual lock deletion; both interruption points produced one document and correct original-batch counts. Two self-audit passes and a local medium review found no remaining issue; these are not formal approval.
Latest cost update after merging main: 55 focused CLI and 126 core tests passed, along with full build, bundle, typecheck, lint and format. Real bundled CLI/HTTP each processed 12 selected sources as [8, 4], with two requests and no extra request on repetition. Synthetic existing-context payload fell from 199,080 to 2,170 bytes. Interrupted-batch, budget-recovery and cross-group ordering probes passed. Production billing savings are not asserted.
Tested on
Environment (optional)
Isolated synthetic HOME, runtime, source and repository directories; deterministic loopback provider only. The full build used an 8 GiB Node heap. No actual user memory or production provider was accessed.
Risk & Scope
Design and API contracts are complete and synchronized in both languages: Design (English), 设计（中文）, API (English), 接口（中文）.
Linked Issues
Supersedes #14149 following the requested backend redesign.
中文说明
本 PR 的变更
提供由 Qwen 自身负责的外部记忆导入服务，供 CLI 和云端系统接入，首版支持 Codex。客户端提交相对 Markdown 路径和内容，查看确定性预览，明确选择章节和目标范围后，启动使用目标工作区模型的持久化后台整理任务。结果进入原生记忆搜索、全文、召回和遗忘流程。主检出与已注册 linked worktree 共享导入项目层，普通记忆的既有隔离语义保持不变。
提供
memory_import_v1、限定工作区的 HTTP 预览/提交/状态/来源/取消/重试接口，以及离线memory import codex --dry-run --json。未知归属保持待分配。目标路径由服务端管理，检查活动工作区信任状态，不回退主运行时。只保留选中来源快照，通过提交日志、幂等、取消和重试保护已有、已编辑和已遗忘的记忆。整理按来源项目和确认范围分成有界批次，发送紧凑的同范围元数据索引及相关证据，并持久化总调用/token预留预算。预览展示估算，不读取原生记忆、不调用模型；失败、取消、重试不会重置预算。整批提案持久化后支持逐来源恢复，继续保护编辑和遗忘。仓库身份采用有界缓存，没有导入层时跳过遍历。
为什么需要
云端运行时无法直接读取用户电脑的文件系统；逐条转换 Markdown 列表还会丢失历史上下文，无法协调历史知识和已有原生记忆。本 PR 以共享底层能力和模型整理替代 #14149。不包含外部产品界面、独立收集器、额外安装要求或无关 CI 修补。
评审测试计划
使用含两个项目和一个引用 Markdown 资源的合成来源，验证预览返回分组、引用和归属原因，不调用模型，不写记忆或任务；歧义章节保持待分配。只选择一个项目并提交到可信次工作区，观察任务状态、查看保留来源并搜索原生结果；未选内容不得进入模型或快照，未知、不可信和退出中的目标不得回退主工作区。
重复同一请求/幂等键，再用新键提交相同内容，文档数量不应增加。编辑或遗忘导入文档后再次导入应返回冲突，不恢复内容。验证取消、失败重试，以及发布前后强制结束进程后的重启恢复：等待旧锁租约后自动继续，计数和引用正确且只发布一次。验证 linked worktree 共享结果、独立 clone 隔离及普通记忆原有行为。
前后证据
基线已安装 CLI 不支持该命令，基线 daemon 无此能力标识。实现后，成品 CLI 的普通、前置 debug 和 safe 模式预览均输出版本化纯 JSON，零网络尝试和零文件变化。确认后的 CLI 导入，以及真实 daemon → 真实 ACP 子进程 → 本地 OpenAI 兼容模拟模型的链路，均生成可搜索原生记忆且不包含未选来源。bare/safe 模式确认导入在读取来源或认证前被拒绝。
本地通过 559 项 CLI、31 项 core 测试及冷启动工作区 ACP 专项测试。全工作区构建和类型检查通过；最后修改后重新构建受影响包，完成最终 bundle 和 CLI 类型检查。ESLint、格式检查通过。真实 SIGKILL 遗留锁目录，约 120 秒后自动恢复，无需手工删锁；两个中断点均只生成一个文档，原批次计数正确。两轮自审和本地 medium 审查无剩余问题，不等同正式批准。
最新成本更新同步主线后，55 项 CLI、126 项 core 定向测试及全构建、bundle、typecheck、lint/format 通过。真实 CLI/HTTP 均将 12 条选中来源分成 8＋4，只调用两次，重复提交不增加调用；预览展示每组和总用量估算。合成已有上下文从 199,080 降至 2,170 字节，中断恢复、预算不重置与交错项目顺序验证通过。这不是生产账单节省率。
平台与环境
macOS 已完成合成 CLI、真实 daemon/ACP/模型传输、原生记忆和进程恢复验证；Windows、Linux 等待 CI。使用隔离的合成 HOME、运行目录、来源和仓库，以及确定性本地模拟模型；全量构建使用 8 GiB Node 堆。未访问真实用户记忆或生产模型。
每任务上限为 32 次逻辑调用及 512,000 个保守预留 token 单位，每次输出最多 8,192 token；不保证供应商实际计费 token 或金额硬上限。元数据或相关证据超限时明确失败。
风险、范围与迁移
本功能跨包并接入原生记忆读取和 ACP，需要维护者架构评审。模型输出经过结构化和原生格式校验且不能覆盖现有文档，但生产模型语义质量仍需评估。不包含外部产品界面、独立收集器、持续同步、跨实例同步或生产部署。v1 拒绝 SSH 工作区及旧式记忆 base/local 路径覆盖；资源和上下文限制已说明，超限失败而不静默截断。
新增能力和接口，普通记忆写入根目录不变。不迁移 #14149 的实验状态。双语设计与接口文档内容完整同步，链接见上方。本 PR 按重新设计要求替代 #14149。